Skip to content

fix(forge-lint): [unused imports] check doc cmnts (inheritdoc) #11003

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jul 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions crates/lint/src/linter.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use foundry_compilers::Language;
use foundry_config::lint::Severity;
use solar_ast::{
Expr, ImportDirective, ItemContract, ItemFunction, ItemStruct, SourceUnit, UsingDirective,
VariableDefinition, visit::Visit,
self as ast, Expr, ImportDirective, ItemContract, ItemFunction, ItemStruct, SourceUnit,
UsingDirective, VariableDefinition, visit::Visit,
};
use solar_interface::{
Session, Span,
Expand Down Expand Up @@ -50,6 +50,10 @@ impl<'s> LintContext<'s> {
Self { sess, with_description, inline_config: config }
}

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

/// Helper method to emit diagnostics easily from passes
pub fn emit<L: Lint>(&self, lint: &'static L, span: Span) {
if self.inline_config.is_disabled(span, lint.id()) {
Expand All @@ -70,7 +74,7 @@ impl<'s> LintContext<'s> {
}

/// Trait for lints that operate directly on the AST.
/// Its methods mirror `solar_ast::visit::Visit`, with the addition of `LintCotext`.
/// Its methods mirror `ast::visit::Visit`, with the addition of `LintCotext`.
pub trait EarlyLintPass<'ast>: Send + Sync {
fn check_expr(&mut self, _ctx: &LintContext<'_>, _expr: &'ast Expr<'ast>) {}
fn check_item_struct(&mut self, _ctx: &LintContext<'_>, _struct: &'ast ItemStruct<'ast>) {}
Expand All @@ -95,11 +99,12 @@ pub trait EarlyLintPass<'ast>: Send + Sync {
}
fn check_item_contract(&mut self, _ctx: &LintContext<'_>, _contract: &'ast ItemContract<'ast>) {
}
fn check_doc_comment(&mut self, _ctx: &LintContext<'_>, _cmnt: &'ast ast::DocComment) {}
// 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>) {}
fn check_full_source_unit(&mut self, _ctx: &LintContext<'ast>, _ast: &'ast SourceUnit<'ast>) {}
}

/// Visitor struct for `EarlyLintPass`es
Expand All @@ -126,6 +131,13 @@ where
{
type BreakValue = Never;

fn visit_doc_comment(&mut self, cmnt: &'ast ast::DocComment) -> ControlFlow<Self::BreakValue> {
for pass in self.passes.iter_mut() {
pass.check_doc_comment(self.ctx, cmnt)
}
self.walk_doc_comment(cmnt)
}

fn visit_expr(&mut self, expr: &'ast Expr<'ast>) -> ControlFlow<Self::BreakValue> {
for pass in self.passes.iter_mut() {
pass.check_expr(self.ctx, expr)
Expand Down
30 changes: 23 additions & 7 deletions crates/lint/src/sol/info/imports.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use solar_ast::{self as ast, SourceUnit, Span, Symbol, visit::Visit};
use solar_data_structures::map::FxIndexSet;
use solar_interface::SourceMap;
use std::ops::ControlFlow;

use super::Imports;
Expand Down Expand Up @@ -36,22 +37,23 @@ impl<'ast> EarlyLintPass<'ast> for Imports {
}
}

fn check_full_source_unit(&mut self, ctx: &LintContext<'_>, ast: &'ast SourceUnit<'ast>) {
let mut checker = UnusedChecker::new();
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();
}
}

/// Visitor that collects all used symbols in a source unit.
struct UnusedChecker {
struct UnusedChecker<'ast> {
used_symbols: FxIndexSet<Symbol>,
source_map: &'ast SourceMap,
}

impl UnusedChecker {
fn new() -> Self {
Self { used_symbols: Default::default() }
impl<'ast> UnusedChecker<'ast> {
fn new(source_map: &'ast SourceMap) -> Self {
Self { source_map, used_symbols: Default::default() }
}

fn clear(&mut self) {
Expand Down Expand Up @@ -93,7 +95,7 @@ impl UnusedChecker {
}
}

impl<'ast> Visit<'ast> for UnusedChecker {
impl<'ast> Visit<'ast> for UnusedChecker<'ast> {
type BreakValue = solar_data_structures::Never;

fn visit_item(&mut self, item: &'ast ast::Item<'ast>) -> ControlFlow<Self::BreakValue> {
Expand Down Expand Up @@ -154,4 +156,18 @@ impl<'ast> Visit<'ast> for UnusedChecker {

self.walk_ty(ty)
}

fn visit_doc_comment(
&mut self,
cmnt: &'ast solar_ast::DocComment,
) -> ControlFlow<Self::BreakValue> {
if let Ok(snip) = self.source_map.span_to_snippet(cmnt.span) {
for line in snip.lines() {
if let Some((_, relevant)) = line.split_once("@inheritdoc") {
self.mark_symbol_used(Symbol::intern(relevant.trim()));
}
}
}
ControlFlow::Continue(())
}
}
8 changes: 8 additions & 0 deletions crates/lint/testdata/Imports.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import {
symbol3,
symbol4,
symbol5,
docSymbol,
docSymbol2,
docSymbolWrongTag, //~NOTE: unused imports should be removed
eventSymbol,
symbolNotUsed, //~NOTE: unused imports should be removed
IContract,
Expand Down Expand Up @@ -35,13 +38,18 @@ import * as OtherUtils from "utils2.sol"; //~NOTE: unused imports should be remo
contract UnusedImport is IContract {
using mySymbol for address;

/// @inheritdoc docSymbol
uint256 constant MY_CONSTANT = CONSTANT_0;

/**
* @inheritdoc docSymbol2
*/
struct FooBar {
symbol3 foo;
myOtherSymbol bar;
}

/// @wrong docSymbolWrongTag
SomeFile.Baz public myStruct;
SomeFile2.Baz public myStruct2;
symbol4 public myVar;
Expand Down
34 changes: 21 additions & 13 deletions crates/lint/testdata/Imports.stderr
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
note[unaliased-plain-import]: use named imports '{A, B}' or alias 'import ".." as X'
--> ROOT/testdata/Imports.sol:LL:CC
|
25 | import "SomeFile.sol";
28 | 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
|
26 | import "AnotherFile.sol";
29 | import "AnotherFile.sol";
| -----------------
|
= help: https://book.getfoundry.sh/reference/forge/forge-lint#unaliased-plain-import
Expand All @@ -23,49 +23,57 @@ note[unused-import]: unused imports should be removed
= 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
|
9 | symbolNotUsed,
| -------------
|
= help: https://book.getfoundry.sh/reference/forge/forge-lint#unused-import
--> ROOT/testdata/Imports.sol:LL:CC
|
10 | docSymbolWrongTag,
| -----------------
|
= 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
|
12 | 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
|
11 | IContractNotUsed
14 | 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
|
16 | CONSTANT_1
19 | 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
|
22 | YetAnotherType
25 | 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
|
29 | import "another_file_2.sol" as AnotherFile2;
32 | 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
|
32 | import * as OtherUtils from "utils2.sol";
35 | import * as OtherUtils from "utils2.sol";
| -----------------------------------------
|
= help: https://book.getfoundry.sh/reference/forge/forge-lint#unused-import
Expand Down