fix(compile): #5984 — computed-key static fields leak synthetic name into cross-module externs

[proggeramlug]

Closes #5984. Found via continued real-world sst/opencode source-compile verification, after #5918/#5922/#5924/#5927/#5928 (PR #5923, merged) and #5928's follow-up fixes (PR #5980) cleared every prior wall — this is a genuinely new, previously-unreached bug that only surfaces once a program needs both the class with the computed field and imports it from a different module than where it's defined.

Bug

A computed-key static field (static [Symbol.for(...)] = init) gets a synthetic HIR name __computed_field_<span.lo>_<span.hi> (perry-hir/src/lower_decl/class_members.rs). module_globals_emit.rs's emission loop correctly skips creating a backing perry_static_<mod>__<class>__<field> global for these — computed fields are stored via a runtime side table instead (see the existing comment referencing #420/#894) — so the defining module never emits that symbol.

But the cross-module class-export metadata built in run_pipeline.rs (ImportedClass.static_field_names, populated at 7 call sites) collected every static field's .name, including the computed ones' synthetic placeholder, with no filter. An importing module took that list at face value and declared an external global reference for the synthetic name — which the source module never defines, producing an undefined-symbol link error.

This is invisible for a same-module class (no cross-module declaration needed) or an unimported one, which is presumably why drizzle-orm's pervasive static readonly [entityKind]: string = "ClassName" tagging pattern (used on dozens of classes) never surfaced this before — it takes a subclass or consumer of the class in a different file that also needs the class's other, non-computed statics to pull the whole ImportedClass metadata across the module boundary.

Fix

Filter out computed-key fields (f.key_expr.is_none()) before collecting static_field_names, matching the filter module_globals_emit.rs already applies on the defining side. Same one-line change at all 7 call sites.

Testing

• New regression fixture (test_issue_5984_computed_static_field_cross_module.ts): base class with a computed-key static field in one module, subclass overriding it in another that also uses the class's instance method — reproduces the undefined-symbol error pre-fix, links and runs correctly post-fix, output verified byte-for-byte against Node.
• Full existing namespace/effect regression suite (16 fixtures) — unaffected, all still MATCH.
• cargo test --release -p perry --bin perry: 669/670 passed (one known flaky, resource-contention-only test — install::lifecycle::run_lifecycle_executes_script — confirmed unrelated and passing in isolation).
• cargo test --release -p perry-codegen --lib: 152/152 passed.
• cargo test --release -p perry --test issue_5920_wrapper_bundled_runtime_async_starvation: passes (with PERRY_LLVM_* env vars set).
• Verified against the real opencode compile: this exact class of "undefined symbol referencing a computed-field synthetic name" (10+ distinct classes across effect-drizzle-sqlite and drizzle-orm itself, including SQLiteEffectSession, SQLiteEffectPreparedQuery, SQLiteEffectTransaction, drizzle's Relation/Relations/Cache/alias-proxy classes, and more) is completely gone after this fix — the compile proceeds past this wall entirely to a different, unrelated undefined symbol (tracked separately, not part of this fix's scope).

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of computed static properties on imported and re-exported classes, including cases with inheritance and unresolved type references.
  • Static field lists now exclude keyed entries where appropriate, helping preserve accurate class metadata.

• Tests

  • Added coverage for cross-module inheritance with computed static fields to verify expected runtime behavior.

[coderabbitai]

📝 Walkthrough

Walkthrough

Modifies ImportedClass construction in run_pipeline.rs to exclude static fields with computed keys (key_expr present) from static_field_names across seven code paths. Adds cross-module fixture files (entity.ts, base.ts) and a test file exercising computed static fields across a class hierarchy.

Changes

Computed static field filtering

Layer / File(s)	Summary
Filter static_field_names in ImportedClass construction crates/perry/src/commands/compile/run_pipeline.rs	Seven ImportedClass construction sites (named imports, namespace re-export, aliased/non-aliased classes, cross-module dispatch enumeration, unresolved-reference fallback, transitive parent closure) now filter static_fields to only those with key_expr.is_none() before collecting static_field_names.
Cross-module computed static field fixtures and test test-files/fixtures/issue_5984_pkg/entity.ts, test-files/fixtures/issue_5984_pkg/base.ts, test-files/test_issue_5984_computed_static_field_cross_module.ts	Adds an exported entityKind unique symbol, a BaseSession abstract class with a computed static field and tag() method, and a test defining EffectSession subclass that overrides the computed static field and logs values.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

