fix(lint): lint only files that we build #11247

grandizzy · 2025-08-08T13:03:37Z

Motivation

closes bug(forge lint): lint only files that we build #11234
if forge build path then lint only path that we build

Solution

PR Checklist

Added Tests
Added Documentation
Breaking changes

…e build

0xrusowsky

the changes look good, but i am wondering if it is desirable to have this behavior? we already exclude gas and codesize lints from tests and scripts.

if we think its desirable (i am not against it), imo it would be better to configure it via a flag in foundry.toml, rather than users having to explicitly include tests and scripts.

this way users would easily know the behavior that they have configured.. having to pass the script/ and test/ paths every time, is more tedious than simply changing a config flag.

grandizzy · 2025-08-08T13:57:11Z

the changes look good, but i am wondering if it is desirable to have this behavior? we already

this way users would easily know the behavior that they have configured.. having to pass the script/ and test/ paths every time, is more tedious than simply changing a config flag.

yep, agree, this makes sense, so we will lint src, test and scripts by default but not libs. Also lint only files we build if they're specified Cc @aviggiano

aviggiano · 2025-08-08T14:06:57Z

Makes sense! Good feature

crates/forge/src/cmd/build.rs

DaniPopes · 2025-08-15T09:43:43Z

crates/forge/src/cmd/build.rs

