Skip to content

Commit 79d4ab2

Browse files
authored
fix(lint): exclude disabled ids in multi-lint passes (#11122)
1 parent a2d5967 commit 79d4ab2

File tree

6 files changed

+122
-31
lines changed

6 files changed

+122
-31
lines changed

crates/forge/tests/cli/lint.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,17 @@ const OTHER_CONTRACT: &str = r#"
3737
}
3838
"#;
3939

40+
const ONLY_IMPORTS: &str = r#"
41+
// SPDX-License-Identifier: MIT
42+
pragma solidity ^0.8.0;
43+
44+
// forge-lint: disable-next-line
45+
import { ContractWithLints } from "./ContractWithLints.sol";
46+
47+
import { _PascalCaseInfo } from "./ContractWithLints.sol";
48+
import "./ContractWithLints.sol";
49+
"#;
50+
4051
forgetest!(can_use_config, |prj, cmd| {
4152
prj.wipe_contracts();
4253
prj.add_source("ContractWithLints", CONTRACT).unwrap();
@@ -359,6 +370,26 @@ forgetest!(can_process_inline_config_regardless_of_input_order, |prj, cmd| {
359370
cmd.arg("lint").assert_success();
360371
});
361372

373+
// <https://github.com/foundry-rs/foundry/issues/11080>
374+
forgetest!(can_use_only_lint_with_multilint_passes, |prj, cmd| {
375+
prj.wipe_contracts();
376+
prj.add_source("ContractWithLints", CONTRACT).unwrap();
377+
prj.add_source("OnlyImports", ONLY_IMPORTS).unwrap();
378+
cmd.arg("lint").args(["--only-lint", "unused-import"]).assert_success().stderr_eq(str![[r#"
379+
note[unused-import]: unused imports should be removed
380+
[FILE]:8:14
381+
|
382+
8 | import { _PascalCaseInfo } from "./ContractWithLints.sol";
383+
| ---------------
384+
|
385+
= help: https://book.getfoundry.sh/reference/forge/forge-lint#unused-import
386+
387+
388+
"#]]);
389+
});
390+
391+
// ------------------------------------------------------------------------------------------------
392+
362393
#[tokio::test]
363394
async fn ensure_lint_rule_docs() {
364395
const FOUNDRY_BOOK_LINT_PAGE_URL: &str =

crates/lint/src/linter/early.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,23 @@ pub trait EarlyLintPass<'ast>: Send + Sync {
4444

4545
/// Should be called after the source unit has been visited. Enables lints that require
4646
/// knowledge of the entire AST to perform their analysis.
47+
///
48+
/// # Performance
49+
///
50+
/// Since a full-AST analysis can be computationally expensive, implementations
51+
/// should guard their logic by first checking if the relevant lint is enabled
52+
/// using [`LintContext::is_lint_enabled`]. This avoids performing costly work
53+
/// if the user has disabled the lint.
54+
///
55+
/// ### Example
56+
/// ```rust,ignore
57+
/// fn check_full_source_unit(&mut self, ctx: &LintContext<'ast>, ast: &'ast ast::SourceUnit<'ast>) {
58+
/// // Check if the lint is enabled before performing expensive work.
59+
/// if ctx.is_lint_enabled(MY_EXPENSIVE_LINT.id) {
60+
/// // ... perform computation and emit diagnostics ...
61+
/// }
62+
/// }
63+
/// ```
4764
fn check_full_source_unit(
4865
&mut self,
4966
_ctx: &LintContext<'ast>,

crates/lint/src/linter/mod.rs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,20 +53,34 @@ pub struct LintContext<'s> {
5353
sess: &'s Session,
5454
with_description: bool,
5555
pub inline_config: InlineConfig,
56+
active_lints: Vec<&'static str>,
5657
}
5758

5859
impl<'s> LintContext<'s> {
59-
pub fn new(sess: &'s Session, with_description: bool, config: InlineConfig) -> Self {
60-
Self { sess, with_description, inline_config: config }
60+
pub fn new(
61+
sess: &'s Session,
62+
with_description: bool,
63+
config: InlineConfig,
64+
active_lints: Vec<&'static str>,
65+
) -> Self {
66+
Self { sess, with_description, inline_config: config, active_lints }
6167
}
6268

6369
pub fn session(&self) -> &'s Session {
6470
self.sess
6571
}
6672

73+
// Helper method to check if a lint id is enabled.
74+
//
75+
// For performance reasons, some passes check several lints at once. Thus, this method is
76+
// required to avoid unintended warnings.
77+
pub fn is_lint_enabled(&self, id: &'static str) -> bool {
78+
self.active_lints.contains(&id)
79+
}
80+
6781
/// Helper method to emit diagnostics easily from passes
6882
pub fn emit<L: Lint>(&self, lint: &'static L, span: Span) {
69-
if self.inline_config.is_disabled(span, lint.id()) {
83+
if self.inline_config.is_disabled(span, lint.id()) || !self.is_lint_enabled(lint.id()) {
7084
return;
7185
}
7286

@@ -87,7 +101,7 @@ impl<'s> LintContext<'s> {
87101
/// For Diff snippets, if no span is provided, it will use the lint's span.
88102
/// If unable to get code from the span, it will fall back to a Block snippet.
89103
pub fn emit_with_fix<L: Lint>(&self, lint: &'static L, span: Span, snippet: Snippet) {
90-
if self.inline_config.is_disabled(span, lint.id()) {
104+
if self.inline_config.is_disabled(span, lint.id()) || !self.is_lint_enabled(lint.id()) {
91105
return;
92106
}
93107

crates/lint/src/sol/info/imports.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,14 @@ impl<'ast> EarlyLintPass<'ast> for Imports {
3838
}
3939

4040
fn check_full_source_unit(&mut self, ctx: &LintContext<'ast>, ast: &'ast SourceUnit<'ast>) {
41-
let mut checker = UnusedChecker::new(ctx.session().source_map());
42-
let _ = checker.visit_source_unit(ast);
43-
checker.check_unused_imports(ast, ctx);
44-
checker.clear();
41+
// Despite disabled lints are filtered inside `ctx.emit()`, we explicitly check
42+
// upfront to avoid the expensive full source unit traversal when unnecessary.
43+
if ctx.is_lint_enabled(UNUSED_IMPORT.id) {
44+
let mut checker = UnusedChecker::new(ctx.session().source_map());
45+
let _ = checker.visit_source_unit(ast);
46+
checker.check_unused_imports(ast, ctx);
47+
checker.clear();
48+
}
4549
}
4650
}
4751

crates/lint/src/sol/macros.rs

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -52,20 +52,23 @@ macro_rules! declare_forge_lint {
5252
/// - `const REGISTERED_LINTS` containing all registered lint objects
5353
#[macro_export]
5454
macro_rules! register_lints {
55-
// 1. Internal rule for declaring structs.
56-
( @declare_structs $( ($pass_id:ident, $pass_type:ident, $lints:tt) ),* $(,)? ) => {
55+
// 1. Internal rule for declaring structs and their associated lints.
56+
( @declare_structs $( ($pass_id:ident, $pass_type:ident, ($($lint:expr),* $(,)?)) ),* $(,)? ) => {
5757
$(
5858
#[derive(Debug, Default, Clone, Copy, Eq, PartialEq)]
5959
pub struct $pass_id;
6060

6161
impl $pass_id {
62+
/// Static slice of lints associated with this pass.
63+
const LINTS: &'static [SolLint] = &[$($lint),*];
64+
6265
register_lints!(@early_impl $pass_id, $pass_type);
6366
register_lints!(@late_impl $pass_id, $pass_type);
6467
}
6568
)*
6669
};
6770

68-
// 2. Internal rule for declaring the const array.
71+
// 2. Internal rule for declaring the const array of ALL lints.
6972
( @declare_consts $( ($pass_id:ident, $pass_type:ident, ($($lint:expr),* $(,)?)) ),* $(,)? ) => {
7073
pub const REGISTERED_LINTS: &[SolLint] = &[
7174
$(
@@ -76,21 +79,21 @@ macro_rules! register_lints {
7679

7780
// 3. Internal rule for declaring the helper functions.
7881
( @declare_funcs $( ($pass_id:ident, $pass_type:ident, $lints:tt) ),* $(,)? ) => {
79-
pub fn create_early_lint_passes<'a>() -> Vec<(Box<dyn EarlyLintPass<'a>>, SolLint)> {
80-
vec![
82+
pub fn create_early_lint_passes<'ast>() -> Vec<(Box<dyn EarlyLintPass<'ast>>, &'static [SolLint])> {
83+
[
8184
$(
82-
register_lints!(@early_create $pass_id, $pass_type, $lints),
85+
register_lints!(@early_create $pass_id, $pass_type),
8386
)*
8487
]
8588
.into_iter()
8689
.flatten()
8790
.collect()
8891
}
8992

90-
pub fn create_late_lint_passes<'hir>() -> Vec<(Box<dyn LateLintPass<'hir>>, SolLint)> {
91-
vec![
93+
pub fn create_late_lint_passes<'hir>() -> Vec<(Box<dyn LateLintPass<'hir>>, &'static [SolLint])> {
94+
[
9295
$(
93-
register_lints!(@late_create $pass_id, $pass_type, $lints),
96+
register_lints!(@late_create $pass_id, $pass_type),
9497
)*
9598
]
9699
.into_iter()
@@ -114,14 +117,14 @@ macro_rules! register_lints {
114117
}
115118
};
116119

117-
(@early_create $_pass_id:ident, late, $_lints:tt) => { vec![] };
118-
(@early_create $pass_id:ident, $other:ident, ($($lint:expr),*)) => {
119-
vec![ $(($pass_id::as_early_lint_pass(), $lint)),* ]
120+
(@early_create $_pass_id:ident, late) => { None };
121+
(@early_create $pass_id:ident, $_other:ident) => {
122+
Some(($pass_id::as_early_lint_pass(), $pass_id::LINTS))
120123
};
121124

122-
(@late_create $_pass_id:ident, early, $_lints:tt) => { vec![] };
123-
(@late_create $pass_id:ident, $other:ident, ($($lint:expr),*)) => {
124-
vec![ $(($pass_id::as_late_lint_pass(), $lint)),* ]
125+
(@late_create $_pass_id:ident, early) => { None };
126+
(@late_create $pass_id:ident, $_other:ident) => {
127+
Some(($pass_id::as_late_lint_pass(), $pass_id::LINTS))
125128
};
126129

127130
// --- ENTRY POINT ---------------------------------------------------------

crates/lint/src/sol/mod.rs

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -118,17 +118,28 @@ impl SolidityLinter {
118118
}
119119

120120
// Filter passes based on linter config
121-
let mut passes: Vec<Box<dyn EarlyLintPass<'_>>> = passes_and_lints
121+
let (mut passes, lints): (Vec<Box<dyn EarlyLintPass<'_>>>, Vec<_>) = passes_and_lints
122122
.into_iter()
123-
.filter_map(|(pass, lint)| if self.include_lint(lint) { Some(pass) } else { None })
124-
.collect();
123+
.fold((Vec::new(), Vec::new()), |(mut passes, mut ids), (pass, lints)| {
124+
let included_ids: Vec<_> = lints
125+
.iter()
126+
.filter_map(|lint| if self.include_lint(*lint) { Some(lint.id) } else { None })
127+
.collect();
128+
129+
if !included_ids.is_empty() {
130+
passes.push(pass);
131+
ids.extend(included_ids);
132+
}
133+
134+
(passes, ids)
135+
});
125136

126137
// Process the inline-config
127138
let comments = Comments::new(file);
128139
let inline_config = parse_inline_config(sess, &comments, InlineConfigSource::Ast(ast));
129140

130141
// Initialize and run the early lint visitor
131-
let ctx = LintContext::new(sess, self.with_description, inline_config);
142+
let ctx = LintContext::new(sess, self.with_description, inline_config, lints);
132143
let mut early_visitor = EarlyLintVisitor::new(&ctx, &mut passes);
133144
_ = early_visitor.visit_source_unit(ast);
134145
early_visitor.post_source_unit(ast);
@@ -158,18 +169,29 @@ impl SolidityLinter {
158169
}
159170

160171
// Filter passes based on config
161-
let mut passes: Vec<Box<dyn LateLintPass<'_>>> = passes_and_lints
172+
let (mut passes, lints): (Vec<Box<dyn LateLintPass<'_>>>, Vec<_>) = passes_and_lints
162173
.into_iter()
163-
.filter_map(|(pass, lint)| if self.include_lint(lint) { Some(pass) } else { None })
164-
.collect();
174+
.fold((Vec::new(), Vec::new()), |(mut passes, mut ids), (pass, lints)| {
175+
let included_ids: Vec<_> = lints
176+
.iter()
177+
.filter_map(|lint| if self.include_lint(*lint) { Some(lint.id) } else { None })
178+
.collect();
179+
180+
if !included_ids.is_empty() {
181+
passes.push(pass);
182+
ids.extend(included_ids);
183+
}
184+
185+
(passes, ids)
186+
});
165187

166188
// Process the inline-config
167189
let comments = Comments::new(file);
168190
let inline_config =
169191
parse_inline_config(sess, &comments, InlineConfigSource::Hir((&gcx.hir, source_id)));
170192

171193
// Run late lint visitor
172-
let ctx = LintContext::new(sess, self.with_description, inline_config);
194+
let ctx = LintContext::new(sess, self.with_description, inline_config, lints);
173195
let mut late_visitor = LateLintVisitor::new(&ctx, &mut passes, &gcx.hir);
174196

175197
// Visit this specific source

0 commit comments

Comments
 (0)