morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #6384] [CLOSED] fix(javascript): exclude type, enum and meta-property names from lexical reads #7325

New issue

Closed

opened 2026-11-01 21:47:04 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:47:04 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6384
Author: @kshitizj03
Created: 10/22/2026
Status: ❌ Closed

Base: main ← Head: fix/nonreference-names

📝 Commits (3)

ad5fb03 fix(javascript): exclude type, enum and meta-property names from lexical reads
93f8b26 test(javascript): require complete parses for non-reference absence checks
93370dc Merge branch 'main' into fix/nonreference-names

📊 Changes

2 files changed (+70 additions, -0 deletions)

View changed files

📝 src/domain/javascript/javascriptSemanticAnalysis.structure.test.ts (+54 -0)
📝 src/domain/javascript/javascriptSemanticProjection.ts (+16 -0)

📄 Description

Summary

Fixes #6380.

TypeScript member keys, enum member names, the import.meta and new.target parts, and import attribute keys are syntax names, but JavaScript semantic analysis treated them as lexical reads. An outer binding with the same name gained a false resolved reads relation, for example const type = ... next to import data from "./data.json" with { type: "json" }, or const meta = ... next to import.meta.url. This is the same defect class as #3310 (class member keys, fixed by #3638), in positions that fix does not cover.

Problem and expected behavior

isNonReferenceKey (src/domain/javascript/javascriptSemanticProjection.ts) decides whether an identifier occurrence is a lexical reference. It had no case for these parents. Reproduced on main at 86655eac (analyzeJavaScriptSemantics(...).references); the branch is rebased on 35710024, which does not touch the JavaScript analysis files:

Input	Outer binding reads before	After
`interface Shape { marker: string }`	1 resolved (`marker`)	0
`interface Shape { marker(): void }`	1 resolved (`marker`)	0
`type Shape = { marker: string; marker(): void }`	1 resolved each (`marker`)	0
`enum Kind { marker }`	1 resolved (`marker`)	0
`import.meta.url`	1 resolved (`meta`)	0
`new.target`	1 resolved (`target`)	0
`import d from "./d.json" with { type: "json" }`	1 resolved (`type`)	0
`interface Shape { [marker]: string }` (computed control)	1 resolved	1 resolved (unchanged)
`enum Kind { first = marker }` (initializer control)	1 resolved	1 resolved (unchanged)

Change and scope

Add an isTypeOrModuleSyntaxName predicate to isNonReferenceKey, next to isNoncomputedClassMemberKey from #3638: noncomputed TSPropertySignature/TSMethodSignature keys, TSEnumMember ids, any MetaProperty identifier, and ImportAttribute keys. All consumers route through this predicate (collectSemanticReferencesSteps, semanticReferenceRole, closure-capture detection in javascriptSemanticCalls.ts, and reference-source require bindings), and JavaScriptSemanticGraphBuilder projects each read reference into a reads relation, so references, graph relations and captures stay consistent.

Not included: TypeScript declaration names (interface marker {}, type marker = ..., enum marker {}, namespace marker {}) and type references (let v: marker) also produce a resolved read on main. enum and namespace create runtime bindings and typeof marker in a type position is a real reference, so those need a decision about TypeScript type-space names; this change keeps to syntax names that can never be references. Described in the linked issue.

Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: isNonReferenceKey in javascriptSemanticProjection.ts (reference classification).
Contribution scope fit and linked maintainer decision: not applicable (bug fix within existing JavaScript analysis).
CLI and MCP/tool-catalog contract: none; schemas unchanged. Fewer false reads relations in semantic Evidence.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: removes false resolved reads relations; no new fields.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.
Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated: javascriptSemanticAnalysis.structure.test.ts, "non-reference names": (1) interface and type-literal property/method keys and an enum member name yield no marker references; (2) new.target, import.meta and an import attribute key yield no target/meta/type references; (3) a computed interface key and an enum initializer keep their resolved reads (lines 3 and 4). Each absence check first asserts coverage.status: "complete" and the declared outer binding, because a parse failure also yields no references; a deliberately unparseable sample fails on that assertion.
Base reproduction or other evidence: on 86655eac, tests (1) and (2) fail (5 false marker references; 3 false type/meta/target references) and test (3) passes before and after.

