From 444010cd1f125315cc70d641a5a44760f4a4d972 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 7 Sep 2026 18:48:53 -0400 Subject: [PATCH] fix(ids): declaration merging mints one id per facet; both facets survive in types{} (#177) Value facets keep the bare id; a colliding type facet is #type; a later type facet of a type/type merge is keyed Name# and gets #. Signatures unchanged; heritage resolves through the type facet; a split is not an L1 collision. Ids move only on collision. --- .claude/SCHEMA_DECISIONS.md | 10 +++ docs/design/specs/declaration-merging-ids.md | 42 +++++++++ src/schema/assignIds.ts | 29 ++++-- src/schema/emit.ts | 6 +- src/syntactic_analysis/builders.ts | 40 +++++---- test/declaration-merging.test.ts | 95 ++++++++++++++++++++ 6 files changed, 199 insertions(+), 23 deletions(-) create mode 100644 docs/design/specs/declaration-merging-ids.md create mode 100644 test/declaration-merging.test.ts diff --git a/.claude/SCHEMA_DECISIONS.md b/.claude/SCHEMA_DECISIONS.md index c3c3d61..05a168c 100644 --- a/.claude/SCHEMA_DECISIONS.md +++ b/.claude/SCHEMA_DECISIONS.md @@ -206,3 +206,13 @@ Spec: `docs/design/specs/span-bytes-are-bytes.md`. Analyzer 1.5.0; Neo4j contrac | D1 | **`span.bytes` meaning** | UTF-8 byte offsets into `module.source` (and an artifact's `source` for `ConfigKey` spans), every node and level; `Buffer.from(source).subarray(lo, hi)` reproduces the text | the keystone's `module.source[span.bytes]` and python's `byte_offsets`/`_span_code` mean bytes; TS emitted UTF-16 char offsets, so the graph's Buffer-sliced `code` ran short by the multibyte surplus inside the span and one slicing rule could not hold across languages | | D2 | **Conversion point** | `src/schema/offsets.ts` (`offsetMapOf`: ASCII identity, else one cumulative table per text, cached per owner). Producers convert on the way out (builders, `dataflow/attach`, `artifacts/yamlKeys`); consumers needing compiler positions convert on the way in (`configUse.nodeAtSpan`, defuse-linker factory lookup, `entrypoints/matching` default export). The dataflow IR keeps char offsets — internal, never on the wire | ts-morph positions stay native inside the analyzer; one definition of the mapping | | D3 | **Version** | 1.5.0 minor; a documented field's values move to the documented contract; ASCII files are byte-identical before and after | not a new field or shape | + +## Declaration merging: one id per facet (2026-09-07, #177 — no sibling precedent) + +Spec: `docs/design/specs/declaration-merging-ids.md`. Analyzer 1.5.0; Neo4j contract unchanged. + +| # | Concept | Decision | Rationale | +|---|---|---|---| +| D1 | **Value facet keeps the bare id** | a type whose bare id is already minted by a callable/field of the same scope becomes `#type` (`src/schema/assignIds.ts`) | callables anchor call edges, body-node ids, `@formal_in:N`, `parameters[i].id`; a type's id is referenced only through `extends_ids`/`implements_ids`, resolved by the analyzer's own map | +| D2 | **Type/type merging** | first type facet (builder order class → interface → enum → alias → namespace) keeps the bare key and id; each later facet is keyed `Name#` in `types{}` (`builders.ts::putType`) and gets `#` | both facets must survive — "later kind wins" dropped one from JSON entirely; the kind suffix is self-describing where `#type` would not tell two types apart | +| D3 | **Only on collision; signatures unchanged** | suffixes appear only when the bare id is already used this run; `signature` stays the dotted name on every facet; `idBySig` keeps the value facet, `typeIdBySig` the first type facet, heritage resolves through the union with the type winning; a split is not an L1 collision | a merge-free project is byte-identical; the resolver computes signatures from the AST (`new X()` → `X.constructor`), so a signature change would break call resolution for every merged class | diff --git a/docs/design/specs/declaration-merging-ids.md b/docs/design/specs/declaration-merging-ids.md new file mode 100644 index 0000000..e2a178d --- /dev/null +++ b/docs/design/specs/declaration-merging-ids.md @@ -0,0 +1,42 @@ +# Declaration merging: one id per facet + +Tracking: codeanalyzer-typescript #177. Ships in 1.5.0. Neo4j contract unchanged at 2.0.0; ids +change ONLY where two declarations of one name shared an id before (a bug), never elsewhere. + +## Problem + +TypeScript lets one name carry several declaration facets in one scope — a value and a type +(`const TableOption = () => …` + `interface TableOption`), a type and a type (`class C` + +`interface C`, `enum E` + `namespace E`), a value and a namespace (`function f` + `namespace f`). +The analyzer minted one `can://` id per *signature*, and every facet of a name has the same +signature, so: + +- `--emit neo4j` `MERGE`d the facets onto one node carrying both labels and the last writer's + `kind` (measured on superset-frontend: `TSCallable`+`TSInterface` with `kind: arrow`, …); +- in `analysis.json`, a value/type pair survived (different maps, same id) but a type/type pair + did not — `types{}` is keyed by name and "the later kind wins", so `class C` + `interface C` + dropped the class from the output entirely. + +Python and Java have no equivalent (a later `def` rebinds; Java forbids it), so the shape is +coined here. + +## Decisions + +| # | Concept | Decision | Rationale | +|---|---|---|---| +| D1 | **Which facet keeps the bare id** | The VALUE facet (callable or field) always; a type whose id is already taken by a value in the same scope becomes `#type`. | Callables anchor the most: call edges, body-node ids `@line:col`, `@formal_in:N` vertices, `parameters[i].id`. Renaming a type touches `extends_ids`/`implements_ids` and decorator targets only, and those resolve through the analyzer's own map. | +| D2 | **Type/type merging** | The first type facet (builder order: class → interface → enum → type alias → namespace) keeps the bare id and the bare `types{}` key; each later facet is keyed `Name#` in `types{}` and gets id `#` (`#interface`, `#enum`, `#type_alias`, `#namespace`). | Both facets must survive in JSON — the old "later wins" silently dropped one. The kind suffix is self-describing; `#type` would not distinguish two types. | +| D3 | **Only on collision** | An id is suffixed only when the bare id is already minted in this run. A project without merging is byte-identical before and after. | Every consumer's stored ids stay valid; adding a colliding facet later changes one type's id, which is the same instability class as any rename. | +| D4 | **Signatures unchanged** | `signature` stays the dotted name on every facet. `idBySig` keeps its value-facet meaning (call-graph re-identification, callee backfill, homing); heritage resolves `extends`/`implements` names through a type-facet map that prefers the type's id. The L1 id-uniqueness gate does not count a facet split as a collision. | The signature is what the resolver computes from the AST (`new X()` → `X.constructor`); changing it would break call resolution for every merged class. | +| D5 | **Neo4j** | Nothing to change: distinct ids are distinct nodes; `TS_DECLARES` from the scope points at each facet; `kind` and labels agree on every node. | The bug was the shared id, not the projection. | + +## Definition of done + +- A fixture with value+interface, type-alias+arrow, type-alias+const, class+interface, + function+namespace: every facet present in `analysis.json` with a distinct id; `types{}` carries + both facets of a type/type merge; `implements` a merged interface resolves to the `#type` facet, + `extends` a merged class to the bare class; no L1 collision reported; body-node ids under the + value facet unchanged. +- The graph projection of that fixture has no node with two `TS*` kind labels, and every node's + `kind` matches its label. +- `bun test` green; ASCII/non-merging fixtures byte-identical (`-j` determinism gate). diff --git a/src/schema/assignIds.ts b/src/schema/assignIds.ts index 8c26f24..580409c 100644 --- a/src/schema/assignIds.ts +++ b/src/schema/assignIds.ts @@ -13,7 +13,8 @@ import type { AnalysisInternal, TSCallable, TSField, TSType } from "./schema"; export interface AssignedIds { appId: string; - idBySig: Map; // signature → can:// id (types + callables) + idBySig: Map; // signature → can:// id (types + callables; the VALUE facet of a merged name) + typeIdBySig: Map; // signature → the first TYPE facet's id (#177; heritage resolves through this) callableBySig: Map; // locates each callable's node for the L3/L4 attach collisions: string[]; // signatures that mapped to two distinct ids (L1 id-uniqueness gate) } @@ -21,8 +22,14 @@ export interface AssignedIds { export function assignIds(app: AnalysisInternal, appName: string): AssignedIds { const appId = applicationIdOf(appName); const idBySig = new Map(); + const typeIdBySig = new Map(); const callableBySig = new Map(); const collisions: string[] = []; + // #177: every id minted this run, so a second declaration facet of one name (declaration + // merging) gets its own id instead of silently sharing. Value facets (callables, fields) are + // always minted before the types of their scope, so a type is the one that yields. + const usedIds = new Set(); + const typeIds = new Set(); const register = (sig: string, id: string): void => { if (idBySig.has(sig) && idBySig.get(sig) !== id) collisions.push(sig); @@ -30,11 +37,15 @@ export function assignIds(app: AnalysisInternal, appName: string): AssignedIds { }; const doFields = (parentId: string, fields: Record | undefined): void => { - for (const [name, f] of Object.entries(fields ?? {})) f.id = `${parentId}/${name}`; + for (const [name, f] of Object.entries(fields ?? {})) { + f.id = `${parentId}/${name}`; + usedIds.add(f.id); + } }; const doCallable = (moduleId: string, modulePrefix: string, c: TSCallable): void => { c.id = idFromSig(moduleId, modulePrefix, c.signature); + usedIds.add(c.id); register(c.signature, c.id); callableBySig.set(c.signature, c); for (const nested of Object.values(c.callables ?? {})) doCallable(moduleId, modulePrefix, nested); @@ -42,8 +53,16 @@ export function assignIds(app: AnalysisInternal, appName: string): AssignedIds { }; const doType = (moduleId: string, modulePrefix: string, t: TSType): void => { - t.id = idFromSig(moduleId, modulePrefix, t.signature); - register(t.signature, t.id); + const bare = idFromSig(moduleId, modulePrefix, t.signature); + // Declaration merging (#177): `#type` when a VALUE of this name holds the bare id, `#` + // when an earlier TYPE facet does (class+interface, enum+namespace). Never otherwise. + t.id = !usedIds.has(bare) ? bare : typeIds.has(bare) ? `${bare}#${t.kind}` : `${bare}#type`; + usedIds.add(t.id); + typeIds.add(bare); + if (!typeIdBySig.has(t.signature)) typeIdBySig.set(t.signature, t.id); + // The value facet keeps the signature → id slot (call-graph re-identification, callee + // backfill, homing all mean the callable); a split is not a collision. + if (t.id === bare) register(t.signature, t.id); doFields(t.id, t.fields); for (const m of Object.values(t.callables ?? {})) doCallable(moduleId, modulePrefix, m); for (const f of Object.values(t.functions ?? {})) doCallable(moduleId, modulePrefix, f); // namespace @@ -88,5 +107,5 @@ export function assignIds(app: AnalysisInternal, appName: string): AssignedIds { dep.declared_in = artifactIdOf(appName, artPath.startsWith("can://") ? artPath.split("/").slice(4).join("/") : artPath); } - return { appId, idBySig, callableBySig, collisions }; + return { appId, idBySig, typeIdBySig, callableBySig, collisions }; } diff --git a/src/schema/emit.ts b/src/schema/emit.ts index 271ea7b..9679b66 100644 --- a/src/schema/emit.ts +++ b/src/schema/emit.ts @@ -97,9 +97,11 @@ export function finalizeAnalysis( const appName = (opts.appName ?? (opts.input ? path.basename(opts.input) : "") ?? "").trim() || "app"; // L1 — stamp ids, derive body{}, project heritage (all overwrite-idempotent per-run passes). - const { appId, idBySig, callableBySig, collisions } = assignIds(app, appName); + const { appId, idBySig, typeIdBySig, callableBySig, collisions } = assignIds(app, appName); populateL1Body(app); - resolveHeritageIds(app, idBySig); + // `extends`/`implements` name TYPES: on a merged name (#177) the type facet's id wins over the + // value facet's, which is what `idBySig` holds for every other pass. + resolveHeritageIds(app, new Map([...idBySig, ...typeIdBySig])); // Level-free, after heritage: unit 4 matches on resolved extends_ids. const entrypoint_report = detectEntrypoints(app, opts, rules); diff --git a/src/syntactic_analysis/builders.ts b/src/syntactic_analysis/builders.ts index 9d95497..8f3174b 100644 --- a/src/syntactic_analysis/builders.ts +++ b/src/syntactic_analysis/builders.ts @@ -32,6 +32,15 @@ import { import { computeSignatureForDecl } from "../schema"; import { memberKey } from "../schema/ids"; import { offsetMapFor } from "../schema/offsets"; + +/** + * #177: a later type facet of a merged name (`class C` + `interface C`, `enum E` + `namespace E`) + * keeps its own slot as `Name#` instead of overwriting the first. assignIds mints the + * matching `#` from the same collision. + */ +function putType(types: Record, key: string, t: TSType): void { + types[key in types ? `${key}#${t.kind}` : key] = t; +} import { importTable, resolveWritten } from "./importResolver"; // ---------------------------------------------------------------------------------------------- @@ -544,7 +553,7 @@ export function buildCallable( }, onNestedClass: (n: Node) => { const r = buildClass(n, root); - types[memberKey(r.sig)] = r.cls; + putType(types, memberKey(r.sig), r.cls); }, }; const body = (fnNode as unknown as { getBody?: () => Node | undefined }).getBody?.(); @@ -640,14 +649,12 @@ function resolveHeritage(expr: Node, root: string): string { if (sym) { const aliased = (sym as { getAliasedSymbol?: () => typeof sym }).getAliasedSymbol?.(); if (aliased) sym = aliased; - const d = sym?.getDeclarations?.()?.[0]; - if ( - d && - (Node.isClassDeclaration(d) || - Node.isInterfaceDeclaration(d) || - Node.isEnumDeclaration(d) || - Node.isClassExpression(d)) - ) { + // A merged symbol (#177: `const X = …` + `interface X`) lists every facet; `extends` / + // `implements` name the TYPE, so take the first type-like declaration, not the first one. + const d = (sym?.getDeclarations?.() ?? []).find( + (x) => Node.isClassDeclaration(x) || Node.isInterfaceDeclaration(x) || Node.isEnumDeclaration(x) || Node.isClassExpression(x), + ); + if (d) { const s = computeSignatureForDecl(d, root); if (s) return s; } @@ -912,24 +919,25 @@ function buildStatemented(container: Node, root: string, varScope: "module" | "n getVariableStatements: () => Node[]; }; // One types{} map. Fill order (classes → interfaces → enums → aliases → namespaces) is the - // canonical precedence: on a member-key collision (e.g. class/interface declaration merging) - // the later kind wins, exactly as the historical per-kind bucket merge did. + // canonical precedence: on a member-key collision (declaration merging, #177) the FIRST kind + // keeps the bare key and every later facet is keyed `Name#` — both survive, where the + // historical per-kind bucket merge silently dropped the earlier one. const types: Record = {}; for (const cl of c.getClasses()) { const r = buildClass(cl, root); - types[memberKey(r.sig)] = r.cls; + putType(types, memberKey(r.sig), r.cls); } for (const it of c.getInterfaces()) { const r = buildInterface(it, root); - types[memberKey(r.sig)] = r.intf; + putType(types, memberKey(r.sig), r.intf); } for (const en of c.getEnums()) { const r = buildEnum(en, root); - types[memberKey(r.sig)] = r.en; + putType(types, memberKey(r.sig), r.en); } for (const ta of c.getTypeAliases()) { const r = buildTypeAlias(ta, root); - types[memberKey(r.sig)] = r.ta; + putType(types, memberKey(r.sig), r.ta); } const functions: Record = {}; for (const fn of c.getFunctions()) { @@ -967,7 +975,7 @@ function buildStatemented(container: Node, root: string, varScope: "module" | "n }); for (const ns of c.getModules()) { const r = buildNamespace(ns, root); - types[memberKey(r.sig)] = r.ns; + putType(types, memberKey(r.sig), r.ns); } return { types, functions, fields }; } diff --git a/test/declaration-merging.test.ts b/test/declaration-merging.test.ts new file mode 100644 index 0000000..63c664d --- /dev/null +++ b/test/declaration-merging.test.ts @@ -0,0 +1,95 @@ +/** + * #177 — declaration merging mints one id per FACET (spec: docs/design/specs/declaration-merging-ids.md). + * + * Value facets keep the bare id; a type sharing a value's name becomes `#type`; a later type + * facet of a type/type merge is keyed `Name#` in `types{}` and gets `#`. Ids only + * move on collision, heritage resolves to the right facet, and the graph never carries two kind + * labels on one node. + */ +import { describe, expect, test } from "bun:test"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { project } from "../src/build/neo4j"; +import { analyze } from "../src/core"; +import type { AnalysisOptions } from "../src/options"; +import type { TSAnalysis, TSModule } from "../src/schema"; + +const dir = fs.mkdtempSync(path.join(os.tmpdir(), "cants-merge-")); +fs.mkdirSync(path.join(dir, "src")); +fs.writeFileSync(path.join(dir, "tsconfig.json"), JSON.stringify({ compilerOptions: { target: "ES2020" }, include: ["src/**/*.ts"] })); +fs.writeFileSync(path.join(dir, "src", "m.ts"), [ + "export const TableOption = () => 1;", + "export interface TableOption { a: string }", + "export type Dyn = string;", + "export const Dyn = () => 2;", + 'export type TG = "d";', + "export const TG = 3;", + "export class C { m(): number { return 1; } }", + "export interface C { x: number }", + "export function f(): void {}", + "export namespace f { export const y = 1; }", + "export class D implements TableOption { a = 'x'; }", + "export class E extends C {}", + "export class Plain {}", +].join("\n")); + +const opts = { input: dir, appName: "dm", analysisLevel: 2, noBuild: true, emit: "json", eager: true } as unknown as AnalysisOptions; +const res = await analyze(opts); +const app = res.application as TSAnalysis; +const mod = app.application.symbol_table["src/m.ts"] as TSModule; +const M = mod.id; + +describe("#177 declaration merging", () => { + test("value facets keep the bare id; the colliding type facet is #type", () => { + expect(mod.functions.TableOption!.id).toBe(`${M}/TableOption`); + expect(mod.types.TableOption!.id).toBe(`${M}/TableOption#type`); + expect(mod.types.TableOption!.kind).toBe("interface"); + expect(mod.functions.Dyn!.id).toBe(`${M}/Dyn`); + expect(mod.types.Dyn!.id).toBe(`${M}/Dyn#type`); + expect(mod.fields.TG!.id).toBe(`${M}/TG`); + expect(mod.types.TG!.id).toBe(`${M}/TG#type`); + expect(mod.functions.f!.id).toBe(`${M}/f`); + expect(mod.types.f!.id).toBe(`${M}/f#type`); + expect(mod.types.f!.kind).toBe("namespace"); + }); + + test("type/type merging keeps BOTH facets: the later kind is keyed and suffixed by its kind", () => { + expect(mod.types.C!.kind).toBe("class"); + expect(mod.types.C!.id).toBe(`${M}/C`); + expect(mod.types["C#interface"]!.kind).toBe("interface"); + expect(mod.types["C#interface"]!.id).toBe(`${M}/C#interface`); + // the class facet's members hang off the bare id + expect(Object.values(mod.types.C!.callables!)[0]!.id).toBe(`${M}/C/m`); + }); + + test("ids move only on collision; signatures never move", () => { + expect(mod.types.Plain!.id).toBe(`${M}/Plain`); + expect(mod.types.TableOption!.signature).toBe("src/m.TableOption"); + expect(mod.functions.TableOption!.signature).toBe("src/m.TableOption"); + expect(res.collisions).toEqual([]); + }); + + test("heritage resolves to the right facet", () => { + expect(mod.types.D!.implements_ids).toEqual([`${M}/TableOption#type`]); + expect(mod.types.E!.extends_ids).toEqual([`${M}/C`]); + }); + + test("the graph has one node per facet, kind and labels agreeing", () => { + const rows = project(app); + const kindLabels = (labels: string[]) => labels.filter((l) => l.startsWith("TS") && l !== "TSCanNode" && l !== "TSAnonymousCallable"); + for (const n of rows.nodes) expect(kindLabels(n.labels).length, `${n.value} ${n.labels.join(":")}`).toBeLessThanOrEqual(1); + const by = (id: string) => rows.nodes.find((n) => n.value === id)!; + expect(kindLabels(by(`${M}/TableOption`).labels)).toEqual(["TSCallable"]); + expect(kindLabels(by(`${M}/TableOption#type`).labels)).toEqual(["TSInterface"]); + expect(by(`${M}/TableOption#type`).props.kind).toBe("interface"); + expect(kindLabels(by(`${M}/C`).labels)).toEqual(["TSClass"]); + expect(kindLabels(by(`${M}/C#interface`).labels)).toEqual(["TSInterface"]); + expect(kindLabels(by(`${M}/TG`).labels)).toEqual(["TSField"]); + expect(kindLabels(by(`${M}/TG#type`).labels)).toEqual(["TSTypeAlias"]); + // both facets are declared by the module + const declared = rows.edges.filter((e) => e.type === "TS_DECLARES" && e.from.value === M).map((e) => e.to.value); + expect(declared).toContain(`${M}/C`); + expect(declared).toContain(`${M}/C#interface`); + }); +});