Name a dependency API only where the package supplies the name; reach the file that calls it - #142
Merged
Merged
Conversation
One app per reachability rung, asserting what the map SAYS about a dependency — which is what a consumer grades into a verdict, and a different question from whether a flow compiles to a rule. A package can be plainly reachable and still produce no rule. Each rung has its own way of being wrong: reachable must not read as a bare import, api-called must not be promoted, imported must not be demoted to "not imported", not-a-code-question has no consumer artifact to gate it, and unknown must DECLINE. The last is load-bearing — every other rung is a positive claim that shows up as a bad rule someone notices, while a wrong unknown is a confident negative that makes a real finding disappear silently. The suite is layered so a failure localises: positive controls that the source was parsed at all (an app landing on `imported` because nothing parsed would otherwise pass for the wrong reason), then imports, invocations, absent invocations, and flows. One case fails deliberately and is left failing: a local function is named as a dependency API, and the dependency API called inside the module it lives in is not recorded. This branch is the regression harness for that fix, and is NOT ready to merge while the assertion is red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`new Pool().query()` — and, more importantly, `function getDb() { return new Pool() }` called as
`getDb().query()` — rooted at nothing, because the root walk followed call expressions but not
constructions. The generated-app shape is a module-level getter returning a client, so this was not
an exotic gap: the receiver came back untraceable, and untraceable is reported the same way as app
code — no sink, no invocation record, nothing saying a link was skipped.
A construction is a chain link like any other, so the walk now follows it.
An invocation record makes two claims — a package and an API name — and tracing the value only
established the first. A local helper re-exported from another module (`loadConfig()`, whose body
returns `JSON5.parse(...)`) was recorded as an API of json5, which exports no such thing. So did a
factory result called under the app's own name (`const log = createLogger(); log(...)`), and an
aliased destructure (`const { pick: choose } = require("lodash")`) was recorded as `choose`.
This is worse than recording nothing. A consumer comparing the inventory against an advisory's
affected functions reads a name that appears in no advisory, and reads the absence of the real name
as evidence about it.
A trace now carries whether the NAME is the package's own — true for a binding that came straight
out of the package here or through a pass-through re-export, false for anything the app derived —
and only a named trace produces a record. For a pass-through, the name comes from the module that
imported it, since an intermediate module may have renamed it on the way in.
Calls we can trace but cannot name are counted as ambiguous, not as recorded dependency calls and
not as correct local exclusions: it is a real miss, and the bucket that measures resolver quality is
the honest place for it.
Unchanged, and tested so it stays that way: a METHOD on a dependency-derived value
(`getDb().query()`) is still that dependency's API — the method name comes from the package's
surface even when the receiver's name does not.
The receiver rule admitted any binding that came straight from the package, which includes the
module object itself: `const JSON5 = require("json5"); JSON5.parse(x)` was recorded with symbol
`JSON5.parse`. That name is convention, not API — the same require is written `J5` or `json5` in the
next codebase — so a consumer matching the inventory against an advisory that names `parse` matches
nothing, and reads the miss as evidence about the call.
A receiver is now reported only when it is a NAMED member of the package, under the package's own
name for it: `const { promises: fsp } = require("node:fs"); fsp.readFile(x)` stays
`promises.readFile`, because `promises` is genuinely part of node:fs's surface, while the app's alias
`fsp` never appears. A module object, a factory result, and a re-exported value all yield the method
alone.
The bindings now distinguish the two, since both are `direct` and only one carries a name the
package chose.
The ladder's `api-called` rung stays red here: `json5.parse` is written in a file the invocation pass
does not reach yet, which the next commit addresses.
An entry file rarely holds the dependency call. `src/server.js` imports `loadConfig` from `./config`, and the `JSON5.parse(...)` call lives in config.js — a file with no entry-point signal, so the walk only scanned it for imports. The map therefore reported json5 as imported and reported no call to it, which is precisely the evidence an advisory gated on *calling* the function needs. Files reachable from an entry file by a relative import are now parsed for their invocations. ONE hop, entry files only, invocations only — no endpoints, no sinks, no recursion into what the hop file itself imports. The bound is structural rather than a file budget, so coverage follows the app's shape instead of its size, and it is asserted in both directions: a call one hop away is recorded, a call two hops away is not. Sites carry the hop file, so a coordinate names the line that makes the call rather than the entry that reaches it. Its calls feed the same four buckets, so resolver quality is still measured over every call analysed. The count is reported separately from `filesParsed` — these files were already counted as pre-filtered, and the two answer different questions — and the limitations now state the new depth, in the terms the tests check. An unparseable hop file is fail-open and counted, but deliberately does NOT join the skipped-file list: that list gates the import-completeness flag, and this is a different question about a file whose imports were already read. Closes the `api-called` rung of the reachability ladder: 33/33.
|
Implements one-hop module parsing and precise API naming to avoid spurious dependency calls. 🎯 Quality: 86% Excellent · 📦 Size: Extra Large — strongly consider breaking this down 🛡️ Standards: no pre-flight fit check ran for this change — wire 📈 This month: Your 77th PR — above team average · Averaging Good |
This was referenced Aug 18, 2026
Contributor
Author
|
/review |
mariojgt
approved these changes
Aug 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The invocation inventory made two claims per record — a package and an API name — and only the first
was ever established by evidence. Tracing a value to a package does not make the name it is known by
part of that package's surface, and four shapes exploited that gap:
loadConfig(), a local function re-exported from./configwhose body returnsJSON5.parse(...)json5.loadConfigjson5.parse, from config.jsconst log = createLogger(); log(...)winston.logwinston.createLoggeris still recorded)const { pick: choose } = require("lodash"); choose(...)lodash.chooselodash.pickconst JSON5 = require("json5"); JSON5.parse(x)json5.JSON5.parsejson5.parseEach invents a name the package does not export. That is worse than recording nothing: a consumer
comparing the inventory against an advisory's affected functions finds a name no advisory contains,
and takes the absence of the real name as evidence about the real call.
The last row is the one that mattered most in practice — the app's alias for a module object appeared
in every
symbolfor a CommonJS default require, so the inventory's most common shape was also itsleast matchable.
The commits, in the order they are meant to be read
newexpression.function getDb() { return new Pool() }called as
getDb().query()rooted at nothing, so the receiver came back untraceable — anduntraceable is reported exactly like app code. A construction is a chain link like any other.
name is the package's own; only a named trace produces a record. For a pass-through re-export the
name comes from the module that imported it, since an intermediate module may have renamed it.
Calls we can trace but cannot name are counted ambiguous — not as records, and not as correct
local exclusions.
name (
promises.readFilefornode:fs); a module object, a factory result and a re-export allyield the method alone.
invocations only — no endpoints, no sinks, no recursion. This is where the removed records are
replaced by the real ones:
json5.parseis now read from the file that writes it, with a sitepointing at that file. The bound is structural (reachable from an entry by a relative import) and
asserted in both directions.
Method calls on dependency-derived values are deliberately untouched and now pinned by a test:
getDb().query()is stillpg.query, because the method name comes from the package's surface evenwhen the receiver's name does not.
Verification
The first commit in this branch is a five-app reachability-ladder harness — one fixture app per rung
(reachable / api-called / imported / not-a-code-question / unknown) asserting what the map says
about a dependency, which is the input a consumer grades into a verdict. It reproduced this defect,
and the
api-calledrung goes green here: 33/33.Each behavioural rule was mutation-checked — removed, and the suite confirmed to fail — so no test in
this branch passes for reasons other than the rule it names. Full suite 1100/1100, typecheck clean.