From 4317ff27d8782e0390a6cb27021a24b016a59407 Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Sat, 15 Aug 2026 22:09:21 -0600 Subject: [PATCH] fix(js/ts): seed typeMap from as-casts, resolving the #2235 real-world repro (#2397) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #2235's scoping fix was correct and complete for the general collision case, but the specific real-world repro that motivated it — this repo's own src/db/connection.ts — was still divergent: openReadonlyOrFail's own local (`const db = new Database(...) as unknown as BetterSqlite3Database`) is an as-cast, not a type annotation or bare `new X()`, so neither engine's handleVarDeclaratorTypeMap had a branch for it. With no scoped entry, `db`'s resolution fell through to the bare "db" key, whose winner depended on confidence/insertion-order luck in the return-type-propagation branch — luck that differed between engines (wasm happened to resolve correctly via that path, native didn't). Rather than chasing that fragile propagation-order divergence, this seeds the typeMap directly from the as-cast's target type — the cast is what the rest of the file actually treats the value as — at confidence 0.9 (the same tier as an explicit type annotation), checked with the same priority as the existing constructor branch (an explicit initializer shape wins over a declared annotation). `X as unknown as Y` is handled by extracting from the OUTERMOST as_expression's own type child, which naturally yields the final Y without special-casing the intermediate unknown hop. The Rust side required restructuring handle_var_declarator_type_map's ordering: dedup_type_map is first-write-wins on confidence TIES, and the cast is pushed at the same 0.9 tier as a type annotation, so simply pushing both and relying on confidence comparison (as the constructor branch's unambiguous 1.0-vs-0.9 gap already could) would let the annotation win the tie instead of the cast. The cast/constructor checks now run first and skip the annotation push when either already seeded a more authoritative entry. Verified against the actual reported repro end-to-end: rebuilding this repo's own src/ with both engines now produces byte-for-byte identical edges (kind, target, confidence, technique) for openReadonlyOrFail, including the receiver edge and the previously-missing db.prepare/db.pragma call edges. docs check acknowledged Impact: 2 functions changed, 0 affected --- .../src/extractors/javascript.rs | 181 +++++++++++++++--- src/extractors/javascript.ts | 49 +++++ tests/engines/parity.test.ts | 43 +++++ tests/parsers/javascript.test.ts | 41 ++++ 4 files changed, 291 insertions(+), 23 deletions(-) diff --git a/crates/codegraph-core/src/extractors/javascript.rs b/crates/codegraph-core/src/extractors/javascript.rs index a71244f64..76ddca618 100644 --- a/crates/codegraph-core/src/extractors/javascript.rs +++ b/crates/codegraph-core/src/extractors/javascript.rs @@ -213,6 +213,41 @@ fn extract_simple_type_name<'a>(node: &Node<'a>, source: &'a [u8]) -> Option<&'a None } +/// Extract the target type name from an `as_expression` (`value as Type`), +/// mirroring TS `extractAsExpressionTypeName`. +/// +/// `as_expression` has no named fields in tree-sitter-typescript's grammar — +/// its two named children (the expression and the type) are distinguished +/// only by kind, not a field name. Scanning from the END and matching on +/// `type_identifier`/`generic_type`/`parenthesized_type` (never plain +/// `identifier`, unlike `extract_simple_type_name`) is safe because the +/// expression side can never produce those node kinds — TS's grammar keeps +/// "type" and "expression" as disjoint node-kind namespaces — so there is no +/// risk of matching the cast's INPUT instead of its target type, even when +/// that input is itself a bare identifier. +/// +/// `X as unknown as Y` parses as nested as_expressions, `(X as unknown) as +/// Y` — called on the outermost node, this naturally extracts `Y` (the +/// final, intended type) without needing to special-case the `unknown` hop; +/// called on a bare `X as unknown`, it correctly finds no nameable type +/// (`unknown` is a `predefined_type`, not handled here) and returns `None`. +fn extract_as_expression_type_name<'a>(node: &Node<'a>, source: &'a [u8]) -> Option<&'a str> { + for i in (0..node.child_count()).rev() { + if let Some(child) = node.child(i) { + match child.kind() { + "type_identifier" => return Some(node_text(&child, source)), + "generic_type" => { + let base = child.child(0).map(|n| node_text(&n, source)); + return base.filter(|b| !OPAQUE_TYPE_TRANSFORM_WRAPPERS.contains(b)); + } + "parenthesized_type" => return extract_simple_type_name(&child, source), + _ => {} + } + } + } + None +} + /// Extract constructor type name from a new_expression node. fn extract_new_expr_type_name<'a>(node: &Node<'a>, source: &'a [u8]) -> Option<&'a str> { if node.kind() != "new_expression" { @@ -293,33 +328,64 @@ fn handle_var_declarator_type_map(node: &Node, source: &[u8], symbols: &mut File // differently-typed local of this same name would otherwise silently // collide under the bare key. Mirrors TS handleVarDeclaratorTypeMap. let enclosing_qualifier = find_enclosing_function_qualifier(node, source); - // Type annotation: confidence 0.9 - if let Some(type_anno) = find_child(node, "type_annotation") { - if let Some(type_name) = extract_simple_type_name(&type_anno, source) { - push_scoped_type_map_entry( - symbols, - enclosing_qualifier.as_deref(), - var_name, - type_name.to_string(), - 0.9, - ); + let value_n = node.child_by_field_name("value"); + + // Constructor and `as`-cast both win over a same-declaration type + // annotation (checked first, before the annotation push below) — mirrors + // TS handleVarDeclaratorTypeMap's early-return priority exactly. This + // isn't just style: dedup_type_map is first-write-wins on confidence + // TIES, and the cast is pushed at the SAME 0.9 tier as the annotation + // (#2397 — `const db = new Database(...) as unknown as BetterSqlite3Database` + // must resolve to BetterSqlite3Database, not whatever an unrelated + // annotation on the same declarator might say), so simply pushing both + // and relying on confidence comparison — as the constructor branch's + // unambiguous 1.0-vs-0.9 gap already could — would silently let the + // annotation win the tie instead of the cast. + let mut explicit_initializer_seeded = false; + if let Some(v) = &value_n { + if v.kind() == "new_expression" { + // Constructor: confidence 1.0 (overrides annotation in edge builder) + if let Some(type_name) = extract_new_expr_type_name(v, source) { + push_scoped_type_map_entry( + symbols, + enclosing_qualifier.as_deref(), + var_name, + type_name.to_string(), + 1.0, + ); + explicit_initializer_seeded = true; + } + } else if v.kind() == "as_expression" { + if let Some(type_name) = extract_as_expression_type_name(v, source) { + push_scoped_type_map_entry( + symbols, + enclosing_qualifier.as_deref(), + var_name, + type_name.to_string(), + 0.9, + ); + explicit_initializer_seeded = true; + } } } - let Some(value_n) = node.child_by_field_name("value") else { - return; - }; - // Constructor: confidence 1.0 (overrides annotation in edge builder) - if value_n.kind() == "new_expression" { - if let Some(type_name) = extract_new_expr_type_name(&value_n, source) { - push_scoped_type_map_entry( - symbols, - enclosing_qualifier.as_deref(), - var_name, - type_name.to_string(), - 1.0, - ); + // Type annotation: confidence 0.9 — only when neither of the above + // already seeded a more authoritative entry from the initializer itself. + if !explicit_initializer_seeded { + if let Some(type_anno) = find_child(node, "type_annotation") { + if let Some(type_name) = extract_simple_type_name(&type_anno, source) { + push_scoped_type_map_entry( + symbols, + enclosing_qualifier.as_deref(), + var_name, + type_name.to_string(), + 0.9, + ); + } } } + let Some(value_n) = value_n else { + return; + }; // Phase 8.3e: Object.create({ key: fn }) → composite pts key per property if value_n.kind() == "call_expression" { seed_object_create_entries(var_name, &value_n, source, symbols); @@ -10550,6 +10616,75 @@ mod tests { assert_eq!(tm.unwrap().confidence, 0.7); } + // Issue #2397: `as`-cast target must seed the typeMap directly, at the + // source, rather than leaving the local unresolvable and dependent on + // fragile bare-key propagation from an unrelated function in the file — + // exactly the divergence #2235's scoping fix didn't reach for + // `src/db/connection.ts`'s `openReadonlyOrFail`. + #[test] + fn as_cast_seeds_type_map_at_point_nine_confidence() { + let s = parse_ts("const db = new Database(path) as BetterSqlite3Database;"); + let tm = s.type_map.iter().find(|t| t.name == "db"); + assert!( + tm.is_some(), + "expected 'db' to be typed; got {:?}", + s.type_map + ); + assert_eq!(tm.unwrap().type_name, "BetterSqlite3Database"); + assert_eq!(tm.unwrap().confidence, 0.9); + } + + #[test] + fn as_cast_extracts_final_target_type_from_a_chained_as_unknown_as_x() { + let s = parse_ts("const db = new Database(path) as unknown as BetterSqlite3Database;"); + let tm = s.type_map.iter().find(|t| t.name == "db"); + assert!( + tm.is_some(), + "expected 'db' to be typed; got {:?}", + s.type_map + ); + assert_eq!(tm.unwrap().type_name, "BetterSqlite3Database"); + assert_eq!(tm.unwrap().confidence, 0.9); + } + + #[test] + fn as_cast_seeds_nothing_for_a_bare_as_unknown_with_no_further_cast() { + let s = parse_ts("const db = new Database(path) as unknown;"); + assert!(s.type_map.iter().all(|t| t.name != "db")); + } + + #[test] + fn as_cast_wins_over_a_same_declaration_type_annotation() { + // dedup_type_map is first-write-wins on confidence TIES — this proves + // the cast is checked (and skips the annotation push) BEFORE the + // annotation branch, not merely pushed alongside it at the same 0.9 + // and left to an ambiguous tie. + let s = parse_ts("const db: RawHandle = new Database(path) as BetterSqlite3Database;"); + let tm = s.type_map.iter().find(|t| t.name == "db"); + assert!( + tm.is_some(), + "expected 'db' to be typed; got {:?}", + s.type_map + ); + assert_eq!(tm.unwrap().type_name, "BetterSqlite3Database"); + } + + #[test] + fn as_cast_does_not_mistake_a_bare_identifier_cast_input_for_the_target_type() { + // Regression guard: extract_as_expression_type_name must scan for + // type_identifier specifically, not identifier, or `raw` (the cast's + // INPUT, an ordinary identifier) would be wrongly returned instead of + // the actual target type `Handle`. + let s = parse_ts("const db = raw as Handle;"); + let tm = s.type_map.iter().find(|t| t.name == "db"); + assert!( + tm.is_some(), + "expected 'db' to be typed; got {:?}", + s.type_map + ); + assert_eq!(tm.unwrap().type_name, "Handle"); + } + /// `this.prop = new Ctor()` outside any class declaration (function-style /// constructor) falls back to the un-scoped `this.prop` key. #[test] diff --git a/src/extractors/javascript.ts b/src/extractors/javascript.ts index a7645ef45..57da76c3f 100644 --- a/src/extractors/javascript.ts +++ b/src/extractors/javascript.ts @@ -2727,6 +2727,40 @@ function extractSimpleTypeName(typeAnnotationNode: TreeSitterNode): string | nul return null; } +/** + * Extract the target type name from an `as_expression` (`value as Type`). + * + * `as_expression` has no named fields in tree-sitter-typescript's grammar — + * its two named children (the expression and the type) are distinguished + * only by position/kind, not a field name. Scanning from the END and + * matching on `type_identifier`/`generic_type`/`parenthesized_type` (never + * `identifier`, unlike `extractSimpleTypeName`) is safe because the + * expression side can never produce those node kinds — TS's grammar keeps + * "type" and "expression" as disjoint node-kind namespaces — so there is no + * risk of matching the cast's INPUT instead of its target type, even when + * that input is itself a bare identifier. + * + * `X as unknown as Y` parses as nested as_expressions, `(X as unknown) as + * Y` — called on the outermost node, this naturally extracts `Y` (the final, + * intended type) without needing to special-case the `unknown` hop; called + * on a bare `X as unknown`, it correctly finds no nameable type (`unknown` + * is a `predefined_type`, not handled here) and returns null. + */ +function extractAsExpressionTypeName(asExprNode: TreeSitterNode): string | null { + for (let i = asExprNode.childCount - 1; i >= 0; i--) { + const child = asExprNode.child(i); + if (!child) continue; + const t = child.type; + if (t === 'type_identifier') return child.text; + if (t === 'generic_type') { + const base = child.child(0)?.text || null; + return base && OPAQUE_TYPE_TRANSFORM_WRAPPERS.has(base) ? null : base; + } + if (t === 'parenthesized_type') return extractSimpleTypeName(child); + } + return null; +} + function extractNewExprTypeName(newExprNode: TreeSitterNode): string | null { if (newExprNode?.type !== 'new_expression') return null; const ctor = newExprNode.childForFieldName('constructor') || newExprNode.child(1); @@ -3678,6 +3712,21 @@ function handleVarDeclaratorTypeMap( } } + // 2b. `as`-cast wins over annotation too, same rationale as the constructor + // branch above: `const db = new Database(...) as unknown as BetterSqlite3Database` + // must resolve to the CAST's target type, not the annotation (there usually + // isn't one) or the inner constructor's own name — the cast is what the rest + // of the file actually treats the value as from this point on (#2397). + // Confidence 0.9, matching the type-annotation tier below: both are explicit, + // developer-declared types, just via different syntax. + if (valueN?.type === 'as_expression') { + const castType = extractAsExpressionTypeName(valueN); + if (castType) { + setScopedTypeMapEntry(typeMap, enclosingQualifier, nameN.text, castType, 0.9); + return; + } + } + // 3. Type annotation — confidence 0.9. if (typeAnno) { const typeName = extractSimpleTypeName(typeAnno); diff --git a/tests/engines/parity.test.ts b/tests/engines/parity.test.ts index aa2904bb0..90d79ef9e 100644 --- a/tests/engines/parity.test.ts +++ b/tests/engines/parity.test.ts @@ -1031,6 +1031,49 @@ const result = utils.create(); expect(entries?.find((e) => e.name === 'result')).toBeUndefined(); }); + // Explicit guard for issue #2397: the real-world repro that motivated #2235 + // was still divergent after that fix landed, because neither engine's + // handleVarDeclaratorTypeMap had a branch for `as`-cast values at all — + // `const db = new Database(...) as unknown as BetterSqlite3Database` (this + // repo's own src/db/connection.ts:389) contributed NOTHING to the typeMap + // on either engine, leaving `db`'s resolution dependent on fragile + // bare-key propagation luck that happened to differ between engines. The + // normalize() loop above strips typeMap from the structural comparison, so + // this regression would otherwise slip through undetected. + it('TS — as-cast seeds typeMap at 0.9 on both engines, using the FINAL target type (issue #2397)', () => { + const code = `const db = new Database(path) as unknown as BetterSqlite3Database;`; + const wasm = wasmExtract(code, 'service.ts'); + expect(wasm?.typeMap).toBeInstanceOf(Map); + expect(wasm?.typeMap?.get('db')).toEqual({ type: 'BetterSqlite3Database', confidence: 0.9 }); + + if (!hasNative) return; + const raw = nativeExtract(code, 'service.ts'); + if (raw?.typeMap === undefined) return; + const entries = raw?.typeMap as Array<{ name: string; typeName: string; confidence: number }>; + expect(Array.isArray(entries)).toBe(true); + const dbEntry = entries?.find((e) => e.name === 'db'); + expect(dbEntry, 'native typeMap missing "db" key').toBeDefined(); + expect(dbEntry).toMatchObject({ + name: 'db', + typeName: 'BetterSqlite3Database', + confidence: 0.9, + }); + }); + + it('TS — as-cast wins over a same-declaration type annotation on both engines (issue #2397)', () => { + const code = `const db: RawHandle = new Database(path) as BetterSqlite3Database;`; + const wasm = wasmExtract(code, 'service.ts'); + expect(wasm?.typeMap?.get('db')).toEqual({ type: 'BetterSqlite3Database', confidence: 0.9 }); + + if (!hasNative) return; + const raw = nativeExtract(code, 'service.ts'); + if (raw?.typeMap === undefined) return; + const entries = raw?.typeMap as Array<{ name: string; typeName: string; confidence: number }>; + const dbEntry = entries?.find((e) => e.name === 'db'); + expect(dbEntry, 'native typeMap missing "db" key').toBeDefined(); + expect(dbEntry?.typeName).toBe('BetterSqlite3Database'); + }); + // Explicit guard for the WASM Python fix in #1189. The structural parity // loop above strips `self` from both sides via normalize(), so a regression // where WASM re-emits self/cls would slip through. Assert it directly. diff --git a/tests/parsers/javascript.test.ts b/tests/parsers/javascript.test.ts index 61b13cc7e..97c1ccde7 100644 --- a/tests/parsers/javascript.test.ts +++ b/tests/parsers/javascript.test.ts @@ -713,6 +713,47 @@ describe('JavaScript parser', () => { expect(symbols.typeMap.get('x')).toEqual({ type: 'Derived', confidence: 1.0 }); }); + // Issue #2397: `as`-cast target must seed the typeMap directly, at the + // source, rather than leaving `db` unresolvable and dependent on + // fragile bare-key propagation from an unrelated function in the file. + it('extracts the target type from a single as-cast at confidence 0.9', () => { + const symbols = parseTS(`const db = new Database(path) as BetterSqlite3Database;`); + expect(symbols.typeMap.get('db')).toEqual({ + type: 'BetterSqlite3Database', + confidence: 0.9, + }); + }); + + it('extracts the FINAL target type from a chained "as unknown as X" cast', () => { + const symbols = parseTS(`const db = new Database(path) as unknown as BetterSqlite3Database;`); + expect(symbols.typeMap.get('db')).toEqual({ + type: 'BetterSqlite3Database', + confidence: 0.9, + }); + }); + + it('does not seed anything for a bare "as unknown" cast with no further cast', () => { + const symbols = parseTS(`const db = new Database(path) as unknown;`); + expect(symbols.typeMap.has('db')).toBe(false); + }); + + it('as-cast wins over a same-declaration type annotation', () => { + const symbols = parseTS(`const db: RawHandle = new Database(path) as BetterSqlite3Database;`); + expect(symbols.typeMap.get('db')).toEqual({ + type: 'BetterSqlite3Database', + confidence: 0.9, + }); + }); + + it('does not mistake a bare-identifier cast input for the target type', () => { + // Regression guard: extractAsExpressionTypeName must scan for + // type_identifier specifically, not identifier, or `raw` (the cast's + // INPUT, an ordinary identifier) would be wrongly returned instead of + // the actual target type `Handle`. + const symbols = parseTS(`const db = raw as Handle;`); + expect(symbols.typeMap.get('db')).toEqual({ type: 'Handle', confidence: 0.9 }); + }); + it('extracts factory method patterns with confidence 0.7', () => { const symbols = parseJS(`const client = HttpClient.create();`); expect(symbols.typeMap.get('client')).toEqual({ type: 'HttpClient', confidence: 0.7 });