feat(lint): refactor LinterConfig and add 3 new linting rules
- Added a new linting rule that checks for multiple contract-like items (contracts, abstract contracts, interfaces and libraries) per
.solfile . - Refactored
LinterConfig - Added 2 new linting rules that checks for interface names and interface file names
Motivation
Enforce best practice.
Solution
I have some doubts regarding this, would like to hear your opinion.
- Should the rule skip interfaces and libraries and count only contracts?
- Should the rule show linting note for every contract-like item in the file, or only one time per file is enough?
My solution count interfaces and libraries too, and it shows linting note only once per file. Let me know if you want something to be changed :)
PR Checklist
- [x] Added Tests
- [ ] Added Documentation
- [ ] Breaking changes
Keep up the good work guys, cheers!
@milosdjurica first of all thanks for the PR and the initiative, we love new contributions to forge's linter!
i think the idea overall makes sense, however imo we can improve it.
- i'd like each contract to be flagged, not just the 2nd one
- imo it would make sense to add a custom config for this lint, so that users can decide if they want to allow interfaces and/or libs in the same file. To do so, you'd want to modify the LinterConfig to have a dedicated config for this lint. If we do that, i think we then want to create a new intermediate struct that group all lint configs (i.e. the pre-existing
mixed_case_exceptions+ your newly introduced config)
lmk if this seems reasonable and if u have any doubts regarding impl
@milosdjurica first of all thanks for the PR and the initiative, we love new contributions to forge's linter!
i think the idea overall makes sense, however imo we can improve it.
- i'd like each contract to be flagged, not just the 2nd one
- imo it would make sense to add a custom config for this lint, so that users can decide if they want to allow interfaces and/or libs in the same file. To do so, you'd want to modify the LinterConfig to have a dedicated config for this lint. If we do that, i think we then want to create a new intermediate struct that group all lint configs (i.e. the pre-existing
mixed_case_exceptions+ your newly introduced config)lmk if this seems reasonable and if u have any doubts regarding impl
Sounds great, I like it! Will look to implement it ASAP :) Thanks for the response!
@milosdjurica lmk when the PR is ready for re-review!
Hi @0xrusowsky , it should be good now 👍🏻
great, tysm! will review it today at some point
@milosdjurica btw, i haven't tested the my code compiles, those are just suggestions, so you may need to tweak them a bit, but u get the gist of the requested simplifications
Hi @0xrusowsky, thanks for suggestions - it's much cleaner now. I have applied them with some small changes, lmk if this is okay now
Hi guys, @0xrusowsky @grandizzy - just a quick ping in case you missed the latest changes 👀
@grandizzy good to merge?
Yep, let's send it 👍