From 1fbc8ebb088a0c77612fb4f8084d6a7ba7166b4e Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Thu, 20 Apr 2023 13:18:26 +0200 Subject: [PATCH] feat: provide diagnostic for unused colocated fragments (#42) * provide message for unused fragments * remove existing imports from diagnostics * updates * update readme * add todo * Update .changeset/sour-feet-hear.md --- .changeset/sour-feet-hear.md | 6 + .gitattributes | 4 +- README.md | 1 + packages/example/src/Pokemon.generated.ts | 127 ++++++++++++++++++---- packages/example/src/Pokemon.ts | 9 +- packages/example/src/index.generated.ts | 65 ----------- packages/example/src/index.ts | 8 +- packages/graphqlsp/README.md | 8 ++ packages/graphqlsp/src/ast.ts | 45 ++++++++ packages/graphqlsp/src/index.ts | 119 ++++++++++++++++++-- packages/graphqlsp/src/resolve.ts | 3 +- packages/graphqlsp/src/utils.ts | 37 ------- 12 files changed, 280 insertions(+), 152 deletions(-) create mode 100644 .changeset/sour-feet-hear.md create mode 100644 packages/graphqlsp/src/ast.ts diff --git a/.changeset/sour-feet-hear.md b/.changeset/sour-feet-hear.md new file mode 100644 index 0000000..d0098e0 --- /dev/null +++ b/.changeset/sour-feet-hear.md @@ -0,0 +1,6 @@ +--- +'@0no-co/graphqlsp': minor +--- + +Add a `message` diagnostic when we see an import from a file that has `fragment` exports we'll warn you when they are not imported, this because of the assumption that to use this file one would have to adhere to the data requirements of said file. +You can choose to disable this setting by setting `shouldCheckForColocatedFragments` to `false` diff --git a/.gitattributes b/.gitattributes index e1edcaf..57bd243 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,3 +1,3 @@ * text=auto -./**/*.generated.ts linguist-generated -./**/*.graphql linguist-generated +**/*.generated.ts linguist-generated +**/*.graphql linguist-generated diff --git a/README.md b/README.md index d0ae612..69e0ce7 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,7 @@ auto-complete and automatically generating [Typed-Document-nodes](https://the-gu - Diagnostics for adding fields that don't exist, are deprecated, missmatched argument types, ... - Auto-complete inside your editor for fields - When you save it will generate `typed-document-nodes` for your documents and cast them to the correct type +- Will warn you when you are importing from a file that is exporting fragments that you're not using ## Installation diff --git a/packages/example/src/Pokemon.generated.ts b/packages/example/src/Pokemon.generated.ts index 754b1d0..784eb56 100644 --- a/packages/example/src/Pokemon.generated.ts +++ b/packages/example/src/Pokemon.generated.ts @@ -1,9 +1,108 @@ -import * as Types from '../__generated__/baseGraphQLSP'; import { TypedDocumentNode as DocumentNode } from '@graphql-typed-document-node/core'; -export type FieldsFragment = { +export type Maybe = T | null; +export type InputMaybe = Maybe; +export type Exact = { + [K in keyof T]: T[K]; +}; +export type MakeOptional = Omit & { + [SubKey in K]?: Maybe; +}; +export type MakeMaybe = Omit & { + [SubKey in K]: Maybe; +}; +/** All built-in and custom scalars, mapped to their actual values */ +export type Scalars = { + ID: string; + String: string; + Boolean: boolean; + Int: number; + Float: number; +}; + +/** Elemental property associated with either a Pokémon or one of their moves. */ +export type PokemonType = + | 'Grass' + | 'Poison' + | 'Fire' + | 'Flying' + | 'Water' + | 'Bug' + | 'Normal' + | 'Electric' + | 'Ground' + | 'Fairy' + | 'Fighting' + | 'Psychic' + | 'Rock' + | 'Steel' + | 'Ice' + | 'Ghost' + | 'Dragon' + | 'Dark'; + +/** Move a Pokémon can perform with the associated damage and type. */ +export type Attack = { + __typename?: 'Attack'; + name?: Maybe; + type?: Maybe; + damage?: Maybe; +}; + +/** Requirement that prevents an evolution through regular means of levelling up. */ +export type EvolutionRequirement = { + __typename?: 'EvolutionRequirement'; + amount?: Maybe; + name?: Maybe; +}; + +export type PokemonDimension = { + __typename?: 'PokemonDimension'; + minimum?: Maybe; + maximum?: Maybe; +}; + +export type AttacksConnection = { + __typename?: 'AttacksConnection'; + fast?: Maybe>>; + special?: Maybe>>; +}; + +export type Pokemon = { __typename?: 'Pokemon'; - classification?: string | null; - id: string; + id: Scalars['ID']; + name: Scalars['String']; + classification?: Maybe; + types?: Maybe>>; + resistant?: Maybe>>; + weaknesses?: Maybe>>; + evolutionRequirements?: Maybe>>; + weight?: Maybe; + height?: Maybe; + attacks?: Maybe; + /** Likelihood of an attempt to catch a Pokémon to fail. */ + fleeRate?: Maybe; + /** Maximum combat power a Pokémon may achieve at max level. */ + maxCP?: Maybe; + /** Maximum health points a Pokémon may achieve at max level. */ + maxHP?: Maybe; + evolutions?: Maybe>>; +}; + +export type Query = { + __typename?: 'Query'; + /** List out all Pokémon, optionally in pages */ + pokemons?: Maybe>>; + /** Get a single Pokémon by its ID, a three character long identifier padded with zeroes */ + pokemon?: Maybe; +}; + +export type QueryPokemonsArgs = { + limit?: InputMaybe; + skip?: InputMaybe; +}; + +export type QueryPokemonArgs = { + id: Scalars['ID']; }; export type PokemonFieldsFragment = { @@ -12,26 +111,6 @@ export type PokemonFieldsFragment = { name: string; }; -export const FieldsFragmentDoc = { - kind: 'Document', - definitions: [ - { - kind: 'FragmentDefinition', - name: { kind: 'Name', value: 'fields' }, - typeCondition: { - kind: 'NamedType', - name: { kind: 'Name', value: 'Pokemon' }, - }, - selectionSet: { - kind: 'SelectionSet', - selections: [ - { kind: 'Field', name: { kind: 'Name', value: 'classification' } }, - { kind: 'Field', name: { kind: 'Name', value: 'id' } }, - ], - }, - }, - ], -} as unknown as DocumentNode; export const PokemonFieldsFragmentDoc = { kind: 'Document', definitions: [ diff --git a/packages/example/src/Pokemon.ts b/packages/example/src/Pokemon.ts index b4168fd..796d7d0 100644 --- a/packages/example/src/Pokemon.ts +++ b/packages/example/src/Pokemon.ts @@ -1,15 +1,10 @@ import { gql } from '@urql/core'; -export const fields = gql` - fragment fields on Pokemon { - classification - id - } -` as typeof import('./Pokemon.generated').FieldsFragmentDoc; - export const PokemonFields = gql` fragment pokemonFields on Pokemon { id name } ` as typeof import('./Pokemon.generated').PokemonFieldsFragmentDoc; + +export const Pokemon = () => 'hi'; diff --git a/packages/example/src/index.generated.ts b/packages/example/src/index.generated.ts index eb02b41..f6c9d9f 100644 --- a/packages/example/src/index.generated.ts +++ b/packages/example/src/index.generated.ts @@ -12,12 +12,6 @@ export type PokemonsQuery = { } | null> | null; }; -export type PokemonFieldsFragment = { - __typename?: 'Pokemon'; - id: string; - name: string; -}; - export type PokemonQueryVariables = Types.Exact<{ id: Types.Scalars['ID']; }>; @@ -28,30 +22,9 @@ export type PokemonQuery = { __typename: 'Pokemon'; id: string; fleeRate?: number | null; - name: string; } | null; }; -export const PokemonFieldsFragmentDoc = { - kind: 'Document', - definitions: [ - { - kind: 'FragmentDefinition', - name: { kind: 'Name', value: 'pokemonFields' }, - typeCondition: { - kind: 'NamedType', - name: { kind: 'Name', value: 'Pokemon' }, - }, - selectionSet: { - kind: 'SelectionSet', - selections: [ - { kind: 'Field', name: { kind: 'Name', value: 'id' } }, - { kind: 'Field', name: { kind: 'Name', value: 'name' } }, - ], - }, - }, - ], -} as unknown as DocumentNode; export const PokemonsDocument = { kind: 'Document', definitions: [ @@ -72,31 +45,12 @@ export const PokemonsDocument = { { kind: 'Field', name: { kind: 'Name', value: 'name' } }, { kind: 'Field', name: { kind: 'Name', value: '__typename' } }, { kind: 'Field', name: { kind: 'Name', value: 'fleeRate' } }, - { - kind: 'FragmentSpread', - name: { kind: 'Name', value: 'pokemonFields' }, - }, ], }, }, ], }, }, - { - kind: 'FragmentDefinition', - name: { kind: 'Name', value: 'pokemonFields' }, - typeCondition: { - kind: 'NamedType', - name: { kind: 'Name', value: 'Pokemon' }, - }, - selectionSet: { - kind: 'SelectionSet', - selections: [ - { kind: 'Field', name: { kind: 'Name', value: 'id' } }, - { kind: 'Field', name: { kind: 'Name', value: 'name' } }, - ], - }, - }, ], } as unknown as DocumentNode; export const PokemonDocument = { @@ -137,10 +91,6 @@ export const PokemonDocument = { selections: [ { kind: 'Field', name: { kind: 'Name', value: 'id' } }, { kind: 'Field', name: { kind: 'Name', value: 'fleeRate' } }, - { - kind: 'FragmentSpread', - name: { kind: 'Name', value: 'pokemonFields' }, - }, { kind: 'Field', name: { kind: 'Name', value: '__typename' } }, ], }, @@ -148,20 +98,5 @@ export const PokemonDocument = { ], }, }, - { - kind: 'FragmentDefinition', - name: { kind: 'Name', value: 'pokemonFields' }, - typeCondition: { - kind: 'NamedType', - name: { kind: 'Name', value: 'Pokemon' }, - }, - selectionSet: { - kind: 'SelectionSet', - selections: [ - { kind: 'Field', name: { kind: 'Name', value: 'id' } }, - { kind: 'Field', name: { kind: 'Name', value: 'name' } }, - ], - }, - }, ], } as unknown as DocumentNode; diff --git a/packages/example/src/index.ts b/packages/example/src/index.ts index e145c94..80079c7 100644 --- a/packages/example/src/index.ts +++ b/packages/example/src/index.ts @@ -1,5 +1,5 @@ import { gql, createClient } from '@urql/core'; -import { PokemonFields } from './Pokemon'; +import { Pokemon, PokemonFields } from './Pokemon'; const PokemonsQuery = gql` query Pokemons { @@ -8,11 +8,8 @@ const PokemonsQuery = gql` name __typename fleeRate - ...pokemonFields } } - - ${PokemonFields} ` as typeof import('./index.generated').PokemonsDocument; const client = createClient({ @@ -31,12 +28,9 @@ const PokemonQuery = gql` pokemon(id: $id) { id fleeRate - ...pokemonFields __typename } } - - ${PokemonFields} ` as typeof import('./index.generated').PokemonDocument; client diff --git a/packages/graphqlsp/README.md b/packages/graphqlsp/README.md index d0ae612..01e1d30 100644 --- a/packages/graphqlsp/README.md +++ b/packages/graphqlsp/README.md @@ -10,6 +10,7 @@ auto-complete and automatically generating [Typed-Document-nodes](https://the-gu - Diagnostics for adding fields that don't exist, are deprecated, missmatched argument types, ... - Auto-complete inside your editor for fields - When you save it will generate `typed-document-nodes` for your documents and cast them to the correct type +- Will warn you when you are importing from a file that is exporting fragments that you're not using ## Installation @@ -36,6 +37,13 @@ Go to your `tsconfig.json` and add now restart your TS-server and you should be good to go +### Configuration + +- `schema` allows you to specify a url, `.json` or `.graphql` file as your schema +- `scalars` allows you to pass an object of scalars that we'll feed into `graphql-code-generator` +- `shouldCheckForColocatedFragments` when turned on (default), this will scan your imports to find + unused fragments and provide a message notifying you about them + ## Local development Run `pnpm i` at the root. Open `packages/example` by running `code packages/example` or if you want to leverage diff --git a/packages/graphqlsp/src/ast.ts b/packages/graphqlsp/src/ast.ts new file mode 100644 index 0000000..3975dd7 --- /dev/null +++ b/packages/graphqlsp/src/ast.ts @@ -0,0 +1,45 @@ +import ts from 'typescript/lib/tsserverlibrary'; +import { + isImportDeclaration, + isNoSubstitutionTemplateLiteral, + isTaggedTemplateExpression, +} from 'typescript'; + +export function findNode( + sourceFile: ts.SourceFile, + position: number +): ts.Node | undefined { + function find(node: ts.Node): ts.Node | undefined { + if (position >= node.getStart() && position < node.getEnd()) { + return ts.forEachChild(node, find) || node; + } + } + return find(sourceFile); +} + +export function findAllTaggedTemplateNodes( + sourceFile: ts.SourceFile | ts.Node +): Array { + const result: Array< + ts.TaggedTemplateExpression | ts.NoSubstitutionTemplateLiteral + > = []; + function find(node: ts.Node) { + if ( + isTaggedTemplateExpression(node) || + isNoSubstitutionTemplateLiteral(node) + ) { + result.push(node); + return; + } else { + ts.forEachChild(node, find); + } + } + find(sourceFile); + return result; +} + +export function findAllImports( + sourceFile: ts.SourceFile +): Array { + return sourceFile.statements.filter(isImportDeclaration); +} diff --git a/packages/graphqlsp/src/index.ts b/packages/graphqlsp/src/index.ts index c4f0239..75a636b 100644 --- a/packages/graphqlsp/src/index.ts +++ b/packages/graphqlsp/src/index.ts @@ -8,7 +8,8 @@ import { isTemplateExpression, isImportTypeNode, ImportTypeNode, - CompletionEntry, + isNamespaceImport, + isNamedImportBindings, } from 'typescript'; import { getHoverInformation, @@ -25,12 +26,11 @@ import { OperationDefinitionNode, } from 'graphql'; +import { findAllImports, findAllTaggedTemplateNodes, findNode } from './ast'; import { Cursor } from './cursor'; import { loadSchema } from './getSchema'; import { getToken } from './token'; import { - findAllTaggedTemplateNodes, - findNode, getSource, getSuggestionsForFragmentSpread, isFileDirty, @@ -65,6 +65,8 @@ function create(info: ts.server.PluginCreateInfo) { const tagTemplate = info.config.template || 'gql'; const scalars = info.config.scalars || {}; + const shouldCheckForColocatedFragments = + info.config.shouldCheckForColocatedFragments || true; const proxy = createBasicDecorator(info); @@ -179,7 +181,108 @@ function create(info: ts.server.PluginCreateInfo) { return result; }); - if (!newDiagnostics.length) { + const imports = findAllImports(source); + if (imports.length && shouldCheckForColocatedFragments) { + const typeChecker = info.languageService.getProgram()?.getTypeChecker(); + imports.forEach(imp => { + if (!imp.importClause) return; + + const importedNames: string[] = []; + if (imp.importClause.name) { + importedNames.push(imp.importClause?.name.text); + } + + if ( + imp.importClause.namedBindings && + isNamespaceImport(imp.importClause.namedBindings) + ) { + // TODO: we might need to warn here when the fragment is unused as a namespace import + return; + } else if ( + imp.importClause.namedBindings && + isNamedImportBindings(imp.importClause.namedBindings) + ) { + imp.importClause.namedBindings.elements.forEach(el => { + importedNames.push(el.name.text); + }); + } + + const symbol = typeChecker?.getSymbolAtLocation(imp.moduleSpecifier); + if (!symbol) return; + + const moduleExports = typeChecker?.getExportsOfModule(symbol); + if (!moduleExports) return; + + const missingImports = moduleExports + .map(exp => { + if (importedNames.includes(exp.name)) { + return; + } + + const declarations = exp.getDeclarations(); + const declaration = declarations?.find(x => { + // TODO: check whether the sourceFile.fileName resembles the module + // specifier + return true; + }); + + if (!declaration) return; + + const [template] = findAllTaggedTemplateNodes(declaration); + if (template) { + let node = template; + if ( + isNoSubstitutionTemplateLiteral(node) || + isTemplateExpression(node) + ) { + if (isTaggedTemplateExpression(node.parent)) { + node = node.parent; + } else { + return; + } + } + + const text = resolveTemplate( + node, + node.getSourceFile().fileName, + info + ); + const parsed = parse(text); + if ( + parsed.definitions.every( + x => x.kind === Kind.FRAGMENT_DEFINITION + ) + ) { + return `'${exp.name}'`; + } + } + }) + .filter(Boolean); + + if (missingImports.length) { + // TODO: we could use getCodeFixesAtPosition + // to build on this + newDiagnostics.push({ + file: source, + length: imp.getText().length, + start: imp.getStart(), + category: ts.DiagnosticCategory.Message, + code: 51001, + messageText: `Missing Fragment import(s) ${missingImports.join( + ', ' + )} from ${imp.moduleSpecifier.getText()}.`, + }); + } + }); + } + + if ( + !newDiagnostics.filter( + x => + x.category === ts.DiagnosticCategory.Error || + x.category === ts.DiagnosticCategory.Warning + ).length + ) { try { const parts = source.fileName.split('/'); const name = parts[parts.length - 1]; @@ -246,10 +349,7 @@ function create(info: ts.server.PluginCreateInfo) { if (typeImport) { // We only want the oldExportName here to be present // that way we can diff its length vs the new one - const oldExportName = typeImport - .getText() - .split('.') - .pop() + const oldExportName = typeImport.getText().split('.').pop(); // Remove ` as ` from the beginning, // this because getText() gives us everything @@ -257,7 +357,8 @@ function create(info: ts.server.PluginCreateInfo) { // around. imp = imp.slice(4); text = source.text.replace(typeImport.getText(), imp); - span.length = imp.length + ((oldExportName || '').length - exportName.length); + span.length = + imp.length + ((oldExportName || '').length - exportName.length); } else { text = source.text.substring(0, span.start) + diff --git a/packages/graphqlsp/src/resolve.ts b/packages/graphqlsp/src/resolve.ts index 2b8aa07..ef4eade 100644 --- a/packages/graphqlsp/src/resolve.ts +++ b/packages/graphqlsp/src/resolve.ts @@ -6,7 +6,8 @@ import { TaggedTemplateExpression, } from 'typescript'; import ts from 'typescript/lib/tsserverlibrary'; -import { findNode, getSource } from './utils'; +import { findNode } from './ast'; +import { getSource } from './utils'; export function resolveTemplate( node: TaggedTemplateExpression, diff --git a/packages/graphqlsp/src/utils.ts b/packages/graphqlsp/src/utils.ts index af517e3..cf85b79 100644 --- a/packages/graphqlsp/src/utils.ts +++ b/packages/graphqlsp/src/utils.ts @@ -1,8 +1,4 @@ import ts from 'typescript/lib/tsserverlibrary'; -import { - isNoSubstitutionTemplateLiteral, - isTaggedTemplateExpression, -} from 'typescript'; import fs from 'fs'; import { CompletionItem, @@ -36,39 +32,6 @@ export function isFileDirty(fileName: string, source: ts.SourceFile) { return currentText !== contents; } -export function findNode( - sourceFile: ts.SourceFile, - position: number -): ts.Node | undefined { - function find(node: ts.Node): ts.Node | undefined { - if (position >= node.getStart() && position < node.getEnd()) { - return ts.forEachChild(node, find) || node; - } - } - return find(sourceFile); -} - -export function findAllTaggedTemplateNodes( - sourceFile: ts.SourceFile -): Array { - const result: Array< - ts.TaggedTemplateExpression | ts.NoSubstitutionTemplateLiteral - > = []; - function find(node: ts.Node) { - if ( - isTaggedTemplateExpression(node) || - isNoSubstitutionTemplateLiteral(node) - ) { - result.push(node); - return; - } else { - ts.forEachChild(node, find); - } - } - find(sourceFile); - return result; -} - export function getSource(info: ts.server.PluginCreateInfo, filename: string) { const program = info.languageService.getProgram(); if (!program) return undefined; -- knotmirror 0.0.1