Skip to content

feat: add nargo check --fix to remove unused imports and unnecessary mut - #13390

Draft
TomAFrench wants to merge 2 commits into
masterfrom
tf/remove-unused-imports
Draft

feat: add nargo check --fix to remove unused imports and unnecessary mut#13390
TomAFrench wants to merge 2 commits into
masterfrom
tf/remove-unused-imports

Conversation

@TomAFrench

@TomAFrench TomAFrench commented Jul 22, 2026

Copy link
Copy Markdown
Member

Description

Problem

Resolves the manual-cleanup gap around warnings whose fix is mechanical: the compiler knows exactly where every unused import and unnecessary mut is, but fixing them meant editing them one by one (or one use statement at a time via the LSP quick fix).

Summary

Adds a compiler-level rewrite that applies removal-only fixes in one go, exposed as nargo check --fix (hidden flag for now). Fixes never add code — they only delete or simplify what a warning points at. Two fixes are supported:

Unused imports (noirc_frontend::fix):

  • Matches imports by the same (Ident, Location) key the usage tracker records, so same-named imports never alias each other.
  • Handles complex composite forms: prunes entries from bracketed lists (use foo::{bar, spam, baz::{qux, corge}}use foo::{bar, baz::qux}), collapses lists left with a single entry (including through nesting), understands aliases (as b) and self imports (a list collapsing to bare self becomes use foo;, not the invalid use foo::self;).
  • Fully-unused use items are deleted together with their line, and adjacent deletions / doubled-up blank lines are cleaned so no whitespace debris is left. Rewritten trees render in canonical single-line form; run nargo fmt afterwards for style.

Unnecessary mut modifiers:

  • Located via the typed VariableDoesNotNeedToBeMutable errors, exposed by a new check_crate_returning_frontend_errors entry point in noirc_driver (the existing check_crate delegates to it, behavior unchanged).
  • The fix collector is an AST visitor, so mut is dropped wherever the binding lives — let bindings, tuple-destructuring patterns, nested modules. The elaborator does not currently warn for never-mutated mut parameters; a characterization test documents that boundary and will flag if it changes.

CLI: nargo check --fix rewrites only files belonging to the package's own crate (dependencies are never touched), only when the check succeeds (no rewriting on top of compilation errors), and filters the warnings for the code it just fixed out of the report so nargo check doesn't warn about code that no longer exists.

LSP: the "Remove unused import" quick fix now delegates to the shared core instead of carrying its own copy of the tree-pruning logic (~100 lines deduplicated), and gains self-import removal in the process.

Deliberate limitation: single round, not a fixpoint

Resolving an import's path marks its first segment as used, so an import whose only consumer is another use statement is only reported unused after that use is removed and the program re-elaborated. Each invocation fixes exactly what the current compilation reports; re-running fixes the next level of such cascades. This is intentional (a fixpoint loop could re-elaborate many times) and is documented by the import_used_only_by_a_removed_import_is_only_removed_on_a_second_run test.

Test coverage

  • 17 frontend tests (compiler/noirc_frontend/src/tests/fix.rs) covering composite/bracketed pruning, brace collapse (incl. nested), whole-item deletion, aliases, self imports and self collapse, nested inline modules, visibility preservation, multi-line use items, adjacent deletions, the cascade-over-two-runs behaviour, mut removal from let and tuple-pattern bindings, combined import+mut fixing in one pass, the parameter-mut boundary, and the nothing-to-fix case.
  • New LSP test for self-import removal; existing quick-fix tests unchanged and passing.
  • Unit test for the fixed-warning filtering in check_cmd.rs (warnings at other locations and non-warning diagnostics survive).

Full noirc_frontend (2225) and noir_lsp (351) suites pass; fmt and clippy clean.

Adds a compiler-level rewrite that removes all imports reported unused by
the elaborator in one go, including complex composite forms: pruning
individual entries from bracketed lists (`use foo::{bar, baz}` ->
`use foo::bar`), collapsing nested lists, handling aliases and `self`
imports, and deleting fully-unused `use` items along with their line.

The core lives in `noirc_frontend::remove_unused_imports` and matches
imports by the same `(Ident, Location)` key the usage tracker records.
The LSP's "Remove unused import" quick fix now reuses this shared core
(gaining `self`-import support) instead of its own tree-pruning copy.

`nargo check --remove-unused-imports` rewrites only files belonging to
the package's own crate, and warnings for the imports it just removed
are filtered out of the report. Removal is deliberately a single round,
not a fixpoint: an import only consumed by another `use` statement's
path is revealed unused only after re-elaboration, so a second run may
remove more (documented by a dedicated test).
@TomAFrench
TomAFrench marked this pull request as draft July 22, 2026 14:30
Renames the flag from --remove-unused-imports to --fix and generalizes
the frontend rewrite (now `noirc_frontend::fix`) to apply any warning
fix that is a pure removal. Alongside unused imports it now drops `mut`
modifiers the elaborator reported as unnecessary, located via the typed
`VariableDoesNotNeedToBeMutable` errors that a new
`check_crate_returning_frontend_errors` driver entry point exposes.

The fix collector is now an AST visitor, since `mut` patterns live
inside function bodies rather than at item level. The elaborator does
not currently warn for never-mutated `mut` parameters; a
characterization test documents that boundary.
@TomAFrench TomAFrench changed the title feat: add nargo check --remove-unused-imports to prune unused imports feat: add nargo check --fix to remove unused imports and unnecessary mut Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant