fix(scanner): Match JS imports structurally instead of by quote style

[JordanCoin]

Third of the Graph accuracy milestone (#172). Fixes #147, reported by @nicknsheth-beep.

The report is accurate and the root cause is exactly as described. Reproduced with ast-grep against the rule verbatim:

single.js    -> 0 match(es)     const memberRoutes = require('./routes/members');
double.js    -> 1 match(es)     const memberRoutes = require("./routes/members");

Two things beyond the report

Correction after independent review: an earlier version of this body said single-quoted ESM import in .js was also invisible. That is true of javascript.yml in isolation, but ast-grep's jsx language already covers .js and jsx.yml used kind: import_statement, so import x from './mod' resolved on main. The routes/admin.js fixture row pins that existing behaviour so the rule rewrite cannot regress it; it is not a new fix.

TypeScript, TSX and JSX miss require() entirely — in both quote styles. They use kind: import_statement, and require() is a call expression, not an import statement:

req.ts   (require('./mod'))  -> 0 match(es)
req2.ts  (require("./mod"))  -> 0 match(es)
imp.ts   (import c from …)   -> 1 match

This matters for the fix you suggested: "switch to a structural kind:-based rule the way typescript.yml already does" would have regressed the very case you reported — kind: import_statement never matches require(), so a pure-kind: rule would have dropped CommonJS from .js altogether. The fix needs both halves.

What changed

rule:
  any:
    - kind: import_statement
    - pattern: require($$$)

in all four of javascript, typescript, tsx and jsx.

$$$ rather than $PATH is deliberate. Path extraction prefers a $PATH metavariable when one exists and otherwise falls back to extractImportPath on the matched text — and that fallback already handles ", ' and backticks. Binding $PATH here would capture the string node including its quotes; binding nothing keeps extraction on the quote-agnostic path and makes a dynamic require(someVariable) return "", which the caller skips. A fabricated edge would be worse than a missing one.

The fixture that proves it

testdata/commonjs-single-quotes/ — modelled on your Express layout, including the services/layoutInputService.js case:

                                     main    after
services/layoutInputService.js  ->   []      [routes/admin.js, routes/members.js]
routes/members.js               ->   []      [app.js]
routes/admin.js                 ->   []      [app.js]
app.js                          ->   []      []

It covers single-quoted require in both ../ and ./../ forms, a double-quoted require, a single-quoted ESM import, and a dynamic require(which). app.js imports resolve to exactly [routes/admin.js, routes/members.js] — the dynamic require adds nothing.

TestCommonJSSingleQuoteImportersResolve fails against main's rules with services/layoutInputService.js importers = [], want exactly [routes/admin.js routes/members.js].

Bundled ast-grep

Checked against 0.42.1 (the version scripts/download-bundled-astgrep.sh pins for releases) as well as the 0.45.1 on PATH, since one unsupported construct fails the entire --inline-rules document and CI only ever exercises latest. Both versions match all cases identically.

Per-ecosystem summary (for release notes)

Ecosystem	Before	After
JavaScript (.js, .jsx, .mjs)	only double-quoted import/require seen; single-quoted invisible	all quote styles, plus template literals
TypeScript / TSX / JSX	require() invisible in every quote style	require() resolved
Dynamic require(variable)	n/a	resolves to nothing, never a guessed edge
Every other ecosystem	unchanged	unchanged

Note on scope

The TS/TSX/JSX half goes beyond #147 as filed, which is about javascript.yml. It's the identical defect — a language's import rule blind to a whole syntax form — and a one-line change per file, so leaving it broken while editing the file next door seemed worse than the small widening. Happy to split it into its own PR if you'd rather keep this one strictly to the reported issue.

Dynamic import('./lazy') is still not matched. That's a miss rather than a wrong answer, and it wants its own thinking about whether a lazily-imported module should count as an edge.

Verification

go vet ./... clean, gofmt clean, go test ./... at the main baseline (only the three known root-environment permission failures). Fixture verified end-to-end with a built binary for --importers, --importers --json and --deps --json.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PEUvjGsJemDFSV8nbxvxBo

---

Generated by Claude Code

Scope note added after review: the scanner/rustcargo.go commit ports a 9-line hunk from #171 (byte-identical) so the cargo-metadata timeout no longer fails the whole graph build; that fixed this PR's red CI legs. #171 also adds TestCargoMetadataDeadlinePreservesFallbackTopology, the only caller that passes a non-constant timeout; it is not ported here, so the buildRustWorkspaceIndexWithTimeout seam is exercised only through its wrapper until #171 lands.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[JordanCoin]

Test (ubuntu-latest, 1.24) and Test (ubuntu-latest, 1.26) were red on 7470e63. Not this PR's, and a fix already existed — I've ported it rather than waiting. Pushed 24af8fb.

The failure

mcp/TestRustGraphContextHandlersDisclosePartialCoverage
    main_more_test.go:487: importers MCP output omits partial coverage:
        Failed to build file graph: context deadline exceeded

codemap/scanner — the only package these rule changes affect — passed on both legs. This is the same mcp cargo-metadata timeout that took a leg off #169 and #170, so it's now cost three PRs.

Root cause, and why it's @reneleonhardt's fix

buildRustWorkspaceIndex shadowed its caller's ctx with the cargo-metadata deadline:

ctx, cancel := context.WithTimeout(ctx, cargoMetadataTimeout)   // 3s

Once that deadline passed, the loop's ctx.Err() check returned DeadlineExceeded and the whole graph build failed — surfacing a bare context deadline exceeded instead of falling back to the manually derived Rust workspace. Three seconds isn't always enough for cargo metadata on a cold runner, and cargoMetadataTimeout is a const, so there's no seam to test it against.

#171 already fixes this — its "preserves manual Rust workspace topology when Cargo metadata reaches its deadline" bullet. It separates metadataCtx from the caller's ctx, so an expired metadata deadline breaks out and keeps the fallback index. That's exactly right: a cargo timeout should mean partial coverage disclosed, which is what the test asserts, not a failed graph.

I applied that hunk from #171 verbatim (scanner/rustcargo.go, 9 insertions), credited in the commit. It's a no-op once #171 lands or main otherwise carries it, and it lets this PR go green now rather than blocking on that merge. It does not touch #171's larger filegraph.go work.

Verification

The previously-failing test passes 8/8 locally with the port. go build, go vet and gofmt clean; full suite at the main baseline. codemap/scanner still green, so the JS rule change is unaffected.

@reneleonhardt — flagging that I'm carrying this hunk so it isn't a surprise if it shows up as an overlap when #171 merges.

---

Generated by Claude Code

[JordanCoin]

The correction in e00b405 is right and the error was mine. Verified it myself rather than take it on trust — a .js fixture with all three forms, against the main binary:

mod.js importers = ['double.js', 'single.js']
   single.js  ->  import { a } from './mod'      single-quoted ESM   ← already resolved
   double.js  ->  import { a } from "./mod"      double-quoted ESM   ← already resolved
   req.js     ->  require('./mod')               single-quoted CJS   ← missing, the real bug

So my claim in the PR description — "Single-quoted ESM import was equally invisible, not just require()" — is false. Single-quoted ESM imports already worked on main.

How I got it wrong is worth naming, because it's a method error rather than a typo. I ran the js-imports rule in isolation through ast-grep, saw zero matches for a single-quoted import, and concluded the system behaved that way. I never checked it end-to-end against the main binary. .js files are also matched by jsx.yml's quote-agnostic kind: import_statement, so the system already covered the case the isolated rule missed. Testing a component and asserting a system-level conclusion is exactly the mistake I've flagged in other people's findings today.

What survives, and is still the substance of this PR:

• Single-quoted require() was invisible — the reported bug, and the reason services/layoutInputService.js went from [] to two importers.
• TypeScript, TSX and JSX missed require() entirely, in both quote styles.
• The reporter's suggested "switch to a structural kind: rule like typescript.yml" would still have regressed the CommonJS case they reported, since kind: import_statement never matches a call expression.

The ESM row now correctly reads as pinning existing behaviour so the rule rewrite can't regress it, which is what it always was.

Two follow-ups from that review I agree with and haven't actioned (not pushing here while the branch is being patched): #171's TestCargoMetadataDeadlinePreservesFallbackTopology wasn't ported alongside the rustcargo hunk, so the timeout seam is untested until #171 lands; and .cjs is missing from the extension map at scanner/types.go:182-184, which is pre-existing and worth its own issue.

---

Generated by Claude Code