Skip to content

Comments

fix: deduplicate submodule status check logic (foundry-rs#13010) #317

Merged
Dargon789 merged 8 commits intoDargon789:masterfrom
foundry-rs:master
Jan 8, 2026
Merged

fix: deduplicate submodule status check logic (foundry-rs#13010) #317
Dargon789 merged 8 commits intoDargon789:masterfrom
foundry-rs:master

Conversation

@Dargon789
Copy link
Owner

@Dargon789 Dargon789 commented Jan 8, 2026

Motivation

Solution

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes
    68844bf

Summary by Sourcery

Deduplicate git submodule status checking and streamline related utilities while tightening Solar source selection and updating dependency advisories.

Enhancements:

  • Reuse the existing missing-dependencies check for detecting uninitialized git submodules instead of duplicating git submodule status parsing logic.
  • Extend Solar source extraction from compile output to accept ignored paths so imports under lint/build ignore lists are not passed to Solar.
  • Remove unused helper methods and utilities across CLI, chisel, benches, common contracts/serde, EVM args, and cheatcodes to reduce dead code.

Build:

  • Update cargo-deny advisory ignore list to reflect current RUSTSEC advisories and unused dependencies.

@codesandbox
Copy link

codesandbox bot commented Jan 8, 2026

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@vercel
Copy link

vercel bot commented Jan 8, 2026

@maximevtush is attempting to deploy a commit to the Foundry development Team on Vercel.

A member of the Team first needs to authorize it.

@sourcery-ai
Copy link

sourcery-ai bot commented Jan 8, 2026

Reviewer's Guide

Refactors and cleans up various utilities across the codebase while deduplicating the git submodule status logic and improving Solar source selection for lint/build flows. Also updates security advisories configuration.

Sequence diagram for deduplicated git submodule status check

sequenceDiagram
    actor Developer
    participant ForgeCLI
    participant GitUtils
    participant GitCommand as GitCommandRunner

    Developer->>ForgeCLI: run command requiring submodules_uninitialized
    ForgeCLI->>GitUtils: submodules_uninitialized()
    GitUtils->>GitUtils: has_missing_dependencies(empty_iterator)
    GitUtils->>GitCommand: cmd().args(["submodule", "status"])
    GitCommand-->>GitUtils: stdout
    GitUtils->>GitUtils: parse lines, check for '-' prefix
    GitUtils-->>ForgeCLI: bool (uninitialized_submodules)
    ForgeCLI-->>Developer: report based on uninitialized_submodules
Loading

Sequence diagram for Solar source selection with ignored paths in lint/build

sequenceDiagram
    actor User
    participant ForgeCLI
    participant LintArgs
    participant BuildArgs
    participant ProjectCompiler
    participant BuildUtils as BuildUtils_get_solar_sources
    participant Solar

    User->>ForgeCLI: forge lint
    ForgeCLI->>LintArgs: execute()
    LintArgs->>ProjectCompiler: compile(input_files)
    ProjectCompiler-->>LintArgs: ProjectCompileOutput
    LintArgs->>BuildUtils: get_solar_sources_from_compile_output(config, output, target_paths, ignored_paths)
    BuildUtils->>BuildUtils: collect target source_paths
    BuildUtils->>BuildUtils: traverse imports, skip is_ignored(path)
    BuildUtils-->>LintArgs: SolcVersionedInput (filtered sources)
    LintArgs->>Solar: run lint with SolcVersionedInput
    Solar-->>User: lint results

    User->>ForgeCLI: forge build
    ForgeCLI->>BuildArgs: execute()
    BuildArgs->>ProjectCompiler: compile(input_files)
    ProjectCompiler-->>BuildArgs: ProjectCompileOutput
    BuildArgs->>BuildUtils: get_solar_sources_from_compile_output(config, output, target_paths, ignored_paths=None)
    BuildUtils-->>BuildArgs: SolcVersionedInput
    BuildArgs->>Solar: optional Solar checks
    Solar-->>User: build diagnostics
Loading

Class diagram for updated Solar source selection utilities

classDiagram
    class BuildUtils {
        +get_solar_sources_from_compile_output(config: Config, output: ProjectCompileOutput, target_paths: Option~PathBuf[]~, ignored_paths: Option~PathBuf[]~) Result~SolcVersionedInput~
        +configure_pcx_from_compile_output(pcx: PcxConfig, config: Config, output: ProjectCompileOutput, target_paths: Option~PathBuf[]~) Result~()~
    }

    class LintArgs {
        +execute() Result~()~
        -ignored: Vec~PathBuf~
    }

    class BuildArgs {
        +execute() Result~()~
    }

    class ProjectCompiler {
        +new() ProjectCompiler
        +files(files: Vec~PathBuf~) ProjectCompiler
        +compile(project: Project) ProjectCompileOutput
    }

    class ProjectCompileOutput {
        +graph() DependencyGraph
    }

    class DependencyGraph {
        +imports(path: Path) Path[]
    }

    class SolcVersionedInput {
        +input: SolcInput
    }

    class SolcInput {
        +sources: HashMap~PathBuf, SourceUnit~
    }

    LintArgs --> BuildUtils : uses get_solar_sources_from_compile_output
    BuildArgs --> BuildUtils : uses get_solar_sources_from_compile_output
    BuildUtils --> ProjectCompileOutput : reads imports via graph()
    ProjectCompileOutput --> DependencyGraph : returns
    SolcVersionedInput --> SolcInput : contains
Loading

File-Level Changes

Change Details Files
Remove unused helper methods and dead APIs to simplify public surfaces in several crates.
  • Delete SessionSource::clear_global and clear_contract since they are unused
  • Remove to_script_source constructor from SessionSource which is no longer referenced
  • Drop get_benchmark_versions and setup_benchmark_repos helpers from benches library
  • Remove get_cached_entry_by_name helper from CLI compiler utilities
  • Delete from_int_or_hex_opt deserializer from serde helpers
  • Remove ContractsByArtifact::find_abi_by_name_or_identifier helper
  • Drop EvmArgs::ensure_fork_url helper as unused
  • Remove Wallets::add_private_key wrapper in cheatcodes
crates/chisel/src/source.rs
benches/src/lib.rs
crates/cli/src/utils/cmd.rs
crates/common/src/serde_helpers.rs
crates/common/src/contracts.rs
crates/cli/src/opts/evm.rs
crates/cheatcodes/src/script.rs
Extend Solar source extraction to support ignored paths and wire it into build/lint commands.
  • Add ignored_paths parameter and is_ignored helper to get_solar_sources_from_compile_output
  • Skip imports that are in the ignored set when traversing the compile graph
  • Update configure_pcx_from_compile_output to pass a None ignored list
  • Update forge lint and build commands to call get_solar_sources_from_compile_output with the new parameter, providing ignored paths for lint and None for build
crates/cli/src/opts/build/utils.rs
crates/forge/src/cmd/lint.rs
crates/forge/src/cmd/build.rs
Deduplicate git submodule status handling by reusing has_missing_dependencies in submodules_uninitialized.
  • Change submodules_uninitialized to delegate to has_missing_dependencies with an empty iterator instead of shelling out to git submodule status directly
  • Add comment documenting intent to keep behavior consistent and avoid duplicate "-prefix" parsing logic
crates/cli/src/utils/mod.rs
Update cargo-deny configuration to track current advisories.
  • Replace ignored advisory RUSTSEC-2025-0137 with new advisories RUSTSEC-2025-0141 and RUSTSEC-2026-0002 along with rationale comments
deny.toml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@snyk-io
Copy link

snyk-io bot commented Jan 8, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@gemini-code-assist
Copy link

Summary of Changes

Hello @Dargon789, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request primarily focuses on refining the codebase through significant cleanup and refactoring efforts. It removes a substantial amount of unused and redundant code across various modules, improving maintainability and reducing the project's footprint. A notable functional enhancement is the introduction of an ignored_paths parameter to the source collection utility, providing more granular control over which files are processed during compilation. These changes collectively aim to streamline the project's internal logic and dependencies.

Highlights

  • Code Deduplication: Refactored the submodules_uninitialized function in crates/cli/src/utils/mod.rs to reuse existing logic from has_missing_dependencies, eliminating redundant code for checking submodule status.
  • Code Cleanup and Removal of Unused Logic: Removed several unused functions and imports across multiple crates, including get_benchmark_versions and setup_benchmark_repos in benches/src/lib.rs, add_private_key in crates/cheatcodes/src/script.rs, clear_global, clear_contract, and to_script_source in crates/chisel/src/source.rs, ensure_fork_url in crates/cli/src/opts/evm.rs, get_cached_entry_by_name in crates/cli/src/utils/cmd.rs, find_abi_by_name_or_identifier in crates/common/src/contracts.rs, and from_int_or_hex_opt in crates/common/src/serde_helpers.rs.
  • Enhanced Source Collection with Ignored Paths: Updated the get_solar_sources_from_compile_output utility in crates/cli/src/opts/build/utils.rs to accept an ignored_paths parameter, allowing specific imports to be skipped during source collection. This new functionality is utilized in the forge lint command.
  • Dependency Management: Updated the deny.toml file to ignore new RUSTSEC advisories, specifically RUSTSEC-2025-0141 (bincode) and RUSTSEC-2026-0002 (lru), reflecting ongoing dependency maintenance.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • In get_solar_sources_from_compile_output, the is_ignored check uses exact Path equality; if ignored_paths are specified in a different form than output.graph().imports() (e.g., relative vs absolute, or directory-level ignores), this may not behave as intended—consider normalizing paths and/or supporting prefix/directory-based matching.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `get_solar_sources_from_compile_output`, the `is_ignored` check uses exact `Path` equality; if `ignored_paths` are specified in a different form than `output.graph().imports()` (e.g., relative vs absolute, or directory-level ignores), this may not behave as intended—consider normalizing paths and/or supporting prefix/directory-based matching.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request is a nice cleanup and refactoring effort. It successfully deduplicates the submodule status check logic by reusing the has_missing_dependencies function, which is the main goal of this PR. A significant amount of unused code has been removed across various crates, which improves the overall maintainability of the codebase. Additionally, it introduces a useful feature to get_solar_sources_from_compile_output to accept ignored paths, which is correctly utilized in the linting command. The dependency advisory list in deny.toml has also been updated. I have one minor suggestion for improving code conciseness.

Repository owner deleted a comment from vercel bot Jan 8, 2026
@Dargon789 Dargon789 merged commit 74d697c into Dargon789:master Jan 8, 2026
3 of 8 checks passed
@Dargon789 Dargon789 linked an issue Jan 16, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

5 participants