Skip to content

Expand used solhint rules #4825

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.

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

Closed
RenanSouza2 opened this issue Jan 11, 2024 · 4 comments · Fixed by #4836 or #5661
Closed

Expand used solhint rules #4825

RenanSouza2 opened this issue Jan 11, 2024 · 4 comments · Fixed by #4836 or #5661

Comments

@RenanSouza2
Copy link
Contributor

🧐 Motivation

Hey, I was looking into solhint rules and there are ones that may be interesting to use here but I wanted to discuss it before opening an PR

📝 Details

'custom-errors',
'explicit-types',
'no-unused-import',

contracts/interfaces/IERC1155.sol
  6:9  error  imported name IERC1155 is not used  no-unused-import

contracts/interfaces/IERC1155MetadataURI.sol
  6:9  error  imported name IERC1155MetadataURI is not used  no-unused-import

contracts/interfaces/IERC1155Receiver.sol
  6:9  error  imported name IERC1155Receiver is not used  no-unused-import

contracts/interfaces/IERC165.sol
  6:9  error  imported name IERC165 is not used  no-unused-import

contracts/interfaces/IERC20.sol
  6:9  error  imported name IERC20 is not used  no-unused-import

contracts/interfaces/IERC20Metadata.sol
  6:9  error  imported name IERC20Metadata is not used  no-unused-import

contracts/interfaces/IERC3156.sol
  6:9  error  imported name IERC3156FlashBorrower is not used  no-unused-import
  7:9  error  imported name IERC3156FlashLender is not used    no-unused-import

contracts/interfaces/IERC721.sol
  6:9  error  imported name IERC721 is not used  no-unused-import

contracts/interfaces/IERC721Enumerable.sol
  6:9  error  imported name IERC721Enumerable is not used  no-unused-import

contracts/interfaces/IERC721Metadata.sol
  6:9  error  imported name IERC721Metadata is not used  no-unused-import
@Amxx
Copy link
Collaborator

Amxx commented Jan 15, 2024

Hello @RenanSouza2

  • no-unused-imports
    The "unused" import here are false positive. "fixing" that would be breaking a lot of usages of the repo.

  • custom-errors
    That triggers a lot of errors for mocks. Some of which we cold change, but some of which be don't want to change because want to propagation of such errors.

  • explicit-types
    Not sure what that rules does.

@Amxx Amxx mentioned this issue Jan 15, 2024
3 tasks
@RenanSouza2
Copy link
Contributor Author

RenanSouza2 commented Jan 15, 2024

solhint accepts a .solhintignore that can be used to ignore all mocks

explicit types enforces things like uint256 instead of uint

@Amxx
Copy link
Collaborator

Amxx commented Jan 15, 2024

solhint accepts a .solhintignore that can be used to ignore all mocks

That is interresting, but if we do that then no rule is checked on the mocks, right? We would like the other rules to continue running.

An alternative would be to add // solhint-disable-next-line were needed, but that feels like a lot.

@RenanSouza2
Copy link
Contributor Author

Yeah it is a lot of // solhint-disable-next-line

Maybe the custom error is not that hard to verify without solhint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants