fix: suppress false E609 for builtin ADTs shared across imported modules by aallan · Pull Request #409 · aallan/vera

aallan · 2026-03-28T17:55:30Z

Fixes #360.

Root cause

Every CodeGenerator registers Option, Result, Ordering, UrlParts, Tuple, MdInline, and MdBlock via _register_builtin_adts() — these are global infrastructure, not owned by any particular module. The collision detector in _register_modules iterated over each temp generator's _adt_layouts and attributed these builtins to whichever module was processed first. When a second module was processed, those same names appeared again and triggered E609.

Fix

Call self._register_builtin_adts() at the start of _register_modules to snapshot the set of builtin ADT names, then skip those names entirely in the ADT collision detection loop. User-defined ADT collisions (two modules both defining their own Color type) still produce E609 correctly — only the pre-registered builtins are exempted.

Test plan

vera run examples/modules.vera --fn clamp -- 100 0 42 returns 42 (was E609 × 7)
Regression test test_prelude_types_not_flagged_as_collision added to TestNameCollisionDetection
Existing E609/E610 collision tests still pass (user-defined ADT collisions still detected)
All 3,096 tests pass, all 22 pre-commit hooks pass

Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Corrected false-positive collision detection for builtin abstract data types appearing across imported modules.
Tests
- Added a regression test to ensure shared builtin abstract data types across imports are not flagged as collisions.
Documentation
- Updated testing metrics and removed the known-issue entry for duplicate prelude types.

Every CodeGenerator registers Option, Result, Ordering, UrlParts, Tuple, MdInline, and MdBlock via _register_builtin_adts(). When two modules are imported, the collision detector in _register_modules saw these types in both modules' _adt_layouts and emitted spurious E609 errors, preventing any program that imports from two different modules from running. Fix: call _register_builtin_adts() at the start of _register_modules to snapshot the builtin set, then skip those names in the ADT collision detection loop. User-defined ADT collisions (e.g. two modules both defining 'Color') still produce E609 correctly. Adds regression test and updates TESTING.md counts. Fixes #360. Co-Authored-By: Claude <noreply@anthropic.invalid>

coderabbitai · 2026-03-28T17:55:44Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2088bb3a-f314-46bb-bb83-597ad7491ece

📥 Commits

Reviewing files that changed from the base of the PR and between 3848f78 and edc05ae.

📒 Files selected for processing (2)

TESTING.md
tests/test_codegen_modules.py

📝 Walkthrough

Walkthrough

Pre-registers builtin prelude ADT names and skips them when harvesting ADT layouts from imported modules to avoid duplicate-prelude collision diagnostics (E609/E610). Adds a regression test and updates testing docs and KNOWN_ISSUES accordingly.

Changes

Cohort / File(s)	Summary
Codegen: builtin ADT handling `vera/codegen/modules.py`	Adds pre-registration of builtin ADTs (`self._register_builtin_adts()`), computes `builtin_adt_names` from `_adt_layouts`, and skips those names when harvesting ADT layouts from imported modules to prevent false E609/E610 collisions.
Regression test `tests/test_codegen_modules.py`	Adds `test_prelude_types_not_flagged_as_collision`, a cross-module regression test that ensures builtin prelude ADTs (e.g. `Option`) exported via multiple imports do not trigger E609 and that compilation succeeds.
Docs / Test metrics `TESTING.md`, `KNOWN_ISSUES.md`	Increments total documented test count in `TESTING.md` (3,095 → 3,096) and updates `test_codegen_modules.py` metrics (18 tests / 529 lines → 19 tests / 565 lines). Removes the “Duplicate prelude types across imported modules (E609)” entry from `KNOWN_ISSUES.md`.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

compiler, tests

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and precisely describes the main fix: suppressing false E609 errors for builtin ADTs shared across imported modules, which is the core change across all modified files.
Linked Issues check	✅ Passed	The PR fully addresses issue `#360` by preventing builtin ADTs from triggering false E609 collisions when shared across imported modules, as verified by the regression test and removal of the known issue.
Out of Scope Changes check	✅ Passed	All changes—builtin ADT pre-registration in modules.py, regression test addition, documentation updates—are directly scoped to fixing the E609 false-positive for builtin ADTs. No unrelated modifications present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/e609-prelude-type-collision

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2026-03-28T17:56:47Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.30%. Comparing base (bcf22f9) to head (edc05ae).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #409   +/-   ##
=======================================
  Coverage   90.30%   90.30%           
=======================================
  Files          49       49           
  Lines       19100    19104    +4     
  Branches      220      220           
=======================================
+ Hits        17248    17252    +4     
  Misses       1848     1848           
  Partials        4        4

Flag	Coverage Δ
javascript	`50.58% <ø> (ø)`
python	`95.31% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/test_codegen_modules.py`:
- Around line 453-479: Test test_prelude_types_not_flagged_as_collision
currently never references builtin ADTs, so strengthen it by changing the mod_a
and mod_b module source strings passed to self._resolved so they explicitly use
a builtin ADT (e.g., Option or Result) in the function signature or return
value; update the two _resolved calls for "mod_a" and "mod_b" used in
test_prelude_types_not_flagged_as_collision to declare/return or consume
Option/Result (so CodeGenerator._register_builtin_adts() initialization is
exercised when layouts for those ADTs are actually used), leaving the rest of
the test assertions intact.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 751f7ceb-be8b-452d-8001-4a1616312fd8

📥 Commits

Reviewing files that changed from the base of the PR and between bcf22f9 and 96b7282.

📒 Files selected for processing (3)

TESTING.md
tests/test_codegen_modules.py
vera/codegen/modules.py

Duplicate prelude types across imported modules (E609) is resolved by the _register_builtin_adts() pre-seeding in _register_modules. Co-Authored-By: Claude <noreply@anthropic.invalid>

@option

…gnatures Modules now explicitly return @option<Int> so that the builtin ADT layouts appear in each temp CodeGenerator — more clearly demonstrating the exact scenario that triggered the false positive (builtin ADTs showing up in two imported module layouts and being mistaken for user-defined collisions). Co-Authored-By: Claude <noreply@anthropic.invalid>

coderabbitai Bot reviewed Mar 28, 2026

View reviewed changes

Comment thread tests/test_codegen_modules.py

docs: remove #360 from KNOWN_ISSUES.md — fixed in this branch

3848f78

Duplicate prelude types across imported modules (E609) is resolved by the _register_builtin_adts() pre-seeding in _register_modules. Co-Authored-By: Claude <noreply@anthropic.invalid>

aallan mentioned this pull request Mar 28, 2026

Module-qualified call disambiguation via name mangling #187

Open

aallan merged commit 6ad5006 into main Mar 28, 2026
17 checks passed

aallan deleted the fix/e609-prelude-type-collision branch March 28, 2026 18:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: suppress false E609 for builtin ADTs shared across imported modules#409

fix: suppress false E609 for builtin ADTs shared across imported modules#409
aallan merged 3 commits into
mainfrom
fix/e609-prelude-type-collision

aallan commented Mar 28, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Mar 28, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Suggested labels

Uh oh!

codecov Bot commented Mar 28, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

aallan commented Mar 28, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Root cause

Fix

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested labels

Uh oh!

codecov Bot commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

aallan commented Mar 28, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Mar 28, 2026 •

edited

Loading

codecov Bot commented Mar 28, 2026 •

edited

Loading