fix(registry): refuse a name-only match the receiver chain contradicts - #1897
fix(registry): refuse a name-only match the receiver chain contradicts#1897CaptainMittens wants to merge 1 commit into
Conversation
A dotted callee whose first segment starts upper-case names a type. That
receiver chain is evidence, and the two name-only strategies threw it away:
they matched on the final segment alone, so Foundation's
URLSession.shared.data bound to a project's own PickedFile.data — at 0.75
confidence, with nothing in the graph to show the answer was wrong.
receiver_chain_admits() now requires the candidate's own parent segment to
appear somewhere in that chain. Calendar.utcGregorian.startOfDayUTC still
resolves to AuthDTOs.Calendar.startOfDayUTC, because the project really does
extend Calendar and Calendar is in the chain.
Three shapes pass through untouched:
- a callee with no separator, which has no chain to judge;
- a lower-case root, which names a value whose declared type the chain does
not show (vm.load, http.Get, os.path.join);
- a name in capitals with underscores, which is a constant holding a value
rather than a type. Measured: without this carve-out the gate refused
ISO_4217_URL.lower -> builtins.str.lower, which is correct. JSON and URL
carry no underscore and stay guarded.
The gate applies only at the two name-only exits of resolve_name_lookup.
import_map, same_module and qualified_suffix already carry real evidence and
are left alone.
Language agnostic by design: the registry holds no language, and every
language that writes receiver chains gains the same protection.
Fixes DeusData#1893
Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
|
Approved, and I want to be explicit that this cleared a bar most changes of its shape do not. There is a standing position here that a precision fix restricting receiver resolution globally is the wrong shape — suppressors are written as per-language helpers gated on the file language, and all three existing ones (Perl, TS/JS, cross-language suffix) are built that way. This PR is global. It was accepted anyway, on four things it does that the usual version of this change does not: The upstream strategies are provably untouched. I checked rather than took it: You measured the blast radius instead of arguing it. Three real codebases, split by language, with every non-Swift refusal read by hand. 16 edges out of 8281 name-only ones on the Go/TypeScript project is the number that settles this, and Your reason for going global is factually right. And you ruled out the cheaper fix with evidence. "The clearest errors were the highest-confidence ones" is the sentence that closes off the threshold approach: Swift's import map is empty, so the 0.5 penalty never applies and the worst edges kept the full 0.75. A confidence cut would have removed correct edges and left these. Two smaller things worth namingYour guard tests pass without the change, deliberately. "A guard test that only passes with the change proves nothing" is exactly right, and showing the reverted run with 2 FAIL / 3 PASS is the clearest way to demonstrate a gate that refuses the right things without over-refusing. Most reproduce-first evidence stops at the failing half. The StatusCleared, reviewed, approved. Thank you — this is the most carefully evidenced call-resolution change we have had. |
What does this PR do?
A Swift project's
URLSession.shared.databound to its ownPickedFile.data,UserDefaults.standard.stringto a DTO'sstring, andJSONEncoder().encodeto a model's
encode. "Who calls this?" then answers with calls that neverhappened.
The cause is that the two name-only resolution strategies match on the final
segment alone. A dotted callee whose first segment starts upper-case names a
type —
URLSession,Calendar,JSONEncoder— and that receiver chain isevidence the scorers were discarding.
receiver_chain_admits()now requires the candidate's own parent segment toappear somewhere in that chain:
URLSession.shared.dataHomeboxUI.PickedFile.dataPickedFile— noCalendar.utcGregorian.startOfDayUTCAuthDTOs.Calendar.startOfDayUTCCalendar— yesThree shapes pass through untouched, so ordinary code is unaffected:
not show —
vm.load,http.Get,os.path.join;rather than a type. This carve-out is measured, not guessed: without it the
gate refused
ISO_4217_URL.lower→builtins.str.lower, which is correct.JSONandURLcarry no underscore and stay guarded.The gate applies only at the two name-only exits of
resolve_name_lookup.import_map,same_moduleandqualified_suffixalready carry real evidenceand are left alone.
Language agnostic by design — the registry holds no language, and every language
that writes receiver chains gains the same protection.
Why the confidence score could not do this instead
The clearest errors were the highest-confidence ones. Swift's import map is
empty (system frameworks are never indexed nodes, so
cbm_pxc_build_import_mapdrops them), and the 0.5 penalty in
resolve_name_lookuponly applies when animport map exists. So the worst calls kept the full 0.75. A threshold would have
cut correct edges and left these.
A "never point a call at a
Variable" guard is also wrong:Variableis a legaltarget on purpose (
cbm_label_is_registry_symbol), one project here has 645 suchedges and another 984, and
Variablenodes carry notypeproperty, so nothingin the graph separates a callable variable from stored
Data.Fixes #1893
Validation
Reproduce-first. With the fix reverted and the five tests kept, exactly the two
refusal tests fail and the three guard tests already pass — which is the point,
since a guard test that only passes with the change proves nothing:
Blast radius, measured on three real codebases
The gate changes resolution for every language, so I counted what it refuses
rather than reasoning about it. Read from live graphs, applying the rule to every
unique_nameandsuffix_matchedge:unique_namesuffix_matchThe Swift project is where the bug lives and it moves most. The Go/TypeScript
project loses 16 edges out of 8281 name-only ones — 0.2% — because its import
map already resolves most calls before the gate is reached.
I read every non-Swift refusal by hand. They are all wrong bindings the gate
should refuse:
import_map,same_moduleandqualified_suffixcounts are unchangedeverywhere, since the gate never runs on those paths.
Suites
pipelinemake -f Makefile.cbm testThe 28 failures are all in
tests/test_cli.c(client install/uninstall) and arepre-existing in my environment. Measured rather than assumed:
origin/mainhereruns 7630 passed / 28 failed, and this branch adds exactly the 5 new tests with
the same 28 failures in the same file. If they are green on your runners they
are environmental on mine.
Lint:
make -f Makefile.cbm lint-cppcheckandlint-no-suppressboth exit 0.clang-formatwants no change on any line this PR adds. I could not runclang-tidylocally — it is not in my toolchain — so that one is unverified onmy side.
Checklist
git commit -s) — required, CI rejectsunsigned commits (DCO, see CONTRIBUTING.md)
make -f Makefile.cbm test)make -f Makefile.cbm lint-ci)