feat(javascript): migrate JavaScript to scope-based resolution (RFC #909 Ring 3, issue #928)#1640
Conversation
…atwari#909 Ring 3, issue abhigyanpatwari#928) Implements the scope-resolution pipeline for JavaScript, making it the fourth language to migrate after TypeScript, Python, and C#. New package: `languages/javascript/` (9 files) - `scope-resolver.ts` — Registers `javascriptScopeResolver` in `SCOPE_RESOLVERS`. Enables `fieldFallbackOnMethodLookup` (JS is dynamically typed), `allowGlobalFreeCallFallback` (CJS patterns), `propagatesReturnTypesAcrossImports`, and `hoistTypeBindingsToModule` so JSDoc @returns bindings reach cross-file callers. - `query.ts` — Tree-sitter query for JS scope captures compiled against `tree-sitter-javascript`. Subset of the TypeScript query with TS-only nodes dropped. Key differences: uses `(class)` not `class_expression`, adds `field_definition` for modern class fields. - `captures.ts` — `emitJsScopeCaptures` with four JS-specific synthesis passes on top of the shared tree-sitter query: 1. CJS `require()` decomposition — turns `const { X } = require('./m')` and `const X = require('./m')` into the same `@import.*` markers that the ESM decomposer produces, so `interpretJsImport` needs no new branches. 2. JSDoc bindings — scans AST nodes for preceding `/** */` comments, extracts `@param {T} name` and `@returns {T}` tags, and emits synthetic `@type-binding.parameter` / `.return` captures. Handles `export function` wrappers by looking at the parent `export_statement`'s preceding sibling, not the inner function's. 3. Constructor field bindings — walks constructor bodies for `this.X = new Y()` assignments and emits `@type-binding.class-field` anchored inside the constructor (hoisted to Class scope by `tsBindingScopeFor`). JSDoc `@type {T}` takes priority over `new Y()` inference when both are present. 4. Destructuring / for-of / instanceof synthesis (shared with TS). - `interpret.ts`, `simple-hooks.ts`, `merge-bindings.ts`, `arity.ts`, `import-target.ts` — thin adapters delegating to their TypeScript counterparts. ESM and CJS shapes are unified by the synthesizer, so no new interpreter branches are needed. Shared pipeline changes - `typescript/simple-hooks.ts` — `tsBindingScopeFor` now hoists `@type-binding.class-field` to the enclosing Class scope (mirrors the existing `@type-binding.parameter-property` branch). - `typescript/interpret.ts` — assigns `source: 'annotation'` to `@type-binding.class-field` captures so the binding strength correctly overrides constructor-inferred fallbacks. - `languages/typescript.ts` — wires `emitJsScopeCaptures` into `javascriptProvider.emitScopeCaptures`. - `registry-primary-flag.ts` — adds `JavaScript` to `MIGRATED_LANGUAGES` behind `REGISTRY_PRIMARY_JAVASCRIPT` env flag (default `true`). - `scope-resolution/pipeline/registry.ts` — registers `javascriptScopeResolver`. Test fixture fix - `test/fixtures/cross-file-binding/js-cross-file/src/models.js` — added `/** @returns {User} */` to `getUser()` so the cross-file return type propagation pass has a binding to mirror into callers. Tests (48 total, parity gate passes at both legacy=0 and registry-primary=1) The integration suite `test/integration/resolvers/javascript.test.ts` covers 13 describe blocks: - Pipeline skipGraphPhases option (4 tests) - `this`-resolution: `this.save()` inside a method resolves to the correct class, not a same-named method on another class (3 tests) - Parent/extends resolution: EXTENDS edges + class hierarchy (4 tests) - Nullable receiver: `/** @returns {User|null} */` strips null and resolves correctly (5 tests) - Super resolution: `super.save()` routes to the parent class (4 tests) - Chained method call: `svc.getUser().save()` via JSDoc @returns (4 tests) - Field type resolution: Property nodes, HAS_PROPERTY edges, static fields, field metadata (5 tests) - Write-access ACCESSES edges: `user.name = "Alice"` emits write edge with confidence 1.0 (3 tests) - Object destructuring: `const { address } = user; address.save()` resolves through the class-field binding (3 tests) - Post-fixpoint for-loop replay: `for...of` iterable resolved after fixpoint (1 test) - Cross-file ESM binding propagation: `const u = getUser(); u.save()` resolves to `User#save` across two files (5 tests) - Method enrichment: HAS_METHOD edges, EXTENDS, isStatic, CALLS (7 tests) - Inherited method resolution (SM-9): MRO first-wins walk (3 tests) Co-authored-by: Cursor <cursoragent@cursor.com>
|
@ReidenXerx is attempting to deploy a commit to the NexusCore Team on Vercel. A member of the Team first needs to authorize it. |
CI Report✅ All checks passed Pipeline Status
Test Results
✅ All 9217 tests passed 1 test(s) skipped — expand for details
Code CoverageTests
📋 View full run · Generated by CI |
The js-consumer-before-provider fixture was missing a JSDoc @returns annotation on getUser(), preventing the scope-based resolution pipeline from propagating the return type and resolving u.save() to User#save. Mirrors the same pattern already applied to js-cross-file/src/models.js, and matches how the TypeScript equivalent uses an explicit `: User` return type annotation. Fixes the CI test failure introduced when JavaScript was added to MIGRATED_LANGUAGES in the JS scope-resolution migration (PR abhigyanpatwari#1640). Co-authored-by: Cursor <cursoragent@cursor.com>
|
Following up on @kratos's request for a solid understanding of ECMAScript semantics before review: Quick note on the spec linkThe link you shared ( Tracking issueI've opened #1646 following the same format as @magyargergo's C++ tracking issue (#1564): LANG-javascript: Remaining work after PR #1640 scope-resolution parity → #1646 It documents:
Happy to turn any row in the divergences table into a dedicated sub-issue if that's the preferred workflow. |
Adds JSDoc @type {T} support for variable declarations: /** @type {User} */ const u = getUser(); Previously only @param and @returns were extracted from JSDoc. Now @type on a lexical_declaration / variable_declaration emits a @type-binding.annotation capture (source = 'annotation', strength 2), which overrides any weaker constructor-inferred or alias bindings on the same name. Closes gap §14.3.1/§14.3.2 from the ECMA-262 conformance audit in abhigyanpatwari#1646. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Claude finished @magyargergo's task in 6m 21s —— View job Adversarial Production-Readiness Review — PR #1640Todo
Review Bar for This PRDoD.md §2.2 requires no language-specific logic to leak into shared infrastructure without a documented reason. §2.7 requires tests that cover ambiguous/suppression cases, not just positive resolution. DoD §6 "Not Done" signals include "A language-specific concern leaked into shared ingestion code" and "tests do not actually exercise the changed path." These are the primary bars I'm applying. Current PR State
Branch Hygiene AssessmentVerdict: merge-from-main commits present but harmless and merge-safe. The two merge commits (
The Understanding of the ChangeJavaScript becomes the 8th language in FindingsFinding 1 — Stale Documentation in
|
|
@ReidenXerx Can you please look into claude's findings before we merge this into main? 🙏 |
Yes will do EOD |
Finding 1 — stale index.ts comment: @type {T} on variable declarations is now synthesized; update the Known Limitations note to list only @typedef as unimplemented. Finding 2 — isExprStmt over-broad in synthesizeJsDocBindings: Removing `isExprStmt` from the gate condition eliminates the false- positive @param binding that would be emitted for JSDoc comments preceding call-expression statements (e.g. `/** @param {User} user */ users.forEach(user => { ... })`). The expression_statement branch provided no useful emission anyway (fnName stayed null so @returns was never emitted either). Finding 3 — JS-specific logic leaking into shared TypeScript files: - Export walkToScope from typescript/simple-hooks.ts so JS hooks can reuse the traversal without duplicating it. - Replace the javascript/simple-hooks.ts re-export with a proper jsBindingScopeFor wrapper that handles @type-binding.class-field (hoist to Class scope) and then delegates to tsBindingScopeFor. - Remove the @type-binding.class-field branch from tsBindingScopeFor (TypeScript never emits this tag). - In interpretJsTypeBinding, remap @type-binding.class-field → @type-binding.annotation before delegating to interpretTsTypeBinding, so source = 'annotation' is set correctly without a JS-specific branch in shared TS code. Remove the branch from interpretTsTypeBinding. Finding 5 (cosmetic) — query.ts header listed class_expression: Correct the "What IS shared" comment to say (class) — the actual JS grammar node type used in the query — and explain why class_expression does not exist in tree-sitter-javascript. All 48 JS + 236 TS integration tests pass. Co-authored-by: Cursor <cursoragent@cursor.com>
|
All 4 actionable findings addressed — pushed as a single commit (
Finding 4 (missing suppression test for All 48 JS + 236 TS integration tests pass after the changes. |
2 participants
What this PR does
Implements the RFC #909 Ring 3 scope-resolution pipeline for JavaScript, closing issue #928. JavaScript becomes the fourth language to migrate after TypeScript, Python, and C#.
The new pipeline replaces the legacy call-processor path (controlled by
REGISTRY_PRIMARY_JAVASCRIPT, defaulting totrue) and passes a full parity gate: 48/48 tests in bothlegacy=0andregistry-primary=1modes.New package:
gitnexus/src/core/ingestion/languages/javascript/Nine new files, each with a focused responsibility:
scope-resolver.tsRegisters
javascriptScopeResolverinSCOPE_RESOLVERS. The key flags vs TypeScript:fieldFallbackOnMethodLookup: true— JavaScript is dynamically typed, so the field-fallback heuristic is on (TypeScript keeps it off because its type-binding layer is precise).allowGlobalFreeCallFallback: true— CJSrequirepatterns and global helpers benefit from workspace-wide unique-name fallback.propagatesReturnTypesAcrossImports: true+hoistTypeBindingsToModule: true— JSDoc@returns {T}bindings are synthesized on the function scope, hoisted, and propagated to importers so cross-file chains likeconst u = getUser(); u.save()resolve correctly.query.tsTree-sitter query for JS scope captures compiled against
tree-sitter-javascript. It's a purposeful subset of the TypeScript query — all TypeScript-only node types (interface_declaration,type_annotation,public_field_definition, etc.) are dropped because the JS grammar doesn't define them.Two important differences from what you might expect:
(class) @scope.class—class_expressiondoesn't exist in the JS grammar and would throw a hardTSQueryErrorNodeTypeat runtime.(field_definition property: …) @declaration.propertyfor modern class fields (name = valuesyntax).captures.tsemitJsScopeCapturesruns the standard tree-sitter query and then applies four JavaScript-specific synthesis passes:CJS
require()decomposition — Walks the AST forrequire()calls and synthesizes the same@import.kind/name/alias/sourcemarkers that the ESM decomposer produces. This meansinterpretJsImportdelegates unchanged tointerpretTsImportfor all cases (named, namespace, aliased, side-effect).JSDoc type bindings — Scans each function-like node for a preceding
/** */block comment, then extracts@param {T} name,@returns {T}, and@type {T}tags. One important subtlety: forexport function foo() {}, the JSDoc comment is the sibling of the wrappingexport_statement, not the innerfunction_declaration, so we walk up to the parent when needed.Constructor field bindings — Walks constructor bodies for
this.X = new Y()assignments. Emits@type-binding.class-fieldanchored inside the constructor sotsBindingScopeForhoists it to the enclosing Class scope. This makes object-destructuring chains likeconst { address } = user; address.save()resolve end-to-end. JSDoc@type {T}takes priority overnew Y()inference when both are present.Destructuring / for-of / instanceof synthesis — Shared with TS captures (pure AST logic, no grammar-specific code).
interpret.ts,simple-hooks.ts,merge-bindings.ts,arity.ts,import-target.tsThin adapters that delegate to their TypeScript counterparts. Because the synthesizer unifies ESM and CJS shapes, no new interpreter branches are needed anywhere.
Shared pipeline changes
typescript/simple-hooks.tstsBindingScopeFornow hoists@type-binding.class-fieldto the Class scope (mirrors the existing@type-binding.parameter-propertybranch for TypeScript constructor parameter properties). Only fires for JavaScript captures, no TypeScript behaviour changes.typescript/interpret.tsAssigns
source: 'annotation'to@type-binding.class-fieldso its binding strength (2) correctly overrides constructor-inferred fallbacks (1).languages/typescript.tsWires
emitJsScopeCapturesintojavascriptProvider.emitScopeCaptures.registry-primary-flag.tsAdds
JavaScripttoMIGRATED_LANGUAGESbehindREGISTRY_PRIMARY_JAVASCRIPTenv flag (defaults totrue).scope-resolution/pipeline/registry.tsRegisters
javascriptScopeResolver.Bugs found and fixed during implementation
These were discovered while getting the parity gate to green:
(class_expression)doesn't exist intree-sitter-javascript; should be(class)field_definitionnodes (modern class fields) weren't captured, so they never entered the semantic modelfield_definitionpattern to querysvc.getUser().save()failingsynthesizeJsDocBindingsskippedmethod_definitionnodes, so@returnson methods was never synthesizedmethod_definitionbranchexport functionnot foundexport_statement, not the innerfunction_declarationexport_statementwhen looking for the preceding commentUser.address → Addressin the class scopesynthesizeConstructorFieldBindingssynthesizes class-field type bindings from constructor bodiesu.save()not resolvinggetUser()in the fixture had no@returns {User}, sopropagateImportedReturnTypeshad nothing to mirrorTest coverage
48 integration tests in
test/integration/resolvers/javascript.test.ts, covering 13 scenarios:this-resolution —this.save()routes to the right class, not a same-named method elsewhereUser|nullstripped, correct CALLS edgesuper.save()routes to parent classsvc.getUser().save()via JSDoc@returnsuser.name = "Alice"emits write edge (confidence 1.0)const { address } = user; address.save()resolvesconst u = getUser(); u.save()across two filesParity gate:
REGISTRY_PRIMARY_JAVASCRIPT=0andREGISTRY_PRIMARY_JAVASCRIPT=1both pass all 48 tests.ECMA-262 conformance posture
Tracked in #1646 — LANG-javascript: Remaining work after PR #1640 scope-resolution parity (same format as @magyargergo's C++ tracking issue #1564).
The resolver is a graph-safe under-approximation of ECMAScript name lookup — suppresses rather than guesses when semantics are ambiguous. The full tables of what's implemented and what's intentionally deferred live in #1646, but the headline summary is:
36 ECMA-262 constructs covered in this PR, including: module/class/function scopes, all
var/let/constdeclaration forms, ESM named/default/namespace/re-export/dynamic imports, CJSrequire()(named, namespace, side-effect), all call shapes (free, member, constructor,super), member write/read access,instanceofnarrowing, JSDoc@param/@returns/@typesynthesis, constructor field inference (this.X = new Y()), object/array destructuring,for...ofiteration,awaitunwrapping, class inheritance (MRO), JSX components, HOC-wrapped declarations.Why the remaining 12 gaps are deferred to #1646
Each deferred item falls into one of three categories:
Statically unresolvable — impossible to analyze without runtime information; suppression is the only correct answer:
obj[expr](§13.3.3) —expris a runtime valueSymbol.*well-known symbol keys (§6.1.5.1) — keys are object identity, not stringsObject.assign(T.prototype, M)mixin patterns — resolvingMrequires dataflow analysiswithandevalscope injection (§14.11, §19.2.1) — deprecated / runtime-only semanticsSignificant standalone scope — each is its own PR-sized chunk of work, the same way C++ ADL, SFINAE, and template partial ordering were separate sub-issues after #1520:
T.prototype.m = fn(§10.1.1) — requires detecting the assignment shape, retroactively binding methods to the owning class, and handling aliased prototype references (const p = T.prototype; p.m = fn)#field(§15.7.3.1) — requires addingprivate_property_identifierthroughout the query and interpreter, plus deciding the graph identity for private membersfn({ x })(§15.1.3) — requires JSDoc@paramname matching against the destructured binding, which needs changes in the parameter interpreterCorrect but low real-world impact — not worth the complexity cost in this PR:
let/const(§9.1) — only affects shadow-variable disambiguation; no edges are suppressed todaystatic {}initialization blocks (§15.7.3.3) — ES2022+, rare in practiceThe same deferral philosophy was used for the C++ migration (#935 → #1564) and the Python migration. Landing a working, flag-gated, fully-tested baseline and iterating in tracked follow-ups is the established pattern for Ring 3 migrations in this repo.
Risk & rollout
REGISTRY_PRIMARY_JAVASCRIPT(env var, defaulttrue). Setting it to0falls back to the legacy call-processor path with zero code changes.tsBindingScopeForandinterpretTsTypeBinding— both changes are additive (newelse ifbranches that only fire for the new@type-binding.class-fieldtag, which is only emitted by the JavaScript captures).npx gitnexus analyzeafter merge to refresh the index.Checklist
REGISTRY_PRIMARY_JAVASCRIPTflag wired — zero impact on other languagesMade with Cursor