Skip to content

feat(forge): run lint on forge build #10748

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 4 commits into from
Jun 11, 2025

Conversation

yash-atreya
Copy link
Member

Summary

This PR integrates the SolidityLinter into the forge build command to provide automatic linting feedback
before compilation. This is similar to how cargo suggests lints on cargo build.

Changes

Core Integration

  • Added linter support to ProjectCompiler: Optional SolidityLinter field with .linter() builder method
  • Pre-compilation linting: Linter runs before compilation to catch issues early
  • Configuration integration: Uses foundry.toml [lint] section settings (severity, exclude_lints)
  • Quiet mode support: Respects --quiet flag to suppress linting output
  • JSON output: Uses JSON formatter when --json flag is provided

Implementation Details

  • Linter runs on all .sol files in the project root
  • Only enabled for Solidity projects (project.compiler.solc.is_some())
  • Filters files using source_files_iter for efficient file discovery
  • Maintains backward compatibility - existing workflows unchanged

Test Coverage

Added comprehensive tests in crates/forge/tests/cli/lint.rs:

  • ✅ build_runs_linter_by_default - Verifies linting output appears before compilation
  • ✅ build_respects_quiet_flag_for_linting - Ensures no linting output with --quiet
  • ✅ build_with_json_uses_json_linter_output - Validates JSON formatter usage

🤖 Generated with https://claude.ai/code

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

Add automatic linting during forge build to provide early feedback on
Solidity code quality and security issues.

Changes:
- Add optional linter field to ProjectCompiler with builder method
- Run linting before compilation when linter is configured
- Respect --quiet flag to suppress linting output
- Configure linter from foundry.toml LinterConfig settings
- Support JSON output format for linting diagnostics
- Add comprehensive tests for build-time linting behavior

The linter runs before compilation and uses all settings from the [lint]
section in foundry.toml including severity filtering and lint exclusions.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@yash-atreya yash-atreya changed the title feat: integrate SolidityLinter into forge build command feat(forge): run lint on forge build Jun 10, 2025
yash-atreya and others added 3 commits June 10, 2025 15:34
Add new boolean config option `lint_on_build` in LinterConfig that allows
users to disable automatic linting during `forge build` while maintaining
the improved developer experience by default.

Changes:
- Add `lint_on_build` field to LinterConfig (defaults to true)
- Update forge build to check lint_on_build before configuring linter
- Add test for lint_on_build = false behavior
- Update existing tests to include new config field
- Modify can_build_after_failure test to disable linting

Users can now disable build-time linting in foundry.toml:
```toml
[lint]
lint_on_build = false
```

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
The test_default_config was failing after adding the lint_on_build property to LintConfig.
Updated the expected output snapshots to include the new property with its default value of true.

- Updated TOML format snapshot to include `lint_on_build = true` in [lint] section
- Updated JSON format snapshot to include `"lint_on_build": true` in lint object

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@yash-atreya yash-atreya marked this pull request as ready for review June 10, 2025 15:09
Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

Copy link
Member

@zerosnacks zerosnacks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍

@grandizzy grandizzy merged commit d544ae2 into master Jun 11, 2025
22 checks passed
@grandizzy grandizzy deleted the yash/integrate-linter-into-forge-build branch June 11, 2025 07:31
@github-project-automation github-project-automation bot moved this to Done in Foundry Jun 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants