Script API IntelliSense: infer types for undocumented helpers from usage#586
Open
taurgis wants to merge 44 commits into
Open
Script API IntelliSense: infer types for undocumented helpers from usage#586taurgis wants to merge 44 commits into
taurgis wants to merge 44 commits into
Conversation
Undocumented helper functions widen to `any`, silencing hover/completion for every caller downstream. Adds an opt-in, heuristic usage-based inference engine to the b2c-script-types tsserver plugin that infers plausible types from call-site arguments and return statements, chasing through undocumented call chains, and surfaces the result as a labeled hover note and synthesized member completions. Off by default via the new b2c-dx.features.scriptTypesInferUsage setting. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018cSJNMGwtCicNegmdA83e6
- Handle the common export patterns that previously produced no inference:
bare `module.exports = function(){}`, destructured `const {x} = require(...)`
(including renamed bindings), and ES6 method-shorthand exports. Previously
only `exports.foo = function(){}` consumed via property access worked.
- Restore the `maximumLength` parameter on the getQuickInfoAtPosition
override, which was being silently dropped for all users regardless of
the inferUsage setting.
- Gate the new hover/completion overrides on the main `enabled` flag and
`isCartridgeFile()`, matching every other feature in this plugin, and fix
the VS Code extension side so `inferUsage` can't stay active once the
parent scriptTypes feature is disabled.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018cSJNMGwtCicNegmdA83e6
- Bound inferenceCache growth: clear the whole cache on project version change instead of leaving stale per-key entries around indefinitely. - Add request-scoped, depth-safe memoization plus a hard reference-count budget per inference request, so a widely-referenced helper can't make a single hover/completion synchronously fan out unboundedly. - Skip inference for parameters/returns that already have an explicit type annotation (TS syntax or JSDoc `@param`/`@returns`), even if that annotation is literally `any` — never override a type the developer or JSDoc already resolved on purpose. - Preserve every other CompletionInfo field (isIncomplete, optionalReplacementSpan, metadata, etc.) from the original completion result when merging in synthesized entries. - Correct a misleading comment claiming plugin state is shared across projects in a multi-root workspace — verified against the tsserver source that the plugin factory is invoked fresh per project, so this was never actually a cross-project leak. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018cSJNMGwtCicNegmdA83e6
- Convert usage-inference.ts's function-level comments to JSDoc /** */ blocks with @param/@returns where the SDK convention already does this for every top-level function (exported or not); index.ts's own internal closures already use plain // comments consistently, so left as-is. - Match the VS Code setting description to the established "(Preview) ... In development — off by default." wording used by other not-yet-stable features, instead of an ad hoc "Experimental:" prefix. - Match doc phrasing in ide-integration.md to the sibling scriptTypes setting's plain-backtick + "(default: `false`)" convention. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018cSJNMGwtCicNegmdA83e6
Correctness (empirically verified with repro scripts): - Check ctx.memo before the MAX_INFERENCE_DEPTH cap in inferParameterType/ inferReturnType, so an already-computed result isn't discarded just because the current call path happens to run over the depth budget. - Add PropertyAccessExpression handling to resolveExpressionTypes so `return x.prop` on an undocumented parameter chases the base's inferred type instead of giving up. - Use checker.getApparentType() before getPropertiesOfType() so inferred primitive types (string/number/boolean) get their wrapper-object member completions. - Widen literal call-site argument types to their general type, so hover shows `string` instead of a union of every literal ever passed. - Add a ctx.visiting cycle guard to inferParameterType (inferReturnType already had one), preventing re-entrant recomputation for self-forwarding helpers. - Cap how much of the shared reference budget a single collectCallSites call can spend, so one widely-referenced helper can't starve sibling branches processed later in the same request. Tooling: - Fix test/test:agent/test:unit/test:watch to explicitly rebuild first — pnpm's pretest hook only fires for the literal `test` script, so the other three were silently running against stale or missing compiled output. - Fix the shared test fixture host to include the default lib file (a LanguageService, unlike a Program, never adds it automatically), needed to test apparent-type-dependent behavior at all. Coverage: added regression tests for every fix above, plus previously- untested fixes (maximumLength forwarding, cache invalidation on project version change) — 33 tests total, up from 25. Conventions: clarified in the changeset why @salesforce/b2c-cli is listed despite no direct file changes, and matched log message phrasing / VS Code setting punctuation to this file's established style. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018cSJNMGwtCicNegmdA83e6
… real dw.* type test coverage Completions silently fell back to plain global suggestions whenever the inferred type was nullable (e.g. ProductMgr.getProduct(): Product | null), since getPropertiesOfType on a union only returns members common to every constituent and null contributes none — strip the nullable part first. Also teach resolveExpressionTypes to chase a return expression that is a multi-hop method chain on the function's own untyped parameter (e.g. `return product.getPriceModel().getPrice();`), so an undocumented helper's own return type resolves instead of stopping at `any`. Add an end-to-end VS Code integration test exercising real hover/completion against a cartridge fixture, and a new test matrix exercising the inference engine against the actual bundled dw.catalog.Product/dw.order.Order/etc. types (not toy ambient stand-ins) across happy paths, deep nesting, and edge cases. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018cSJNMGwtCicNegmdA83e6
- resolveExpressionTypes's method-chain branch (and the pre-existing property-access branch) didn't strip nullability before looking up a member, so a chain rooted in a real nullable SFCC getter (e.g. ProductMgr.getProduct(): Product | null) silently resolved to nothing. Extracted a shared getNonNullableApparentType()/getMemberOfType() helper used by both branches and typesToCompletionEntries, so the fix applies everywhere a candidate type's members are walked. - The new try/catch wrappers around the underlying language service calls unconditionally swallowed ts.OperationCanceledException (thrown whenever the host's CancellationToken fires, e.g. the user kept typing), turning ordinary request cancellation into a logged failure. Consolidated the four call sites into one guarded() helper that always rethrows cancellation. - The new chain-hop recursion had no cap distinct from MAX_INFERENCE_DEPTH and didn't dedupe per hop; added a dedicated MAX_CHAIN_HOPS bound and dedupeTypes() on the accumulated results. - Deduplicated the findFunctionDeclaration test helper into fixture-language-service.js and collapsed repetitive setup in usage-inference.real-types.test.js via a shared setupInference() helper. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018cSJNMGwtCicNegmdA83e6
Correctness: - Chase local-variable indirection: a chain split across intermediate vars (var priceModel = product.getPriceModel(); return priceModel.getPrice();) — the idiomatic SFCC style — now infers the same types as the inline expression, for return inference, hover on the variable, and member completions. Explicit @type annotations on a variable are respected (deliberate `any` is left alone), and mutually-referencing initializers are cycle-guarded. - Offer inferred completions when the receiver is a chained call (product.getPriceModel().|), not just a bare identifier. - Stop memoizing results whose computation hit a cycle guard — they are truncated by what happened to be on the call stack, and a later out-of-cycle query in the same request deserves the full answer. Error handling: - Only guard the plugin's own inference additions; exceptions from the underlying language-service calls propagate to tsserver exactly as they would without the plugin installed (cancellation was already rethrown; now genuine TS errors are no longer swallowed either). Packaging/infra: - Keep the license header in the emitted plugin/usage-inference.js (detach it from the type-only import that tsc elides). - Distinguish method vs property completion kinds; document sortText. - failZero on the infer-usage vscode-test label so a run that discovers zero tests fails instead of passing vacuously. - Trigger extension CI on packages/b2c-script-types/** changes and run CI for PRs targeting feature/** branches. - Add c8 coverage to the package test script; dedupe test:unit; ignore package-level coverage/ output. - Correct the reference-budget comment: it caps result processing and fan-out, not the cost of a single project-wide reference search. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W3fJk97pe2QcEeMKFJqQv6
… inference
Studying real SFRA helper modules (app_storefront_base scripts/helpers,
scripts/cart) surfaced patterns the E2E suite never exercised: cross-file
call sites reached through */ and ~/ cartridge requires, the canonical
`module.exports = {name: name}` alias-map export, chain hops parked in
intermediate variables, and deep property chains with a nullable middle
step (availabilityModel.inventoryRecord: ProductInventoryRecord | null).
Engine fix found by this exercise: a reference search on a function
exported through an alias map dead-ends at the map's initializer, so the
SFRA-canonical export shape inferred nothing cross-file.
resolveIndirectReferenceTarget now hops from a property-assignment
initializer to the property name and searches on from there.
E2E fixture is now a realistic mini-cartridge: an undocumented
productHelpers module whose only call sites live in cartService.js via a
~/ require, plus a jsconfig.json so all cartridge files share one
configured project (the setup `b2c setup ide vscode-types` recommends) —
without it each open file gets its own inferred project and cross-file
reference search has nothing to look at.
New E2E tests (5): cross-file param inference through the plugin's own
module resolution, intermediate-variable hover, chained-receiver
completions, nullable deep-property-chain hover and completions. All
completion assertions now require at least one member that appears
nowhere in any fixture document, so VS Code's word-based suggestions
cannot satisfy them vacuously — the pre-existing test asserted only
members that literally occur in the fixture text.
Matching unit tests: alias-map export cross-file (mechanics) and the
nullable deep chain against the real dw.* types.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3fJk97pe2QcEeMKFJqQv6
…s end-to-end SFRA plugin cartridges extend base modules through module.superModule — the runtime handle to the same-path module in the next cartridge down the cartridge path. TypeScript knows nothing about it, so `var base = module.superModule;` and everything derived from base was opaque `any`. Engine: resolveExpressionTypes gets a module.superModule leaf. The plugin supplies a resolver mapping a cartridge file to the same-subpath file in the next cartridge down (probing existence through the language-service host, not ts.sys); the super module's export type is read off its `module.exports = X` assignment via resolveExpressionTypes, so a pass-through overlay (`module.exports = base`) recurses another cartridge down naturally, with a visiting-guard against overlay cycles. When a member found this way is an undocumented function (declared return type `any`), inference recurses into its actual declaration instead of surfacing `any` — and `any`-typed members are never offered as candidates anywhere. Fixture host fix surfaced by these tests: directoryExists forwarded to the real filesystem, so TS's directoryProbablyExists pre-check silently failed nested relative requires between in-memory fixture files — directories implied by the file map now exist. Tests: five engine-level unit tests (export-type resolution, recursion into undocumented base helpers, no-lower-cartridge, no-resolver, and a three-cartridge pass-through chain), a real-dw-types composition test (superModule + alias map + var chains -> Money), a plugin-level test of the cartridge-path resolver wiring, and three VS Code E2E tests against a new app_custom_cartridge overlay fixture (ordered above test_cartridge via dw.json's cartridges field): hover through the full composition, base-module member completions, and Money member completions — E2E completion assertions filter out word-based (Text) suggestions so only typed entries can satisfy them. Docs: ide-integration guide documents superModule support and the one-project requirement (jsconfig) for cross-file inference. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W3fJk97pe2QcEeMKFJqQv6
…est server.append
Four more real-SFRA capabilities for usage-based inference:
Callback parameters: a function expression in argument position has no
name to run a reference search on, so its first parameter is now typed
from the element type of a collection-like sibling argument (anything
with iterator()/next() — dw.util.Collection and friends), covering the
`collections.forEach(coll, function (item) {...})` idiom. The collection
argument is resolved through the engine, so it works even when the
collection itself only exists via inference of the enclosing helper's
parameter. reduce-style callees are skipped (accumulator first), and
only the first parameter is mapped.
Manual iterator loops needed no engine change — iter/next() chains
already resolve through the chain machinery — but are now pinned down by
real-dw-types unit tests and an E2E test.
Multi-cartridge superModule stacks: the checker's merged module.exports
type is used when concrete (it carries `module.exports.name = fn`
augmentations), pass-through re-exports additionally recurse another
cartridge down, and members that exist ONLY as augmentations on a
pass-through level — invisible to any candidate type — are resolved by
a dedicated member walk down the cartridge chain (and listed in
completions). superModule-derived expressions also bypass the direct
checker-type short-circuit and the hover/completion any-gates: TS's own
type for them is garbage either way (any or an opaque circular typeof).
server.append middleware params turn out to need no inference: with a
modules cartridge present the plugin injects its SFRA ambient
declarations and TypeScript types (req, res, next) contextually from the
typed append() signature. Tests pin down both halves: the params ARE
typed, and inference does NOT decorate them.
Tests: 12 new unit tests (callback heuristics and guards, three-level
overlay stack with mid-level augmentation, plugin-level server.append
and overlay-stack wiring, real-dw-types callback/iterator coverage) and
7 new E2E tests against extended fixtures (modules + plugin_promo
cartridges, an SFRA-style collections util, variantHelpers, and a
controller), all completion assertions filtered to typed entries so
word-based suggestions can't satisfy them. Docs updated.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W3fJk97pe2QcEeMKFJqQv6
The engine runs synchronously inside tsserver on every hover/completion keystroke, and its dominant cost is getReferencesAtPosition (a project-wide scan per call). These tests pin down worst-case cost with deterministic search *counters* rather than wall-clock timings, so they hold on slow CI runners and point at exactly which cap regressed: - widely-referenced helper (300 call sites): <= 4 searches, per-call budget engages, correctness survives the cap - overlong method chain (60 hops): chain cap fires before any search - no-parameter helper chain: delegated to TS's native inference, 0 searches - 12-level parameter-forwarding chain: depth cap keeps cost flat in both directions (0 searches for returns, <= 6 from the deep end) - 20-branch fan-out through a shared sub-helper: request memo collapses it to <= 4 searches - repeated identical hover at the same project version: 0 new searches (served from the position cache) - mutual recursion with heavy call-site fan-in: terminates, budget never goes negative A generous wall-clock ceiling (5s) rides along on each scenario purely as a tripwire for catastrophic regressions (lost cap, infinite loop); the counters are the real baseline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W3fJk97pe2QcEeMKFJqQv6
Performance audit of the usage-inference engine at SFRA scale (~1,900 cartridge JS files, 300-call-site hot helper, 3-level overlay stack) found two engine-level gaps, both measured with the real plugin driving a real LanguageService: 1. The number of getReferencesAtPosition SEARCHES per request was only indirectly bounded. MAX_REFERENCES_PER_REQUEST counts search *results*, but each search is a full project scan even when it returns almost nothing: a helper whose call sites feed it results of many DISTINCT exported sub-helpers (each searched once, each contributing 2-3 results) drained the result budget at ~2-3 per scan — 76 scans and 114ms p50 (141ms p95) for a single hover, measured before the fix. A new MAX_SEARCHES_PER_REQUEST budget (12; the costliest legitimate baseline scenario needs 6) bounds the scans directly: same fixture now runs 12 searches in ~42ms p50, still inferring the right type from the in-budget call sites. 2. Two implicit-any parameters of the same function each re-ran the identical reference searches within one request (the request memo keys parameter/function nodes, not searches): 4 scans instead of 2, measured. collectCallSites results are now memoized per name node for the request; reuse is sound because budgets only decrease within a request, so a memoized result is never less complete than a re-run would be. Also: getNodeAtPosition now stops scanning a sibling list once past the target position (siblings are ordered and non-overlapping). Every reference hit walks the AST from its file's root, so in a generated file whose array literal has thousands of elements the old full-list walk cost ~25ms per request on top of the search itself. Three new counter-based baseline scenarios pin these bounds: distinctSubHelperTree (<= 12 searches, and asserts the budget actually engages), multiParamHelper (<= 2), hugeGeneratedFile (<= 2 searches, <= 50 hits processed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AFvM84JTUzSx4oaXjMabHb
Memory and churn findings from the same performance audit, both in the plugin-side cache wiring: 1. The per-position inference cache stored checker Type[] objects. A Type pins its checker and, through it, the whole Program it came from — so after an edit, the cache kept the entire previous program graph alive until the next inference-eligible request cleared it (indefinitely, if the user stopped hovering). The cache now stores finished display products instead — the hover note string and the synthesized CompletionEntry array — which retain nothing. "Inference found nothing" is now a cached answer too (it costs the same searches as a hit), and a 512-entry cap bounds a long no-edit session. 2. Invalidation now compares Program instances instead of the project version string. TS constructs a new Program for any semantic change and reuses the instance otherwise, while tsserver's project version also bumps on events that produce no new program — each such bump discarded a perfectly valid cache and re-ran full inference (measured ~13ms per hover on an SFRA-sized project, ~40ms including the request's incremental program update). New baseline scenario versionBumpSameProgram pins the second fix: a version bump with an unchanged program must answer from the cache with 0 new searches. Measured after both commits (SFRA-scale workspace, real edits between requests): hover p50 41ms / p95 80ms, completion p50 32ms / p95 49ms, superModule hover p50 20ms — all inside the 100ms hover / 200ms completion targets; warm-cache repeats ~1ms. Cancellation interrupts mid-request within ~0.3-3ms of the token firing (rethrown, checked inside every reference search). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AFvM84JTUzSx4oaXjMabHb
Follow-up fix for the one audit finding previously left as inspection-only, now measured and confirmed real: candidate types propagate up through every recursion level (parameter -> return -> forwarding helper), and dedupeTypes re-stringified the same Type objects at each level — 192 typeToString calls for 48 unique candidate types, 13ms of a 34ms request, when 50 call sites pass large distinct object literals through a two-hop forwarding chain. dedupeTypes now goes through a request-scoped typeToString memo on the inference context (sound: rendering a type is pure for a given checker, and the context never outlives its checker). describeTypes separately stringified every candidate twice — once inside dedupeTypes, once to render — and now dedupes by display string in the same pass that renders it. Same fixture after the fix: 48 stringifications (exactly one per unique type), request wall time 34ms -> 19.5ms. New baseline scenario nestedForwardingStringifications pins the bound with a deterministic counter on the public checker.typeToString: 30 unique candidates through a three-level chain must stay <= 32 calls (unfixed behavior: one call per candidate per level, 120). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AFvM84JTUzSx4oaXjMabHb
The tsserver plugin's require() resolvers join attacker-controlled import
specifiers (and a cartridge package.json `main`) onto a root directory
without asserting the result stays inside that root. A malicious repository
could therefore make a cartridge file resolve `require()` to a file OUTSIDE
the workspace and pull it into the TS program:
- resolveDwModule: `dw/../../../<abs>` escapes the bundled types dir
- resolveCartridgeModule: `~/..`, `*/..`, `<name>/..` escape the cartridge
- resolveModulesCartridge: `seg/..` and a package.json `main` of `../../..`
escape the modules cartridge
- an in-cartridge symlink pointing outside the root escapes too
Each resolved out-of-root file is returned as a resolvedModule, so TS reads
and parses it — an information-disclosure primitive (go-to-definition,
completion on the imported binding) driven purely by opening a cloned repo.
Add a shared canonicalize-and-contain guard (realpath to defeat symlinks,
then `..`/`.` collapse and case/slash fold) applied at every resolver's
trust boundary, plus resolveSuperModulePath for defense in depth. A
legitimately symlinked cartridge root still resolves because both sides are
realpath'd consistently.
Adds test/index.security.test.js: one regression test per traversal shape
(dw, ~/, */, <name>/, modules bare + package.json main, symlink), each
failing before this change, alongside guards that legitimate requires still
resolve. Tests drive the real create() proxy resolver against on-disk
fixtures, not mocks.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013GzksxTHf9NL8qyj59iRfr
dw.json and a cartridge's package.json are attacker-controlled in a cloned repo and were parsed synchronously on tsserver's thread with no size bound — a multi-hundred-megabyte file is a denial-of-service vector (memory + parse time). Route both reads through a single size-capped reader (1 MiB ceiling; real files are a few KB) that never throws and yields `undefined` for a missing, oversized, or malformed file, which callers already treat as absent. Also guard the BASE_CARTRIDGE_RANK lookup in orderCartridges with a hasOwnProperty check so a cartridge directory named `__proto__`/`constructor` can't read an inherited prototype value and corrupt the sort comparator. Adds a regression test: a valid package.json with a valid in-root `main` but padded past the ceiling must be refused, not parsed (fails before the cap). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013GzksxTHf9NL8qyj59iRfr
The feature forwards workspace-derived cartridge names and filesystem paths
to the tsserver plugin, which resolves require() across the whole project
and (with inferUsage) reads sibling files to synthesize hover/completion —
work that should never act on an unvetted, freshly-cloned repository. It was
only implicitly trust-gated (an extension with a main entry and no declared
Workspace Trust support is disabled in Restricted Mode), which is fragile.
Make the posture explicit and robust:
- declare capabilities.untrustedWorkspaces.supported = false so VS Code
formally withholds the extension until the workspace is trusted;
- additionally gate the script-types push() on vscode.workspace.isTrusted
as defense in depth (forwards a disabled config while untrusted), and
re-push on onDidGrantWorkspaceTrust so the feature turns on without a
reload once the user vouches for the workspace.
This also neutralizes a workspace .vscode/settings.json shipped by a repo
force-enabling the preview inferUsage flag: such settings only take effect
in a trusted workspace, which is now a prerequisite for the feature at all.
Adds a changeset covering the full security-hardening pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013GzksxTHf9NL8qyj59iRfr
The inference engine had grown to a single ~1,200-line src/usage-inference.ts (and a ~1,070-line compiled twin), which is hard to navigate. Split it by responsibility into small, single-purpose modules under src/inference/ behind a re-exporting barrel, keeping the public API (and the `../plugin/usage-inference` import path the tests use) unchanged: constants - the tunable limits that bound a request context - the per-request scratchpad (program, budgets, memo, guards) ast-helpers - pure AST navigation (find node, enclosing access, returns) call-sites - find where a function is called across the project type-helpers - Type utilities + hover text / completion entry synthesis super-module - module.superModule detection and export scanning (leaf ops) core - the recursive engine that ties the leaf modules together Dependencies flow one way (core -> leaves; leaves never import core), so there are no import cycles. The recursive functions that must call each other stay together in core.ts; only genuinely independent helpers were extracted. Pure code move — no behavior change. Largest module is now 503 source / 476 compiled lines. All 104 unit tests pass unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013GzksxTHf9NL8qyj59iRfr
index.ts had grown to ~910 lines. Move the pieces that don't touch the
plugin's mutable per-project state into two focused modules under
src/resolver/, leaving index.ts as just the plugin lifecycle (config,
module-resolution wiring, hover/completion overrides):
resolver/constants - shared cartridge types + lookup tables
(ambient module names, candidate extensions,
base-cartridge ranks, discovery ignore list,
JSON size ceiling)
resolver/cartridge-discovery - the size-capped JSON reader, .project
discovery walk, dw.json cartridge-order
parsing, ordering, and declare-module range
scanning — all pure/standalone functions
taking `ts` (+ a fileExists probe) as plain
arguments
The bundled-types path constants (TYPES_DIR/GLOBAL_DTS/SFRA_SERVER_DTS) stay
in index.ts because they resolve against `__dirname`, which must point at the
plugin's own output location. Pure code move — no behavior change. index.ts
is now 724 source / 659 compiled lines. All 104 unit tests and the 17 VS Code
E2E tests pass unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013GzksxTHf9NL8qyj59iRfr
fallow/ESLint flagged these two as the codebase's complexity hotspots.
Decompose them without changing behavior:
- resolveExpressionTypes is now a small dispatcher that routes an `any`
expression to one of three focused resolvers by kind — resolveCallResultTypes
(call / method-chain), resolvePropertyTypes (property access), and
resolveIdentifierTypes (parameter / local variable). Each returns [] where
the original fell through, so the dispatch is equivalent.
Cyclomatic 29 -> 7, cognitive 61 -> 6.
- collectCallSites now delegates the inner per-reference processing loop to
collectCallsFromName, which classifies each reference hit into a call site
or a next-hop name and threads the remaining local budget back via its
return value. Cyclomatic 19 -> 9, cognitive 37 -> 12.
The worst function in the package drops from cognitive 61 to 14. Pure
mechanical extraction — all 104 unit tests and the 17 VS Code E2E tests pass
unchanged, and core.ts stays under 800 lines.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013GzksxTHf9NL8qyj59iRfr
The b2c-script-types package pulled c8 into its own importer purely to run a coverage-wrapped `test` script, which was the only net-new entry in pnpm-lock.yaml on this branch and tripped the third-party-dependency review gate. The package's tests run fine under plain `node --test` (test:agent, used by CI, never invoked c8), so drop the c8 devDependency and the `c8` wrapper from the `test` script. With c8 gone from the only package that added it, the lockfile is byte-for-byte identical to main again (the remaining diff was a cosmetic importer reorder), so `pnpm install --frozen-lockfile` passes and there are no net-new third-party dependencies to review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013GzksxTHf9NL8qyj59iRfr
When call-site inference finds nothing (a helper only reachable indirectly, or genuinely dead code), match a parameter or local variable's own member/method usage against the vendored dw.* ambient classes to recover its type. Also covers the manual-indexing loop idiom (var item = items[i]) where the element type can only come from the loop variable's own downstream usage. Fixes found while dogfooding this against a real cartridge: - Hover on a chained access's member name (not just the receiver) resolved nothing even when the receiver inferred fine. - The ambient class index was cached per-Program instead of per LanguageService, so it rebuilt on nearly every keystroke while editing a cartridge file, making completions unreliably slow on large real projects. - Hover now borrows the real declaration's display header, doc comment, and JSDoc tags instead of a bare "Inferred from usage: X" note. - A class's nested custom-attributes interface (ICustomAttributes.X) rendered with the same display name as the unrelated top-level class X. - A dangling, mid-edit member access (`shipment.` immediately followed by more code on later lines) could parse together with the next statement, poisoning usage-based matching with a phantom member and silently producing no completions for the position being typed. Adds unit coverage in b2c-script-types and end-to-end VS Code integration coverage (hover + completions) in b2c-vs-extension for all of the above. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Debugging output left over from diagnosing the earlier flaky assertions; the failure-path assert messages already report the full completion label list. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Extracts security-critical cartridge module resolution and path-containment logic out of index.ts's closures into a testable resolver/module-resolution.ts module, deduplicates the legacy/modern module-resolution host hooks, hardens the JSON size-cap fallback, fixes a workspace-trust label bug in the VS Code extension, and cleans up docs/changeset/CI trigger inconsistencies found during review. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…cess Reported from real-world dogfooding (neuhaus-core's addressHelpers.js): removing JSDoc from getAddressBookAddressByForm(addressBook, form) lost hover/completion for addressBook entirely, since its only direct member access is addressBook.addresses — a single-member usage signature, below the existing two-member ambiguity threshold. That threshold exists to reject common member names (e.g. .custom) shared by many ambient classes, but .addresses is declared by exactly one: dw.customer.AddressBook. matchAmbientTypesByUsage now accepts a single-member signature when it's globally unambiguous, while still rejecting one that ties across multiple classes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Covers the real-world case fixed in 63d67de: a no-call-site parameter resolved via the single-unique-member ambient-class match. Asserts the ambient-class index (every dw.* class's member-name set) is built once per LanguageService and a repeated hover at the same position adds zero further getPropertiesOfType calls, per the existing deterministic-counter convention in this file (wall-clock is only a catastrophic-regression tripwire). Measured directly: cold hover ~34ms (1470 getPropertiesOfType calls to build the index against this fixture's real dw.* program), warm hover 0.005ms (0 additional calls) — confirms the fallback stays cheap on repeated keystrokes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Surveyed a large real-world SFRA storefront (omoda-core) for patterns our usage-inference engine might miss. The `'member' in obj` existence-check guard — used 261 times across 107 files there to test an optional custom attribute before reading it — was invisible to collectMemberUsageInScope, which only recognized direct `obj.member` property access. A parameter whose only usage evidence was such a guard (no direct read nearby) produced no inference at all. Also expands regression coverage with real-world shapes found in the same survey: mutually-exclusive boolean-flag branches (dw.catalog.Category), and an object literal built from a parameter's properties and passed to a call argument rather than returned — both already handled correctly by the existing control-flow-agnostic AST walk, now locked in as characterization tests. Other surveyed patterns (ES6 classes, arrow-function exports, destructured function parameters) don't appear in this codebase's ES5-style scripts, so were not pursued. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Surveyed a second real-world SFRA storefront (mul-core) for usage-inference
gaps. Confirmed the same constructor-function "class" model idiom found in
omoda-core (StoreModel, ProductLineItem, CartModel, AccountModel, Contact,
...) is invoked almost exclusively via `new Helper(x)`, never a plain call
— a shape collectCallSites/findCallInCalleePosition couldn't see at all,
since `new` expressions are a distinct AST node kind from CallExpression.
call-sites.ts now treats a CallExpression and a NewExpression uniformly as
a `CallSite` wherever a call site is collected or matched (findCallInCalleePosition,
resolveIndirectReferenceTarget, collectCallSites/collectCallsFromName,
InferenceContext.callSiteMemo). inferParameterType guards against a bare
`new Helper` (no parens) whose `arguments` is `undefined`, unlike a plain
call's always-present array.
Also surveyed: this codebase, like omoda-core, is pure ES5-style SFRA (no
ES6 classes/arrow-exports/destructured parameters); the two genuine
destructured-return-value cases found (`var {a,b} = undocumentedFn()`) and
the `Foo.prototype = Base.prototype` constructor-inheritance idiom are real
but substantially more complex to support — deferred given effort/value
tradeoff.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
All six changesets touching @salesforce/b2c-cli and b2c-vs-extension in this PR describe facets of the same shipped feature (usage inference) and its accompanying security hardening. Merged into a single minor changeset so the changelog reads as one coherent entry instead of six fragments. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous commit removed the five now-redundant changeset files but missed staging the actual merged content in script-types-infer-usage.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e test DocumentRegistry Real-world bug reported from mul-core's plugin_marketing_cloud/accountHelpers.js: `var profile = resettingCustomer.profile;` inferred as dw.customer. ProductListRegistrant instead of dw.customer.Profile. Root cause: both classes expose the same email/firstName/lastName/custom field subset touched in that function, and matchAmbientTypesByUsage's "fewest total members" tiebreak picked ProductListRegistrant (21 members) over Profile (92 members) every time, purely because it has less surface area — never the large, contextually correct class. A parameter/variable conventionally named after the SFCC class it holds is now checked first: when exactly one matching candidate's own class name matches the identifier (case- insensitively), that candidate wins outright, before size-based tiebreaking ever runs. Ambient class names are unique, so at most one candidate can ever match this way. Also fixes the test suite's own real-world performance problem, found while investigating why this suite intermittently timed out: every fixture LanguageService created its own brand-new ts.DocumentRegistry, discarding TypeScript's built-in mechanism for reusing an already-parsed-and-bound SourceFile across LanguageServices — forcing the entire vendored dw/* declaration tree to be re-parsed and re-bound from scratch on every single test. Fixture LanguageServices now share one module-level DocumentRegistry; in-memory fixture files use their own text as their "version" (so two tests reusing the same path with different content are never confused for each other), while real on-disk files (the expensive part) keep a constant version and get parsed once total. Measured: a cold fixture build costs ~300ms; a warm one sharing the same real dw.* files now costs ~17ms. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
index.test.js and index.security.test.js build their LanguageService directly (not via createFixtureLanguageService), so the earlier DocumentRegistry-sharing fix didn't reach them. index.test.js alone created 13 fresh DocumentRegistry instances across its tests, each forcing a full re-parse/re-bind of the vendored dw/* tree. Switching those call sites to the same shared, content-versioned registry cuts this file's run time from ~4.1s to ~2.0s, with identical pass/fail results before and after. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two LanguageService instances in usage-inference.perf.test.js still created their own fresh DocumentRegistry instead of reusing the shared one, missed by the earlier fixture-language-service.js and index.test.js fixes. Closes out that cleanup for consistency. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
NormalizedCartridge.root is lowercased on case-insensitive filesystems so ownerCartridge/isCartridgeFile can do prefix comparisons safely, but resolveCartridgeModule, resolveModulesCartridge, and resolveSuperModulePath were also using it to build the actual resolved path handed back to TypeScript — silently lowercasing every resolved cartridge file path on macOS/Windows. This mostly went unnoticed because case-insensitive filesystems open the file anyway, but it broke exact-match containment checks (as seen in the index.security.test.js path-traversal suite) and, more significantly, silently broke the superModule overlay-chain resolution whenever a fixture or real project used mixed-case paths, since that comparison is an exact string lookup rather than a filesystem call. Adds a `rawRoot` (original case) alongside `root` (folded, comparison- only) on NormalizedCartridge, and switches every path-construction site to use it while leaving comparison sites on the folded form. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… coverage Prefer silence over noisy unions when call sites disagree or evidence is only ubiquitous members; rank ambient matches by distinctiveness; cover map/filter/ find callbacks; resolve cartridge requires through the LS host; add a golden corpus and hardening tests drawn from real storefront patterns. Co-authored-by: Cursor <cursoragent@cursor.com>
Drop call-site types that can't satisfy a parameter's body members (mul-core duck-typed Store models), and allow a named parameter with one strong member (customer + .profile) to resolve even when that member is shared. Co-authored-by: Cursor <cursoragent@cursor.com>
…matches Dogfood-derived golden cases (anonymized) cover Customer/Basket/Order/Profile silence traps, and ambient matching now includes Product<T> as Product<any> so JSDoc-less product helpers match IntelliJ-style Script API IntelliSense. Co-authored-by: Cursor <cursoragent@cursor.com>
Treat @param {Object}/{obj}/{*}/{} as non-explicit while still respecting
deliberate any and real dw.* annotations, matching IntelliJ's JSDoc-first
model without silencing the common undocumented storefront helpers.
Co-authored-by: Cursor <cursoragent@cursor.com>
Recover lineItem/pli/priceModel aliases, collections.first ternary returns, and instanceof class checks that JetBrains surfaces from typings and naming conventions. Co-authored-by: Cursor <cursoragent@cursor.com>
Recognize resettingCustomer/apiProduct/currentBasket-style identifiers, expand the storefront corpus, and cover naming/instanceof/collections.first through the VS Code hover suite. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…inference
Two user-facing fixes to the (still-Preview) Script API usage-inference engine,
plus regression coverage that closes the gap the failing VS Code integration
tests exposed.
1. Hover/completion entry gate rejected `@param {Object}`. checkJs resolves the
ubiquitous SFRA `@param {Object}` placeholder to the global `Object`
interface — not `any`, not the lowercase `object` non-primitive — so
isOpenForUsageInference() treated it as a real type and skipped inference in
the live editor, breaking the feature's headline "undocumented SFRA helper"
case. The unit/corpus suites call inferParameterType() directly and never
exercised the gate, so this only surfaced in the VS Code host (the 3 CI
failures: resettingCustomer/lineItem naming tests, all @param {Object}).
Weak `{Object}` now opens the gate like the other placeholders. Added a
proxy-wiring unit test that goes through getQuickInfoAtPosition so the fast
layer catches this next time.
2. Specific dw.order line-item subclasses mis-resolved to ProductLineItem. A
parameter named bonusDiscountLineItem / productShippingLineItem was forced to
ProductLineItem by the generic bare-`LineItem` naming suffix. Added the two
uncovered subclasses to the alias/suffix maps (ordered before the bare
fallback) so they resolve to their own class when body usage fits, and stay
silent otherwise, rather than guessing the wrong sibling.
Also hardens the corpus silence assertion to check types.length (never
deepEqual against TS Type objects, which hangs on circular structure), adds
three corpus cases and a VS Code integration suite covering the subclass
disambiguation and real-`dw.*`-JSDoc deference.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
2 participants
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.
Summary
Today, an undocumented SFCC helper function (no JSDoc) gets its parameters and return value widened to
anyby TypeScript, and thatanysilently poisons every downstream caller — hover and autocomplete just stop working for anything that touches it. This is extremely common in real SFRA cartridges.This PR adds an opt-in, heuristic fallback to
@salesforce/b2c-script-types: when the checker has already given up withany, the plugin looks at how the value is actually used elsewhere in the project — call-site arguments, return expressions, chained property/method access,module.superModuleoverlays — and infers a plausible type from that usage. It surfaces the result as hover text ("Inferred from usage: …") and synthesized member completions, without ever overriding a type TypeScript or JSDoc already resolved correctly.@salesforce/b2c-script-types, wired into the plugin'sgetQuickInfoAtPosition/getCompletionsAtPosition, behind a newinferUsageconfig flag (off by default).b2c-dx.features.scriptTypesInferUsage(off by default, labeled Preview/heuristic) plumbed through to the plugin.dw.*classes, callback/iterator element types, and multi-cartridgesuperModuleoverlays.Also included, hardening the feature before it ships:
require()path — including a cartridge'spackage.jsonmain— is canonicalized and contained, so a crafted import specifier or an in-repo symlink can no longer resolve to a file outside the bundled types directory or the cartridge roots.dw.json/package.jsonparsing is size-bounded to guard against oversized-file denial-of-service. The VS Code extension declarescapabilities.untrustedWorkspacesand refuses to forward cartridge paths or run inference until the workspace is trusted.Screenshots
Setting is off by default and clearly labeled as a preview/heuristic feature:
Hover surfaces the inferred type plus the real declaration's own docs, labeled so it's clear it's a heuristic guess, not a resolved type:
Completions for an otherwise-
anyvalue are populated from the inferred type's real members:Testing
@salesforce/b2c-script-types(node --test): the inference engine against both ambient stand-ins and the real bundleddw.*declarations (happy paths, deep chains, nullable types, unioned call-site types, budget/depth caps, cancellation), the tsserver plugin's proxy wiring (hover + completions), and a dedicated path-traversal/JSON-size security suite with one regression test per traversal shape.vscode-test) drives the real extension headless and asserts hover/completion against the realdw.catalog.Producttype across single-file, cross-file, chained, nullable, callback/iterator, and multi-cartridgesuperModulescenarios.Dependencies
3pl-approvedis set by a maintainerpnpm test)pnpm run format)