Skip to content

Fix shorthand property resolution in typed contexts#441

Open
robbieh-noname wants to merge 1 commit intosourcegraph:mainfrom
robbieh-noname:fix/shorthand-property-typed-context
Open

Fix shorthand property resolution in typed contexts#441
robbieh-noname wants to merge 1 commit intosourcegraph:mainfrom
robbieh-noname:fix/shorthand-property-typed-context

Conversation

@robbieh-noname
Copy link
Copy Markdown

@robbieh-noname robbieh-noname commented Mar 27, 2026

Summary

Fixes #440. Possibly related: #415 (same class of bug in decorator context — I haven't tested that case, so it may need a separate fix).

When a shorthand property is passed to a typed context (constructor or function call), visitSymbolOccurrence() emits references to the type definition's properties but misses the local variable reference. This is because getDeclarationsForPropertyAssignment() returns non-empty declarations, causing isDefinitionNode to be false, which gates out handleShorthandPropertyDefinition() — the method specifically designed to emit that local variable reference.

Example

const resolvers = { Query: { books: () => [...] } };

// Bug: only emits ApolloServerOptions#resolvers (type-side)
// Missing: src/index.ts/resolvers. (local variable)
const server = new ApolloServer({ resolvers });

// Workaround: explicit syntax emits both correctly
const server = new ApolloServer({ resolvers: resolvers });

Root Cause

In FileIndexer.ts, visitSymbolOccurrence():

  1. getDeclarationsForPropertyAssignment(node) succeeds — declarations is non-empty
  2. isDefinitionNode = declarations.length === 0 && isDefinition(node)false (short-circuits)
  3. The for loop emits type-property references correctly
  4. handleShorthandPropertyDefinition is gated behind if (isDefinitionNode)never called

handleShorthandPropertyDefinition uses checker.getShorthandAssignmentValueSymbol() to find the local variable — the logic is correct, it's just unreachable in typed contexts.

Fix

After the for (const declaration of declarations) loop, add a block that calls checker.getShorthandAssignmentValueSymbol() directly when declarations.length > 0 and the node is a ShorthandPropertyAssignment. This emits the missing local variable reference. pushOccurrence deduplication (isEqualOccurrence) prevents double-emission.

The fix is ~15 lines of logic (21 including comments), in one location.

Test plan

  • Existing snapshot test covers this case: snapshots/input/syntax/src/object-literals-call-signatures.ts has a handleShorthand() function that passes shorthand properties to a typed function (consumesInterface({ interfaceMethod, property })). The snapshot update (+2 lines) shows the local variable references (reference local 26, reference local 23) are now correctly emitted alongside the existing type-property references.
  • Full test suite passes: 31/31 tests pass on current main (cherry-picked from v0.3.15 where 30/30 passed — the extra test is from the @deprecated feature in v0.4.0).
  • Real-world validation: I tested this against 13 Apollo Server repos — 9 were affected by this bug, 8 verified fixed by comparing SCIP indexes before and after the fix. The bug also affects makeExecutableSchema() from @graphql-tools/schema and any typed function/constructor with interface parameters.
  • No regressions: SCIP index size comparison across 14 TypeScript test projects: 6 identical (no shorthand in those projects), 8 grew slightly (+1 to +25KB from the added local variable references), 0 shrank.
  • Deduplication safety: pushOccurrence uses isEqualOccurrence to prevent double-emission — if the local variable reference was already emitted through another path, it won't be duplicated.

Impact

This bug affects any TypeScript code using shorthand property syntax in typed constructor/function contexts — not just Apollo Server. It breaks tooling that relies on SCIP reference edges to connect local variable definitions to their usage sites (e.g., code navigation, static analysis).

Note on #415 (NestJS decorators): Issue #415 reports the same symptom in decorator context (@Module({ imports })). I haven't tested whether this fix covers decorators — they may go through a different code path. If not, that'd need a separate look.

When a shorthand property (e.g., `new ApolloServer({ resolvers })`) is
passed to a typed parameter, SCIP emits the type-property reference but
misses the local variable reference. This adds the missing reference by
checking if the original node is a shorthand property assignment and
emitting an additional occurrence for the local binding.

Fixes behavior reported in sourcegraph#415 and sourcegraph#440.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant