fix(generate): reject modules whose declarations and references disagree

[marc0olo]

Closes #157.

Behaviour

The one PR in the chain that makes generation fail where it previously succeeded. A .did whose generated module contradicts itself is rejected with a diagnostic instead of written out:

generated wrapper uses the name `Variant_a_b` twice in type space (as enum and as class).
Candid type names, the .did basename, variant tags, the conversion functions and the imports
the generated module needs all feed into this namespace; two of them collided here. TypeScript
would merge them silently rather than reject them, producing a type that claims members the
value does not have. Rename one of them in the .did file; where a side is the class or the
interface named after the file, renaming the file works too. Generating only the declarations
(`output.actor.disabled`) is unaffected.

The class named after the file already steps aside from a candid type of that name (#174), so the checks are the backstop for what cannot be escaped up front: an enum named after an inline variant's tags, or a conversion function, is named while the module is built. collide_class.did therefore generates (Collide_class_), and variant_a_b.did is the fixture that still refuses. The names taken from the file are not in the set the locals step past either, so _Foo.did beside type Foo — where the class and the local for Foo’s shape both want _Foo — is refused here rather than named around.

Everything the PRs below this one fix would land in that category, which is why they come first — nothing legal is rejected by the time this lands, with one deliberate exception: two methods whose keys escape to one name (class and class_) are refused until #180 stops escaping method keys. Existing output is unchanged; no snapshot moves.

Two inputs that generated broken output now generate correctly: a .did named after its own service type (backend.did with type backend = service { … }) got an interface extending itself, and a candid type named _Foo collided with the local under which the wrapper imports Foo's candid shape. The locals are now allocated together, once per module, each stepping past the names the module binds itself, every candid type’s identifier and every local before it, so no two coincide (Foo, Foo_, _Foo, _Foo_ → _Foo__, _Foo___, __Foo, __Foo_).

What is checked

• No two top-level bindings collide. Candid type ids, the .did basename, joined variant tags, the conversion functions named after the types they convert, and the module's imports all feed into one namespace, and nothing reconciled them.
• Every referenced type resolves to a type the module declares or imports — a function or a value import of that name does not count — so a declaration that was interned away cannot leave a type consumers are unable to import. A reference to a type parameter resolves to the parameter, so those are tracked by the scope that binds them — the preamble's Option<T> does not excuse an unrelated T elsewhere.
• No object type declares a member twice — type literals, interfaces and the class alike: the __kind__ discriminant beside a candid tag of that name, or two methods escaping to one key.
• No candid name is __proto__, checked ahead of both generators and whichever files are wanted. JavaScript treats it as the object's prototype in every position it would appear — record key, enum member, method — so the field, tag or method is missing at runtime, and IDL.Record({ '__proto__': … }) in the declarations loses it too. Verified on main: such a field never round-tripped, so refusing costs no capability.

compile returns Result, surfaced through generate() alongside the existing candid parse errors. The module checks describe the actor files, so they do not run when only declarations/ is wanted, and their diagnostics say so; the __proto__ refusal is the one that applies to the declarations as well.

Reviewer note. The reason to reject is not that TypeScript refuses these — it mostly does not, verified with tsc --strict. What comes out is a type claiming members the runtime value lacks: a wrapper class that silently gains a candid record's fields, or a string enum accepting members its variant never declared, which the conversions then put on the wire.

A declaration occupies TypeScript's type space, its value space, or both, and two collide only where they overlap: a function beside a same-named interface is the merge TypeScript intends and is left alone, while a function beside a same-named enum is two bindings of one name. Import locals take part in the same namespace, and heritage clauses (interface X extends Y) are checked as references — both are names the AST holds differently from an ordinary type reference. Syntactic validity is not checked here: export enum 'Variant_my-tag' is self-consistent and still does not parse, which #180 closes.

[Copilot]

🟡 Changes recommended

The review identified a failing success-path test and four unresolved validation gaps.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds validation to reject inconsistent generated TypeScript modules and propagates generation errors.

Changes:

• Validates names, declarations, members, and type references.
• Adds collision and invalid-name fixtures.
• Integrates validation and actor-disabled generation handling.

File summaries

File	Summary
tests/validation.test.ts	Adds validation tests; success-path toThrow assertion needs correction.
tests/assets/unrepresentable_tag.did	Adds invalid __proto__ tag fixture.
tests/assets/unrepresentable_field.did	Adds invalid __proto__ field fixture.
tests/assets/duplicate_member.did	Adds duplicate-member fixture.
tests/assets/collide_escaped_names.did	Adds escaped-name collision fixture.
tests/assets/collide_class.did	Adds class/type collision fixture.
src/core/generate/rs/src/lib.rs	Propagates compilation errors and actor-disabled handling.
src/core/generate/rs/src/bindings/typescript_native/validate.rs	Adds module checks; unresolved type-parameter scope, duplicate imports, generated-function collisions, and service method keys remain insufficiently validated.
src/core/generate/rs/src/bindings/typescript_native/mod.rs	Registers validation integration.
src/core/generate/rs/src/bindings/typescript_native/compile.rs	Returns compilation errors.
src/core/generate/rs/src/bindings/typescript_native/compile_wrapper.rs	Applies validation to wrapper output.
src/core/generate/rs/src/bindings/typescript_native/compile_interface.rs	Applies validation to interface output.
src/core/generate/rs/Cargo.toml	Adds SWC visitor/parser features.
src/core/generate/index.ts	Forwards actor-disabled configuration.
Cargo.lock	Updates locked dependencies.

Review details

Suppressed comments (4)

src/core/generate/rs/src/bindings/typescript_native/validate.rs:121

• Collecting type parameters module-wide lets an unresolved reference to T pass as if it were declared. For example, type T = record { nat }; service : () -> { f : (T) -> () } uses the tuple type T, which add_type_definitions intentionally omits, but the generated preamble's Some<T>/Option<T> causes references.type_params to contain T, so this validator returns Ok and still emits a wrapper with an out-of-scope T. Track type-parameter scopes (or otherwise exclude preamble-local T from module-level bindings) and add this regression fixture.

    // Type parameters are collected module-wide rather than per-scope. The generated module is
    // flat and its only generics come from the fixed preamble (`Option<T>`, `Some<T>`), so
    // scope tracking would buy nothing; the cost is that a reference is not flagged if some
    // unrelated declaration happens to bind the same name as a type parameter.
    known.extend(references.type_params);

src/core/generate/rs/src/bindings/typescript_native/validate.rs:81

• This loop silently overwrites an existing entry when two imports use the same local name, so the validator does not actually enforce the namespace invariant for imports. For example, a named type SERVICE that needs a wrapper conversion adds SERVICE as _SERVICE through OriginalTypescriptTypes, while the fixed preamble already imports _SERVICE; the emitted module then has duplicate local import bindings but this check proceeds. Reject duplicate import locals here (or otherwise check the previous insert result) before scanning declarations.

    for name in imported_names(module) {
        seen.insert(name, "import");

src/core/generate/rs/src/bindings/typescript_native/validate.rs:85

• type_decl filters out Decl::Fn, so a generated conversion function can still collide with a value-bearing enum (or class) declaration. For example, a payload variant Foo causes to_candid_Foo_n1 to be emitted; an all-null Candid variant named to_candid_Foo_n1 emits an enum with that same identifier. This check accepts the module even though the wrapper now has two conflicting value bindings. Keep the intentional function/interface compatibility, but also compare generated functions with enum/class names (or reserve those generated names).

    for (name, kind) in module.body.iter().filter_map(as_decl).filter_map(type_decl) {
        if let Some(previous) = seen.insert(name.clone(), kind) {

src/core/generate/rs/src/bindings/typescript_native/validate.rs:230

• Service methods are emitted as TsMethodSignature nodes, whose keys are Expr values; they do not pass through visit_prop_name or visit_ts_property_signature. Thus a named nested service such as type S = service { "__proto__" : () -> () }; passes this check and emits an unusable __proto__ method in SInterface. Add a visitor for TsMethodSignature keys so method names are validated too.

    fn visit_prop_name(&mut self, node: &PropName) {
        match node {
            PropName::Ident(ident) => self.names.push(ident.sym.to_string()),
            PropName::Str(s) => self.names.push(s.value.to_string()),
            PropName::Computed(_) | PropName::Num(_) | PropName::BigInt(_) => {}
        }
        node.visit_children_with(self);

• Files reviewed: 14/15 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[marc0olo]

The four suppressed comments, in order.

Duplicate import locals not rejected — correct, and reproduced: a candid type named SERVICE had its shape imported as _SERVICE, the local the service type already binds, so ActorSubclass<_SERVICE> silently meant the user's type. Fixed on both sides — #173 steps the alias aside (_SERVICE_), and the check here now treats import locals as part of the same namespace, with duplicate_import_locals_are_reported covering it.

Generated function vs. enum/class — correct, and reproduced: type foo plus type to_candid_foo_n1 emitted export enum to_candid_foo_n1 beside function to_candid_foo_n1. The check now models both of TypeScript's namespaces and reports a collision only where they overlap, so a function beside a same-named interface still merges as TypeScript intends while a function beside an enum is rejected. collide_conversion_function.did and function_colliding_with_an_enum_is_reported cover it.

TsMethodSignature keys not visited — correct for a nested service type, and fixed in #180 where the method-name refusals live: type S = service { "__proto__" : () -> () } emitted __proto__(): Promise<void> unchecked, and now fails generation. unrepresentable_nested_method.did pins it. The top-level service was already covered, because its methods reach the class as well as the interface.

Type parameters collected module-wide — did not reproduce. The premise is that the tuple type is omitted from the declarations; on the stack tip type T = record { nat } emits export type T = [bigint], so the reference resolves to a real declaration and nothing relies on the preamble's T. With every named candid type now declared (#173), a reference masked by a preamble type parameter needs a type that is referenced and undeclared, which is the case the other check closes. The comment in the code stays as the record of the limitation.

[Copilot]

🟢 Approval recommended

All reviewed changes have no unresolved blocking issues.

Review details

• Files reviewed: 32/33 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Import-local allocation can still produce duplicate locals for certain valid type-name combinations.

Review details

Suppressed comments (1)

src/core/generate/rs/src/bindings/typescript_native/utils.rs:604

• This collision check is not sufficient to make import locals unique. For a module containing candid types Foo, _Foo, and Foo_, the Foo shape starts at _Foo and steps to _Foo_ because _Foo is a type, while the Foo_ shape independently uses _Foo_; both imports therefore bind the same local and check_unique_declarations rejects this otherwise representable input. Derive/allocate locals against the other generated import locals as well (or include this case in the collision search) so legal names are not refused.

    let mut local = format!("_{id}");
    while taken(&local) {
        local.push('_');

• Files reviewed: 32/33 changed files
• Comments generated: 0 new
• Review effort level: Lite

[marc0olo]

Re the suppressed comment on candid_import_local: agreed, stepping aside per type in isolation could land two locals on one name — Foo beside _Foo stepped to _Foo_, which is also the plain local for Foo_. The locals are now allocated together, in the sorted order of the candid ids, each stepping past the names the module occupies, every type's identifier and every local allocated before it, so no two can coincide. import_local_collision.did now holds Foo, Foo_, _Foo and _Foo_ together (locals _Foo__, _Foo___), and a unit test pins the four distinct results.

[Copilot]

🟢 Approval recommended

No unresolved review issues remain, and validation is covered by tests and fixtures.

Review details

• Files reviewed: 32/33 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Copilot review overview

🟡 Changes recommended

Unresolved import-local and class-member collision findings require changes, and the documentation needs clarification.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

• Detect constructor parameter-property method name collisions · New
• Reserve generated actor names when allocating import locals · New

[Copilot]

Copilot review overview

🔵 Needs a closer look

Import-local allocation must use deterministic sorted traversal to prevent non-reproducible generated output.

Review effort: Lite
Findings: 1 · 1

Open (2)

• Detect constructor parameter-property method name collisions
• Reserve generated actor names when allocating import locals