Report unresolvable imports so completeness never certifies a blind spot - #141
Merged
Merged
Conversation
`coverage.importsComplete` is the one field that licenses a NEGATIVE
conclusion — "the package is not imported, so the vulnerability does not
apply". It was computed only from environmental failures (unreadable files,
unscannable files, unwalked paths), so a tree the scanner structurally could
not enumerate still reported as complete.
Neither import scanner can see an import whose module is not a literal.
`scanFileImports` reads `ts.preProcessFile(...).importedFiles`, which reports
resolved literal specifiers only, and `collectFileImports` requires a string
literal. A computed specifier therefore produces no entry at all — the import
is not recorded as unresolved, it is absent — while every read-and-scan check
still passes.
`countComputedSpecifiers()` finds those imports by tokenising (not parsing:
this runs on files deliberately kept out of the AST pass, and a text match
would count `require` in a comment and hold the flag false forever). It
reports:
- computed specifiers: `require(REGISTRY[kind])`, `import(name)`,
a template with substitutions, and `require("a" + b)` — which opens with
a string literal and is still computed
- escaped loaders: `const r = require`, `(require)(x)`,
`module.exports = require` — once the loader is behind another name,
following it needs dataflow this scan does not do
Three call forms are distinguished, because the deciding token sits in a
different place in each: `require(x)`, the optional `require?.(x)` where `?.`
separates the name from the paren, and `loader.require(x)` where a dot before
the name means it is someone else's method. Missing the optional form leaves
the flag true over an unresolvable import; counting a member method makes the
flag permanently false for an app that named a method `require`, which trains
a reader to ignore it. Both end in an unchecked negative.
Escaped loaders are counted conservatively — an alias counts even when every
call through it passes a literal — so a UMD-style wrapper reads as incomplete.
That withholds a negative conclusion rather than granting a wrong one.
`coverage.importCoverageGaps` reports why, splitting environmental gaps (a
re-run may resolve them) from inherent ones (a permanent property of the
source). `importsComplete` remains the single gate a consumer reads, and any
non-zero count makes it false, so the diagnostic can be ignored without ever
licensing a wrong negative.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The extractor now reports inherently unresolvable imports separately from environmental gaps. 🎯 Quality: 99% Elite · 📦 Size: Large — consider splitting if possible 📈 This month: Your 76th PR — above team average · Averaging Excellent |
`countComputedSpecifiers` gained escaped-loader detection (`const r = require`) but kept a name and docblock describing only computed specifiers, so the function no longer matched either its own behaviour or the `unresolvableImports` field it feeds. Renamed to `countUnresolvableImports`, with the test file and the docblock following. No behaviour change. Terminology drift is worth a commit of its own here: the emitted field is what a consumer reads to decide whether a package's absence is evidence, and a name that describes half the behaviour invites the next reader to add the other half again somewhere else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
patchstackdave
force-pushed
the
feature/import-inventory-completeness
branch
from
August 18, 2026 15:33
f01977d to
6d8eda7
Compare
Contributor
Author
|
/review |
daniloradovic
approved these changes
Aug 18, 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.
coverage.importsCompleteis the one field in the map that licenses a negative conclusion — "the package is not imported, so this vulnerability does not apply". It was computed only from environmental failures (unreadable files, unscannable files, unwalked paths), so a source tree the scanner structurally could not enumerate still reported as complete.Neither import scanner can see an import whose module is not a literal:
scanFileImportsreadsts.preProcessFile(...).importedFiles, which reports resolved literal specifiers onlycollectFileImportsrequires a string-literal specifierSo a computed specifier produces no entry at all — the import is not recorded as unresolved, it is simply absent — while every read-and-scan check still passes.
What is now reported
countComputedSpecifiers()tokenises rather than parses. It runs on files deliberately kept out of the AST pass, so it must stay at scanner cost; and a text match would countrequireinside a comment, which would hold the flag false forever and turn it into noise.Computed specifiers —
require(REGISTRY[kind]),import(name), a template with substitutions, andrequire("a" + b), which opens with a string literal and is still computed.Escaped loaders —
const r = require,(require)(x),module.exports = require. Once the loader is behind another name, following it needs dataflow this scan does not do, so the gap is reported at the escape rather than at the later call.Three call forms are distinguished, because the deciding token sits somewhere different in each:
require(x)require?.(x)loader.require(x)The two mistakes fail in opposite directions and both end in an unchecked negative: missing the optional form leaves the flag true over an import nobody can resolve, while counting a member method makes the flag permanently false for an app that named a method
require— and a flag that is always false stops being read.Deliberate conservatism
An escaped loader counts even when every call through it passes a literal, so a UMD-style wrapper reads as incomplete. This is the accepted direction: it withholds a negative conclusion rather than granting a wrong one. Asked about
requireonly —importlegitimately appears without a following paren in every ESM file (import qs from "qs",import.meta.url), and applying the rule there would report a gap for ordinary static imports.Diagnostics
coverage.importCoverageGapssplits environmental gaps (a re-run in a different context may resolve them) from inherent ones (a permanent property of the source). Collapsed into one number, a reviewer re-runs the scan against something no re-run can change and reads the identical result as a flake.importsCompleteremains the single gate a consumer reads, and any non-zero count makes it false — so the diagnostic can be ignored entirely without ever licensing a wrong negative.Tests
35 cases in
tests/map/computed-specifiers.test.ts, covering both directions: computed and aliased forms detected, and literal/comment/string/member-method/static-import controls not counted.Mutation-tested rather than assumed — each rule was removed in turn to confirm the tests fail without it: dropping the optional-call shift fails 3, dropping the qualifier check fails 6, removing the escape check fails 6, and extending the escape rule to
importfails the static-import controls.Full suite green;
npm run typecheckclean.