A041 DuplicateLocalName

[0xGeorgii]

Closes #217

Confidence Score: 4/5

Safe to merge; the new rule correctly replaces a codegen panic with a structured diagnostic, and all collision asserts in wasm-codegen are now properly attributed to A041.

The rule implementation faithfully mirrors pre_scan_locals' DFS descent, the per-body accumulator is fresh and correctly scoped, and the first-location tracking behaves as the triple-duplicate test verifies. The only finding is a minor test-helper redundancy in rules_a041.rs where the same analysis pipeline is invoked twice for the same source string within a single test.

tests/src/analysis/rules_a041.rs — the triple-duplicate test runs the analysis pipeline twice unnecessarily.

Important Files Changed

Filename	Overview
core/analysis/src/rules/duplicate_local_name.rs	New A041 rule implementation; correctly mirrors pre_scan_locals DFS via walk_block_stmts, fresh per-body accumulator, and proper first/repeat location tracking
core/analysis/src/errors.rs	Adds DuplicateLocalName variant to AnalysisDiagnostic, wires into location() and rule_id(); diagnostic template correctly embeds first_location in message text
core/wasm-codegen/src/compiler.rs	Updates four assert messages to reference A041; adds two previously-absent collision asserts for array_offsets and struct_offsets insertions; all consistent with A041 covering compound locals
tests/src/analysis/rules_a041.rs	Comprehensive integration tests; minor: count_a041 call in triple-duplicate test is redundant with the subsequent a041_diags().len() check, running the analysis pipeline twice for the same input
tests/src/codegen/wasm/negative.rs	Adds backstop test verifying that codegen's assert fires on the no-analysis path for the original panic-inducing repro shape
core/analysis/src/rules/mod.rs	Adds duplicate_local_name module and registers DuplicateLocalName in all_rules(); straightforward plumbing change
core/analysis/src/lib.rs	Adds A041 doc comment and DuplicateLocalName to the exhaustive-diagnostic smoke-test list; no logic change

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Source file] --> B[Type Checker]
    B -->|"rejects ancestor/parameter shadowing (VariableShadowed)"| ERR1[Type error]
    B -->|"accepts sibling-block reuse (different scopes)"| C[Analysis pass]
    C --> D{A041: DuplicateLocalName}
    D -->|"first_seen map: name not yet seen"| E[Record first_location]
    D -->|"first_seen map: name already seen"| F["Emit diagnostic\n(location = duplicate site,\nfirst_location = first site)"]
    F --> ERR2[Analysis error - stops pipeline]
    E --> G{More declarations?}
    G -->|Yes| D
    G -->|No| H[wasm-codegen]
    H --> I[pre_scan_locals DFS]
    I -->|"defense-in-depth assert (A041 invariant)"| J{Name already in locals_map?}
    J -->|"No (expected path)"| K[Assign WASM local index]
    J -->|"Yes (should never happen after A041)"| L[assert! panic]
    K --> M[Compiled WASM output]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[Source file] --> B[Type Checker]
    B -->|"rejects ancestor/parameter shadowing (VariableShadowed)"| ERR1[Type error]
    B -->|"accepts sibling-block reuse (different scopes)"| C[Analysis pass]
    C --> D{A041: DuplicateLocalName}
    D -->|"first_seen map: name not yet seen"| E[Record first_location]
    D -->|"first_seen map: name already seen"| F["Emit diagnostic\n(location = duplicate site,\nfirst_location = first site)"]
    F --> ERR2[Analysis error - stops pipeline]
    E --> G{More declarations?}
    G -->|Yes| D
    G -->|No| H[wasm-codegen]
    H --> I[pre_scan_locals DFS]
    I -->|"defense-in-depth assert (A041 invariant)"| J{Name already in locals_map?}
    J -->|"No (expected path)"| K[Assign WASM local index]
    J -->|"Yes (should never happen after A041)"| L[assert! panic]
    K --> M[Compiled WASM output]

Loading

Comments Outside Diff (1)

1. tests/src/analysis/rules_a041.rs, line 626-633 (link)

   Redundant analysis run in triple-duplicate test

   count_a041(source) and a041_diags(source) both run the full parse → type-check → analyze pipeline on the same source string. The assert_eq!(count_a041(source), 2, …) assertion is then immediately re-checked as assert_eq!(diags.len(), 2, …) from the result of the second run, so the first call — and its pipeline invocation — is dead work. Calling a041_diags once and asserting on .len() is sufficient.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

_{Reviews (1): Last reviewed commit: "A041 DuplicateLocalName" | Re-trigger Greptile}

[codecov]

Codecov Report

❌ Patch coverage is 98.88889% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
core/analysis/src/rules/duplicate_local_name.rs	97.14%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!