Cargo Clippy Triggers Warning from #[pallet::call]

[shawntabrizi]

Ideally such warnings would not be produced by macro code.

➜  substrate-collectables-workshop git:(starting-template) ✗ cargo +nightly clippy
    Checking pallet-kitties v0.1.0 (/Users/shawntabrizi/Desktop/substrate-collectables-workshop)
warning: unreachable pattern
  --> src/lib.rs:29:12
   |
29 |     #[pallet::call]
   |               ^^^^ matches no values because `polkadot_sdk_frame::deps::frame_support::Never` is uninhabited
   |
   = note: to learn more about uninhabited types, see https://doc.rust-lang.org/nomicon/exotic-sizes.html#empty-types
   = note: `#[warn(unreachable_patterns)]` on by default

warning: `pallet-kitties` (lib) generated 1 warning
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.27s

[bkchr]

Should be fixed by: #5676

[ntn-x2]

I opened an issue a year ago about the same problem: #163, which is more about allowing parachain teams to define their own set of linting rules they want to meet, and the generated code from the SDK should not get in the way. I was trying to work on a PR to address this. @bkchr would this require us to use a specific Rust version? Or could it work for older versions, e.g., 1.74? Regardless of the polkadot-sdk version we would be based on, if simply cherry-picking the fix to an older branch could work, that would be great.

[bkchr]

I opened an issue a year ago about the same problem: #163, which is more about allowing parachain teams to define their own set of linting rules they want to meet, and the generated code from the SDK should not get in the way. I was trying to work on a PR to address this

Sounds reasonable to me. I mean in the best case these macros should not generate any warning at all in no clippy configuration.

[ntn-x2]

One example for all: https://github.com/KILTprotocol/polkadot-sdk/blob/4aa29a41cf731b8181f03168240e8dedb2adfa7a/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs#L237.

If we want to forbid any use of any constructs that can panic, including expect, the macro expansion will always get in the way. So I think the only way to actually make it transparent is to add a [allow(clippy::all)] wherever is needed. @bkchr I did take a look at the codebase, but do you think there is a way to add it once somewhere and forget it, without forcing all parachains to also accept all lints? Maybe somewhere around here? Putting it of corse in the top-level mod pallet is not ideal for the reason above.

[bkchr]

There is probably no way around putting the "allow(all)" everywhere it is needed. IMO the best would also be to fix the issues clippy reports as best as we can.

[shawntabrizi]

Going to close this. Errors went away when i bumped to stable2409:

shawntabrizi/substrate-collectables-workshop@699a23d