Skip to content

Add --fix to apply machine-applicable suggestions #204

Description

@rohaquinlop

Problem

Applicability::MachineApplicable ("Safe to apply") marks suggestions with high confidence in the generated code, but nothing backs that confidence up: today a user must hand-copy suggestion.replacement out of the CLI output or JSON. Three rules produce a concrete suggestion at that tier: C002 (loop guards), C005 (extract predicate), C007 (collapsible if). --fix would apply it directly to the source file, the way ruff --fix or cargo clippy --fix do.

Why this is nontrivial

  • C002 and C007 are simple span replacementssuggestion.replacement is a drop-in substitute for source_lines[plan.line_start - 1 : plan.line_end]. The plans already carry exact line_start/line_end/column_start.
  • C005 is structurally different. It generates a whole new helper function definition and requires editing the call site to invoke it — two separate edits in two different places, not one span swap. It needs its own mechanism; don't assume one universal splice helper covers all three rules.
  • Multiple plans in one file shift line numbers. If a file gets two fixes applied (even in different functions), plans computed against the original source get stale line_start/line_end after the first edit. Applying fixes bottom-to-top (highest line number first) avoids invalidating not-yet-applied plans.
  • Already-resolved overlaps help here — the registry guarantees surfaced plans for a single function don't overlap each other, but plans from different functions in the same file are independent and still need the bottom-to-top ordering.
  • Validation before write. A patched file must still parse as valid Python before it's written to disk — a bad patch must never corrupt the user's file. Write to a temp file, parse-check it, then atomically rename over the original.

Open questions

  • CLI shape. --fix as its own flag (implying --suggest-refactors), or --suggest-refactors --fix requiring both explicitly? Dry-run/preview mode (--fix --diff printing a unified diff without writing) before or alongside a real --fix?
  • Rule scope. Ship for C002/C007 (simple span replacement) first and leave C005 (multi-location patch) for a follow-up? Or hold the whole feature until all three work?
  • Safety gate. Refuse to run against a dirty git working tree (uncommitted changes) so a bad fix is always one git checkout away from undone? Or is that too opinionated for a linter?
  • Scope of a single --fix invocation. All applicable plans across all analyzed files in one pass, or one plan at a time with confirmation? Per-rule filtering (--fix --only C007) once per-rule suppression (--allow/--deny) exists? Building both around a shared "select which rules apply" mechanism is probably right.
  • Idempotency. After a fix is applied and the file is re-analyzed, the original finding should no longer fire. Is a regression test needed asserting "fixed code produces zero new plans of the same kind"?
  • Interaction with MaybeIncorrect. That tier is currently unused by any rule. In clippy, MaybeIncorrect is exactly the tier --fix does not touch by default. If a rule ever moves to that tier, --fix must never apply it without an explicit opt-in flag.
  • Where the patching logic lives. Rust (alongside the rule that generates the suggestion, so span math and replacement generation stay co-located) or Python (CLI-level file I/O, keeping Rust purely analytical)? Precedent in this codebase leans Rust-does-analysis / Python-does-I/O-and-rendering — but C005's two-location patch may be easier to get right in Rust where it's generated.

Acceptance criteria (draft)

  • --fix applies at least C002 and C007 suggestions to real files; the patched file parses successfully and re-analyzing it shows the complexity actually dropped by (at least) the plan's estimated_reduction.
  • C005's two-location patch either ships in the same pass or is explicitly out of scope for the first cut, with that decision stated up front.
  • Multiple fixes in the same file apply correctly together (bottom-to-top or equivalent ordering verified).
  • A fix that would produce invalid Python is never written to disk — caught by a parse-check before the atomic write, with a test forcing that failure path.
  • Existing --suggest-refactors output is completely unaffected when --fix is not passed.
  • pytest tests/ -q and cargo test --features python both green; wasm build unaffected if any shared code changes.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions