actually fix name collisions in derive closures

[mathpirate]

follow-up to derive closures feature

---

Summary by cubic

Fix name collisions between the derive input parameter and captured variables. Colliding captures are renamed and references are updated so callbacks run correctly and types stay accurate.

• Bug Fixes

  • Detect collisions with the input param name and rename captures (multiplier → multiplier_1, multiplier_2, …).
  • Apply renaming in the merged input object and the callback’s destructured params.
  • Rewrite the callback body to use renamed captures, leaving property names unchanged and expanding shorthand ({ x } → { x: x_1 }).
  • Reflect renamed capture names in the generated input schema.
  • Preserve result type inference for shorthand returns by registering inferred types in schema injection.
  • Added fixtures for property-name and shorthand collisions to ensure keys aren’t rewritten.

• Refactors

  • Remove $schema from emitted JSON schemas.

^{Written for commit a7908a7. Summary will update automatically on new commits.}

[cubic-dev-ai]

1 issue found across 3 files

Prompt for AI agents (all 1 issues)

Understand the root cause of the following 1 issues and fix them.


<file name="packages/ts-transformers/src/closures/transformer.ts">

<violation number="1" location="packages/ts-transformers/src/closures/transformer.ts:1430">
This identifier substitution needs to guard against property-access/property-name contexts; otherwise we rename property names (e.g. `input.input` becomes `input_1.input_1`).</violation>
</file>

_{React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

[cubic-dev-ai]

This identifier substitution needs to guard against property-access/property-name contexts; otherwise we rename property names (e.g. input.input becomes input_1.input_1).

Prompt for AI agents

Address the following comment on packages/ts-transformers/src/closures/transformer.ts at line 1430:

<comment>This identifier substitution needs to guard against property-access/property-name contexts; otherwise we rename property names (e.g. `input.input` becomes `input_1.input_1`).</comment>

<file context>
@@ -1351,15 +1385,61 @@ function buildDeriveInputObject(
+  const visitor = (node: ts.Node): ts.Node =&gt; {
+    // Only substitute root-level identifiers that match captured variable names
+    // Don&#39;t substitute property names or nested references
+    if (ts.isIdentifier(node)) {
+      const substituteName = substitutions.get(node.text);
+      if (substituteName) {
</file context>

✅ Addressed in 789ba6b

[cubic-dev-ai]

1 issue found across 3 files (reviewed changes from recent commits).

1 issue found across 3 files

Prompt for AI agents (all 1 issues)

Understand the root cause of the following 1 issues and fix them.


<file name="packages/ts-transformers/src/closures/transformer.ts">

<violation number="1" location="packages/ts-transformers/src/closures/transformer.ts:1444">
Skipping substitutions for shorthand property assignments leaves renamed captures (e.g., multiplier → multiplier_1) still referenced as `{ multiplier }`, so the body now uses an identifier that no longer exists after the rename and will break at compile/runtime.</violation>
</file>

_{React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

[cubic-dev-ai]

Skipping substitutions for shorthand property assignments leaves renamed captures (e.g., multiplier → multiplier_1) still referenced as { multiplier }, so the body now uses an identifier that no longer exists after the rename and will break at compile/runtime.

Prompt for AI agents

Address the following comment on packages/ts-transformers/src/closures/transformer.ts at line 1444:

<comment>Skipping substitutions for shorthand property assignments leaves renamed captures (e.g., multiplier → multiplier_1) still referenced as `{ multiplier }`, so the body now uses an identifier that no longer exists after the rename and will break at compile/runtime.</comment>

<file context>
@@ -1424,20 +1424,42 @@ function rewriteCaptureReferences(
+
+      // Skip if this identifier is a shorthand property name (e.g., &#39;foo&#39; in &#39;{ foo }&#39;)
+      if (
+        parent &amp;&amp; ts.isShorthandPropertyAssignment(parent) &amp;&amp;
+        parent.name === node
+      ) {
</file context>

✅ Addressed in 4514773

[cubic-dev-ai]

1 issue found across 3 files (reviewed changes from recent commits).

1 issue found across 3 files

Prompt for AI agents (all 1 issues)

Understand the root cause of the following 1 issues and fix them.


<file name="packages/ts-transformers/src/closures/transformer.ts">

<violation number="1" location="packages/ts-transformers/src/closures/transformer.ts:1434">
Expanding shorthand captures to property assignments needs to preserve `objectAssignmentInitializer`; otherwise defaults in patterns like `({ multiplier = fallback } = source)` are erased when the capture is renamed.</violation>
</file>

_{React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

[cubic-dev-ai]

Expanding shorthand captures to property assignments needs to preserve objectAssignmentInitializer; otherwise defaults in patterns like ({ multiplier = fallback } = source) are erased when the capture is renamed.

Prompt for AI agents

Address the following comment on packages/ts-transformers/src/closures/transformer.ts at line 1434:

<comment>Expanding shorthand captures to property assignments needs to preserve `objectAssignmentInitializer`; otherwise defaults in patterns like `({ multiplier = fallback } = source)` are erased when the capture is renamed.</comment>

<file context>
@@ -1425,6 +1425,21 @@ function rewriteCaptureReferences(
+      const substituteName = substitutions.get(node.name.text);
+      if (substituteName) {
+        // Expand shorthand into full property assignment
+        return factory.createPropertyAssignment(
+          node.name, // Property name stays the same
+          factory.createIdentifier(substituteName), // Value uses renamed identifier
</file context>

[cubic-dev-ai]

1 issue found across 80 files (reviewed changes from recent commits).

1 issue found across 80 files

Prompt for AI agents (all 1 issues)

Understand the root cause of the following 1 issues and fix them.


<file name="packages/schema-generator/src/schema-generator.ts">

<violation number="1" location="packages/schema-generator/src/schema-generator.ts:705">
Returning the bare root schema removes the `$schema` declaration, yet this generator still emits `$defs`. Validators that assume draft-07 when `$schema` is missing will treat `$defs` as unknown and break validation. Please keep `$schema` on the returned schema.</violation>
</file>

_{React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.}

[cubic-dev-ai]

Returning the bare root schema removes the $schema declaration, yet this generator still emits $defs. Validators that assume draft-07 when $schema is missing will treat $defs as unknown and break validation. Please keep $schema on the returned schema.

Prompt for AI agents

Address the following comment on packages/schema-generator/src/schema-generator.ts at line 705:

<comment>Returning the bare root schema removes the `$schema` declaration, yet this generator still emits `$defs`. Validators that assume draft-07 when `$schema` is missing will treat `$defs` as unknown and break validation. Please keep `$schema` on the returned schema.</comment>

<file context>
@@ -701,18 +700,14 @@ export class SchemaGenerator implements ISchemaGenerator {
-        $schema: &quot;https://json-schema.org/draft/2020-12/schema&quot;,
-        ...(rootSchema as Record&lt;string, unknown&gt;),
-      } as SchemaDefinition;
+      return rootSchema;
     }
 
</file context>