Skip to content

fix(lint): exclude disabled ids in multi-lint passes #11122

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

0xrusowsky
Copy link
Contributor

ref: #11080

@0xrusowsky 0xrusowsky linked an issue Jul 28, 2025 that may be closed by this pull request
2 tasks
@DaniPopes
Copy link
Member

doesn't this happen because we create 2 "Imports" passes? it looks like passes with more than 1 lint will appear multiple times in the create function

@0xrusowsky
Copy link
Contributor Author

@DaniPopes i think that the issue emerged because a pass with several lints would emit all lints, and my commit fixes it.

However, what you point out is true... i totally overlooked that when flattening the vec in the macro i'd be creating duplicated passes.

should i change the fn sigs and the macro so that they return:

pub fn create_early_lint_passes<'ast>() -> Vec<(Box<dyn EarlyLintPass<'ast>>, Vec<SolLint>)>
pub fn create_late_lint_passes<'hir>() -> Vec<(Box<dyn LateLintPass<'hir>>, Vec<SolLint>)>

alternatively, i could force each pass to be limited to one lint.

imo, despite creating an unnecessary heap allocation for each 1pass:1lint, the performance benefits of running multiple lints in one pass could still be worth it. what do u recommend doing though?

@DaniPopes
Copy link
Member

Passes should have multiple lints, if their logic is very similar just slightly different conditions and messages then you can batch most of the work

You can return (_, &'static [SolLint]) I believe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready For Review
Development

Successfully merging this pull request may close these issues.

bug(forge-lint): improper --only-lint filtering
2 participants