-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
fix(rust): scope-resolution coverage gaps — F66,F68,F71,F72 (#1934) #1974
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e86b500
03dac44
93e4096
622053a
61543b9
49d2940
8ceb08c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| /** | ||
| * `MacroRegistry` — scope-aware lookup for macro definitions | ||
| * (`macro_rules!` in Rust; `#define` in C/C++) referenced from a macro | ||
| * invocation site. | ||
| * | ||
| * Thin wrapper over `lookupCore`, specialized for the macro namespace: | ||
| * | ||
| * - `acceptedKinds` = `MACRO_KINDS` (`['Macro']` only). Crucially this | ||
| * does NOT include `Function`/`Method`, so a `log!(…)` invocation can | ||
| * never resolve to a same-named free function `fn log` — macros and | ||
| * functions are disjoint namespaces (the false-`CALLS`-edge class the | ||
| * #1934 review flagged). | ||
| * - `useReceiverTypeBinding` is **false** — a macro invocation has no | ||
| * receiver; resolution is name-through-the-lexical-chain + the global | ||
| * qualified fallback, exactly like `ClassRegistry`. | ||
| * - Arity is not applied — macros are variadic by nature. | ||
| */ | ||
|
|
||
| import type { Resolution, ScopeId } from '../types.js'; | ||
| import { lookupCore, type CoreLookupParams } from './lookup-core.js'; | ||
| import { MACRO_KINDS, type RegistryContext } from './context.js'; | ||
|
|
||
| export interface MacroRegistry { | ||
| /** | ||
| * Look up a macro definition by simple or scoped name anchored at | ||
| * `scope`. Returns a confidence-ranked `Resolution[]`; consume `[0]` | ||
| * for the best answer. | ||
| */ | ||
| lookup(name: string, scope: ScopeId): readonly Resolution[]; | ||
| } | ||
|
|
||
| export function buildMacroRegistry(ctx: RegistryContext): MacroRegistry { | ||
| const params: CoreLookupParams = { | ||
| acceptedKinds: MACRO_KINDS, | ||
| useReceiverTypeBinding: false, | ||
| ownerScopedContributor: null, | ||
| }; | ||
| return { | ||
| lookup(name: string, scope: ScopeId) { | ||
| return lookupCore(name, scope, params, ctx); | ||
| }, | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ const RUST_SCOPE_QUERY = ` | |
| (trait_item) @scope.class | ||
| (impl_item) @scope.class | ||
| (enum_item) @scope.class | ||
| (union_item) @scope.class | ||
| (function_item) @scope.function | ||
| (closure_expression) @scope.function | ||
| (block) @scope.block | ||
|
|
@@ -30,6 +31,26 @@ const RUST_SCOPE_QUERY = ` | |
| (enum_item | ||
| name: (type_identifier) @declaration.name) @declaration.enum | ||
|
|
||
| ;; Declarations — union | ||
| ;; Deliberately tagged @declaration.struct (→ Struct label), NOT a | ||
| ;; @declaration.union: every registry-primary resolution gate — | ||
| ;; isLinkableLabel (node-lookup.ts), CALLABLE_OR_TYPE_LIKE | ||
| ;; (finalize-algorithm.ts), ClassLikeNodeLabel (class-types.ts) — includes | ||
| ;; Struct but EXCLUDES Union, so a Union-labeled node would be an | ||
| ;; unresolvable orphan. A Rust union is a type whose literal is a real | ||
| ;; constructor, so Struct is both the resolvable and the semantically | ||
| ;; honest label here. #1934 F71. | ||
| (union_item | ||
| name: (type_identifier) @declaration.name) @declaration.struct | ||
|
|
||
| ;; Declarations — macro (macro_rules! foo { ... }) | ||
| ;; Captured as @declaration.macro → Macro label. A macro invocation | ||
| ;; (@reference.macro, below) resolves to this definition via MacroRegistry, | ||
| ;; whose acceptedKinds is ['Macro'] ONLY — so an invoked macro never binds | ||
| ;; to a same-named free function (log!() is not fn log). #1934 F72. | ||
| (macro_definition | ||
| name: (identifier) @declaration.name) @declaration.macro | ||
|
|
||
| ;; Declarations — function (top-level or inside mod) | ||
| (function_item | ||
| name: (identifier) @declaration.name) @declaration.function | ||
|
|
@@ -40,6 +61,10 @@ const RUST_SCOPE_QUERY = ` | |
| type: (_) @declaration.field-type) @declaration.field | ||
|
|
||
| ;; Declarations — variables (let bindings) | ||
| ;; Uses pattern:(identifier) — works for let x and let mut x (mutable_specifier | ||
| ;; is a sibling, not a wrapper). Destructuring patterns like let (a, b) use | ||
| ;; tuple_pattern etc. which pattern:(identifier) intentionally does not match; | ||
| ;; capturing them with (_) would produce "(a, b)" as the name, which is useless. | ||
| (let_declaration | ||
| pattern: (identifier) @declaration.name) @declaration.variable | ||
|
|
||
|
|
@@ -109,9 +134,24 @@ const RUST_SCOPE_QUERY = ` | |
| name: (identifier) @reference.name)) @reference.call.free | ||
|
|
||
| ;; References — constructor calls (struct literal) | ||
| ;; Covers bare names (Foo {}), scoped (foo::bar::Baz {}), and turbofish | ||
| ;; (Foo::<T> {}) — the name: field resolves to the trailing identifier | ||
| ;; in all cases through tree-sitter-rust's grammar. | ||
| (struct_expression | ||
| name: (_) @reference.name) @reference.call.constructor | ||
|
|
||
| ;; References — macro invocations (disjoint namespace from functions) | ||
| ;; Resolved via MacroRegistry → Macro defs only (never fn of the same name). | ||
| (macro_invocation | ||
| macro: (identifier) @reference.name) @reference.macro | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2 · headline] F72 macro coverage is a silent no-op. This Fix: add [reproduced via tsx + code-read; corroborated by Codex (gpt-5.5/xhigh) + correctness/adversarial/risk lanes]
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resolved by fully wiring F72. Added a |
||
|
|
||
| ;; Scoped macro invocation (log::info!(…)) — capture the tail identifier, | ||
| ;; mirroring the scoped free-call pattern above, so the resolved name is | ||
| ;; the tail (info), not the full path (log::info). | ||
| (macro_invocation | ||
| macro: (scoped_identifier | ||
| name: (identifier) @reference.name)) @reference.macro | ||
|
|
||
| ;; References — field reads | ||
| (field_expression | ||
| value: (_) @reference.receiver | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| // F72 — macro invocations | ||
| fn use_macros() { | ||
| println!("hello"); | ||
| let v = vec![1, 2, 3]; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| // F66/F68 — let binding with various pattern shapes | ||
| fn pattern_shapes() { | ||
| let x = 1; // bare identifier | ||
| let mut y = 2; // identifier with mut | ||
| let (a, b) = (1, 2); // tuple pattern | ||
| let Some(val) = Some(3); // tuple struct pattern | ||
| let Foo { field } = Foo { field: 1 }; // struct pattern | ||
| let ref z = 4; // ref pattern | ||
| let n @ 1..=10 = 5; // captured pattern | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| // F71 — union declaration | ||
| union MyUnion { | ||
| x: i32, | ||
| y: f64, | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| // F72 — a macro invocation resolves to its `macro_rules!` definition (a | ||
| // Macro node) via a USES edge, and NEVER to a same-named free function. | ||
| // Macros and functions are disjoint namespaces. | ||
|
|
||
| macro_rules! greet { | ||
| ($name:expr) => { | ||
| let _ = $name; | ||
| }; | ||
| } | ||
|
|
||
| // Same simple name as the macro, on purpose: proves the macro invocation | ||
| // does not bind to this function (no false CALLS edge) and the function | ||
| // call does not bind to the macro. | ||
| fn greet() -> u32 { | ||
| 0 | ||
| } | ||
|
|
||
| fn run() { | ||
| greet!("world"); // macro invocation -> USES edge to Macro greet | ||
| let _ = greet(); // function call -> CALLS edge to Function greet | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| // F71 — a `union` is captured as a Struct-labeled node (every | ||
| // registry-primary resolution gate includes Struct but excludes Union), | ||
| // and it is resolvable: the union literal is a real type constructor. | ||
|
|
||
| union MyUnion { | ||
| int_val: i32, | ||
| float_val: f64, | ||
| } | ||
|
|
||
| fn make() -> MyUnion { | ||
| MyUnion { int_val: 5 } // constructor -> CALLS edge to the Struct MyUnion | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Union is labeled
Struct, notUnion— intentional/benign, but undocumented & untested.@declaration.structyieldsnormalizeNodeLabel('struct')='Struct'. This is actually the correct pragmatic choice: every registry-primary resolution gate —isLinkableLabel(node-lookup.ts:165-189),CALLABLE_OR_TYPE_LIKE(finalize-algorithm.ts:785-803),ClassLikeNodeLabel(class-types.ts:4-7) — includesStructbut excludesUnion, so a@declaration.unionnode would be an unresolvable orphan. So this is not a bug. But (a) no comment explains the deliberate downgrade, and (b) no pipeline-level test assertsunion.rsresolves to aStructnamedMyUnion.Fix: add a one-line rationale comment here, plus a pipeline-level union-resolution test.
[reproduced + code-read; Codex + correctness/adversarial]
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed, and it went deeper than docs: the union had no graph node at all — the legacy
RUST_QUERIESnever capturedunion_item, so the@declaration.structscope capture had nothing to resolve to. Added(union_item name: (type_identifier) @name) @definition.structto materialize it, plus the rationale comment (Struct chosen because every registry gate includes Struct, excludes Union). New pipeline testrust.test.ts › Rust union resolutionassertsunion.rsresolves to aStructnamedMyUnionand thatMyUnion { .. }emits aCALLSedge to it — runs on both parity halves. (49d2940)