From 1581b779924118e727976cef3176251786453a18 Mon Sep 17 00:00:00 2001 From: Milos Djurica Date: Sun, 16 Nov 2025 15:18:33 +0100 Subject: [PATCH 1/8] feat(lint): add multi-contract-file rule --- crates/lint/src/sol/info/mod.rs | 6 ++- .../lint/src/sol/info/multi_contract_file.rs | 43 +++++++++++++++++++ crates/lint/testdata/Keccak256.stderr | 8 ++++ crates/lint/testdata/MixedCase.stderr | 8 ++++ crates/lint/testdata/MultiContractFile.sol | 12 ++++++ crates/lint/testdata/MultiContractFile.stderr | 8 ++++ .../MultiContractFile_InterfaceLibrary.sol | 12 ++++++ .../MultiContractFile_InterfaceLibrary.stderr | 8 ++++ .../testdata/UncheckedTransferERC20.stderr | 8 ++++ crates/lint/testdata/UnsafeTypecast.stderr | 8 ++++ .../testdata/UnwrappedModifierLogic.stderr | 8 ++++ 11 files changed, 128 insertions(+), 1 deletion(-) create mode 100644 crates/lint/src/sol/info/multi_contract_file.rs create mode 100644 crates/lint/testdata/MultiContractFile.sol create mode 100644 crates/lint/testdata/MultiContractFile.stderr create mode 100644 crates/lint/testdata/MultiContractFile_InterfaceLibrary.sol create mode 100644 crates/lint/testdata/MultiContractFile_InterfaceLibrary.stderr diff --git a/crates/lint/src/sol/info/mod.rs b/crates/lint/src/sol/info/mod.rs index ddb18c6e30960..b87bf900193ca 100644 --- a/crates/lint/src/sol/info/mod.rs +++ b/crates/lint/src/sol/info/mod.rs @@ -18,6 +18,9 @@ use named_struct_fields::NAMED_STRUCT_FIELDS; mod unsafe_cheatcodes; use unsafe_cheatcodes::UNSAFE_CHEATCODE_USAGE; +mod multi_contract_file; +use multi_contract_file::MULTI_CONTRACT_FILE; + register_lints!( (PascalCaseStruct, early, (PASCAL_CASE_STRUCT)), (MixedCaseVariable, early, (MIXED_CASE_VARIABLE)), @@ -25,5 +28,6 @@ register_lints!( (ScreamingSnakeCase, early, (SCREAMING_SNAKE_CASE_CONSTANT, SCREAMING_SNAKE_CASE_IMMUTABLE)), (Imports, early, (UNALIASED_PLAIN_IMPORT, UNUSED_IMPORT)), (NamedStructFields, late, (NAMED_STRUCT_FIELDS)), - (UnsafeCheatcodes, early, (UNSAFE_CHEATCODE_USAGE)) + (UnsafeCheatcodes, early, (UNSAFE_CHEATCODE_USAGE)), + (MultiContractFile, early, (MULTI_CONTRACT_FILE)) ); diff --git a/crates/lint/src/sol/info/multi_contract_file.rs b/crates/lint/src/sol/info/multi_contract_file.rs new file mode 100644 index 0000000000000..234c7315600b2 --- /dev/null +++ b/crates/lint/src/sol/info/multi_contract_file.rs @@ -0,0 +1,43 @@ +use crate::{ + linter::{EarlyLintPass, Lint, LintContext}, + sol::{Severity, SolLint, info::MultiContractFile}, +}; + +use solar::ast::{self as ast}; + +declare_forge_lint!( + MULTI_CONTRACT_FILE, + Severity::Info, + "multi-contract-file", + "prefer having only one contract, interface or library per file" +); + +impl<'ast> EarlyLintPass<'ast> for MultiContractFile { + fn check_full_source_unit( + &mut self, + ctx: &LintContext<'ast, '_>, + unit: &'ast ast::SourceUnit<'ast>, + ) { + if !ctx.is_lint_enabled(MULTI_CONTRACT_FILE.id()) { + return; + } + + // Count contract-like items (contracts, interfaces, libraries) in this source unit. + let count = unit.count_contracts(); + + if count > 1 { + // Point at the second contract's name to make the diagnostic actionable. + if let Some(span) = unit + .items + .iter() + .filter_map(|item| match &item.kind { + ast::ItemKind::Contract(c) => Some(c.name.span), + _ => None, + }) + .nth(1) + { + ctx.emit(&MULTI_CONTRACT_FILE, span); + } + } + } +} diff --git a/crates/lint/testdata/Keccak256.stderr b/crates/lint/testdata/Keccak256.stderr index b980f45732189..bf7484aa22ecf 100644 --- a/crates/lint/testdata/Keccak256.stderr +++ b/crates/lint/testdata/Keccak256.stderr @@ -38,6 +38,14 @@ LL | uint256 Enabled_MixedCase_Variable = 1; | = help: https://book.getfoundry.sh/reference/forge/forge-lint#mixed-case-variable +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/Keccak256.sol:LL:CC + | +LL | contract OtherAsmKeccak256 { + | ^^^^^^^^^^^^^^^^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + note[asm-keccak256]: use of inefficient hashing mechanism; consider using inline assembly --> ROOT/testdata/Keccak256.sol:LL:CC | diff --git a/crates/lint/testdata/MixedCase.stderr b/crates/lint/testdata/MixedCase.stderr index 654b7e2f171c5..04b94c7f9533b 100644 --- a/crates/lint/testdata/MixedCase.stderr +++ b/crates/lint/testdata/MixedCase.stderr @@ -134,3 +134,11 @@ LL | function NOT_ELEMENTARY_RETURN() external view returns (uint256[] memor | = help: https://book.getfoundry.sh/reference/forge/forge-lint#mixed-case-function +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/MixedCase.sol:LL:CC + | +LL | contract MixedCaseTest { + | ^^^^^^^^^^^^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + diff --git a/crates/lint/testdata/MultiContractFile.sol b/crates/lint/testdata/MultiContractFile.sol new file mode 100644 index 0000000000000..02ab04124f3d0 --- /dev/null +++ b/crates/lint/testdata/MultiContractFile.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.18; + +contract A {} + +contract B {} //~NOTE: prefer having only one contract, interface or library per file + +contract C {} + +interface I {} + +library L {} diff --git a/crates/lint/testdata/MultiContractFile.stderr b/crates/lint/testdata/MultiContractFile.stderr new file mode 100644 index 0000000000000..8a2cf790ba8d4 --- /dev/null +++ b/crates/lint/testdata/MultiContractFile.stderr @@ -0,0 +1,8 @@ +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/MultiContractFile.sol:LL:CC + | +LL | contract B {} + | ^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + diff --git a/crates/lint/testdata/MultiContractFile_InterfaceLibrary.sol b/crates/lint/testdata/MultiContractFile_InterfaceLibrary.sol new file mode 100644 index 0000000000000..ca9e6c21c266b --- /dev/null +++ b/crates/lint/testdata/MultiContractFile_InterfaceLibrary.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.18; + +// Interface counts as a contract-like item. +interface I1 {} + +// Library is also a contract-like item and it should be counted. +library L1 {} //~NOTE: prefer having only one contract, interface or library per file + +// Third contract-like item. +contract C1 {} + diff --git a/crates/lint/testdata/MultiContractFile_InterfaceLibrary.stderr b/crates/lint/testdata/MultiContractFile_InterfaceLibrary.stderr new file mode 100644 index 0000000000000..6eba4b3343593 --- /dev/null +++ b/crates/lint/testdata/MultiContractFile_InterfaceLibrary.stderr @@ -0,0 +1,8 @@ +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/MultiContractFile_InterfaceLibrary.sol:LL:CC + | +LL | library L1 {} + | ^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + diff --git a/crates/lint/testdata/UncheckedTransferERC20.stderr b/crates/lint/testdata/UncheckedTransferERC20.stderr index 1afa4d689a871..baed6397fdb05 100644 --- a/crates/lint/testdata/UncheckedTransferERC20.stderr +++ b/crates/lint/testdata/UncheckedTransferERC20.stderr @@ -1,3 +1,11 @@ +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/UncheckedTransferERC20.sol:LL:CC + | +LL | interface IERC20Wrapper { + | ^^^^^^^^^^^^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + warning[erc20-unchecked-transfer]: ERC20 'transfer' and 'transferFrom' calls should check the return value --> ROOT/testdata/UncheckedTransferERC20.sol:LL:CC | diff --git a/crates/lint/testdata/UnsafeTypecast.stderr b/crates/lint/testdata/UnsafeTypecast.stderr index 4a3f35b2275a4..0598bb95dd2ca 100644 --- a/crates/lint/testdata/UnsafeTypecast.stderr +++ b/crates/lint/testdata/UnsafeTypecast.stderr @@ -1,3 +1,11 @@ +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/UnsafeTypecast.sol:LL:CC + | +LL | contract Repros { + | ^^^^^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + warning[unsafe-typecast]: typecasts that can truncate values should be checked --> ROOT/testdata/UnsafeTypecast.sol:LL:CC | diff --git a/crates/lint/testdata/UnwrappedModifierLogic.stderr b/crates/lint/testdata/UnwrappedModifierLogic.stderr index d5d02817d0c04..46d4065da8aa1 100644 --- a/crates/lint/testdata/UnwrappedModifierLogic.stderr +++ b/crates/lint/testdata/UnwrappedModifierLogic.stderr @@ -1,3 +1,11 @@ +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/UnwrappedModifierLogic.sol:LL:CC + | +LL | contract C { + | ^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + note[unwrapped-modifier-logic]: wrap modifier logic to reduce code size --> ROOT/testdata/UnwrappedModifierLogic.sol:LL:CC | From 9712c0607f765aa99bc482c9c17f0317d98161ef Mon Sep 17 00:00:00 2001 From: Milos Djurica Date: Tue, 18 Nov 2025 21:01:36 +0100 Subject: [PATCH 2/8] multi emit per file --- .../lint/src/sol/info/multi_contract_file.rs | 9 +++---- crates/lint/testdata/Keccak256.stderr | 8 +++++++ crates/lint/testdata/MultiContractFile.stderr | 24 +++++++++++++++++++ .../MultiContractFile_InterfaceLibrary.stderr | 8 +++++++ .../testdata/UncheckedTransferERC20.stderr | 24 +++++++++++++++++++ .../testdata/UnwrappedModifierLogic.stderr | 8 +++++++ 6 files changed, 75 insertions(+), 6 deletions(-) diff --git a/crates/lint/src/sol/info/multi_contract_file.rs b/crates/lint/src/sol/info/multi_contract_file.rs index 234c7315600b2..4c9d4da3d55fc 100644 --- a/crates/lint/src/sol/info/multi_contract_file.rs +++ b/crates/lint/src/sol/info/multi_contract_file.rs @@ -27,17 +27,14 @@ impl<'ast> EarlyLintPass<'ast> for MultiContractFile { if count > 1 { // Point at the second contract's name to make the diagnostic actionable. - if let Some(span) = unit - .items + unit.items .iter() .filter_map(|item| match &item.kind { ast::ItemKind::Contract(c) => Some(c.name.span), _ => None, }) - .nth(1) - { - ctx.emit(&MULTI_CONTRACT_FILE, span); - } + .skip(1) + .for_each(|span| ctx.emit(&MULTI_CONTRACT_FILE, span)); } } } diff --git a/crates/lint/testdata/Keccak256.stderr b/crates/lint/testdata/Keccak256.stderr index bf7484aa22ecf..7d481bcd6a4fb 100644 --- a/crates/lint/testdata/Keccak256.stderr +++ b/crates/lint/testdata/Keccak256.stderr @@ -46,6 +46,14 @@ LL | contract OtherAsmKeccak256 { | = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/Keccak256.sol:LL:CC + | +LL | contract YetAnotherAsmKeccak256 { + | ^^^^^^^^^^^^^^^^^^^^^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + note[asm-keccak256]: use of inefficient hashing mechanism; consider using inline assembly --> ROOT/testdata/Keccak256.sol:LL:CC | diff --git a/crates/lint/testdata/MultiContractFile.stderr b/crates/lint/testdata/MultiContractFile.stderr index 8a2cf790ba8d4..2f6bb07c6b6fd 100644 --- a/crates/lint/testdata/MultiContractFile.stderr +++ b/crates/lint/testdata/MultiContractFile.stderr @@ -6,3 +6,27 @@ LL | contract B {} | = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/MultiContractFile.sol:LL:CC + | +LL | contract C {} + | ^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/MultiContractFile.sol:LL:CC + | +LL | interface I {} + | ^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/MultiContractFile.sol:LL:CC + | +LL | library L {} + | ^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + diff --git a/crates/lint/testdata/MultiContractFile_InterfaceLibrary.stderr b/crates/lint/testdata/MultiContractFile_InterfaceLibrary.stderr index 6eba4b3343593..5a34350812095 100644 --- a/crates/lint/testdata/MultiContractFile_InterfaceLibrary.stderr +++ b/crates/lint/testdata/MultiContractFile_InterfaceLibrary.stderr @@ -6,3 +6,11 @@ LL | library L1 {} | = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/MultiContractFile_InterfaceLibrary.sol:LL:CC + | +LL | contract C1 {} + | ^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + diff --git a/crates/lint/testdata/UncheckedTransferERC20.stderr b/crates/lint/testdata/UncheckedTransferERC20.stderr index baed6397fdb05..502a38f6d3bc2 100644 --- a/crates/lint/testdata/UncheckedTransferERC20.stderr +++ b/crates/lint/testdata/UncheckedTransferERC20.stderr @@ -6,6 +6,30 @@ LL | interface IERC20Wrapper { | = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/UncheckedTransferERC20.sol:LL:CC + | +LL | contract UncheckedTransfer { + | ^^^^^^^^^^^^^^^^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/UncheckedTransferERC20.sol:LL:CC + | +LL | library Currency { + | ^^^^^^^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/UncheckedTransferERC20.sol:LL:CC + | +LL | contract UncheckedTransferUsingCurrencyLib { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + warning[erc20-unchecked-transfer]: ERC20 'transfer' and 'transferFrom' calls should check the return value --> ROOT/testdata/UncheckedTransferERC20.sol:LL:CC | diff --git a/crates/lint/testdata/UnwrappedModifierLogic.stderr b/crates/lint/testdata/UnwrappedModifierLogic.stderr index 46d4065da8aa1..1cf45732a9593 100644 --- a/crates/lint/testdata/UnwrappedModifierLogic.stderr +++ b/crates/lint/testdata/UnwrappedModifierLogic.stderr @@ -6,6 +6,14 @@ LL | contract C { | = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/UnwrappedModifierLogic.sol:LL:CC + | +LL | contract UnwrappedModifierLogicTest { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + note[unwrapped-modifier-logic]: wrap modifier logic to reduce code size --> ROOT/testdata/UnwrappedModifierLogic.sol:LL:CC | From 9c68f144d679e00472d142784259bd8aa325bf97 Mon Sep 17 00:00:00 2001 From: Milos Djurica Date: Wed, 19 Nov 2025 19:59:57 +0100 Subject: [PATCH 3/8] feat(lint): add LintSpecificConfig for mixed_case_exceptions and multi_contract_file_exceptions --- crates/config/src/lint.rs | 44 ++- crates/forge/src/cmd/build.rs | 2 +- crates/forge/src/cmd/lint.rs | 2 +- crates/forge/tests/cli/config.rs | 14 +- crates/forge/tests/cli/lint.rs | 289 +++++++++++++++++- crates/lint/src/linter/mod.rs | 7 +- crates/lint/src/sol/info/mixed_case.rs | 14 +- .../lint/src/sol/info/multi_contract_file.rs | 39 ++- crates/lint/src/sol/mod.rs | 18 +- crates/lint/testdata/Keccak256.stderr | 8 + crates/lint/testdata/MixedCase.stderr | 8 + crates/lint/testdata/MultiContractFile.stderr | 8 + .../MultiContractFile_InterfaceLibrary.stderr | 8 + .../testdata/UncheckedTransferERC20.stderr | 8 + crates/lint/testdata/UnsafeTypecast.stderr | 8 + .../testdata/UnwrappedModifierLogic.stderr | 8 + 16 files changed, 448 insertions(+), 37 deletions(-) diff --git a/crates/config/src/lint.rs b/crates/config/src/lint.rs index 6752372767a2b..7a87a47321d21 100644 --- a/crates/config/src/lint.rs +++ b/crates/config/src/lint.rs @@ -26,11 +26,8 @@ pub struct LinterConfig { /// Defaults to true. Set to false to disable automatic linting during builds. pub lint_on_build: bool, - /// Configurable patterns that should be excluded when performing `mixedCase` lint checks. - /// - /// Default's to ["ERC", "URI"] to allow common names like `rescueERC20`, `ERC721TokenReceiver` - /// or `tokenURI`. - pub mixed_case_exceptions: Vec, + /// Configuration specific to individual lints. + pub lint_specific: LintSpecificConfig, } impl Default for LinterConfig { @@ -40,7 +37,44 @@ impl Default for LinterConfig { severity: Vec::new(), exclude_lints: Vec::new(), ignore: Vec::new(), + lint_specific: LintSpecificConfig::default(), + } + } +} + +/// Contract types that can be exempted from the multi-contract-file lint. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ContractException { + Interface, + Library, + AbstractContract, +} + +/// Configuration specific to individual lints. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(default)] +pub struct LintSpecificConfig { + /// Configurable patterns that should be excluded when performing `mixedCase` lint checks. + /// + /// Defaults to ["ERC", "URI"] to allow common names like `rescueERC20`, `ERC721TokenReceiver` + /// or `tokenURI`. + pub mixed_case_exceptions: Vec, + + /// Contract types that are allowed to appear multiple times in the same file. + /// + /// Valid values: "interface", "library", "abstract_contract" + /// + /// Defaults to an empty array (all contract types are flagged when multiple exist). + /// Note: Regular contracts cannot be exempted and will always be flagged when multiple exist. + pub multi_contract_file_exceptions: Vec, +} + +impl Default for LintSpecificConfig { + fn default() -> Self { + Self { mixed_case_exceptions: vec!["ERC".to_string(), "URI".to_string()], + multi_contract_file_exceptions: Vec::new(), } } } diff --git a/crates/forge/src/cmd/build.rs b/crates/forge/src/cmd/build.rs index d37c73fd68cbb..5ae1247de4b5b 100644 --- a/crates/forge/src/cmd/build.rs +++ b/crates/forge/src/cmd/build.rs @@ -151,7 +151,7 @@ impl BuildArgs { .collect(), ) }) - .with_mixed_case_exceptions(&config.lint.mixed_case_exceptions); + .with_lint_specific(&config.lint.lint_specific); // Expand ignore globs and canonicalize from the get go let ignored = expand_globs(&config.root, config.lint.ignore.iter())? diff --git a/crates/forge/src/cmd/lint.rs b/crates/forge/src/cmd/lint.rs index ae4b8389a8ecc..a4663b2bce766 100644 --- a/crates/forge/src/cmd/lint.rs +++ b/crates/forge/src/cmd/lint.rs @@ -104,7 +104,7 @@ impl LintArgs { .with_lints(include) .without_lints(exclude) .with_severity(if severity.is_empty() { None } else { Some(severity) }) - .with_mixed_case_exceptions(&config.lint.mixed_case_exceptions); + .with_lint_specific(&config.lint.lint_specific); let output = ProjectCompiler::new().files(input.iter().cloned()).compile(&project)?; let solar_sources = get_solar_sources_from_compile_output(&config, &output, Some(&input))?; diff --git a/crates/forge/tests/cli/config.rs b/crates/forge/tests/cli/config.rs index 6af175ae15a6d..e84d5d277cfd6 100644 --- a/crates/forge/tests/cli/config.rs +++ b/crates/forge/tests/cli/config.rs @@ -146,10 +146,13 @@ severity = [] exclude_lints = [] ignore = [] lint_on_build = true + +[lint.lint_specific] mixed_case_exceptions = [ "ERC", "URI", ] +multi_contract_file_exceptions = [] [doc] out = "docs" @@ -1339,10 +1342,13 @@ forgetest_init!(test_default_config, |prj, cmd| { "exclude_lints": [], "ignore": [], "lint_on_build": true, - "mixed_case_exceptions": [ - "ERC", - "URI" - ] + "lint_specific": { + "mixed_case_exceptions": [ + "ERC", + "URI" + ], + "multi_contract_file_exceptions": [] + } }, "doc": { "out": "docs", diff --git a/crates/forge/tests/cli/lint.rs b/crates/forge/tests/cli/lint.rs index da766cf099173..0d6e9d0b48e11 100644 --- a/crates/forge/tests/cli/lint.rs +++ b/crates/forge/tests/cli/lint.rs @@ -1,5 +1,5 @@ use forge_lint::{linter::Lint, sol::med::REGISTERED_LINTS}; -use foundry_config::{DenyLevel, LintSeverity, LinterConfig}; +use foundry_config::{DenyLevel, LintSeverity, LinterConfig, lint::LintSpecificConfig}; mod geiger; @@ -112,6 +112,57 @@ contract CounterTest { } "#; +const MULTI_CONTRACT_FILE: &str = r#" +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +interface IToken { + function transfer(address to, uint256 amount) external returns (bool); +} + +library MathLib { + function add(uint256 a, uint256 b) internal pure returns (uint256) { + return a + b; + } +} + +contract FirstContract { + uint256 public value; + + function setValue(uint256 _value) public { + value = _value; + } +} + +abstract contract BaseContract { + function baseFunction() public virtual; +} + +interface IERC20 { + function balanceOf(address account) external view returns (uint256); +} + +contract SecondContract { + address public owner; + + constructor() { + owner = msg.sender; + } +} + +library StringLib { + function toUpperCase(string memory str) internal pure returns (string memory) { + return str; + } +} + +abstract contract AbstractStorage { + mapping(address => uint256) internal balances; + + function getBalance(address account) public view virtual returns (uint256); +} +"#; + forgetest!(can_use_config, |prj, cmd| { prj.add_source("ContractWithLints", CONTRACT); prj.add_source("OtherContractWithLints", OTHER_CONTRACT); @@ -189,12 +240,246 @@ forgetest!(can_use_config_mixed_case_exception, |prj, cmd| { exclude_lints: vec![], ignore: vec!["src/ContractWithLints.sol".into()], lint_on_build: true, - mixed_case_exceptions: vec!["MIXED".to_string()], + lint_specific: LintSpecificConfig { + mixed_case_exceptions: vec!["MIXED".to_string()], + ..Default::default() + }, }; }); cmd.arg("lint").assert_success().stderr_eq(str![[""]]); }); +forgetest!(multi_contract_file_no_exceptions, |prj, cmd| { + prj.add_source("MixedFile", MULTI_CONTRACT_FILE); + + // Without exceptions, should flag all 8 contract-like items + prj.update_config(|config| { + config.lint = LinterConfig { lint_on_build: true, ..Default::default() }; + }); + + let output = cmd.arg("lint").assert_success(); + let stderr = String::from_utf8_lossy(&output.get_output().stderr); + + // Should see 8 instances of multi-contract-file lint + assert_eq!(stderr.matches("note[multi-contract-file]").count(), 8); + assert!(stderr.contains("IToken")); + assert!(stderr.contains("IERC20")); + assert!(stderr.contains("MathLib")); + assert!(stderr.contains("StringLib")); + assert!(stderr.contains("BaseContract")); + assert!(stderr.contains("AbstractStorage")); + assert!(stderr.contains("FirstContract")); + assert!(stderr.contains("SecondContract")); +}); + +forgetest!(multi_contract_file_interface_exception, |prj, cmd| { + use foundry_config::lint::ContractException; + + prj.add_source("MixedFile", MULTI_CONTRACT_FILE); + + // With interface exception, should flag 6 items + prj.update_config(|config| { + config.lint = LinterConfig { + lint_on_build: true, + lint_specific: LintSpecificConfig { + multi_contract_file_exceptions: vec![ContractException::Interface], + ..Default::default() + }, + ..Default::default() + }; + }); + + let output = cmd.arg("lint").assert_success(); + let stderr = String::from_utf8_lossy(&output.get_output().stderr); + + // Should see 6 instances (2 interfaces excluded) + assert_eq!(stderr.matches("note[multi-contract-file]").count(), 6); + assert!(!stderr.contains("IToken")); + assert!(!stderr.contains("IERC20")); + assert!(stderr.contains("MathLib")); + assert!(stderr.contains("FirstContract")); +}); + +forgetest!(multi_contract_file_library_exception, |prj, cmd| { + use foundry_config::lint::ContractException; + + prj.add_source("MixedFile", MULTI_CONTRACT_FILE); + + // With library exception, should flag 6 items + prj.update_config(|config| { + config.lint = LinterConfig { + lint_on_build: true, + lint_specific: LintSpecificConfig { + multi_contract_file_exceptions: vec![ContractException::Library], + ..Default::default() + }, + ..Default::default() + }; + }); + + let output = cmd.arg("lint").assert_success(); + let stderr = String::from_utf8_lossy(&output.get_output().stderr); + + // Should see 6 instances (2 libraries excluded) + assert_eq!(stderr.matches("note[multi-contract-file]").count(), 6); + assert!(stderr.contains("IToken")); + assert!(!stderr.contains("MathLib")); + assert!(!stderr.contains("StringLib")); + assert!(stderr.contains("FirstContract")); +}); + +forgetest!(multi_contract_file_abstract_exception, |prj, cmd| { + use foundry_config::lint::ContractException; + + prj.add_source("MixedFile", MULTI_CONTRACT_FILE); + + // With abstract contract exception, should flag 6 items + prj.update_config(|config| { + config.lint = LinterConfig { + lint_on_build: true, + lint_specific: LintSpecificConfig { + multi_contract_file_exceptions: vec![ContractException::AbstractContract], + ..Default::default() + }, + ..Default::default() + }; + }); + + let output = cmd.arg("lint").assert_success(); + let stderr = String::from_utf8_lossy(&output.get_output().stderr); + + // Should see 6 instances (2 abstract contracts excluded) + assert_eq!(stderr.matches("note[multi-contract-file]").count(), 6); + assert!(stderr.contains("IToken")); + assert!(stderr.contains("MathLib")); + assert!(!stderr.contains("BaseContract")); + assert!(!stderr.contains("AbstractStorage")); + assert!(stderr.contains("FirstContract")); +}); + +forgetest!(multi_contract_file_multiple_exceptions, |prj, cmd| { + use foundry_config::lint::ContractException; + + prj.add_source("MixedFile", MULTI_CONTRACT_FILE); + + // With interface + library exceptions, should flag 4 items + prj.update_config(|config| { + config.lint = LinterConfig { + lint_on_build: true, + lint_specific: LintSpecificConfig { + multi_contract_file_exceptions: vec![ + ContractException::Interface, + ContractException::Library, + ], + ..Default::default() + }, + ..Default::default() + }; + }); + + let output = cmd.arg("lint").assert_success(); + let stderr = String::from_utf8_lossy(&output.get_output().stderr); + + // Should see 4 instances (2 interfaces + 2 libraries excluded) + assert_eq!(stderr.matches("note[multi-contract-file]").count(), 4); + assert!(!stderr.contains("IToken")); + assert!(!stderr.contains("MathLib")); + assert!(stderr.contains("BaseContract")); + assert!(stderr.contains("FirstContract")); +}); + +forgetest!(multi_contract_file_all_exceptions, |prj, cmd| { + use foundry_config::lint::ContractException; + + prj.add_source("MixedFile", MULTI_CONTRACT_FILE); + + // With all exceptions, should still flag 2 regular contracts + prj.update_config(|config| { + config.lint = LinterConfig { + lint_on_build: true, + lint_specific: LintSpecificConfig { + multi_contract_file_exceptions: vec![ + ContractException::Interface, + ContractException::Library, + ContractException::AbstractContract, + ], + ..Default::default() + }, + ..Default::default() + }; + }); + + let output = cmd.arg("lint").assert_success(); + let stderr = String::from_utf8_lossy(&output.get_output().stderr); + + // Should see 2 instances (only the 2 regular contracts) + assert_eq!(stderr.matches("note[multi-contract-file]").count(), 2); + assert!(!stderr.contains("IToken")); + assert!(!stderr.contains("MathLib")); + assert!(!stderr.contains("BaseContract")); + assert!(stderr.contains("FirstContract")); + assert!(stderr.contains("SecondContract")); +}); + +forgetest!(multi_contract_file_invalid_toml_value, |prj, cmd| { + use std::fs; + + prj.add_source("Simple", "contract Simple {}"); + + // Write invalid TOML config with invalid enum value + let config_path = prj.root().join("foundry.toml"); + let invalid_config = r#" +[profile.default] +src = "src" +out = "out" +libs = ["lib"] + +[profile.default.lint.lint_specific] +multi_contract_file_exceptions = ["interface", "bad_contract_type", "library"] +"#; + + fs::write(&config_path, invalid_config).unwrap(); + + // Should fail with deserialization error + let output = cmd.arg("lint").assert_failure(); + let stderr = String::from_utf8_lossy(&output.get_output().stderr); + + // Assert specific error message for invalid enum variant + assert!(stderr.contains("unknown variant")); + assert!(stderr.contains("expected `one of `interface`, `library`, `abstract_contract`")); +}); + +forgetest!(multi_contract_file_valid_toml_values, |prj, cmd| { + use std::fs; + + prj.add_source("MixedFile", MULTI_CONTRACT_FILE); + + // Write valid TOML config with all valid enum values + let config_path = prj.root().join("foundry.toml"); + let valid_config = r#" +[profile.default] +src = "src" +out = "out" +libs = ["lib"] + +[profile.default.lint] +lint_on_build = true + +[profile.default.lint.lint_specific] +multi_contract_file_exceptions = ["interface", "library", "abstract_contract"] +"#; + + fs::write(&config_path, valid_config).unwrap(); + + // Should succeed and only flag the 2 regular contracts + let output = cmd.arg("lint").assert_success(); + let stderr = String::from_utf8_lossy(&output.get_output().stderr); + + assert_eq!(stderr.matches("note[multi-contract-file]").count(), 2); + assert!(stderr.contains("FirstContract")); + assert!(stderr.contains("SecondContract")); +}); + forgetest!(can_override_config_severity, |prj, cmd| { prj.add_source("ContractWithLints", CONTRACT); prj.add_source("OtherContractWithLints", OTHER_CONTRACT); diff --git a/crates/lint/src/linter/mod.rs b/crates/lint/src/linter/mod.rs index 3ac445744a10a..11e1ba17e5705 100644 --- a/crates/lint/src/linter/mod.rs +++ b/crates/lint/src/linter/mod.rs @@ -6,7 +6,10 @@ pub use late::{LateLintPass, LateLintVisitor}; use foundry_common::comments::inline_config::InlineConfig; use foundry_compilers::Language; -use foundry_config::{DenyLevel, lint::Severity}; +use foundry_config::{ + DenyLevel, + lint::{LintSpecificConfig, Severity}, +}; use solar::{ interface::{ Session, Span, @@ -55,7 +58,7 @@ pub struct LintContext<'s, 'c> { pub struct LinterConfig<'s> { pub inline: &'s InlineConfig>, - pub mixed_case_exceptions: &'s [String], + pub lint_specific: &'s LintSpecificConfig, } impl<'s, 'c> LintContext<'s, 'c> { diff --git a/crates/lint/src/sol/info/mixed_case.rs b/crates/lint/src/sol/info/mixed_case.rs index c128d40b55ec8..669772188c2a3 100644 --- a/crates/lint/src/sol/info/mixed_case.rs +++ b/crates/lint/src/sol/info/mixed_case.rs @@ -15,8 +15,11 @@ declare_forge_lint!( impl<'ast> EarlyLintPass<'ast> for MixedCaseFunction { fn check_item_function(&mut self, ctx: &LintContext, func: &'ast ItemFunction<'ast>) { if let Some(name) = func.header.name - && let Some(expected) = - check_mixed_case(name.as_str(), true, ctx.config.mixed_case_exceptions) + && let Some(expected) = check_mixed_case( + name.as_str(), + true, + &ctx.config.lint_specific.mixed_case_exceptions, + ) && !is_constant_getter(&func.header) { ctx.emit_with_suggestion( @@ -47,8 +50,11 @@ impl<'ast> EarlyLintPass<'ast> for MixedCaseVariable { ) { if var.mutability.is_none() && let Some(name) = var.name - && let Some(expected) = - check_mixed_case(name.as_str(), false, ctx.config.mixed_case_exceptions) + && let Some(expected) = check_mixed_case( + name.as_str(), + false, + &ctx.config.lint_specific.mixed_case_exceptions, + ) { ctx.emit_with_suggestion( &MIXED_CASE_VARIABLE, diff --git a/crates/lint/src/sol/info/multi_contract_file.rs b/crates/lint/src/sol/info/multi_contract_file.rs index 4c9d4da3d55fc..25a930793a29a 100644 --- a/crates/lint/src/sol/info/multi_contract_file.rs +++ b/crates/lint/src/sol/info/multi_contract_file.rs @@ -3,6 +3,7 @@ use crate::{ sol::{Severity, SolLint, info::MultiContractFile}, }; +use foundry_config::lint::ContractException; use solar::ast::{self as ast}; declare_forge_lint!( @@ -22,19 +23,33 @@ impl<'ast> EarlyLintPass<'ast> for MultiContractFile { return; } - // Count contract-like items (contracts, interfaces, libraries) in this source unit. - let count = unit.count_contracts(); + // Check which types are exempted + let exceptions = &ctx.config.lint_specific.multi_contract_file_exceptions; + let allow_interfaces = exceptions.contains(&ContractException::Interface); + let allow_libraries = exceptions.contains(&ContractException::Library); + let allow_abstract = exceptions.contains(&ContractException::AbstractContract); - if count > 1 { - // Point at the second contract's name to make the diagnostic actionable. - unit.items - .iter() - .filter_map(|item| match &item.kind { - ast::ItemKind::Contract(c) => Some(c.name.span), - _ => None, - }) - .skip(1) - .for_each(|span| ctx.emit(&MULTI_CONTRACT_FILE, span)); + // Collect spans of all contract-like items, skipping those that are exempted + let relevant_spans: Vec<_> = unit + .items + .iter() + .filter_map(|item| match &item.kind { + ast::ItemKind::Contract(c) => { + let should_skip = match c.kind { + ast::ContractKind::Interface => allow_interfaces, + ast::ContractKind::Library => allow_libraries, + ast::ContractKind::AbstractContract => allow_abstract, + ast::ContractKind::Contract => false, // Regular contracts are never skipped + }; + (!should_skip).then_some(c.name.span) + } + _ => None, + }) + .collect(); + + // Flag all if there's more than one + if relevant_spans.len() > 1 { + relevant_spans.into_iter().for_each(|span| ctx.emit(&MULTI_CONTRACT_FILE, span)); } } } diff --git a/crates/lint/src/sol/mod.rs b/crates/lint/src/sol/mod.rs index ed72b0724a3a4..d6d426a057865 100644 --- a/crates/lint/src/sol/mod.rs +++ b/crates/lint/src/sol/mod.rs @@ -11,7 +11,10 @@ use foundry_common::{ sh_warn, }; use foundry_compilers::{ProjectPathsConfig, solc::SolcLanguage}; -use foundry_config::{DenyLevel, lint::Severity}; +use foundry_config::{ + DenyLevel, + lint::{LintSpecificConfig, Severity}, +}; use rayon::prelude::*; use solar::{ ast::{self as ast, visit::Visit as _}, @@ -49,6 +52,9 @@ static ALL_REGISTERED_LINTS: LazyLock> = LazyLock::new(|| { lints.into_iter().map(|lint| lint.id()).collect() }); +static DEFAULT_LINT_SPECIFIC_CONFIG: LazyLock = + LazyLock::new(LintSpecificConfig::default); + /// Linter implementation to analyze Solidity source code responsible for identifying /// vulnerabilities gas optimizations, and best practices. #[derive(Debug)] @@ -60,7 +66,7 @@ pub struct SolidityLinter<'a> { with_description: bool, with_json_emitter: bool, // lint-specific configuration - mixed_case_exceptions: &'a [String], + lint_specific: &'a LintSpecificConfig, } impl<'a> SolidityLinter<'a> { @@ -72,7 +78,7 @@ impl<'a> SolidityLinter<'a> { lints_included: None, lints_excluded: None, with_json_emitter: false, - mixed_case_exceptions: &[], + lint_specific: &DEFAULT_LINT_SPECIFIC_CONFIG, } } @@ -101,13 +107,13 @@ impl<'a> SolidityLinter<'a> { self } - pub fn with_mixed_case_exceptions(mut self, exceptions: &'a [String]) -> Self { - self.mixed_case_exceptions = exceptions; + pub fn with_lint_specific(mut self, lint_specific: &'a LintSpecificConfig) -> Self { + self.lint_specific = lint_specific; self } fn config(&'a self, inline: &'a InlineConfig>) -> LinterConfig<'a> { - LinterConfig { inline, mixed_case_exceptions: self.mixed_case_exceptions } + LinterConfig { inline, lint_specific: self.lint_specific } } fn include_lint(&self, lint: SolLint) -> bool { diff --git a/crates/lint/testdata/Keccak256.stderr b/crates/lint/testdata/Keccak256.stderr index 7d481bcd6a4fb..199d5217b34ea 100644 --- a/crates/lint/testdata/Keccak256.stderr +++ b/crates/lint/testdata/Keccak256.stderr @@ -38,6 +38,14 @@ LL | uint256 Enabled_MixedCase_Variable = 1; | = help: https://book.getfoundry.sh/reference/forge/forge-lint#mixed-case-variable +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/Keccak256.sol:LL:CC + | +LL | contract AsmKeccak256 { + | ^^^^^^^^^^^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + note[multi-contract-file]: prefer having only one contract, interface or library per file --> ROOT/testdata/Keccak256.sol:LL:CC | diff --git a/crates/lint/testdata/MixedCase.stderr b/crates/lint/testdata/MixedCase.stderr index 04b94c7f9533b..05a759c65f381 100644 --- a/crates/lint/testdata/MixedCase.stderr +++ b/crates/lint/testdata/MixedCase.stderr @@ -134,6 +134,14 @@ LL | function NOT_ELEMENTARY_RETURN() external view returns (uint256[] memor | = help: https://book.getfoundry.sh/reference/forge/forge-lint#mixed-case-function +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/MixedCase.sol:LL:CC + | +LL | interface IERC20 { + | ^^^^^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + note[multi-contract-file]: prefer having only one contract, interface or library per file --> ROOT/testdata/MixedCase.sol:LL:CC | diff --git a/crates/lint/testdata/MultiContractFile.stderr b/crates/lint/testdata/MultiContractFile.stderr index 2f6bb07c6b6fd..38d2300a080b8 100644 --- a/crates/lint/testdata/MultiContractFile.stderr +++ b/crates/lint/testdata/MultiContractFile.stderr @@ -1,3 +1,11 @@ +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/MultiContractFile.sol:LL:CC + | +LL | contract A {} + | ^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + note[multi-contract-file]: prefer having only one contract, interface or library per file --> ROOT/testdata/MultiContractFile.sol:LL:CC | diff --git a/crates/lint/testdata/MultiContractFile_InterfaceLibrary.stderr b/crates/lint/testdata/MultiContractFile_InterfaceLibrary.stderr index 5a34350812095..91e0fcc7960c9 100644 --- a/crates/lint/testdata/MultiContractFile_InterfaceLibrary.stderr +++ b/crates/lint/testdata/MultiContractFile_InterfaceLibrary.stderr @@ -1,3 +1,11 @@ +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/MultiContractFile_InterfaceLibrary.sol:LL:CC + | +LL | interface I1 {} + | ^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + note[multi-contract-file]: prefer having only one contract, interface or library per file --> ROOT/testdata/MultiContractFile_InterfaceLibrary.sol:LL:CC | diff --git a/crates/lint/testdata/UncheckedTransferERC20.stderr b/crates/lint/testdata/UncheckedTransferERC20.stderr index 502a38f6d3bc2..b35774c796198 100644 --- a/crates/lint/testdata/UncheckedTransferERC20.stderr +++ b/crates/lint/testdata/UncheckedTransferERC20.stderr @@ -1,3 +1,11 @@ +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/UncheckedTransferERC20.sol:LL:CC + | +LL | interface IERC20 { + | ^^^^^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + note[multi-contract-file]: prefer having only one contract, interface or library per file --> ROOT/testdata/UncheckedTransferERC20.sol:LL:CC | diff --git a/crates/lint/testdata/UnsafeTypecast.stderr b/crates/lint/testdata/UnsafeTypecast.stderr index 0598bb95dd2ca..ddc451733e506 100644 --- a/crates/lint/testdata/UnsafeTypecast.stderr +++ b/crates/lint/testdata/UnsafeTypecast.stderr @@ -1,3 +1,11 @@ +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/UnsafeTypecast.sol:LL:CC + | +LL | contract UnsafeTypecast { + | ^^^^^^^^^^^^^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + note[multi-contract-file]: prefer having only one contract, interface or library per file --> ROOT/testdata/UnsafeTypecast.sol:LL:CC | diff --git a/crates/lint/testdata/UnwrappedModifierLogic.stderr b/crates/lint/testdata/UnwrappedModifierLogic.stderr index 1cf45732a9593..65a733e8cab6a 100644 --- a/crates/lint/testdata/UnwrappedModifierLogic.stderr +++ b/crates/lint/testdata/UnwrappedModifierLogic.stderr @@ -1,3 +1,11 @@ +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/UnwrappedModifierLogic.sol:LL:CC + | +LL | library Lib { + | ^^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + note[multi-contract-file]: prefer having only one contract, interface or library per file --> ROOT/testdata/UnwrappedModifierLogic.sol:LL:CC | From 036c20c62b8e2a9d67dc9efb00c7db714815e650 Mon Sep 17 00:00:00 2001 From: Milos Djurica Date: Wed, 19 Nov 2025 20:08:56 +0100 Subject: [PATCH 4/8] refactor(lint): refactor multi_contract_file logic to be cleaner --- .../lint/src/sol/info/multi_contract_file.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/crates/lint/src/sol/info/multi_contract_file.rs b/crates/lint/src/sol/info/multi_contract_file.rs index 25a930793a29a..2e56244850f1f 100644 --- a/crates/lint/src/sol/info/multi_contract_file.rs +++ b/crates/lint/src/sol/info/multi_contract_file.rs @@ -25,9 +25,9 @@ impl<'ast> EarlyLintPass<'ast> for MultiContractFile { // Check which types are exempted let exceptions = &ctx.config.lint_specific.multi_contract_file_exceptions; - let allow_interfaces = exceptions.contains(&ContractException::Interface); - let allow_libraries = exceptions.contains(&ContractException::Library); - let allow_abstract = exceptions.contains(&ContractException::AbstractContract); + let should_lint_interfaces = !exceptions.contains(&ContractException::Interface); + let should_lint_libraries = !exceptions.contains(&ContractException::Library); + let should_lint_abstract = !exceptions.contains(&ContractException::AbstractContract); // Collect spans of all contract-like items, skipping those that are exempted let relevant_spans: Vec<_> = unit @@ -35,13 +35,14 @@ impl<'ast> EarlyLintPass<'ast> for MultiContractFile { .iter() .filter_map(|item| match &item.kind { ast::ItemKind::Contract(c) => { - let should_skip = match c.kind { - ast::ContractKind::Interface => allow_interfaces, - ast::ContractKind::Library => allow_libraries, - ast::ContractKind::AbstractContract => allow_abstract, - ast::ContractKind::Contract => false, // Regular contracts are never skipped + let should_lint = match c.kind { + ast::ContractKind::Interface => should_lint_interfaces, + + ast::ContractKind::Library => should_lint_libraries, + ast::ContractKind::AbstractContract => should_lint_abstract, + ast::ContractKind::Contract => true, // Regular contracts are always linted }; - (!should_skip).then_some(c.name.span) + should_lint.then_some(c.name.span) } _ => None, }) From 9ca2234503cc965fc8f083d5ae1d531b99b14cf7 Mon Sep 17 00:00:00 2001 From: Milos Djurica Date: Mon, 24 Nov 2025 20:00:12 +0100 Subject: [PATCH 5/8] feat: add interface-file-naming linting rule --- crates/lint/src/sol/info/interface_naming.rs | 59 ++++++++++++++++++++ crates/lint/src/sol/info/mod.rs | 6 +- crates/lint/testdata/SoloInterfaces.sol | 15 +++++ crates/lint/testdata/SoloInterfaces.stderr | 32 +++++++++++ 4 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 crates/lint/src/sol/info/interface_naming.rs create mode 100644 crates/lint/testdata/SoloInterfaces.sol create mode 100644 crates/lint/testdata/SoloInterfaces.stderr diff --git a/crates/lint/src/sol/info/interface_naming.rs b/crates/lint/src/sol/info/interface_naming.rs new file mode 100644 index 0000000000000..0bb4336a0c3e4 --- /dev/null +++ b/crates/lint/src/sol/info/interface_naming.rs @@ -0,0 +1,59 @@ +use crate::{ + linter::{EarlyLintPass, Lint, LintContext}, + sol::{Severity, SolLint, info::InterfaceFileNaming}, +}; + +use solar::ast::{self as ast}; + +declare_forge_lint!( + INTERFACE_FILE_NAMING, + Severity::Info, + "interface-file-naming", + "interface file names should be prefixed with 'I'" +); + +impl<'ast> EarlyLintPass<'ast> for InterfaceFileNaming { + fn check_full_source_unit( + &mut self, + ctx: &LintContext<'ast, '_>, + unit: &'ast ast::SourceUnit<'ast>, + ) { + if !ctx.is_lint_enabled(INTERFACE_FILE_NAMING.id()) { + return; + } + + // Get first item in file and exit if the unit contains no items + let Some(first_item) = unit.items.first() else { return }; + + // Get file from first item + let file = ctx.session().source_map().lookup_source_file(first_item.span.lo()); + + // Get file name from file + let Some(file_name) = file.name.as_real().and_then(|path| path.file_name()?.to_str()) + else { + return; + }; + + // If file name starts with 'I', skip lint + if file_name.starts_with('I') { + return; + } + + let mut first_interface_span = None; + for item in unit.items.iter() { + if let ast::ItemKind::Contract(c) = &item.kind { + match c.kind { + ast::ContractKind::Interface => { + first_interface_span.get_or_insert(c.name.span); + } + _ => return, // Mixed file, skip lint + } + } + } + + // Emit if file contains ONLY interfaces. Emit only on the first interface. + if let Some(span) = first_interface_span { + ctx.emit(&INTERFACE_FILE_NAMING, span); + } + } +} diff --git a/crates/lint/src/sol/info/mod.rs b/crates/lint/src/sol/info/mod.rs index b87bf900193ca..4c9d0552f6626 100644 --- a/crates/lint/src/sol/info/mod.rs +++ b/crates/lint/src/sol/info/mod.rs @@ -21,6 +21,9 @@ use unsafe_cheatcodes::UNSAFE_CHEATCODE_USAGE; mod multi_contract_file; use multi_contract_file::MULTI_CONTRACT_FILE; +mod interface_naming; +use interface_naming::INTERFACE_FILE_NAMING; + register_lints!( (PascalCaseStruct, early, (PASCAL_CASE_STRUCT)), (MixedCaseVariable, early, (MIXED_CASE_VARIABLE)), @@ -29,5 +32,6 @@ register_lints!( (Imports, early, (UNALIASED_PLAIN_IMPORT, UNUSED_IMPORT)), (NamedStructFields, late, (NAMED_STRUCT_FIELDS)), (UnsafeCheatcodes, early, (UNSAFE_CHEATCODE_USAGE)), - (MultiContractFile, early, (MULTI_CONTRACT_FILE)) + (MultiContractFile, early, (MULTI_CONTRACT_FILE)), + (InterfaceFileNaming, early, (INTERFACE_FILE_NAMING)) ); diff --git a/crates/lint/testdata/SoloInterfaces.sol b/crates/lint/testdata/SoloInterfaces.sol new file mode 100644 index 0000000000000..a343179a52745 --- /dev/null +++ b/crates/lint/testdata/SoloInterfaces.sol @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.18; + +interface ISoloInterfaces { + function foo() external; +} + +interface I2 { + function foo() external; +} + +interface I3 { + function foo() external; +} + diff --git a/crates/lint/testdata/SoloInterfaces.stderr b/crates/lint/testdata/SoloInterfaces.stderr new file mode 100644 index 0000000000000..0e60cb652f6a7 --- /dev/null +++ b/crates/lint/testdata/SoloInterfaces.stderr @@ -0,0 +1,32 @@ +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/SoloInterfaces.sol:LL:CC + | +LL | interface ISoloInterfaces { + | ^^^^^^^^^^^^^^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/SoloInterfaces.sol:LL:CC + | +LL | interface I2 { + | ^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + +note[multi-contract-file]: prefer having only one contract, interface or library per file + --> ROOT/testdata/SoloInterfaces.sol:LL:CC + | +LL | interface I3 { + | ^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file + +note[interface-file-naming]: interface file names should be prefixed with 'I' + --> ROOT/testdata/SoloInterfaces.sol:LL:CC + | +LL | interface ISoloInterfaces { + | ^^^^^^^^^^^^^^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#interface-file-naming + From dafa10834ecc829a4a71261fb88d6f6280a600c0 Mon Sep 17 00:00:00 2001 From: Milos Djurica Date: Mon, 24 Nov 2025 20:16:54 +0100 Subject: [PATCH 6/8] feat: add interface-naming linting rule --- crates/lint/src/sol/info/interface_naming.rs | 24 ++++++++++++++++++++ crates/lint/src/sol/info/mod.rs | 4 ++-- crates/lint/testdata/SoloInterfaces.sol | 2 +- crates/lint/testdata/SoloInterfaces.stderr | 16 +++++++++---- 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/crates/lint/src/sol/info/interface_naming.rs b/crates/lint/src/sol/info/interface_naming.rs index 0bb4336a0c3e4..3989e142d6c54 100644 --- a/crates/lint/src/sol/info/interface_naming.rs +++ b/crates/lint/src/sol/info/interface_naming.rs @@ -12,6 +12,13 @@ declare_forge_lint!( "interface file names should be prefixed with 'I'" ); +declare_forge_lint!( + INTERFACE_NAMING, + Severity::Info, + "interface-naming", + "interface names should be prefixed with 'I'" +); + impl<'ast> EarlyLintPass<'ast> for InterfaceFileNaming { fn check_full_source_unit( &mut self, @@ -56,4 +63,21 @@ impl<'ast> EarlyLintPass<'ast> for InterfaceFileNaming { ctx.emit(&INTERFACE_FILE_NAMING, span); } } + + fn check_item_contract(&mut self, ctx: &LintContext, contract: &'ast ast::ItemContract<'ast>) { + if !ctx.is_lint_enabled(INTERFACE_NAMING.id()) { + return; + } + + // Only check interfaces + if contract.kind != ast::ContractKind::Interface { + return; + } + + // Check if interface name starts with 'I' + let name = contract.name.as_str(); + if !name.starts_with('I') { + ctx.emit(&INTERFACE_NAMING, contract.name.span); + } + } } diff --git a/crates/lint/src/sol/info/mod.rs b/crates/lint/src/sol/info/mod.rs index 4c9d0552f6626..97628a85e1132 100644 --- a/crates/lint/src/sol/info/mod.rs +++ b/crates/lint/src/sol/info/mod.rs @@ -22,7 +22,7 @@ mod multi_contract_file; use multi_contract_file::MULTI_CONTRACT_FILE; mod interface_naming; -use interface_naming::INTERFACE_FILE_NAMING; +use interface_naming::{INTERFACE_FILE_NAMING, INTERFACE_NAMING}; register_lints!( (PascalCaseStruct, early, (PASCAL_CASE_STRUCT)), @@ -33,5 +33,5 @@ register_lints!( (NamedStructFields, late, (NAMED_STRUCT_FIELDS)), (UnsafeCheatcodes, early, (UNSAFE_CHEATCODE_USAGE)), (MultiContractFile, early, (MULTI_CONTRACT_FILE)), - (InterfaceFileNaming, early, (INTERFACE_FILE_NAMING)) + (InterfaceFileNaming, early, (INTERFACE_FILE_NAMING, INTERFACE_NAMING)) ); diff --git a/crates/lint/testdata/SoloInterfaces.sol b/crates/lint/testdata/SoloInterfaces.sol index a343179a52745..f9cc2ee8aa235 100644 --- a/crates/lint/testdata/SoloInterfaces.sol +++ b/crates/lint/testdata/SoloInterfaces.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.18; -interface ISoloInterfaces { +interface SoloInterfaces { function foo() external; } diff --git a/crates/lint/testdata/SoloInterfaces.stderr b/crates/lint/testdata/SoloInterfaces.stderr index 0e60cb652f6a7..999b814b2d24d 100644 --- a/crates/lint/testdata/SoloInterfaces.stderr +++ b/crates/lint/testdata/SoloInterfaces.stderr @@ -1,8 +1,16 @@ +note[interface-naming]: interface names should be prefixed with 'I' + --> ROOT/testdata/SoloInterfaces.sol:LL:CC + | +LL | interface SoloInterfaces { + | ^^^^^^^^^^^^^^ + | + = help: https://book.getfoundry.sh/reference/forge/forge-lint#interface-naming + note[multi-contract-file]: prefer having only one contract, interface or library per file --> ROOT/testdata/SoloInterfaces.sol:LL:CC | -LL | interface ISoloInterfaces { - | ^^^^^^^^^^^^^^^ +LL | interface SoloInterfaces { + | ^^^^^^^^^^^^^^ | = help: https://book.getfoundry.sh/reference/forge/forge-lint#multi-contract-file @@ -25,8 +33,8 @@ LL | interface I3 { note[interface-file-naming]: interface file names should be prefixed with 'I' --> ROOT/testdata/SoloInterfaces.sol:LL:CC | -LL | interface ISoloInterfaces { - | ^^^^^^^^^^^^^^^ +LL | interface SoloInterfaces { + | ^^^^^^^^^^^^^^ | = help: https://book.getfoundry.sh/reference/forge/forge-lint#interface-file-naming From 075310c1ec1106c6d817dcf3b16c7ca65098bd82 Mon Sep 17 00:00:00 2001 From: Milos Djurica Date: Mon, 24 Nov 2025 20:53:14 +0100 Subject: [PATCH 7/8] test: add cli tests --- crates/forge/tests/cli/lint.rs | 89 +++++++++++++++++++++++++++++----- 1 file changed, 78 insertions(+), 11 deletions(-) diff --git a/crates/forge/tests/cli/lint.rs b/crates/forge/tests/cli/lint.rs index 0d6e9d0b48e11..f4a9398e8e284 100644 --- a/crates/forge/tests/cli/lint.rs +++ b/crates/forge/tests/cli/lint.rs @@ -161,6 +161,23 @@ abstract contract AbstractStorage { function getBalance(address account) public view virtual returns (uint256); } + +interface Token { + function transfer(address to, uint256 amount) external returns (bool); +} +"#; + +const SOLO_INTERFACES: &str = r#" +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +interface ERC20 { + function balanceOf(address account) external view returns (uint256); +} + +interface IToken { + function transfer(address to, uint256 amount) external returns (bool); +} "#; forgetest!(can_use_config, |prj, cmd| { @@ -260,10 +277,11 @@ forgetest!(multi_contract_file_no_exceptions, |prj, cmd| { let output = cmd.arg("lint").assert_success(); let stderr = String::from_utf8_lossy(&output.get_output().stderr); - // Should see 8 instances of multi-contract-file lint - assert_eq!(stderr.matches("note[multi-contract-file]").count(), 8); + // Should see 9 instances of multi-contract-file lint + assert_eq!(stderr.matches("note[multi-contract-file]").count(), 9); assert!(stderr.contains("IToken")); assert!(stderr.contains("IERC20")); + assert!(stderr.contains("Token")); assert!(stderr.contains("MathLib")); assert!(stderr.contains("StringLib")); assert!(stderr.contains("BaseContract")); @@ -281,6 +299,7 @@ forgetest!(multi_contract_file_interface_exception, |prj, cmd| { prj.update_config(|config| { config.lint = LinterConfig { lint_on_build: true, + exclude_lints: vec!["interface-naming".into()], lint_specific: LintSpecificConfig { multi_contract_file_exceptions: vec![ContractException::Interface], ..Default::default() @@ -292,10 +311,11 @@ forgetest!(multi_contract_file_interface_exception, |prj, cmd| { let output = cmd.arg("lint").assert_success(); let stderr = String::from_utf8_lossy(&output.get_output().stderr); - // Should see 6 instances (2 interfaces excluded) + // Should see 6 instances (3 interfaces excluded: IToken, IERC20, Token) assert_eq!(stderr.matches("note[multi-contract-file]").count(), 6); assert!(!stderr.contains("IToken")); assert!(!stderr.contains("IERC20")); + assert!(!stderr.contains("Token")); assert!(stderr.contains("MathLib")); assert!(stderr.contains("FirstContract")); }); @@ -305,7 +325,7 @@ forgetest!(multi_contract_file_library_exception, |prj, cmd| { prj.add_source("MixedFile", MULTI_CONTRACT_FILE); - // With library exception, should flag 6 items + // With library exception, should flag 7 items prj.update_config(|config| { config.lint = LinterConfig { lint_on_build: true, @@ -320,8 +340,8 @@ forgetest!(multi_contract_file_library_exception, |prj, cmd| { let output = cmd.arg("lint").assert_success(); let stderr = String::from_utf8_lossy(&output.get_output().stderr); - // Should see 6 instances (2 libraries excluded) - assert_eq!(stderr.matches("note[multi-contract-file]").count(), 6); + // Should see 7 instances (2 libraries excluded) + assert_eq!(stderr.matches("note[multi-contract-file]").count(), 7); assert!(stderr.contains("IToken")); assert!(!stderr.contains("MathLib")); assert!(!stderr.contains("StringLib")); @@ -333,7 +353,7 @@ forgetest!(multi_contract_file_abstract_exception, |prj, cmd| { prj.add_source("MixedFile", MULTI_CONTRACT_FILE); - // With abstract contract exception, should flag 6 items + // With abstract contract exception, should flag 7 items prj.update_config(|config| { config.lint = LinterConfig { lint_on_build: true, @@ -348,13 +368,13 @@ forgetest!(multi_contract_file_abstract_exception, |prj, cmd| { let output = cmd.arg("lint").assert_success(); let stderr = String::from_utf8_lossy(&output.get_output().stderr); - // Should see 6 instances (2 abstract contracts excluded) - assert_eq!(stderr.matches("note[multi-contract-file]").count(), 6); + // Should see 7 instances (2 abstract contracts excluded) + assert_eq!(stderr.matches("note[multi-contract-file]").count(), 7); assert!(stderr.contains("IToken")); assert!(stderr.contains("MathLib")); + assert!(stderr.contains("FirstContract")); assert!(!stderr.contains("BaseContract")); assert!(!stderr.contains("AbstractStorage")); - assert!(stderr.contains("FirstContract")); }); forgetest!(multi_contract_file_multiple_exceptions, |prj, cmd| { @@ -366,6 +386,7 @@ forgetest!(multi_contract_file_multiple_exceptions, |prj, cmd| { prj.update_config(|config| { config.lint = LinterConfig { lint_on_build: true, + exclude_lints: vec!["interface-naming".into()], // Exclude interface-naming to avoid Token being flagged lint_specific: LintSpecificConfig { multi_contract_file_exceptions: vec![ ContractException::Interface, @@ -380,9 +401,11 @@ forgetest!(multi_contract_file_multiple_exceptions, |prj, cmd| { let output = cmd.arg("lint").assert_success(); let stderr = String::from_utf8_lossy(&output.get_output().stderr); - // Should see 4 instances (2 interfaces + 2 libraries excluded) + // Should see 4 instances (3 interfaces + 2 libraries excluded) assert_eq!(stderr.matches("note[multi-contract-file]").count(), 4); assert!(!stderr.contains("IToken")); + assert!(!stderr.contains("IERC20")); + assert!(!stderr.contains("Token")); assert!(!stderr.contains("MathLib")); assert!(stderr.contains("BaseContract")); assert!(stderr.contains("FirstContract")); @@ -480,6 +503,50 @@ multi_contract_file_exceptions = ["interface", "library", "abstract_contract"] assert!(stderr.contains("SecondContract")); }); +forgetest!(interface_naming_fails_for_non_prefixed, |prj, cmd| { + prj.add_source("MixedFile", MULTI_CONTRACT_FILE); + + prj.update_config(|config| { + config.lint = LinterConfig { + severity: vec![], + exclude_lints: vec!["multi-contract-file".into()], + ignore: vec![], + lint_on_build: true, + ..Default::default() + }; + }); + + let output = cmd.arg("lint").assert_success(); + let stderr = String::from_utf8_lossy(&output.get_output().stderr); + + // Should flag only the interface that doesn't start with 'I': Token + assert_eq!(stderr.matches("note[interface-naming]").count(), 1); + assert!(stderr.contains("Token")); +}); + +forgetest!(interface_file_naming_fails_for_non_prefixed_file, |prj, cmd| { + prj.add_source("SoloInterfaces", SOLO_INTERFACES); + + prj.update_config(|config| { + config.lint = LinterConfig { + severity: vec![], + exclude_lints: vec!["multi-contract-file".into()], + ignore: vec![], + lint_on_build: true, + ..Default::default() + }; + }); + + let output = cmd.arg("lint").assert_success(); + let stderr = String::from_utf8_lossy(&output.get_output().stderr); + + // File name "SoloInterfaces" doesn't start with 'I', so interface-file-naming should trigger + assert_eq!(stderr.matches("note[interface-file-naming]").count(), 1); + // ERC20 is not prefixed with 'I', so interface-naming should trigger + assert_eq!(stderr.matches("note[interface-naming]").count(), 1); + assert!(stderr.contains("ERC20")); +}); + forgetest!(can_override_config_severity, |prj, cmd| { prj.add_source("ContractWithLints", CONTRACT); prj.add_source("OtherContractWithLints", OTHER_CONTRACT); From 0a2c6187b513aab5d759c3399d85ce92f8e05542 Mon Sep 17 00:00:00 2001 From: Milos Djurica Date: Mon, 24 Nov 2025 21:04:01 +0100 Subject: [PATCH 8/8] style: remove comment --- crates/forge/tests/cli/lint.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/forge/tests/cli/lint.rs b/crates/forge/tests/cli/lint.rs index f4a9398e8e284..86f67d5966df6 100644 --- a/crates/forge/tests/cli/lint.rs +++ b/crates/forge/tests/cli/lint.rs @@ -386,7 +386,7 @@ forgetest!(multi_contract_file_multiple_exceptions, |prj, cmd| { prj.update_config(|config| { config.lint = LinterConfig { lint_on_build: true, - exclude_lints: vec!["interface-naming".into()], // Exclude interface-naming to avoid Token being flagged + exclude_lints: vec!["interface-naming".into()], lint_specific: LintSpecificConfig { multi_contract_file_exceptions: vec![ ContractException::Interface,