User-visible CLI/MCP output (if applicable): the public semantic graph, built with the graphForJavaScript test fixture (the projection the CLI returns), counting reads relations from the outer binding:

Sample	Base	Fix
`interface Shape { marker: string }`	1	0
`enum Kind { marker }`	1	0
`export const u = import.meta.url;` (binding `meta`)	1	0
`import d from "./d.json" with { type: "json" };` (binding `type`)	1	0
`interface Shape { [marker]: string }`	1	1

Remaining proof gaps: validated on Windows 11 from a source checkout, where CLI analysis is rejected by #3501, so the end-to-end analyze-javascript-application / trace-javascript-semantics CLI steps were not executed.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

npx vitest run src/domain/javascript/javascriptSemanticAnalysis.structure.test.ts -t "non-reference names" (base code, new tests): 2 failed, 1 passed; (fixed code): 3 passed
npx vitest run src/domain/javascript/javascriptSemanticAnalysis.structure.test.ts: 105 passed
npx vitest run src/domain/javascript src/application/javascript: 151 files, 4199 passed, 6 skipped
npx oxfmt --check (changed files): passed
npx oxlint (changed files): no new warnings (max-lines, max-params and the isDeclarationIdentifier complexity warning are identical on main)
npx tsc --noEmit (rebased on 35710024): passed, 0 errors
npm run check:fast (pre-push gate: typecheck and lint): passed

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not applicable (static JavaScript analysis); validated on Windows 11, Node 24.14.0.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none; parse-only change.

Author readiness

