diff --git a/.changeset/fuzzy-snakes-hope.md b/.changeset/fuzzy-snakes-hope.md new file mode 100644 index 0000000..5e12398 --- /dev/null +++ b/.changeset/fuzzy-snakes-hope.md @@ -0,0 +1,5 @@ +--- +'@0no-co/graphqlsp': patch +--- + +Mute co-located fragment warnings, when fragments are directly referenced. diff --git a/packages/graphqlsp/src/ast/checks.ts b/packages/graphqlsp/src/ast/checks.ts index 57505f2..395e825 100644 --- a/packages/graphqlsp/src/ast/checks.ts +++ b/packages/graphqlsp/src/ast/checks.ts @@ -171,13 +171,3 @@ export const getSchemaName = ( return null; }); }; - -/** Checks if node is a maskFragments() call */ -export const isMaskFragmentsCall = ( - node: ts.Node -): node is ts.CallExpression => { - if (!ts.isCallExpression(node)) return false; - if (!ts.isIdentifier(node.expression)) return false; - // Only checks function name, not whether it's from gql.tada - return node.expression.escapedText === 'maskFragments'; -}; diff --git a/packages/graphqlsp/src/ast/index.ts b/packages/graphqlsp/src/ast/index.ts index 0695a5e..96683ff 100644 --- a/packages/graphqlsp/src/ast/index.ts +++ b/packages/graphqlsp/src/ast/index.ts @@ -356,21 +356,6 @@ export function findAllImports( return sourceFile.statements.filter(ts.isImportDeclaration); } -export function findAllMaskFragmentsCalls( - sourceFile: ts.SourceFile -): Array { - const result: Array = []; - - function find(node: ts.Node): void { - if (checks.isMaskFragmentsCall(node)) { - result.push(node); - } - ts.forEachChild(node, find); - } - find(sourceFile); - return result; -} - export function bubbleUpTemplate(node: ts.Node): ts.Node { while ( ts.isNoSubstitutionTemplateLiteral(node) || diff --git a/packages/graphqlsp/src/diagnostics.ts b/packages/graphqlsp/src/diagnostics.ts index a21af00..1c9bcaa 100644 --- a/packages/graphqlsp/src/diagnostics.ts +++ b/packages/graphqlsp/src/diagnostics.ts @@ -13,12 +13,13 @@ import { print } from '@0no-co/graphql.web'; import { findAllCallExpressions, + findAllImports, findAllPersistedCallExpressions, findAllTaggedTemplateNodes, - findAllMaskFragmentsCalls, getSource, unrollFragment, } from './ast'; +import { getValueOfIdentifier } from './ast/declaration'; import { resolveTemplate } from './ast/resolve'; import { templates } from './ast/templates'; import { UNUSED_FIELD_CODE, checkFieldUsageInFile } from './fieldUsage'; @@ -225,6 +226,7 @@ export function getGraphQLDiagnostics( nodes: { node: ts.StringLiteralLike | ts.TaggedTemplateExpression; schema: string | null; + tadaFragmentRefs?: readonly ts.Identifier[] | null; }[]; if (isCallExpression) { const result = findAllCallExpressions(source, info); @@ -461,22 +463,14 @@ export function getGraphQLDiagnostics( } catch (e) {} }); - // check for maskFragments() calls - const maskFragmentsCalls = findAllMaskFragmentsCalls(source); - maskFragmentsCalls.forEach(call => { - const firstArg = call.arguments[0]; - if (!firstArg) return; - - // Handle array of fragments: maskFragments([Fragment1, Fragment2], data) - if (ts.isArrayLiteralExpression(firstArg)) { - firstArg.elements.forEach(element => { - if (ts.isIdentifier(element)) { - const fragmentDefs = unrollFragment(element, info, typeChecker); - fragmentDefs.forEach(def => usedFragments.add(def.name.value)); - } - }); - } - }); + // A fragment referenced directly (as a type/value, or via maskFragments()) + // rather than declared in a fragment-reference array isn't meant to be + // spread here, so don't flag it. + const directlyUsedFragments = getDirectlyUsedFragments( + source, + nodes, + typeChecker + ); Object.keys(moduleSpecifierToFragments).forEach(moduleSpecifier => { const { @@ -485,7 +479,11 @@ export function getGraphQLDiagnostics( length, } = moduleSpecifierToFragments[moduleSpecifier]!; const missingFragments = Array.from( - new Set(fragmentNames.filter(x => !usedFragments.has(x))) + new Set( + fragmentNames.filter( + x => !usedFragments.has(x) && !directlyUsedFragments.has(x) + ) + ) ); if (missingFragments.length) { fragmentDiagnostics.push({ @@ -507,6 +505,100 @@ export function getGraphQLDiagnostics( } } +/** Resolves the fragment name(s) defined directly by an identifier's `graphql()` + * document, excluding fragments it only composes via its reference array. */ +function getFragmentNamesForIdentifier( + identifier: ts.Identifier, + checker: ts.TypeChecker +): string[] { + const value = getValueOfIdentifier(identifier, checker); + if (!value || !ts.isCallExpression(value)) return []; + const documentArg = value.arguments[0]; + if (!documentArg || !ts.isStringLiteralLike(documentArg)) return []; + try { + const parsed = parse(documentArg.getText().slice(1, -1), { + noLocation: true, + }); + return parsed.definitions + .filter( + (definition): definition is FragmentDefinitionNode => + definition.kind === Kind.FRAGMENT_DEFINITION + ) + .map(definition => definition.name.value); + } catch (e) { + return []; + } +} + +/** Collects fragment names whose imported binding is referenced outside both its + * import and any `graphql()` fragment-reference array — i.e. used directly (as a + * type, an unmasking-call argument, or a re-export) and so not meant to be + * spread here. Bindings only in a reference array, or never referenced, are left + * to warn. */ +function getDirectlyUsedFragments( + source: ts.SourceFile, + nodes: Array<{ tadaFragmentRefs?: readonly ts.Identifier[] | null }>, + typeChecker: ts.TypeChecker | undefined +): Set { + const directlyUsedFragments = new Set(); + if (!typeChecker) return directlyUsedFragments; + + // A reference here is an explicit co-location declaration, not a direct use. + const fragmentRefIdentifiers = new Set(); + nodes.forEach(({ tadaFragmentRefs }) => { + if (tadaFragmentRefs) + tadaFragmentRefs.forEach(identifier => + fragmentRefIdentifiers.add(identifier) + ); + }); + + // Index occurrences by symbol once, rather than re-walking per import. + const occurrencesBySymbol = new Map(); + const visit = (node: ts.Node): void => { + if (ts.isIdentifier(node)) { + const symbol = typeChecker.getSymbolAtLocation(node); + if (symbol) { + const existing = occurrencesBySymbol.get(symbol); + if (existing) existing.push(node); + else occurrencesBySymbol.set(symbol, [node]); + } + } + ts.forEachChild(node, visit); + }; + visit(source); + + const bindings: ts.Identifier[] = []; + findAllImports(source).forEach(imp => { + const clause = imp.importClause; + if (!clause) return; + if (clause.name) bindings.push(clause.name); + const namedBindings = clause.namedBindings; + if (namedBindings) { + if (ts.isNamespaceImport(namedBindings)) { + bindings.push(namedBindings.name); + } else { + namedBindings.elements.forEach(element => bindings.push(element.name)); + } + } + }); + + bindings.forEach(binding => { + const symbol = typeChecker.getSymbolAtLocation(binding); + if (!symbol) return; + const occurrences = occurrencesBySymbol.get(symbol) || []; + const usedDirectly = occurrences.some( + identifier => + identifier !== binding && !fragmentRefIdentifiers.has(identifier) + ); + if (!usedDirectly) return; + getFragmentNamesForIdentifier(binding, typeChecker).forEach(name => + directlyUsedFragments.add(name) + ); + }); + + return directlyUsedFragments; +} + const runDiagnostics = ( source: ts.SourceFile, { @@ -516,7 +608,7 @@ const runDiagnostics = ( nodes: { node: ts.TaggedTemplateExpression | ts.StringLiteralLike; schema: string | null; - tadaFragmentRefs?: readonly ts.Identifier[]; + tadaFragmentRefs?: readonly ts.Identifier[] | null; }[]; fragments: FragmentDefinitionNode[]; }, @@ -569,7 +661,10 @@ const runDiagnostics = ( const endPosition = startingPosition + node.getText().length; let docFragments = [...fragments]; - if (originalNode.tadaFragmentRefs !== undefined) { + if ( + originalNode.tadaFragmentRefs !== undefined && + originalNode.tadaFragmentRefs !== null + ) { const fragmentNames = new Set(); for (const identifier of originalNode.tadaFragmentRefs) { const unrolled = unrollFragment(identifier, info, typeChecker); diff --git a/test/e2e/fixture-project-tada/fixtures/used-fragment-direct.ts b/test/e2e/fixture-project-tada/fixtures/used-fragment-direct.ts new file mode 100644 index 0000000..80c3f8d --- /dev/null +++ b/test/e2e/fixture-project-tada/fixtures/used-fragment-direct.ts @@ -0,0 +1,15 @@ +import { graphql, type FragmentOf } from './graphql'; +import { PokemonFields } from './fragment'; + +const x = graphql(` + query Pok($limit: Int!) { + pokemons(limit: $limit) { + id + name + } + } +`); + +export const render = (pokemon: FragmentOf) => pokemon; + +console.log(x); diff --git a/test/e2e/tada.test.ts b/test/e2e/tada.test.ts index 482c7c7..a1eef3a 100644 --- a/test/e2e/tada.test.ts +++ b/test/e2e/tada.test.ts @@ -16,6 +16,10 @@ describe('Fragment + operations', () => { projectPath, 'used-fragment-mask.ts' ); + const outfileUsedFragmentDirect = path.join( + projectPath, + 'used-fragment-direct.ts' + ); const outfileCombinations = path.join(projectPath, 'fragment.ts'); let server: TSServer; @@ -47,6 +51,11 @@ describe('Fragment + operations', () => { fileContent: '// empty', scriptKindName: 'TS', } satisfies ts.server.protocol.OpenRequestArgs); + server.sendCommand('open', { + file: outfileUsedFragmentDirect, + fileContent: '// empty', + scriptKindName: 'TS', + } satisfies ts.server.protocol.OpenRequestArgs); server.sendCommand('updateOpen', { openFiles: [ @@ -85,6 +94,13 @@ describe('Fragment + operations', () => { 'utf-8' ), }, + { + file: outfileUsedFragmentDirect, + fileContent: fs.readFileSync( + path.join(projectPath, 'fixtures/used-fragment-direct.ts'), + 'utf-8' + ), + }, ], } satisfies ts.server.protocol.UpdateOpenRequestArgs); @@ -108,12 +124,17 @@ describe('Fragment + operations', () => { file: outfileUsedFragmentMask, tmpfile: outfileUsedFragmentMask, } satisfies ts.server.protocol.SavetoRequestArgs); + server.sendCommand('saveto', { + file: outfileUsedFragmentDirect, + tmpfile: outfileUsedFragmentDirect, + } satisfies ts.server.protocol.SavetoRequestArgs); }); afterAll(() => { try { fs.unlinkSync(outfileUnusedFragment); fs.unlinkSync(outfileUsedFragmentMask); + fs.unlinkSync(outfileUsedFragmentDirect); fs.unlinkSync(outfileCombinations); fs.unlinkSync(outfileCombo); fs.unlinkSync(outfileTypeCondition); @@ -430,6 +451,28 @@ List out all Pokémon, optionally in pages` expect(res[0].body.diagnostics).toMatchInlineSnapshot(`[]`); }, 30000); + it('should not warn about unused fragments when the fragment is used directly (not co-located)', async () => { + server.sendCommand('saveto', { + file: outfileUsedFragmentDirect, + tmpfile: outfileUsedFragmentDirect, + } satisfies ts.server.protocol.SavetoRequestArgs); + + await server.waitForResponse( + e => + e.type === 'event' && + e.event === 'semanticDiag' && + e.body?.file === outfileUsedFragmentDirect + ); + + const res = server.responses.filter( + resp => + resp.type === 'event' && + resp.event === 'semanticDiag' && + resp.body?.file === outfileUsedFragmentDirect + ); + expect(res[0].body.diagnostics).toMatchInlineSnapshot(`[]`); + }, 30000); + it('gives quick-info at start of word (#15)', async () => { server.send({ seq: 11,