meta (`lint`): tracking issue for linter improvements

[0xrusowsky]

Tracking all tickets related to forge lint:

• [x] https://github.com/foundry-rs/foundry/issues/1970
• [ ] --fix flag for automatic fixing of stable found issues

[sakulstra]

Would love to see "unused imports" as a lint rule. Was removed from slither at some point due to instability, but exists in solhint.

[sakulstra]

One more general thing regarding the linter:

For me it would be huge dx improvement if the lint error would link me to a webpage with:

• the error being properly explained
• steps on how to disable for one occasion
• steps on how to disable globally like it's done on eslint https://eslint.org/docs/latest/rules/no-await-in-loop.

Ran it on a bigger codebase and there were some warnings for which it was not clear to me what the warning was about.

E.g. warning[incorrect-shift]: the order of args in a shift operation is incorrect Quite sure it's not, but would like to understand why does the rule think so.

[0xrusowsky]

One more general thing regarding the linter:

For me it would be huge dx improvement if the lint error would link me to a webpage with:

• the error being properly explained
• steps on how to disable for one occasion
• steps on how to disable globally like it's done on eslint https://eslint.org/docs/latest/rules/no-await-in-loop.

thanks for the suggestions, better devex is always the goal, and all ideas are welcome

Ran it on a bigger codebase and there were some warnings for which it was not clear to me what the warning was about.

E.g. warning[incorrect-shift]: the order of args in a shift operation is incorrect Quite sure it's not, but would like to understand why does the rule think so.

regarding the "incorrect shift" lint, right now it is based on a heuristic, we should probably make the error less reassuring.

here you have some ctx:

• https://github.com/foundry-rs/foundry/blob/master/crates/lint/src/sol/high/incorrect_shift.rs#L32-L43
• https://github.com/foundry-rs/foundry/blob/master/crates/lint/testdata/IncorrectShift.sol

[zerosnacks]

For the note[asm-keccak256]: hash using inline assembly to save gas lint rule triggers quite a few false positives.

In most cases the optimizer will be able to compute the keccak256 statically and inline through constant folding if there is no dynamic component.

It is also raised for tests which does not really make sense.

I think we should put the gas optimizations in a specific category (if we don't already) and disable them for tests / scripts.

A possible improvement would be to detect the constant keyword before it and not raise the lint if there is.

To reproduce run forge lint on https://github.com/ithacaxyz/account

cc @0xrusowsky

[0xrusowsky]

awesome, thanks for raising this up!

gas optimizations are their own category and can easily be disabled with the linting config.

i'll have a look and incorporate all the suggestions

[0xClandestine]

I think it would be valuable if lint checks could be implemented as external plugins.

[zerosnacks]

I think it would be valuable if lint checks could be implemented as external plugins.

I would prefer that we first make the canonical lint suite of Foundry extensive and robust before considering a plugin system or heavy customizability beyond toggling of lint rules.

[0xClandestine]

Feel like quite a few of solhints lints are low hanging fruit if anyone wants to hop on them:

https://github.com/protofire/solhint/blob/develop/docs/rules.md

[adraffy]

Can this be disabled or selectively muted from forge build?

Having to sift through pages of note[...] spam to find a useful warning seems silly.

---

warning[incorrect-shift]: the order of args in a shift operation is incorrect seems incorrect, triggers for 1 << i Looks like this was mentioned above.

[0xrusowsky]

https://getfoundry.sh/config/reference/linter#disabling-linter-on-build

we advise to disable individual lints, but you can turn it off completely