chore(forge lint): unused imports

crates/lint/src/linter.rs

@@ -1,12 +1,15 @@
 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, Symbol,
+    UsingDirective, VariableDefinition,
+};
 use solar_interface::{
     data_structures::Never,
     diagnostics::{DiagBuilder, DiagId, MultiSpan},
     Session, Span,
 };
-use std::{ops::ControlFlow, path::PathBuf};
+use std::{collections::HashMap, ops::ControlFlow, path::PathBuf};
 
 /// Trait representing a generic linter for analyzing and reporting issues in smart contract source
 /// code files. A linter can be implemented for any smart contract language supported by Foundry.
@@ -37,14 +40,37 @@ pub trait Lint {
 pub struct LintContext<'s> {
     sess: &'s Session,
     desc: bool,
+    unused_imports: HashMap<Symbol, Span>,
 }
 
 impl<'s> LintContext<'s> {
     pub fn new(sess: &'s Session, with_description: bool) -> Self {
-        Self { sess, desc: with_description }
+        Self { sess, desc: with_description, unused_imports: HashMap::new() }
+    }
+
+    pub fn add_import(&mut self, import: (Symbol, Span)) {
+        self.unused_imports.insert(import.0, import.1);
+    }
+
+    pub fn use_import(&mut self, import: Symbol) {
+        self.unused_imports.remove(&import);
     }
 
-    // Helper method to emit diagnostics easily from passes
+    /// Helper method to easily emit diagnostics for unused imports.
+    /// Should be called after all passes have finished.
+    ///
+    /// Clears the `unused_imports` map.
+    pub fn emit_unused_imports<L: Lint>(&mut self, lint: &'static L) {
+        let unused = std::mem::take(&mut self.unused_imports);
+        let mut spans = unused.into_values().collect::<Vec<Span>>();
+        spans.sort();
+
+        for span in spans.into_iter() {
+            self.emit(lint, span);
+        }
+    }
+
+    /// Helper method to emit diagnostics easily from passes
     pub fn emit<L: Lint>(&self, lint: &'static L, span: Span) {
         let desc = if self.desc { lint.description() } else { "" };
         let diag: DiagBuilder<'_, ()> = self
@@ -67,17 +93,35 @@ pub trait EarlyLintPass<'ast>: Send + Sync {
     fn check_item_function(&mut self, _ctx: &LintContext<'_>, _func: &'ast ItemFunction<'ast>) {}
     fn check_variable_definition(
         &mut self,
-        _ctx: &LintContext<'_>,
+        _ctx: &mut LintContext<'_>,
         _var: &'ast VariableDefinition<'ast>,
     ) {
     }
+    fn check_import_directive(
+        &mut self,
+        _ctx: &mut LintContext<'_>,
+        _import: &'ast ImportDirective<'ast>,
+    ) {
+    }
+    fn check_using_directive(
+        &mut self,
+        _ctx: &mut LintContext<'_>,
+        _using: &'ast UsingDirective<'ast>,
+    ) {
+    }
+    fn check_item_contract(
+        &mut self,
+        _ctx: &mut LintContext<'_>,
+        _contract: &'ast ItemContract<'ast>,
+    ) {
+    }
 
     // TODO: Add methods for each required AST node type
 }
 
 /// Visitor struct for `EarlyLintPass`es
 pub struct EarlyLintVisitor<'a, 's, 'ast> {
-    pub ctx: &'a LintContext<'s>,
+    pub ctx: &'a mut LintContext<'s>,
     pub passes: &'a mut [Box<dyn EarlyLintPass<'ast> + 's>],
 }
 
@@ -124,6 +168,36 @@ where
         self.walk_item_function(func)
     }
 
