diff --git a/crates/forge/tests/cli/lint.rs b/crates/forge/tests/cli/lint.rs index f11450bcd63f1..112a800cbbf85 100644 --- a/crates/forge/tests/cli/lint.rs +++ b/crates/forge/tests/cli/lint.rs @@ -37,6 +37,17 @@ const OTHER_CONTRACT: &str = r#" } "#; +const ONLY_IMPORTS: &str = r#" + // SPDX-License-Identifier: MIT + pragma solidity ^0.8.0; + + // forge-lint: disable-next-line + import { ContractWithLints } from "./ContractWithLints.sol"; + + import { _PascalCaseInfo } from "./ContractWithLints.sol"; + import "./ContractWithLints.sol"; + "#; + forgetest!(can_use_config, |prj, cmd| { prj.wipe_contracts(); prj.add_source("ContractWithLints", CONTRACT).unwrap(); @@ -359,6 +370,26 @@ forgetest!(can_process_inline_config_regardless_of_input_order, |prj, cmd| { cmd.arg("lint").assert_success(); }); +// +forgetest!(can_use_only_lint_with_multilint_passes, |prj, cmd| { + prj.wipe_contracts(); + prj.add_source("ContractWithLints", CONTRACT).unwrap(); + prj.add_source("OnlyImports", ONLY_IMPORTS).unwrap(); + cmd.arg("lint").args(["--only-lint", "unused-import"]).assert_success().stderr_eq(str![[r#" +note[unused-import]: unused imports should be removed + [FILE]:8:14 + | +8 | import { _PascalCaseInfo } from "./ContractWithLints.sol"; + | --------------- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unused-import + + +"#]]); +}); + +// ------------------------------------------------------------------------------------------------ + #[tokio::test] async fn ensure_lint_rule_docs() { const FOUNDRY_BOOK_LINT_PAGE_URL: &str = diff --git a/crates/lint/src/linter/early.rs b/crates/lint/src/linter/early.rs index ada64ee853eb3..99a4812638f9f 100644 --- a/crates/lint/src/linter/early.rs +++ b/crates/lint/src/linter/early.rs @@ -44,6 +44,23 @@ pub trait EarlyLintPass<'ast>: Send + Sync { /// Should be called after the source unit has been visited. Enables lints that require /// knowledge of the entire AST to perform their analysis. + /// + /// # Performance + /// + /// Since a full-AST analysis can be computationally expensive, implementations + /// should guard their logic by first checking if the relevant lint is enabled + /// using [`LintContext::is_lint_enabled`]. This avoids performing costly work + /// if the user has disabled the lint. + /// + /// ### Example + /// ```rust,ignore + /// fn check_full_source_unit(&mut self, ctx: &LintContext<'ast>, ast: &'ast ast::SourceUnit<'ast>) { + /// // Check if the lint is enabled before performing expensive work. + /// if ctx.is_lint_enabled(MY_EXPENSIVE_LINT.id) { + /// // ... perform computation and emit diagnostics ... + /// } + /// } + /// ``` fn check_full_source_unit( &mut self, _ctx: &LintContext<'ast>, diff --git a/crates/lint/src/linter/mod.rs b/crates/lint/src/linter/mod.rs index e1b66e2bcce4d..d2622d50b640a 100644 --- a/crates/lint/src/linter/mod.rs +++ b/crates/lint/src/linter/mod.rs @@ -53,20 +53,34 @@ pub struct LintContext<'s> { sess: &'s Session, with_description: bool, pub inline_config: InlineConfig, + active_lints: Vec<&'static str>, } impl<'s> LintContext<'s> { - pub fn new(sess: &'s Session, with_description: bool, config: InlineConfig) -> Self { - Self { sess, with_description, inline_config: config } + pub fn new( + sess: &'s Session, + with_description: bool, + config: InlineConfig, + active_lints: Vec<&'static str>, + ) -> Self { + Self { sess, with_description, inline_config: config, active_lints } } pub fn session(&self) -> &'s Session { self.sess } + // Helper method to check if a lint id is enabled. + // + // For performance reasons, some passes check several lints at once. Thus, this method is + // required to avoid unintended warnings. + pub fn is_lint_enabled(&self, id: &'static str) -> bool { + self.active_lints.contains(&id) + } + /// Helper method to emit diagnostics easily from passes pub fn emit(&self, lint: &'static L, span: Span) { - if self.inline_config.is_disabled(span, lint.id()) { + if self.inline_config.is_disabled(span, lint.id()) || !self.is_lint_enabled(lint.id()) { return; } @@ -87,7 +101,7 @@ impl<'s> LintContext<'s> { /// For Diff snippets, if no span is provided, it will use the lint's span. /// If unable to get code from the span, it will fall back to a Block snippet. pub fn emit_with_fix(&self, lint: &'static L, span: Span, snippet: Snippet) { - if self.inline_config.is_disabled(span, lint.id()) { + if self.inline_config.is_disabled(span, lint.id()) || !self.is_lint_enabled(lint.id()) { return; } diff --git a/crates/lint/src/sol/info/imports.rs b/crates/lint/src/sol/info/imports.rs index bee8fdd123f5e..7736b4e8f0f21 100644 --- a/crates/lint/src/sol/info/imports.rs +++ b/crates/lint/src/sol/info/imports.rs @@ -38,10 +38,14 @@ impl<'ast> EarlyLintPass<'ast> for Imports { } fn check_full_source_unit(&mut self, ctx: &LintContext<'ast>, ast: &'ast SourceUnit<'ast>) { - let mut checker = UnusedChecker::new(ctx.session().source_map()); - let _ = checker.visit_source_unit(ast); - checker.check_unused_imports(ast, ctx); - checker.clear(); + // Despite disabled lints are filtered inside `ctx.emit()`, we explicitly check + // upfront to avoid the expensive full source unit traversal when unnecessary. + if ctx.is_lint_enabled(UNUSED_IMPORT.id) { + let mut checker = UnusedChecker::new(ctx.session().source_map()); + let _ = checker.visit_source_unit(ast); + checker.check_unused_imports(ast, ctx); + checker.clear(); + } } } diff --git a/crates/lint/src/sol/macros.rs b/crates/lint/src/sol/macros.rs index d1e393e4c4017..3c83f6760f747 100644 --- a/crates/lint/src/sol/macros.rs +++ b/crates/lint/src/sol/macros.rs @@ -52,20 +52,23 @@ macro_rules! declare_forge_lint { /// - `const REGISTERED_LINTS` containing all registered lint objects #[macro_export] macro_rules! register_lints { - // 1. Internal rule for declaring structs. - ( @declare_structs $( ($pass_id:ident, $pass_type:ident, $lints:tt) ),* $(,)? ) => { + // 1. Internal rule for declaring structs and their associated lints. + ( @declare_structs $( ($pass_id:ident, $pass_type:ident, ($($lint:expr),* $(,)?)) ),* $(,)? ) => { $( #[derive(Debug, Default, Clone, Copy, Eq, PartialEq)] pub struct $pass_id; impl $pass_id { + /// Static slice of lints associated with this pass. + const LINTS: &'static [SolLint] = &[$($lint),*]; + register_lints!(@early_impl $pass_id, $pass_type); register_lints!(@late_impl $pass_id, $pass_type); } )* }; - // 2. Internal rule for declaring the const array. + // 2. Internal rule for declaring the const array of ALL lints. ( @declare_consts $( ($pass_id:ident, $pass_type:ident, ($($lint:expr),* $(,)?)) ),* $(,)? ) => { pub const REGISTERED_LINTS: &[SolLint] = &[ $( @@ -76,10 +79,10 @@ macro_rules! register_lints { // 3. Internal rule for declaring the helper functions. ( @declare_funcs $( ($pass_id:ident, $pass_type:ident, $lints:tt) ),* $(,)? ) => { - pub fn create_early_lint_passes<'a>() -> Vec<(Box>, SolLint)> { - vec![ + pub fn create_early_lint_passes<'ast>() -> Vec<(Box>, &'static [SolLint])> { + [ $( - register_lints!(@early_create $pass_id, $pass_type, $lints), + register_lints!(@early_create $pass_id, $pass_type), )* ] .into_iter() @@ -87,10 +90,10 @@ macro_rules! register_lints { .collect() } - pub fn create_late_lint_passes<'hir>() -> Vec<(Box>, SolLint)> { - vec![ + pub fn create_late_lint_passes<'hir>() -> Vec<(Box>, &'static [SolLint])> { + [ $( - register_lints!(@late_create $pass_id, $pass_type, $lints), + register_lints!(@late_create $pass_id, $pass_type), )* ] .into_iter() @@ -114,14 +117,14 @@ macro_rules! register_lints { } }; - (@early_create $_pass_id:ident, late, $_lints:tt) => { vec![] }; - (@early_create $pass_id:ident, $other:ident, ($($lint:expr),*)) => { - vec![ $(($pass_id::as_early_lint_pass(), $lint)),* ] + (@early_create $_pass_id:ident, late) => { None }; + (@early_create $pass_id:ident, $_other:ident) => { + Some(($pass_id::as_early_lint_pass(), $pass_id::LINTS)) }; - (@late_create $_pass_id:ident, early, $_lints:tt) => { vec![] }; - (@late_create $pass_id:ident, $other:ident, ($($lint:expr),*)) => { - vec![ $(($pass_id::as_late_lint_pass(), $lint)),* ] + (@late_create $_pass_id:ident, early) => { None }; + (@late_create $pass_id:ident, $_other:ident) => { + Some(($pass_id::as_late_lint_pass(), $pass_id::LINTS)) }; // --- ENTRY POINT --------------------------------------------------------- diff --git a/crates/lint/src/sol/mod.rs b/crates/lint/src/sol/mod.rs index 4654f3d0f6715..f3fbcf5eecc6c 100644 --- a/crates/lint/src/sol/mod.rs +++ b/crates/lint/src/sol/mod.rs @@ -118,17 +118,28 @@ impl SolidityLinter { } // Filter passes based on linter config - let mut passes: Vec>> = passes_and_lints + let (mut passes, lints): (Vec>>, Vec<_>) = passes_and_lints .into_iter() - .filter_map(|(pass, lint)| if self.include_lint(lint) { Some(pass) } else { None }) - .collect(); + .fold((Vec::new(), Vec::new()), |(mut passes, mut ids), (pass, lints)| { + let included_ids: Vec<_> = lints + .iter() + .filter_map(|lint| if self.include_lint(*lint) { Some(lint.id) } else { None }) + .collect(); + + if !included_ids.is_empty() { + passes.push(pass); + ids.extend(included_ids); + } + + (passes, ids) + }); // Process the inline-config let comments = Comments::new(file); let inline_config = parse_inline_config(sess, &comments, InlineConfigSource::Ast(ast)); // Initialize and run the early lint visitor - let ctx = LintContext::new(sess, self.with_description, inline_config); + let ctx = LintContext::new(sess, self.with_description, inline_config, lints); let mut early_visitor = EarlyLintVisitor::new(&ctx, &mut passes); _ = early_visitor.visit_source_unit(ast); early_visitor.post_source_unit(ast); @@ -158,10 +169,21 @@ impl SolidityLinter { } // Filter passes based on config - let mut passes: Vec>> = passes_and_lints + let (mut passes, lints): (Vec>>, Vec<_>) = passes_and_lints .into_iter() - .filter_map(|(pass, lint)| if self.include_lint(lint) { Some(pass) } else { None }) - .collect(); + .fold((Vec::new(), Vec::new()), |(mut passes, mut ids), (pass, lints)| { + let included_ids: Vec<_> = lints + .iter() + .filter_map(|lint| if self.include_lint(*lint) { Some(lint.id) } else { None }) + .collect(); + + if !included_ids.is_empty() { + passes.push(pass); + ids.extend(included_ids); + } + + (passes, ids) + }); // Process the inline-config let comments = Comments::new(file); @@ -169,7 +191,7 @@ impl SolidityLinter { parse_inline_config(sess, &comments, InlineConfigSource::Hir((&gcx.hir, source_id))); // Run late lint visitor - let ctx = LintContext::new(sess, self.with_description, inline_config); + let ctx = LintContext::new(sess, self.with_description, inline_config, lints); let mut late_visitor = LateLintVisitor::new(&ctx, &mut passes, &gcx.hir); // Visit this specific source