@@ -118,13 +118,13 @@ impl BuildArgs {

        // Only run the `SolidityLinter` if there are no compilation errors
        if !output.output().errors.iter().any(|e| e.is_error()) {
-            self.lint(&project, &config).map_err(|err| eyre!("Lint failed: {err}"))?;
+            self.lint(&project, &config, &self.paths).map_err(|err| eyre!("Lint failed: {err}"))?;


can we not get the paths from the compiler output instead?

I am not sure we want this as will mean that we won't lint contracts that are already built, e.g. we will show lint errors only first time for forge build src/utils/BaseVault.sol but not on subsequent forge build src/utils/BaseVault.sol (same with forge build entire project). I think we want to always lint the files, wdyt?

that's true for output, however we can check the paths from artifacts_with_files().

with that we can simply pass the paths from artifacts_with_files() to lint and avoid having to input_files_iter

hm, isn't this the same, artifacts_with_files() returns cached and compiled artifacts, so if we call forge build src/utils/BaseVault.sol again (and BaseVault.sol is not recompiled) then we won't know to lint BaseVault.sol only, maybe I am missing something?

we can do both

@DaniPopes artifacts_with_files returns all artifacts including artifacts from deps while input_files_iter returns only src, test and scripts that we want to lint. Could use artifacts_with_files but then have to filter out deps artifacts - IMO is cleaner by input_files_iter but happy to change if you strongly think we should use artifacts_with_files. thank you!

I see, that is fine then

Co-authored-by: DaniPopes <[email protected]>

0xrusowsky

lgtm

zerosnacks

lgtm, makes sense 👍

DaniPopes · 2025-08-18T08:00:24Z

crates/forge/src/cmd/build.rs

@@ -118,13 +118,14 @@ impl BuildArgs {

        // Only run the `SolidityLinter` if there are no compilation errors
        if !output.output().errors.iter().any(|e| e.is_error()) {


the condition above can be merged with this one: config.lint.lint_on_build &&

yep, simplified in 7722419

* fix(lint): explicit message when lint on build failures (#11224) * fix(lint): lint only files that we build (#11247) * fix: force 4844 txtype in blobhashes setter (#11355) * test: add blobhashes repro * fix: force 4844 tx type * fix(forge): handle error if etherscan identifier cannot resolve config (#11356) * fix(forge): handle error if etherscan identifier cannot resolve config * warn on config failures * fix: disable tx gas limit cap (#11347) * fix(forge): write ordered deps in foundry.lock (#11360) * chore: fix clippy (#11361) * chore: bump version 1.3.2 (#11363) * chore: fix cargo deny - update slab to 0.4.11 --------- Co-authored-by: Matthias Seitz <[email protected]>

fix(lint): lint src if ignored not configured, lint only files that w…

36ad52d

…e build

github-project-automation bot added this to Foundry Aug 8, 2025

grandizzy marked this pull request as ready for review August 8, 2025 13:20

grandizzy requested review from DaniPopes, klkvr, mattsse, yash-atreya and zerosnacks as code owners August 8, 2025 13:20

grandizzy requested a review from 0xrusowsky August 8, 2025 13:20

0xrusowsky reviewed Aug 8, 2025

View reviewed changes

grandizzy marked this pull request as draft August 8, 2025 14:12

grandizzy changed the title ~~fix(lint): lint src only by default and files that we build~~ fix(lint): lint src/script/test only by default and files that we build Aug 8, 2025

grandizzy added 2 commits August 15, 2025 11:32

Merge branch 'master' into issue-11234

b105232

LInt only built files

10fbdc8

grandizzy changed the title ~~fix(lint): lint src/script/test only by default and files that we build~~ fix(lint): lint only files that we build Aug 15, 2025

grandizzy requested a review from 0xrusowsky August 15, 2025 09:22

grandizzy marked this pull request as ready for review August 15, 2025 09:22

DaniPopes requested changes Aug 15, 2025

View reviewed changes

grandizzy and others added 2 commits August 15, 2025 13:49

Update crates/forge/src/cmd/build.rs

ccea4c9

Co-authored-by: DaniPopes <[email protected]>

Update crates/forge/src/cmd/build.rs

2123e13

Co-authored-by: DaniPopes <[email protected]>

0xrusowsky previously approved these changes Aug 15, 2025

View reviewed changes

grandizzy added 2 commits August 18, 2025 10:15

Merge remote-tracking branch 'origin' into issue-11234

c5bb8df

Fmt

bb7987e

grandizzy dismissed 0xrusowsky’s stale review via bb7987e August 18, 2025 07:24

grandizzy requested review from 0xrusowsky and DaniPopes August 18, 2025 07:24

0xrusowsky previously approved these changes Aug 18, 2025

View reviewed changes

zerosnacks previously approved these changes Aug 18, 2025

View reviewed changes

DaniPopes reviewed Aug 18, 2025

View reviewed changes

Nit: simplify lint cond

7722419

grandizzy dismissed stale reviews from zerosnacks and 0xrusowsky via 7722419 August 18, 2025 08:09

0xrusowsky approved these changes Aug 18, 2025

View reviewed changes

grandizzy self-assigned this Aug 18, 2025

grandizzy moved this to Ready For Review in Foundry Aug 18, 2025

Merge branch 'master' into issue-11234

3e2a4a2

DaniPopes approved these changes Aug 19, 2025

View reviewed changes

0xrusowsky merged commit ead9577 into foundry-rs:master Aug 19, 2025
22 checks passed

github-project-automation bot moved this from Ready For Review to Done in Foundry Aug 19, 2025

grandizzy mentioned this pull request Aug 19, 2025

Forge build specific file fails in 1.3.1-stable #11352

Closed

2 tasks

grandizzy added a commit to grandizzy/foundry that referenced this pull request Aug 20, 2025

fix(lint): lint only files that we build (foundry-rs#11247)

bc4e7b9

grandizzy mentioned this pull request Aug 20, 2025

v1.3.2 backports #11364

Merged

3 tasks

grandizzy moved this from Done to Completed in Foundry Aug 25, 2025

		@@ -118,13 +118,14 @@ impl BuildArgs {

		// Only run the `SolidityLinter` if there are no compilation errors
		if !output.output().errors.iter().any(\|e\| e.is_error()) {

fix(lint): lint only files that we build #11247

fix(lint): lint only files that we build #11247

Uh oh!

Conversation

grandizzy commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Solution

PR Checklist

Uh oh!

0xrusowsky left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grandizzy commented Aug 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aviggiano commented Aug 8, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DaniPopes Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xrusowsky left a comment

Choose a reason for hiding this comment

Uh oh!

zerosnacks left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

grandizzy commented Aug 8, 2025 •

edited

Loading

0xrusowsky left a comment •

edited

Loading

grandizzy commented Aug 8, 2025 •

edited

Loading

DaniPopes Aug 18, 2025 •

edited

Loading

zerosnacks left a comment •

edited

Loading