Human owner: @kshitizj03

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/6384 **Author:** [@kshitizj03](https://github.com/kshitizj03) **Created:** 10/22/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/nonreference-names` --- ### 📝 Commits (3) - [`ad5fb03`](https://github.com/morluto/rea/commit/ad5fb0364ddbc1530dcc676c3c9ea6cefdf0103e) fix(javascript): exclude type, enum and meta-property names from lexical reads - [`93f8b26`](https://github.com/morluto/rea/commit/93f8b26fa05234d2df5222c65dd42f2e057c9892) test(javascript): require complete parses for non-reference absence checks - [`93370dc`](https://github.com/morluto/rea/commit/93370dc87fd977952f230ec55b4a78111fac8a69) Merge branch 'main' into fix/nonreference-names ### 📊 Changes **2 files changed** (+70 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptSemanticAnalysis.structure.test.ts` (+54 -0) 📝 `src/domain/javascript/javascriptSemanticProjection.ts` (+16 -0) </details> ### 📄 Description ## Summary Fixes #6380. TypeScript member keys, enum member names, the `import.meta` and `new.target` parts, and import attribute keys are syntax names, but JavaScript semantic analysis treated them as lexical reads. An outer binding with the same name gained a false resolved `reads` relation, for example `const type = ...` next to `import data from "./data.json" with { type: "json" }`, or `const meta = ...` next to `import.meta.url`. This is the same defect class as #3310 (class member keys, fixed by #3638), in positions that fix does not cover. ## Problem and expected behavior `isNonReferenceKey` (`src/domain/javascript/javascriptSemanticProjection.ts`) decides whether an identifier occurrence is a lexical reference. It had no case for these parents. Reproduced on `main` at `86655eac` (`analyzeJavaScriptSemantics(...).references`); the branch is rebased on `35710024`, which does not touch the JavaScript analysis files: | Input | Outer binding reads before | After | |---|---|---| | `interface Shape { marker: string }` | 1 resolved (`marker`) | 0 | | `interface Shape { marker(): void }` | 1 resolved (`marker`) | 0 | | `type Shape = { marker: string; marker(): void }` | 1 resolved each (`marker`) | 0 | | `enum Kind { marker }` | 1 resolved (`marker`) | 0 | | `import.meta.url` | 1 resolved (`meta`) | 0 | | `new.target` | 1 resolved (`target`) | 0 | | `import d from "./d.json" with { type: "json" }` | 1 resolved (`type`) | 0 | | `interface Shape { [marker]: string }` (computed control) | 1 resolved | 1 resolved (unchanged) | | `enum Kind { first = marker }` (initializer control) | 1 resolved | 1 resolved (unchanged) | ## Change and scope Add an `isTypeOrModuleSyntaxName` predicate to `isNonReferenceKey`, next to `isNoncomputedClassMemberKey` from #3638: noncomputed `TSPropertySignature`/`TSMethodSignature` keys, `TSEnumMember` ids, any `MetaProperty` identifier, and `ImportAttribute` keys. All consumers route through this predicate (`collectSemanticReferencesSteps`, `semanticReferenceRole`, closure-capture detection in `javascriptSemanticCalls.ts`, and reference-source require bindings), and `JavaScriptSemanticGraphBuilder` projects each `read` reference into a `reads` relation, so references, graph relations and captures stay consistent. Not included: TypeScript declaration names (`interface marker {}`, `type marker = ...`, `enum marker {}`, `namespace marker {}`) and type references (`let v: marker`) also produce a resolved read on `main`. `enum` and `namespace` create runtime bindings and `typeof marker` in a type position is a real reference, so those need a decision about TypeScript type-space names; this change keeps to syntax names that can never be references. Described in the linked issue. Contribution scope: ordinary fix within existing boundaries; maintainer scope decision not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: `isNonReferenceKey` in `javascriptSemanticProjection.ts` (reference classification). - Contribution scope fit and linked maintainer decision: not applicable (bug fix within existing JavaScript analysis). - CLI and MCP/tool-catalog contract: none; schemas unchanged. Fewer false `reads` relations in semantic Evidence. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: removes false resolved `reads` relations; no new fields. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `javascriptSemanticAnalysis.structure.test.ts`, "non-reference names": (1) interface and type-literal property/method keys and an enum member name yield no `marker` references; (2) `new.target`, `import.meta` and an import attribute key yield no `target`/`meta`/`type` references; (3) a computed interface key and an enum initializer keep their resolved reads (lines 3 and 4). Each absence check first asserts `coverage.status: "complete"` and the declared outer binding, because a parse failure also yields no references; a deliberately unparseable sample fails on that assertion. - Base reproduction or other evidence: on `86655eac`, tests (1) and (2) fail (5 false `marker` references; 3 false `type`/`meta`/`target` references) and test (3) passes before and after. - User-visible CLI/MCP output (if applicable): the public semantic graph, built with the `graphForJavaScript` test fixture (the projection the CLI returns), counting `reads` relations from the outer binding: | Sample | Base | Fix | |---|---|---| | `interface Shape { marker: string }` | 1 | 0 | | `enum Kind { marker }` | 1 | 0 | | `export const u = import.meta.url;` (binding `meta`) | 1 | 0 | | `import d from "./d.json" with { type: "json" };` (binding `type`) | 1 | 0 | | `interface Shape { [marker]: string }` | 1 | 1 | - Remaining proof gaps: validated on Windows 11 from a source checkout, where CLI analysis is rejected by #3501, so the end-to-end `analyze-javascript-application` / `trace-javascript-semantics` CLI steps were not executed. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npx vitest run src/domain/javascript/javascriptSemanticAnalysis.structure.test.ts -t "non-reference names"` (base code, new tests): 2 failed, 1 passed; (fixed code): 3 passed - `npx vitest run src/domain/javascript/javascriptSemanticAnalysis.structure.test.ts`: 105 passed - `npx vitest run src/domain/javascript src/application/javascript`: 151 files, 4199 passed, 6 skipped - `npx oxfmt --check` (changed files): passed - `npx oxlint` (changed files): no new warnings (`max-lines`, `max-params` and the `isDeclarationIdentifier` complexity warning are identical on `main`) - `npx tsc --noEmit` (rebased on `35710024`): passed, 0 errors - `npm run check:fast` (pre-push gate: typecheck and lint): passed ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable (static JavaScript analysis); validated on Windows 11, Node 24.14.0. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none; parse-only change. ## Author readiness Human owner: @kshitizj03 - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:47:04 +00:00

closed this issue
added the
pull-request
label

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#7325

No description provided.

Rows
Columns