• Computed-key static class fields leak their synthetic name into cross-module external-global declarations #5984: Addresses the same cross-module computed static field leak in run_pipeline.rs by filtering out computed-key fields, matching the root cause described in that issue.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately states the fix for computed-key static fields leaking synthetic names across module boundaries.
Description check	✅ Passed	The PR description covers the bug, fix, related issue, and testing, though it doesn't use the template headings verbatim.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

crates/perry/src/commands/compile/run_pipeline.rs (1)

2437-2467: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Consider extracting a shared ImportedClass builder.

The exact same ImportedClass literal (method/static-method/static-field/getter/setter/field name+type filtering) is duplicated near-verbatim across 7 call sites in this function. This duplication is precisely what caused the bug this PR fixes: field_names/field_types already filtered by key_expr.is_none() at all sites, but static_field_names was missed at every one of them until now. A shared helper (e.g. fn build_imported_class(class: &Class, source_prefix: String, local_alias: Option<String>) -> ImportedClass) taking only the few varying parameters (local_alias, source_prefix, source_class_id) would collapse ~30 lines × 7 into one definition, eliminating this entire class of drift for any future field added to ImportedClass.

♻️ Sketch of a shared constructor helper

fn build_imported_class(
    class: &perry_hir::Class,
    source_prefix: String,
    local_alias: Option<String>,
) -> perry_codegen::ImportedClass {
    perry_codegen::ImportedClass {
        name: class.name.clone(),
        local_alias,
        source_prefix,
        constructor_param_count: class.constructor.as_ref().map(|c| c.params.len()).unwrap_or(0),
        has_own_constructor: class.constructor.is_some(),
        constructor_has_rest: class
            .constructor
            .as_ref()
            .map(|c| c.params.iter().any(|p| p.is_rest))
            .unwrap_or(false),
        has_instance_fields: !class.fields.is_empty(),
        method_names: class.methods.iter().map(|m| m.name.clone()).collect(),
        method_param_counts: class.methods.iter().map(|m| m.params.len()).collect(),
        method_has_rest: class.methods.iter().map(|m| m.params.iter().any(|p| p.is_rest)).collect(),
        static_method_names: class.static_methods.iter().map(|m| m.name.clone()).collect(),
        static_field_names: class.static_fields.iter().filter(|f| f.key_expr.is_none()).map(|f| f.name.clone()).collect(),
        getter_names: class.getters.iter().map(|(n, _)| n.clone()).collect(),
        setter_names: class.setters.iter().map(|(n, _)| n.clone()).collect(),
        parent_name: class.extends_name.clone(),
        field_names: class.fields.iter().filter(|f| f.key_expr.is_none()).map(|f| f.name.clone()).collect(),
        field_types: class.fields.iter().filter(|f| f.key_expr.is_none()).map(|f| f.ty.clone()).collect(),
        source_class_id: Some(class.id),
    }
}

Also applies to: 2746-2777, 3021-3051, 3089-3111, 3248-3270, 3722-3744, 3922-3944

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry/src/commands/compile/run_pipeline.rs` around lines 2437 - 2467,
The ImportedClass construction is duplicated across multiple paths in
run_pipeline, so extract a shared builder to prevent drift in filtered fields
like static_field_names, field_names, and field_types. Add a helper such as
build_imported_class near the repeated literals and use it at each call site,
passing only the varying values like local_alias, source_prefix, and
source_class_id. Keep the existing name/method/static/getter/setter collection
logic inside the helper so future ImportedClass additions are handled in one
place.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/perry/src/commands/compile/run_pipeline.rs`:
- Around line 2437-2467: The ImportedClass construction is duplicated across
multiple paths in run_pipeline, so extract a shared builder to prevent drift in
filtered fields like static_field_names, field_names, and field_types. Add a
helper such as build_imported_class near the repeated literals and use it at
each call site, passing only the varying values like local_alias, source_prefix,
and source_class_id. Keep the existing name/method/static/getter/setter
collection logic inside the helper so future ImportedClass additions are handled
in one place.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f98e14ac-558a-445b-8090-dc68f6285bfd

📥 Commits

Reviewing files that changed from the base of the PR and between 4500b5d and 8de46fe.

📒 Files selected for processing (4)

• crates/perry/src/commands/compile/run_pipeline.rs
• test-files/fixtures/issue_5984_pkg/base.ts
• test-files/fixtures/issue_5984_pkg/entity.ts
• test-files/test_issue_5984_computed_static_field_cross_module.ts