diff --git a/Cargo.lock b/Cargo.lock index 5ce4dd7e6d149..bc53d31b8e080 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2225,6 +2225,12 @@ dependencies = [ "syn 2.0.104", ] +[[package]] +name = "boxcar" +version = "0.2.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "26c4925bc979b677330a8c7fe7a8c94af2dbb4a2d37b4a20a80d884400f46baa" + [[package]] name = "bs58" version = "0.5.1" @@ -3924,6 +3930,7 @@ dependencies = [ "heck", "rayon", "solar-ast", + "solar-data-structures", "solar-interface", "solar-parse", "thiserror 2.0.12", @@ -4989,10 +4996,6 @@ name = "hashbrown" version = "0.14.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" -dependencies = [ - "ahash", - "allocator-api2", -] [[package]] name = "hashbrown" @@ -5469,6 +5472,19 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "inturn" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "62f17d4bce58d4380de6432e6b1a0ebb561dfbbe21fc123204870b7006189677" +dependencies = [ + "boxcar", + "bumpalo", + "dashmap", + "hashbrown 0.14.5", + "thread_local", +] + [[package]] name = "ipnet" version = "2.11.0" @@ -5716,16 +5732,6 @@ dependencies = [ "rustversion", ] -[[package]] -name = "lasso" -version = "0.7.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6e14eda50a3494b3bf7b9ce51c52434a761e383d7238ce1dd5dcec2fbc13e9fb" -dependencies = [ - "dashmap", - "hashbrown 0.14.5", -] - [[package]] name = "lazy_static" version = "1.5.0" @@ -8647,8 +8653,7 @@ dependencies = [ [[package]] name = "solar-ast" version = "0.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "acb0d9abd88c1325e5c0f9bb153ebfb02ed40bc9639067d0ad120f5d29ee8777" +source = "git+https://github.com/paradigmxyz/solar.git?branch=main#06653a27f6bb75538e38af9583f6fd57e9976fcc" dependencies = [ "alloy-primitives", "bumpalo", @@ -8666,8 +8671,7 @@ dependencies = [ [[package]] name = "solar-config" version = "0.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "908d455bb7c04acd783bd157b63791bf010cf6a495a845e48f7aee334aad319f" +source = "git+https://github.com/paradigmxyz/solar.git?branch=main#06653a27f6bb75538e38af9583f6fd57e9976fcc" dependencies = [ "strum 0.27.1", ] @@ -8675,8 +8679,7 @@ dependencies = [ [[package]] name = "solar-data-structures" version = "0.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a8b5a697cab81241c4623b4546c69e57182202847a75d7c0047c0dd10b923d8c" +source = "git+https://github.com/paradigmxyz/solar.git?branch=main#06653a27f6bb75538e38af9583f6fd57e9976fcc" dependencies = [ "bumpalo", "index_vec", @@ -8690,8 +8693,7 @@ dependencies = [ [[package]] name = "solar-interface" version = "0.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "33e49e5a714ec80d7f9c8942584e5e86add1c5e824aa175d0681e9ac7a10e5cc" +source = "git+https://github.com/paradigmxyz/solar.git?branch=main#06653a27f6bb75538e38af9583f6fd57e9976fcc" dependencies = [ "annotate-snippets", "anstream", @@ -8700,9 +8702,9 @@ dependencies = [ "derive_builder", "derive_more 2.0.1", "dunce", - "itertools 0.10.5", + "inturn", + "itertools 0.14.0", "itoa", - "lasso", "match_cfg", "normalize-path", "rayon", @@ -8720,8 +8722,7 @@ dependencies = [ [[package]] name = "solar-macros" version = "0.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e17f8b99f28f358b41acb6efe4d7b8f326541b3c58a15dd1931d6fe581feefef" +source = "git+https://github.com/paradigmxyz/solar.git?branch=main#06653a27f6bb75538e38af9583f6fd57e9976fcc" dependencies = [ "proc-macro2", "quote", @@ -8731,8 +8732,7 @@ dependencies = [ [[package]] name = "solar-parse" version = "0.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c6982ef2ecfb1338c774c743ab7166ce8c3dcc92089ab77fad58eeb632b201e9" +source = "git+https://github.com/paradigmxyz/solar.git?branch=main#06653a27f6bb75538e38af9583f6fd57e9976fcc" dependencies = [ "alloy-primitives", "bitflags 2.9.1", @@ -8752,8 +8752,7 @@ dependencies = [ [[package]] name = "solar-sema" version = "0.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fe8e0a3a6dfc7f4987bfb93f9b56079150407ef55c459964a1541c669554b74d" +source = "git+https://github.com/paradigmxyz/solar.git?branch=main#06653a27f6bb75538e38af9583f6fd57e9976fcc" dependencies = [ "alloy-json-abi", "alloy-primitives", diff --git a/Cargo.toml b/Cargo.toml index e2fbc4a9f9bd4..a1bec76c0e573 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -211,6 +211,7 @@ solar-ast = { version = "=0.1.4", default-features = false } solar-parse = { version = "=0.1.4", default-features = false } solar-interface = { version = "=0.1.4", default-features = false } solar-sema = { version = "=0.1.4", default-features = false } +solar-data-structures = { version = "=0.1.4", default-features = false } ## alloy alloy-consensus = { version = "1.0.11", default-features = false } @@ -410,3 +411,10 @@ zip-extract = "=0.2.1" ## foundry # foundry-compilers = { git = "https://github.com/foundry-rs/compilers.git", rev = "e4a9b04" } # foundry-fork-db = { git = "https://github.com/foundry-rs/foundry-fork-db", rev = "811a61a" } + +# solar +solar-ast = { git = "https://github.com/paradigmxyz/solar.git", branch = "main" } +solar-parse = { git = "https://github.com/paradigmxyz/solar.git", branch = "main" } +solar-interface = { git = "https://github.com/paradigmxyz/solar.git", branch = "main" } +solar-sema = { git = "https://github.com/paradigmxyz/solar.git", branch = "main" } +solar-data-structures = { git = "https://github.com/paradigmxyz/solar.git", branch = "main" } diff --git a/crates/lint/Cargo.toml b/crates/lint/Cargo.toml index f10e774d40921..5e78a55abbc3a 100644 --- a/crates/lint/Cargo.toml +++ b/crates/lint/Cargo.toml @@ -22,6 +22,7 @@ foundry-config.workspace = true solar-parse.workspace = true solar-ast.workspace = true solar-interface = { workspace = true, features = ["json"] } +solar-data-structures.workspace = true heck.workspace = true rayon.workspace = true diff --git a/crates/lint/src/linter.rs b/crates/lint/src/linter.rs index e69d78dc8d0c9..fd40d0076449c 100644 --- a/crates/lint/src/linter.rs +++ b/crates/lint/src/linter.rs @@ -1,6 +1,9 @@ use foundry_compilers::Language; use foundry_config::lint::Severity; -use solar_ast::{visit::Visit, Expr, ItemFunction, ItemStruct, VariableDefinition}; +use solar_ast::{ + visit::Visit, Expr, ImportDirective, ItemContract, ItemFunction, ItemStruct, SourceUnit, + UsingDirective, VariableDefinition, +}; use solar_interface::{ data_structures::Never, diagnostics::{DiagBuilder, DiagId, MultiSpan}, @@ -47,7 +50,7 @@ impl<'s> LintContext<'s> { Self { sess, with_description, inline_config: config } } - // Helper method to emit diagnostics easily from passes + /// 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()) { return; @@ -78,8 +81,25 @@ pub trait EarlyLintPass<'ast>: Send + Sync { _var: &'ast VariableDefinition<'ast>, ) { } - + fn check_import_directive( + &mut self, + _ctx: &LintContext<'_>, + _import: &'ast ImportDirective<'ast>, + ) { + } + fn check_using_directive( + &mut self, + _ctx: &LintContext<'_>, + _using: &'ast UsingDirective<'ast>, + ) { + } + fn check_item_contract(&mut self, _ctx: &LintContext<'_>, _contract: &'ast ItemContract<'ast>) { + } // TODO: Add methods for each required AST node type + + /// Should be called after the source unit has been visited. Enables lints that require + /// knowledge of the entire AST to perform their analysis. + fn check_full_source_unit(&mut self, _ctx: &LintContext<'_>, _ast: &'ast SourceUnit<'ast>) {} } /// Visitor struct for `EarlyLintPass`es @@ -88,6 +108,18 @@ pub struct EarlyLintVisitor<'a, 's, 'ast> { pub passes: &'a mut [Box + 's>], } +/// Extends the [`Visit`] trait functionality with a hook that can run after the initial traversal. +impl<'s, 'ast> EarlyLintVisitor<'_, 's, 'ast> +where + 's: 'ast, +{ + pub fn post_source_unit(&mut self, ast: &'ast SourceUnit<'ast>) { + for pass in self.passes.iter_mut() { + pass.check_full_source_unit(self.ctx, ast); + } + } +} + impl<'s, 'ast> Visit<'ast> for EarlyLintVisitor<'_, 's, 'ast> where 's: 'ast, @@ -131,6 +163,36 @@ where self.walk_item_function(func) } + fn visit_import_directive( + &mut self, + import: &'ast ImportDirective<'ast>, + ) -> ControlFlow { + for pass in self.passes.iter_mut() { + pass.check_import_directive(self.ctx, import); + } + self.walk_import_directive(import) + } + + fn visit_using_directive( + &mut self, + using: &'ast UsingDirective<'ast>, + ) -> ControlFlow { + for pass in self.passes.iter_mut() { + pass.check_using_directive(self.ctx, using); + } + self.walk_using_directive(using) + } + + fn visit_item_contract( + &mut self, + contract: &'ast ItemContract<'ast>, + ) -> ControlFlow { + for pass in self.passes.iter_mut() { + pass.check_item_contract(self.ctx, contract); + } + self.walk_item_contract(contract) + } + // TODO: Add methods for each required AST node type, mirroring `solar_ast::visit::Visit` method // sigs + adding `LintContext` } diff --git a/crates/lint/src/sol/info/imports.rs b/crates/lint/src/sol/info/imports.rs new file mode 100644 index 0000000000000..5a523e2d73c2c --- /dev/null +++ b/crates/lint/src/sol/info/imports.rs @@ -0,0 +1,147 @@ +use solar_ast::{self as ast, visit::Visit, SourceUnit, Span, Symbol}; +use solar_data_structures::map::FxIndexSet; +use std::ops::ControlFlow; + +use super::Imports; +use crate::{ + linter::{EarlyLintPass, LintContext}, + sol::{Severity, SolLint}, +}; + +declare_forge_lint!( + UNUSED_IMPORT, + Severity::Info, + "unused-import", + "unused imports should be removed" +); + +declare_forge_lint!( + UNALIASED_PLAIN_IMPORT, + Severity::Info, + "unaliased-plain-import", + "use named imports '{A, B}' or alias 'import \"..\" as X'" +); + +impl<'ast> EarlyLintPass<'ast> for Imports { + fn check_import_directive( + &mut self, + ctx: &LintContext<'_>, + import: &'ast ast::ImportDirective<'ast>, + ) { + // Non-aliased plain imports like `import "File.sol";`. + if let ast::ImportItems::Plain(_) = &import.items { + if import.source_alias().is_none() { + ctx.emit(&UNALIASED_PLAIN_IMPORT, import.path.span); + } + } + } + + fn check_full_source_unit(&mut self, ctx: &LintContext<'_>, ast: &'ast SourceUnit<'ast>) { + let mut checker = UnusedChecker::new(); + let _ = checker.visit_source_unit(ast); + checker.check_unused_imports(ast, ctx); + checker.clear(); + } +} + +/// Visitor that collects all used symbols in a source unit. +struct UnusedChecker { + used_symbols: FxIndexSet, +} + +impl UnusedChecker { + fn new() -> Self { + Self { used_symbols: Default::default() } + } + + fn clear(&mut self) { + self.used_symbols.clear(); + } + + /// Mark a symbol as used in a source. + fn mark_symbol_used(&mut self, symbol: Symbol) { + self.used_symbols.insert(symbol); + } + + /// Check for unused imports and emit warnings. + fn check_unused_imports(&self, ast: &SourceUnit<'_>, ctx: &LintContext<'_>) { + for (span, import) in ast.imports() { + match &import.items { + ast::ImportItems::Plain(_) | ast::ImportItems::Glob(_) => { + if let Some(alias) = import.source_alias() { + if !self.used_symbols.contains(&alias.name) { + self.unused_import(ctx, span); + } + } + } + ast::ImportItems::Aliases(symbols) => { + for &(orig, alias) in symbols.iter() { + let name = alias.unwrap_or(orig); + if !self.used_symbols.contains(&name.name) { + self.unused_import(ctx, orig.span.to(name.span)); + } + } + } + } + } + } + + fn unused_import(&self, ctx: &LintContext<'_>, span: Span) { + ctx.emit(&UNUSED_IMPORT, span); + } +} + +impl<'ast> Visit<'ast> for UnusedChecker { + type BreakValue = solar_data_structures::Never; + + fn visit_item(&mut self, item: &'ast ast::Item<'ast>) -> ControlFlow { + if let ast::ItemKind::Import(_) = &item.kind { + return ControlFlow::Continue(()); + } + + self.walk_item(item) + } + + fn visit_using_directive( + &mut self, + using: &'ast ast::UsingDirective<'ast>, + ) -> ControlFlow { + match &using.list { + ast::UsingList::Single(path) => { + self.mark_symbol_used(path.first().name); + } + ast::UsingList::Multiple(items) => { + for (path, _) in items.iter() { + self.mark_symbol_used(path.first().name); + } + } + } + + self.walk_using_directive(using) + } + + fn visit_modifier( + &mut self, + modifier: &'ast ast::Modifier<'ast>, + ) -> ControlFlow { + self.mark_symbol_used(modifier.name.first().name); + + self.walk_modifier(modifier) + } + + fn visit_expr(&mut self, expr: &'ast ast::Expr<'ast>) -> ControlFlow { + if let ast::ExprKind::Ident(id) = expr.kind { + self.mark_symbol_used(id.name); + } + + self.walk_expr(expr) + } + + fn visit_ty(&mut self, ty: &'ast ast::Type<'ast>) -> ControlFlow { + if let ast::TypeKind::Custom(path) = &ty.kind { + self.mark_symbol_used(path.first().name); + } + + self.walk_ty(ty) + } +} diff --git a/crates/lint/src/sol/info/mod.rs b/crates/lint/src/sol/info/mod.rs index df458bacef992..bf1e18af2fa0e 100644 --- a/crates/lint/src/sol/info/mod.rs +++ b/crates/lint/src/sol/info/mod.rs @@ -9,9 +9,13 @@ use pascal_case::PASCAL_CASE_STRUCT; mod screaming_snake_case; use screaming_snake_case::{SCREAMING_SNAKE_CASE_CONSTANT, SCREAMING_SNAKE_CASE_IMMUTABLE}; +mod imports; +use imports::{UNALIASED_PLAIN_IMPORT, UNUSED_IMPORT}; + register_lints!( (PascalCaseStruct, (PASCAL_CASE_STRUCT)), (MixedCaseVariable, (MIXED_CASE_VARIABLE)), (MixedCaseFunction, (MIXED_CASE_FUNCTION)), - (ScreamingSnakeCase, (SCREAMING_SNAKE_CASE_CONSTANT, SCREAMING_SNAKE_CASE_IMMUTABLE)) + (ScreamingSnakeCase, (SCREAMING_SNAKE_CASE_CONSTANT, SCREAMING_SNAKE_CASE_IMMUTABLE)), + (Imports, (UNALIASED_PLAIN_IMPORT, UNUSED_IMPORT)) ); diff --git a/crates/lint/src/sol/mod.rs b/crates/lint/src/sol/mod.rs index dd1aac2a0c3b0..591c8c2a65044 100644 --- a/crates/lint/src/sol/mod.rs +++ b/crates/lint/src/sol/mod.rs @@ -132,6 +132,7 @@ impl SolidityLinter { let ctx = LintContext::new(sess, self.with_description, inline_config); let mut visitor = EarlyLintVisitor { ctx: &ctx, passes: &mut passes }; _ = visitor.visit_source_unit(&ast); + visitor.post_source_unit(&ast); Ok(()) }); diff --git a/crates/lint/testdata/Imports.sol b/crates/lint/testdata/Imports.sol new file mode 100644 index 0000000000000..31083206dc3b2 --- /dev/null +++ b/crates/lint/testdata/Imports.sol @@ -0,0 +1,57 @@ +import { + symbol0 as mySymbol, + symbol1 as myOtherSymbol, + symbol2 as notUsed, //~NOTE: unused imports should be removed + symbol3, + symbol4, + symbol5, + symbolNotUsed, //~NOTE: unused imports should be removed + IContract, + IContractNotUsed //~NOTE: unused imports should be removed +} from "File.sol"; + +import { + CONSTANT_0, + CONSTANT_1 //~NOTE: unused imports should be removed +} from "Constants.sol"; + +import { + MyTpe, + MyOtherType, + YetAnotherType //~NOTE: unused imports should be removed +} from "Types.sol"; + +import "SomeFile.sol"; //~NOTE: use named imports '{A, B}' or alias 'import ".." as X' +import "AnotherFile.sol"; //~NOTE: use named imports '{A, B}' or alias 'import ".." as X' + +import "some_file_2.sol" as SomeFile2; +import "another_file_2.sol" as AnotherFile2; //~NOTE: unused imports should be removed + +import * as Utils from "utils.sol"; +import * as OtherUtils from "utils2.sol"; //~NOTE: unused imports should be removed + + +contract UnusedImport is IContract { + using mySymbol for address; + + uint256 constant MY_CONSTANT = CONSTANT_0; + + struct FooBar { + symbol3 foo; + myOtherSymbol bar; + } + + SomeFile.Baz public myStruct; + SomeFile2.Baz public myStruct2; + symbol4 public myVar; + + function foo(uint256 a, symbol5 b) public view returns (uint256) { + uint256 c = Utils.calculate(a, b); + return c; + } + + function convert(address addr) public pure returns (MyOtherType) { + MyType a = MyTpe.wrap(123); + return MyOtherType.wrap(a); + } +} diff --git a/crates/lint/testdata/Imports.stderr b/crates/lint/testdata/Imports.stderr new file mode 100644 index 0000000000000..fb59ca2dc38a1 --- /dev/null +++ b/crates/lint/testdata/Imports.stderr @@ -0,0 +1,72 @@ +note[unaliased-plain-import]: use named imports '{A, B}' or alias 'import ".." as X' + --> ROOT/testdata/Imports.sol:LL:CC + | +24 | import "SomeFile.sol"; + | -------------- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unaliased-plain-import + +note[unaliased-plain-import]: use named imports '{A, B}' or alias 'import ".." as X' + --> ROOT/testdata/Imports.sol:LL:CC + | +25 | import "AnotherFile.sol"; + | ----------------- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unaliased-plain-import + +note[unused-import]: unused imports should be removed + --> ROOT/testdata/Imports.sol:LL:CC + | +4 | symbol2 as notUsed, + | ------------------ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unused-import + +note[unused-import]: unused imports should be removed + --> ROOT/testdata/Imports.sol:LL:CC + | +8 | symbolNotUsed, + | ------------- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unused-import + +note[unused-import]: unused imports should be removed + --> ROOT/testdata/Imports.sol:LL:CC + | +10 | IContractNotUsed + | ---------------- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unused-import + +note[unused-import]: unused imports should be removed + --> ROOT/testdata/Imports.sol:LL:CC + | +15 | CONSTANT_1 + | ---------- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unused-import + +note[unused-import]: unused imports should be removed + --> ROOT/testdata/Imports.sol:LL:CC + | +21 | YetAnotherType + | -------------- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unused-import + +note[unused-import]: unused imports should be removed + --> ROOT/testdata/Imports.sol:LL:CC + | +28 | import "another_file_2.sol" as AnotherFile2; + | -------------------------------------------- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unused-import + +note[unused-import]: unused imports should be removed + --> ROOT/testdata/Imports.sol:LL:CC + | +31 | import * as OtherUtils from "utils2.sol"; + | ----------------------------------------- + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#unused-import +