+    fn visit_import_directive(
+        &mut self,
+        import: &'ast ImportDirective<'ast>,
+    ) -> ControlFlow<Self::BreakValue> {
+        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<Self::BreakValue> {
+        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<Self::BreakValue> {
+        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`
 }

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

@@ -33,7 +33,7 @@ declare_forge_lint!(
 impl<'ast> EarlyLintPass<'ast> for MixedCaseVariable {
     fn check_variable_definition(
         &mut self,
-        ctx: &LintContext<'_>,
+        ctx: &mut LintContext<'_>,
         var: &'ast VariableDefinition<'ast>,
     ) {
         if var.mutability.is_none() {

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

@@ -12,9 +12,13 @@ use pascal_case::PASCAL_CASE_STRUCT;
 mod screaming_snake_case;
 use screaming_snake_case::{SCREAMING_SNAKE_CASE_CONSTANT, SCREAMING_SNAKE_CASE_IMMUTABLE};
 
+mod unused_import;
+pub use unused_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)),
+    (UnusedImport, (UNUSED_IMPORT))
 );

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

@@ -23,7 +23,7 @@ declare_forge_lint!(
 impl<'ast> EarlyLintPass<'ast> for ScreamingSnakeCase {
     fn check_variable_definition(
         &mut self,
-        ctx: &LintContext<'_>,
+        ctx: &mut LintContext<'_>,
         var: &'ast VariableDefinition<'ast>,
     ) {
         if let (Some(name), Some(mutability)) = (var.name, var.mutability) {

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

@@ -0,0 +1,113 @@
+use solar_ast::{Expr, ExprKind, ImportItems, PathSlice, Symbol, TypeKind, UsingList};
+
+use super::UnusedImport;
+use crate::{
+    declare_forge_lint,
+    linter::{EarlyLintPass, LintContext},
+    sol::{Severity, SolLint},
+};
+
+declare_forge_lint!(
+    UNUSED_IMPORT,
+    Severity::Info,
+    "unused-import",
+    "unused imports should be removed"
+);
+
+impl<'ast> EarlyLintPass<'ast> for UnusedImport {
+    /// Collects all the file imports and caches them in `LintContext`.
+    fn check_import_directive(
+        &mut self,
+        ctx: &mut LintContext<'_>,
+        import: &'ast solar_ast::ImportDirective<'ast>,
+    ) {
+        match import.items {
+            ImportItems::Aliases(ref items) => {
+                for item in &**items {
+                    let (name, span) = if let Some(ref i) = &item.1 {
+                        (&i.name, &i.span)
+                    } else {
+                        (&item.0.name, &item.0.span)
+                    };
+
+                    ctx.add_import((*name, *span));
+                }
+            }
+            ImportItems::Glob(ref ident) => {
+                ctx.add_import((ident.name, ident.span));
+            }
+            ImportItems::Plain(ref maybe) => match maybe {
+                Some(ident) => ctx.add_import((ident.name, ident.span)),
+                None => {
+                    let path = import.path.value.to_string();
+                    let len = path.len() - 4;
+                    ctx.add_import((Symbol::intern(&path[..len]), import.path.span));
+                }
+            },
+        }
+    }
+
+    /// Marks contract modifiers as used, effectively removing them from the `LintContext` cache.
+    fn check_item_contract(
+        &mut self,
+        ctx: &mut LintContext<'_>,
+        contract: &'ast solar_ast::ItemContract<'ast>,
+    ) {
+        for modifier in &*contract.bases {
+            use_import_type(ctx, &modifier.name);
+        }
+    }
+
+    /// Marks variable definitions (both, variable type and initializer name) as used,
+    /// effectively removing them from the `LintContext` cache.
+    fn check_variable_definition(
+        &mut self,
+        ctx: &mut LintContext<'_>,
+        var: &'ast solar_ast::VariableDefinition<'ast>,
+    ) {
+        if let TypeKind::Custom(ty) = &var.ty.kind {
+            use_import_type(ctx, ty);
+        }
+
+        if let Some(expr) = &var.initializer {
+            use_import_expr(ctx, expr);
+        }
+    }
+
+    /// Marks the types in a using directive as used, effectively removing them from the
+    /// `LintContext` cache.
+    fn check_using_directive(
+        &mut self,
+        ctx: &mut LintContext<'_>,
+        using: &'ast solar_ast::UsingDirective<'ast>,
+    ) {
+        match &using.list {
+            UsingList::Single(ty) => use_import_type(ctx, ty),
+            UsingList::Multiple(types) => {
+                for (ty, _operator) in &**types {
+                    use_import_type(ctx, ty);
+                }
+            }
+        }
+    }
+}
+
+/// Marks the type as used.
+/// If the type has more than one segment, it marks both, the first, and the last one.
+fn use_import_type(ctx: &mut LintContext<'_>, ty: &&mut PathSlice) {
+    ctx.use_import(ty.last().name);
+    if ty.segments().len() != 1 {
+        ctx.use_import(ty.first().name);
+    }
+}
+
+/// Marks the type as used.
+/// If the type has more than one segment, it marks both, the first, and the last one.
+fn use_import_expr<'ast>(ctx: &mut LintContext<'_>, expr: &&mut Expr<'ast>) {
+    match &expr.kind {
+        ExprKind::Ident(ident) => ctx.use_import(ident.name),
+        ExprKind::Member(ref expr, _) => use_import_expr(ctx, expr),
+        ExprKind::Call(ref expr, _) => use_import_expr(ctx, expr),
+        _ => (),
+    }
+}

crates/lint/src/sol/mod.rs

@@ -1,6 +1,7 @@
 use crate::linter::{EarlyLintPass, EarlyLintVisitor, Lint, LintContext, Linter};
 use foundry_compilers::{solc::SolcLanguage, ProjectPathsConfig};
 use foundry_config::lint::Severity;
+use info::UNUSED_IMPORT;
 use rayon::iter::{IntoParallelIterator, ParallelIterator};
 use solar_ast::{visit::Visit, Arena};
 use solar_interface::{
@@ -114,9 +115,10 @@ impl SolidityLinter {
             let ast = parser.parse_file().map_err(|e| e.emit())?;
 
             // Initialize and run the visitor
-            let ctx = LintContext::new(sess, self.with_description);
-            let mut visitor = EarlyLintVisitor { ctx: &ctx, passes: &mut passes };
+            let mut ctx = LintContext::new(sess, self.with_description);
+            let mut visitor = EarlyLintVisitor { ctx: &mut ctx, passes: &mut passes };
             _ = visitor.visit_source_unit(&ast);
+            ctx.emit_unused_imports(&UNUSED_IMPORT);
 
             Ok(())
         });

crates/lint/testdata/UnusedImport.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";
+import "AnotherFile.sol"; //~NOTE: unused imports should be removed
+
+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);
+    }
+}