From 88db961ee2d51968608fd30517017053e7253d64 Mon Sep 17 00:00:00 2001 From: 0xrusowsky <0xrusowsky@proton.me> Date: Mon, 28 Jul 2025 09:01:01 +0200 Subject: [PATCH 1/3] fix(lint): exclude disabled ids in multi-lint passes --- crates/forge/tests/cli/lint.rs | 31 +++++++++++++++++++++++++++++ crates/lint/src/linter/early.rs | 17 ++++++++++++++++ crates/lint/src/linter/mod.rs | 24 +++++++++++++++++----- crates/lint/src/sol/info/imports.rs | 12 +++++++---- crates/lint/src/sol/mod.rs | 21 +++++++++++-------- 5 files changed, 88 insertions(+), 17 deletions(-) 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 dcd86d16e2c6a..11f7d5177ce75 100644 --- a/crates/lint/src/linter/mod.rs +++ b/crates/lint/src/linter/mod.rs @@ -11,7 +11,7 @@ use solar_interface::{ diagnostics::{DiagBuilder, DiagId, DiagMsg, MultiSpan, Style}, }; use solar_sema::ParsingContext; -use std::path::PathBuf; +use std::{collections::HashSet, path::PathBuf}; use crate::inline_config::InlineConfig; @@ -49,20 +49,34 @@ pub struct LintContext<'s> { sess: &'s Session, with_description: bool, pub inline_config: InlineConfig, + active_lints: HashSet<&'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: HashSet<&'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; } @@ -83,7 +97,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/mod.rs b/crates/lint/src/sol/mod.rs index 0d357a3e9404b..13edf86b76bde 100644 --- a/crates/lint/src/sol/mod.rs +++ b/crates/lint/src/sol/mod.rs @@ -19,6 +19,7 @@ use solar_sema::{ hir::{self, Visit as VisitHIR}, }; use std::{ + collections::HashSet, path::{Path, PathBuf}, sync::{Arc, LazyLock}, }; @@ -115,17 +116,19 @@ impl SolidityLinter { } // Filter passes based on linter config - let mut passes: Vec>> = passes_and_lints + let (mut passes, lints): (Vec>>, HashSet<_>) = passes_and_lints .into_iter() - .filter_map(|(pass, lint)| if self.include_lint(lint) { Some(pass) } else { None }) - .collect(); + .filter_map( + |(pass, lint)| if self.include_lint(lint) { Some((pass, lint.id)) } else { None }, + ) + .unzip(); // 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); @@ -154,10 +157,12 @@ impl SolidityLinter { } // Filter passes based on config - let mut passes: Vec>> = passes_and_lints + let (mut passes, lints): (Vec>>, HashSet<_>) = passes_and_lints .into_iter() - .filter_map(|(pass, lint)| if self.include_lint(lint) { Some(pass) } else { None }) - .collect(); + .filter_map( + |(pass, lint)| if self.include_lint(lint) { Some((pass, lint.id)) } else { None }, + ) + .unzip(); // Process the inline-config let comments = Comments::new(file); @@ -165,7 +170,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 From be5b01bd30b85f8aa5789f9302a569498a4b4b5b Mon Sep 17 00:00:00 2001 From: 0xrusowsky <0xrusowsky@proton.me> Date: Mon, 28 Jul 2025 18:53:53 +0200 Subject: [PATCH 2/3] fix: duplicate pass creation for multi-lint passes --- crates/lint/src/linter/mod.rs | 8 +++---- crates/lint/src/sol/macros.rs | 33 +++++++++++++++-------------- crates/lint/src/sol/mod.rs | 39 +++++++++++++++++++++++++---------- 3 files changed, 50 insertions(+), 30 deletions(-) diff --git a/crates/lint/src/linter/mod.rs b/crates/lint/src/linter/mod.rs index 11f7d5177ce75..46132539c9fb7 100644 --- a/crates/lint/src/linter/mod.rs +++ b/crates/lint/src/linter/mod.rs @@ -11,7 +11,7 @@ use solar_interface::{ diagnostics::{DiagBuilder, DiagId, DiagMsg, MultiSpan, Style}, }; use solar_sema::ParsingContext; -use std::{collections::HashSet, path::PathBuf}; +use std::path::PathBuf; use crate::inline_config::InlineConfig; @@ -49,7 +49,7 @@ pub struct LintContext<'s> { sess: &'s Session, with_description: bool, pub inline_config: InlineConfig, - active_lints: HashSet<&'static str>, + active_lints: Vec<&'static str>, } impl<'s> LintContext<'s> { @@ -57,7 +57,7 @@ impl<'s> LintContext<'s> { sess: &'s Session, with_description: bool, config: InlineConfig, - active_lints: HashSet<&'static str>, + active_lints: Vec<&'static str>, ) -> Self { Self { sess, with_description, inline_config: config, active_lints } } @@ -71,7 +71,7 @@ impl<'s> LintContext<'s> { // 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) + self.active_lints.contains(&id) } /// Helper method to emit diagnostics easily from passes 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 13edf86b76bde..05ba7ffad02bf 100644 --- a/crates/lint/src/sol/mod.rs +++ b/crates/lint/src/sol/mod.rs @@ -19,7 +19,6 @@ use solar_sema::{ hir::{self, Visit as VisitHIR}, }; use std::{ - collections::HashSet, path::{Path, PathBuf}, sync::{Arc, LazyLock}, }; @@ -116,12 +115,21 @@ impl SolidityLinter { } // Filter passes based on linter config - let (mut passes, lints): (Vec>>, HashSet<_>) = 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, lint.id)) } else { None }, - ) - .unzip(); + .fold((Vec::new(), Vec::new()), |(mut passes, mut ids), (pass, lints)| { + let included_ids: Vec<_> = lints + .into_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); @@ -157,12 +165,21 @@ impl SolidityLinter { } // Filter passes based on config - let (mut passes, lints): (Vec>>, HashSet<_>) = 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, lint.id)) } else { None }, - ) - .unzip(); + .fold((Vec::new(), Vec::new()), |(mut passes, mut ids), (pass, lints)| { + let included_ids: Vec<_> = lints + .into_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); From c0b425f768af18add5e2260b874f67402d8d005e Mon Sep 17 00:00:00 2001 From: 0xrusowsky <0xrusowsky@proton.me> Date: Mon, 28 Jul 2025 23:38:24 +0200 Subject: [PATCH 3/3] style: clippy --- crates/lint/src/sol/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/lint/src/sol/mod.rs b/crates/lint/src/sol/mod.rs index 79efe03980d18..f3fbcf5eecc6c 100644 --- a/crates/lint/src/sol/mod.rs +++ b/crates/lint/src/sol/mod.rs @@ -122,7 +122,7 @@ impl SolidityLinter { .into_iter() .fold((Vec::new(), Vec::new()), |(mut passes, mut ids), (pass, lints)| { let included_ids: Vec<_> = lints - .into_iter() + .iter() .filter_map(|lint| if self.include_lint(*lint) { Some(lint.id) } else { None }) .collect(); @@ -173,7 +173,7 @@ impl SolidityLinter { .into_iter() .fold((Vec::new(), Vec::new()), |(mut passes, mut ids), (pass, lints)| { let included_ids: Vec<_> = lints - .into_iter() + .iter() .filter_map(|lint| if self.include_lint(*lint) { Some(lint.id) } else { None }) .collect();