diff --git a/linting/Cargo.toml b/linting/Cargo.toml
index bd5024e310..a5861b1f8e 100644
--- a/linting/Cargo.toml
+++ b/linting/Cargo.toml
@@ -18,18 +18,18 @@ include = ["Cargo.toml", "*.rs", "LICENSE"]
crate-type = ["cdylib"]
[dependencies]
-clippy_utils = { git = "https://github.com/rust-lang/rust-clippy", rev = "1480cea393d0cee195e59949eabdfbcf1230f7f9" }
-dylint_linting = "2.0.0"
+clippy_utils = { git = "https://github.com/rust-lang/rust-clippy", rev = "1d334696587ac22b3a9e651e7ac684ac9e0697b2" }
+dylint_linting = "2.1.12"
if_chain = "1.0.2"
log = "0.4.14"
regex = "1.5.4"
[dev-dependencies]
-dylint_testing = "2.0.0"
+dylint_testing = "2.1.12"
# The following are ink! dependencies, they are only required for the `ui` tests.
ink_env = { path = "../crates/env", default-features = false }
-ink = { path = "../crates/ink", default-features = false }
+ink = { path = "../crates/ink", default-features = false, features = ["std"] }
ink_metadata = { path = "../crates/metadata", default-features = false }
ink_primitives = { path = "../crates/primitives", default-features = false }
ink_storage = { path = "../crates/storage", default-features = false }
@@ -45,6 +45,12 @@ scale-info = { version = "2.6", default-features = false, features = ["derive"]
#
# Those files require the ink! dependencies though, by having
# them as examples here, they inherit the `dev-dependencies`.
+[[example]]
+name = "primitive_topic_pass"
+path = "ui/pass/primitive_topic.rs"
+[[example]]
+name = "primitive_topic_fail"
+path = "ui/fail/primitive_topic.rs"
[package.metadata.rust-analyzer]
rustc_private = true
diff --git a/linting/rust-toolchain.toml b/linting/rust-toolchain.toml
index fdec8427c3..cac65e4fce 100644
--- a/linting/rust-toolchain.toml
+++ b/linting/rust-toolchain.toml
@@ -2,5 +2,5 @@
# https://github.com/trailofbits/dylint/blob/ef7210cb08aac920c18d2141604efe210029f2a2/internal/template/rust-toolchain
[toolchain]
-channel = "nightly-2023-01-27"
+channel = "nightly-2023-07-14"
components = ["llvm-tools-preview", "rustc-dev"]
diff --git a/linting/src/lib.rs b/linting/src/lib.rs
index 266d33d027..5b1dc50b4b 100644
--- a/linting/src/lib.rs
+++ b/linting/src/lib.rs
@@ -1,18 +1,16 @@
// Copyright (C) Parity Technologies (UK) Ltd.
-// This file is part of cargo-contract.
//
-// cargo-contract is free software: you can redistribute it and/or modify
-// it under the terms of the GNU General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
//
-// cargo-contract is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-// GNU General Public License for more details.
+// http://www.apache.org/licenses/LICENSE-2.0
//
-// You should have received a copy of the GNU General Public License
-// along with cargo-contract. If not, see .
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
#![doc(
html_logo_url = "https://use.ink/img/crate-docs/logo.png",
@@ -30,12 +28,16 @@ extern crate rustc_middle;
extern crate rustc_session;
extern crate rustc_span;
+mod primitive_topic;
+
#[doc(hidden)]
#[no_mangle]
pub fn register_lints(
_sess: &rustc_session::Session,
- _lint_store: &mut rustc_lint::LintStore,
+ lint_store: &mut rustc_lint::LintStore,
) {
+ lint_store.register_lints(&[primitive_topic::PRIMITIVE_TOPIC]);
+ lint_store.register_late_pass(|_| Box::new(primitive_topic::PrimitiveTopic));
}
#[test]
diff --git a/linting/src/primitive_topic.rs b/linting/src/primitive_topic.rs
new file mode 100644
index 0000000000..14bcdd2451
--- /dev/null
+++ b/linting/src/primitive_topic.rs
@@ -0,0 +1,223 @@
+// Copyright (C) Parity Technologies (UK) Ltd.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+use clippy_utils::{
+ diagnostics::span_lint_and_then,
+ is_lint_allowed,
+ match_def_path,
+ source::snippet_opt,
+};
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir::{
+ self,
+ def::{
+ DefKind,
+ Res,
+ },
+ def_id::DefId,
+ Arm,
+ AssocItemKind,
+ ExprKind,
+ Impl,
+ ImplItemKind,
+ ImplItemRef,
+ Item,
+ ItemKind,
+ Node,
+ PatKind,
+ QPath,
+};
+use rustc_lint::{
+ LateContext,
+ LateLintPass,
+};
+use rustc_middle::ty::{
+ self,
+ Ty,
+ TyKind,
+};
+use rustc_session::{
+ declare_lint,
+ declare_lint_pass,
+};
+
+declare_lint! {
+ /// **What it does:** Checks for ink! contracts that use
+ /// the [`#[ink(topic)]`](https://use.ink/macros-attributes/topic) annotation with primitive
+ /// number types. Topics are discrete events for which it makes sense to filter. Typical
+ /// examples of fields that should be filtered are `AccountId`, `bool` or `enum` variants.
+ ///
+ /// **Why is this bad?** It typically doesn't make sense to annotate types like `u32` or `i32`
+ /// as a topic, if those fields can take continuous values that could be anywhere between
+ /// `::MIN` and `::MAX`. An example of a case where it doesn't make sense at all to have a
+ /// topic on the storage field is something like `value: Balance` in the examle below.
+ ///
+ /// **Example:**
+ ///
+ /// ```rust
+ /// // Good
+ /// // Filtering transactions based on source and destination addresses.
+ /// #[ink(event)]
+ /// pub struct Transaction {
+ /// #[ink(topic)]
+ /// src: Option,
+ /// #[ink(topic)]
+ /// dst: Option,
+ /// value: Balance,
+ /// }
+ /// ```
+ ///
+ /// ```rust
+ /// // Bad
+ /// // It typically makes no sense to filter `Balance`, since its value may varies from `::MAX`
+ /// // to `::MIN`.
+ /// #[ink(event)]
+ /// pub struct Transaction {
+ /// #[ink(topic)]
+ /// src: Option,
+ /// #[ink(topic)]
+ /// dst: Option,
+ /// #[ink(topic)]
+ /// value: Balance,
+ /// }
+ /// ```
+ pub PRIMITIVE_TOPIC,
+ Warn,
+ "`#[ink(topic)]` should not be used with a number primitive"
+}
+
+declare_lint_pass!(PrimitiveTopic => [PRIMITIVE_TOPIC]);
+
+/// Returns `true` if `item` is an implementation of `::ink::env::Event` for a storage
+/// struct. If that's the case, it returns the name of this struct.
+fn is_ink_event_impl<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>) -> bool {
+ if let Some(trait_ref) = cx.tcx.impl_trait_ref(item.owner_id) {
+ match_def_path(
+ cx,
+ trait_ref.skip_binder().def_id,
+ &["ink_env", "event", "Event"],
+ )
+ } else {
+ false
+ }
+}
+
+/// Returns `true` if `impl_item` is the `topics` function
+fn is_topics_function(impl_item: &ImplItemRef) -> bool {
+ impl_item.kind == AssocItemKind::Fn { has_self: true }
+ && impl_item.ident.name.as_str() == "topics"
+}
+
+/// Returns `true` if `ty` is a numerical primitive type that should not be annotated with
+/// `#[ink(topic)]`
+fn is_primitive_number_ty(ty: &Ty) -> bool {
+ matches!(ty.kind(), ty::Int(_) | ty::Uint(_))
+}
+
+/// Reports a topic-annotated field with a numerical primitive type
+fn report_field(cx: &LateContext, event_def_id: DefId, field_name: &str) {
+ if_chain! {
+ if let Some(Node::Item(event_node)) = cx.tcx.hir().get_if_local(event_def_id);
+ if let ItemKind::Struct(ref struct_def, _) = event_node.kind;
+ if let Some(field) = struct_def.fields().iter().find(|f|{ f.ident.as_str() == field_name });
+ if !is_lint_allowed(cx, PRIMITIVE_TOPIC, field.hir_id);
+ then {
+ span_lint_and_then(
+ cx,
+ PRIMITIVE_TOPIC,
+ field.span,
+ "using `#[ink(topic)]` for a field with a primitive number type",
+ |diag| {
+ let snippet = snippet_opt(cx, field.span).expect("snippet must exist");
+ diag.span_suggestion(
+ field.span,
+ "consider removing `#[ink(topic)]`".to_string(),
+ snippet,
+ Applicability::Unspecified,
+ );
+ },
+ )
+ }
+ }
+}
+
+/// Returns `DefId` of the event struct for which `Topics` is implemented
+fn get_event_def_id(topics_impl: &Impl) -> Option {
+ if_chain! {
+ if let rustc_hir::TyKind::Path(qpath) = &topics_impl.self_ty.kind;
+ if let QPath::Resolved(_, path) = qpath;
+ if let Res::Def(DefKind::Struct, def_id) = path.res;
+ then { Some(def_id) }
+ else { None }
+ }
+}
+
+impl<'tcx> LateLintPass<'tcx> for PrimitiveTopic {
+ fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
+ if_chain! {
+ if let ItemKind::Impl(topics_impl) = &item.kind;
+ if is_ink_event_impl(cx, item);
+ if let Some(event_def_id) = get_event_def_id(topics_impl);
+ then {
+ topics_impl.items.iter().for_each(|impl_item| {
+ if_chain! {
+ // We need to extract field patterns from the event sturct matched in the
+ // `topics` function to access their inferred types.
+ // Here is the simplified example of the expanded code:
+ // ```
+ // fn topics(/* ... */) {
+ // match self {
+ // MyEvent {
+ // field_1: __binding_0,
+ // field_2: __binding_1,
+ // /* ... */
+ // ..
+ // } => { /* ... */ }
+ // }
+ // }
+ // ```
+ if is_topics_function(impl_item);
+ let impl_item = cx.tcx.hir().impl_item(impl_item.id);
+ if let ImplItemKind::Fn(_, eid) = impl_item.kind;
+ let body = cx.tcx.hir().body(eid).value;
+ if let ExprKind::Block (block, _) = body.kind;
+ if let Some(match_self) = block.expr;
+ if let ExprKind::Match(_, [Arm { pat: arm_pat, .. }], _) = match_self.kind;
+ if let PatKind::Struct(_, pat_fields, _) = &arm_pat.kind;
+ then {
+ pat_fields.iter().for_each(|pat_field| {
+ cx.tcx
+ .has_typeck_results(pat_field.hir_id.owner.def_id)
+ .then(|| {
+ if let TyKind::Ref(_, ty, _) = cx
+ .tcx
+ .typeck(pat_field.hir_id.owner.def_id)
+ .pat_ty(pat_field.pat)
+ .kind()
+ {
+ if is_primitive_number_ty(ty) {
+ report_field(cx,
+ event_def_id,
+ pat_field.ident.as_str())
+ }
+ }
+ });
+ })
+ }
+ }
+ })
+ }
+ }
+ }
+}
diff --git a/linting/ui/fail/primitive_topic.rs b/linting/ui/fail/primitive_topic.rs
new file mode 100644
index 0000000000..6c0cb5d00e
--- /dev/null
+++ b/linting/ui/fail/primitive_topic.rs
@@ -0,0 +1,36 @@
+pub type TyAlias1 = i32;
+pub type TyAlias2 = TyAlias1;
+
+#[ink::contract]
+pub mod primitive_topic {
+
+ #[ink(event)]
+ pub struct Transaction {
+ // Bad
+ #[ink(topic)]
+ value_1: u8,
+ // Bad
+ #[ink(topic)]
+ value_2: Balance,
+ // Bad
+ #[ink(topic)]
+ value_3: crate::TyAlias1,
+ // Bad
+ #[ink(topic)]
+ value_4: crate::TyAlias2,
+ }
+
+ #[ink(storage)]
+ pub struct PrimitiveTopic {}
+
+ impl PrimitiveTopic {
+ #[ink(constructor)]
+ pub fn new() -> Self {
+ Self {}
+ }
+ #[ink(message)]
+ pub fn do_nothing(&mut self) {}
+ }
+}
+
+fn main() {}
diff --git a/linting/ui/fail/primitive_topic.stderr b/linting/ui/fail/primitive_topic.stderr
new file mode 100644
index 0000000000..634dacf4b8
--- /dev/null
+++ b/linting/ui/fail/primitive_topic.stderr
@@ -0,0 +1,28 @@
+error: using `#[ink(topic)]` for a field with a primitive number type
+ --> $DIR/primitive_topic.rs:11:9
+ |
+LL | value_1: u8,
+ | ^^^^^^^^^^^ help: consider removing `#[ink(topic)]`: `value_1: u8`
+ |
+ = note: `-D primitive-topic` implied by `-D warnings`
+
+error: using `#[ink(topic)]` for a field with a primitive number type
+ --> $DIR/primitive_topic.rs:14:9
+ |
+LL | value_2: Balance,
+ | ^^^^^^^^^^^^^^^^ help: consider removing `#[ink(topic)]`: `value_2: Balance`
+
+error: using `#[ink(topic)]` for a field with a primitive number type
+ --> $DIR/primitive_topic.rs:17:9
+ |
+LL | value_3: crate::TyAlias1,
+ | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `#[ink(topic)]`: `value_3: crate::TyAlias1`
+
+error: using `#[ink(topic)]` for a field with a primitive number type
+ --> $DIR/primitive_topic.rs:20:9
+ |
+LL | value_4: crate::TyAlias2,
+ | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider removing `#[ink(topic)]`: `value_4: crate::TyAlias2`
+
+error: aborting due to 4 previous errors
+
diff --git a/linting/ui/pass/primitive_topic.rs b/linting/ui/pass/primitive_topic.rs
new file mode 100644
index 0000000000..6cc5428072
--- /dev/null
+++ b/linting/ui/pass/primitive_topic.rs
@@ -0,0 +1,31 @@
+#[ink::contract]
+pub mod primitive_topic {
+
+ #[ink(event)]
+ pub struct Transaction {
+ #[ink(topic)]
+ src: Option,
+ #[ink(topic)]
+ dst: Option,
+ // Good: no topic annotation
+ value_1: Balance,
+ // Good: suppressed warning
+ #[ink(topic)]
+ #[cfg_attr(dylint_lib = "ink_linting", allow(primitive_topic))]
+ value_2: Balance,
+ }
+
+ #[ink(storage)]
+ pub struct PrimitiveTopic {}
+
+ impl PrimitiveTopic {
+ #[ink(constructor)]
+ pub fn new() -> Self {
+ Self {}
+ }
+ #[ink(message)]
+ pub fn do_nothing(&mut self) {}
+ }
+}
+
+fn main() {}