Repository navigation
fix(kernel): a JS built-in method name needs receiver evidence (#1987) - #588
Conversation
…mchenry#1987) A call like `list.map()` or `cache.get()` on a receiver the resolver cannot type fell through to name-based method matching, so it linked a same-named project method. The free matching path now declines a built-in method name unless the receiver is rooted at `this`/`super` (a field chain) or at a class name; constructed, imported and typed receivers answer before that path. Fold the js-builtins.ts names into the existing JS_BUILT_IN_METHODS table so the prefilter and the resolver agree on the list. Restores the 11 failing suites on fork/consolidated: full suite 934 files, 11,157 passed, 44 skipped, 0 failed. Golden dumps unchanged; precision holds on vite (6/6 absent, 2/2 present) and svelte (1/1 absent, 0 gained).
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
![bompus-ai-review[bot]](https://avatars.githubusercontent.com/u/253635?s=60&v=4){kind=small}
| // rooted at `this`/`super` (a field chain) or at a class name is | ||
| // evidence of its own; constructed, imported and typed receivers | ||
| // resolve before this free path. | ||
| if is_js_family(&r.language) && JS_BUILT_IN_METHODS.contains(method_name.as_str()) { |
There was a problem hiding this comment.
medium: Preserve type inference for component receivers used in markup
The new gate returns Ok(None) before infer_local_receiver_type. However, pipeline.rs deliberately bypasses the bound-receiver claim when is_component_receiver_out_of_scope(r) is true, including constructed Svelte/Astro values used outside their script block.
Failure: In a Svelte component declaring class Counter { get() { return 1; } } and const counter = new Counter() inside its script, markup {counter.get()} now loses its call edge to Counter::get. Previously, this free path inferred Counter from the constructor. The existing component-receiver tests cover equivalent calls named double/getCount, which evade the new gate.
Suggested fix: Apply the built-in gate after the free path's type and object-literal resolution, immediately before name-only matching. Add a component markup case using get or set.
| "toLocaleLowerCase", "toLocaleUpperCase", "padStart", "padEnd", "charAt", "charCodeAt", | ||
| "codePointAt", "substring", "substr", "repeat", "localeCompare", "normalize", | ||
| "toString", "toLocaleString", "valueOf", "hasOwnProperty", "toJSON", "call", "apply", "bind", | ||
| "on", "off", "once", "emit", "addListener", "removeListener", "removeAllListeners", |
There was a problem hiding this comment.
medium: Preserve newly listed methods on parenthesized this receivers
Adding "emit" also activates is_unknown_receiver_built_in_call, which rejects calls when member_call_receiver(r) == Some(false). Its source-text check recognizes receivers ending in this or super, but (this) matches neither. The earlier this-call matcher requires strip_prefix("this.").
Failure: Indexing export class Bus { emit() {} run() { (this).emit(); } } now omits Bus::run -> Bus::emit. Extraction peels the parentheses and emits bare emit, while the resolver classifies the written receiver (this) as unknown and rejects both exact and fuzzy matching. Before this table expansion, the same-file method resolved.
Suggested fix: Recognize transparent wrappers around this in the receiver guard or early this-call matcher before expanding the shared rejection table; cover a newly added name such as emit.
There was a problem hiding this comment.
Fixed differently in 4a59fa5: the gate keeps its own JS_BUILTIN_METHODS list, so the prefilter/name-matcher JS_BUILT_IN_METHODS stays at its base contents and (this).emit() is no longer treated as a rejected built-in. (this).emit() is covered in the __tests__/js-builtin-method-calls.test.ts regression test (d6fa6ba).
…nry#1987) The gate sat at the free path's entry, ahead of its field, literal and typed receiver arms, and it widened the prefilter's skip list, which the name matchers consult before any receiver evidence. Both dropped real links found in review of #588: `self.emit()` and `new this().emit()` through the prefilter list, and a class-name receiver that a parameter shadows. Keep the gate list separate from the prefilter's skip list, move the gate below the evidence arms, and exempt a class-name root only while the file does not bind that name to a value. Add a regression test for the bound `self` alias, `new this()`, an unshadowed static call, and the shadowing parameter. Full suite 11,161 passed / 44 skipped / 0 failed; precision holds on vite (6/6 absent, 2/2 present) and svelte (1/1 absent, 0 gained).
…y#1987) Cover the two cases raised in review of #588: a parenthesized `this` receiver keeps a name from the gate list, and a Svelte markup call on a script-typed component receiver resolves before the gate. Both hold on 4a59fa5; the tests keep the table split and the gate's placement from regressing.
…chenry#1987) Binding rows carry line ranges, not columns, so a parameter of another one-line function on the call's own line read as shadowing the class receiver, and the built-in gate declined Bus.get(). Keep a same-line param/local as shadowing only when its name sits inside the caller's own span (its signature, or its body before the call), and cover both sides of the case: the sibling parameter and the caller's own one.
| if r.reference_kind == "calls" && is_js_family(&r.language) | ||
| && JS_BUILTIN_METHODS.contains(method_name.as_str()) { | ||
| let root = object_or_class.split('.').next().unwrap_or(&object_or_class); | ||
| let class_name = root.chars().next().is_some_and(|c| c.is_ascii_uppercase()) |
There was a problem hiding this comment.
medium: Preserve declared class receivers whose names start with an underscore or lowercase letter
Class evidence requires root.chars().next().is_some_and(|c| c.is_ascii_uppercase()); otherwise the gate returns Ok(None) before class_method_scan. Svelte/Astro markup deliberately reaches this fallback when script binding scopes do not cover the call.
Failure: A Svelte file containing <script>\nclass _Cache { static get() { return 1; } }\n</script>\n<p>{_Cache.get()}</p> now loses its call edge to _Cache::get. The receiver is a declared project class, but type and object-literal inference do not resolve class receivers here, and _Cache fails the uppercase check. Previously, the following class scan resolved it.
Suggested fix: Allow a matching visible class declaration, including a script-level declaration used from component markup, to supply receiver evidence regardless of capitalization. Preserve the shadowing check and add a regression for _Cache.get().
There was a problem hiding this comment.
Fixed in f94d877: the direct class-name scan now runs before the gate, so a declared _Cache or cache receiver is validated by its class rather than its spelling; the parameter/local shadow check still runs first. Added the Svelte markup case {_Cache.get()}{cache.get()} to __tests__/js-family-receiver-fixes.test.ts (resolves at @0.85).
…ymchenry#1987) The gate required an uppercase first letter as class evidence, so a declared class named `_Cache` or `cache` lost its `get()` link, including a Svelte component's markup where the class scan is the only reader. Run the direct class-name scan before the gate and let the value-binding shadow check stop a parameter or local first; the gate then covers only the name-similarity guesses after that scan.
What changed
match_method_call_free— the fallback for a receiver the resolver cannot type — runs the direct class-name scan first, then stops a JavaScript built-in method call unless the receiver carries evidence. The gate covers only the name-similarity guesses after the validated class scan; typed, field and object-literal receivers resolved above it.JS_BUILTIN_METHODStable (upstream'sjs-builtins.tslist) backs the gate. The prefilter'sJS_BUILT_IN_METHODSskip list is unchanged: widening it changed which same-named member calls the name matchers could keep, which review caught.list.map()andcache.get()stay unlinked. Binding rows carry lines, not columns, so a same-line sibling function's parameter is checked against the caller's own source span before it counts.Why the receiver check
A blanket gate on every built-in name fixed the 11 failures but broke 21
this.<field>.<builtin>links across 8 suites (field-chain-owner,ts-this-field-call,ts-this-field-bases,ts-imported-type-receivers,js-object-literal-members,ts-generic-class-heads). Review found four more cases, all fixed with regression tests: a boundself.emit()andnew this().emit(), a class-name root shadowed by a parameter, a sibling function's parameter on the same line asBus.get(), and a declared class receiver named_Cacheorcache(including Svelte markup).Tests
js-builtin-method-calls(ts, tsx, js, jsx),library-receiver-calls,route-inline-handler-calls,sync-typed-local-member-retry.js-family-receiver-fixes,browser-host-globals,new-expression-receiver,unknown-receiver-builtinsandkernel-golden-dumps.npx vitest run): 934 files, 11,163 passed, 44 skipped, 0 failed. Before this branch,fork/consolidatedwas 11,146 passed / 11 failed.EVAL_REPOS=~/cg-scratch/eval-repos npm run eval:precision -- vite: absent 6/6 held, present 2/2 held;-- svelte: absent 1/1 held, 0 gained. Reports forf94d8770are in__tests__/evaluation/results/.