Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Linter: Warn when number primitive annotated with #[ink(topic)] #1436

Closed
cmichi opened this issue Oct 17, 2022 · 1 comment · Fixed by #1837
Closed

Linter: Warn when number primitive annotated with #[ink(topic)] #1436

cmichi opened this issue Oct 17, 2022 · 1 comment · Fixed by #1837
Assignees
@jubnzv
Labels
A-linter Issue regarding the ink! linter. B-feature-request A request for a new feature.

Comments

@cmichi
Copy link
Collaborator

cmichi commented Oct 17, 2022

The #[ink(topic)] annotation enables third party tooling to filter for "topics" ‒ discrete events for which it makes sense to filter. Typical examples where this can make sense are AccountId, bool or enum variants.

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.

We should provide a linter rule that warns developers when they annotate a number primitive with #[ink(topic)].
@cmichi cmichi added B-feature-request A request for a new feature. A-linter Issue regarding the ink! linter. labels Oct 17, 2022
@cmichi
Copy link
Collaborator Author

cmichi commented Jun 16, 2023 •
edited
Loading

I'm putting some notes here on how to approach this issue:

The linting stage in ink! is optional by now. You have to install the linting dependencies and can then execute cargo contract build --lint on a contract. With the ink! 4.0 release we deprecated the only one lint we had. So at the moment there won't be any lint run.

A lint was introduced in this PR. The files to get you started are in ink_linting/src/. Those two are most relevant:

There are some more interesting things in this PR (tests), but the above should serve as a starting point.

jubnzv pushed a commit to jubnzv/ink that referenced this issue Jun 26, 2023
The initial structure of the linting crate and tests
7ca2cf7 
Nedeed for use-ink#1436
@jubnzv jubnzv mentioned this issue Jul 5, 2023
Merged
jubnzv added a commit that referenced this issue Aug 15, 2023
@jubnzv
Linter: Warn when number primitive is annotated with #[ink(topic)] (#…
cfada84 
…1837)

* The initial structure of the linting crate and tests

Needed for #1436

* Implement the lint that checks primitive numeric types in expanded code

* The initial implementation of the `primitive_topic` lint

* chore: Run clippy

* fix(fmt)

* fix: Lint suppressions

The problem with suppressions is that `rustc` saves the state of the
last visited node and checks its attributes to decide if the lint
warning should be suppressed there (see:
[LateContext::last_node_with_lint_attrs](https://github.com/rust-lang/rust/blob/c44324a4fe8e96f9d6473255df6c3a481caca76f/compiler/rustc_lint/src/context.rs#L1105)).

This commit adds a call to the utility function from clippy that checks
if the lint is suppressed for the field definition node, not for the
last node (which is `Topics` impl).

* chore: cleanup

* chore: make warning messages more consistent

* feat: support events 2.0

Rework the lint after #1827 is merged

* chore: Fix comments; remove needless empty file

* chore: Update `clippy_utils` version

* fix: Cargo.toml

---------

Co-authored-by: Georgiy Komarov <georgiy@parity.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jubnzv jubnzv

Labels
A-linter Issue regarding the ink! linter. B-feature-request A request for a new feature.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Linter: Warn when number primitive is annotated with #[ink(topic)] jubnzv/ink
2 participants
@cmichi @jubnzv