diff --git a/.changeset/dry-moose-camp.md b/.changeset/dry-moose-camp.md new file mode 100644 index 00000000..3b8e6d4b --- /dev/null +++ b/.changeset/dry-moose-camp.md @@ -0,0 +1,5 @@ +--- +'@0no-co/graphqlsp': patch +--- + +Address potential crashes on malformed TypeScript AST input (such as missing function arguments where they were previously assumed to be passed) diff --git a/packages/graphqlsp/src/ast/checks.ts b/packages/graphqlsp/src/ast/checks.ts index a22fce21..22496d4c 100644 --- a/packages/graphqlsp/src/ast/checks.ts +++ b/packages/graphqlsp/src/ast/checks.ts @@ -43,7 +43,7 @@ export const isTadaGraphQLCall = ( return false; } else if (node.arguments.length < 1 || node.arguments.length > 2) { return false; - } else if (!ts.isStringLiteralLike(node.arguments[0])) { + } else if (!ts.isStringLiteralLike(node.arguments[0]!)) { return false; } return checker ? isTadaGraphQLFunction(node.expression, checker) : false; @@ -70,11 +70,16 @@ export const isTadaPersistedCall = ( } }; +// As per check in `isGraphQLCall()` below, enforces arguments length +export type GraphQLCallNode = ts.CallExpression & { + arguments: [ts.Expression] | [ts.Expression, ts.Expression]; +}; + /** Checks if node is a gql.tada or regular graphql() call */ export const isGraphQLCall = ( node: ts.Node, checker: ts.TypeChecker | undefined -): node is ts.CallExpression => { +): node is GraphQLCallNode => { return ( ts.isCallExpression(node) && node.arguments.length >= 1 && diff --git a/packages/graphqlsp/src/ast/index.ts b/packages/graphqlsp/src/ast/index.ts index f5330962..a3e17e28 100644 --- a/packages/graphqlsp/src/ast/index.ts +++ b/packages/graphqlsp/src/ast/index.ts @@ -60,9 +60,8 @@ function unrollFragment( element.getStart() ); - if (!definitions || !definitions.length) return fragments; - - const [fragment] = definitions; + const fragment = definitions && definitions[0]; + if (!fragment) return fragments; const externalSource = getSource(info, fragment.fileName); if (!externalSource) return fragments; @@ -263,6 +262,7 @@ export function getAllFragments( if ( ts.isVariableStatement(node) && node.declarationList && + node.declarationList.declarations[0] && node.declarationList.declarations[0].name.getText() === 'documents' ) { const [declaration] = node.declarationList.declarations; diff --git a/packages/graphqlsp/src/ast/resolve.ts b/packages/graphqlsp/src/ast/resolve.ts index f1e79ff5..5097ee0f 100644 --- a/packages/graphqlsp/src/ast/resolve.ts +++ b/packages/graphqlsp/src/ast/resolve.ts @@ -38,9 +38,10 @@ export function resolveTemplate( filename, span.expression.getStart() ); - if (!definitions || !definitions.length) return; - const def = definitions[0]; + const def = definitions && definitions[0]; + if (!def) return; + const src = getSource(info, def.fileName); if (!src) return; diff --git a/packages/graphqlsp/src/ast/token.ts b/packages/graphqlsp/src/ast/token.ts index 2914428c..67d83ade 100644 --- a/packages/graphqlsp/src/ast/token.ts +++ b/packages/graphqlsp/src/ast/token.ts @@ -63,7 +63,7 @@ export const getToken = ( } } - cPos += input[line].length + 1; + cPos += input[line]!.length + 1; } return foundToken; diff --git a/packages/graphqlsp/src/autoComplete.ts b/packages/graphqlsp/src/autoComplete.ts index e29dd38d..76307165 100644 --- a/packages/graphqlsp/src/autoComplete.ts +++ b/packages/graphqlsp/src/autoComplete.ts @@ -299,7 +299,7 @@ function runOnlineParser( let stream: CharacterStream = new CharacterStream(''); for (let i = 0; i < lines.length; i++) { - stream = new CharacterStream(lines[i]); + stream = new CharacterStream(lines[i]!); while (!stream.eol()) { style = parser.token(stream, state); const code = callback(stream, state, style, i); diff --git a/packages/graphqlsp/src/checkImports.ts b/packages/graphqlsp/src/checkImports.ts index d1d55ed0..76499643 100644 --- a/packages/graphqlsp/src/checkImports.ts +++ b/packages/graphqlsp/src/checkImports.ts @@ -37,8 +37,8 @@ export const getColocatedFragmentNames = ( source.fileName, imp.importClause.name.getStart() ); - if (definitions && definitions.length) { - const [def] = definitions; + const def = definitions && definitions[0]; + if (def) { if (def.fileName.includes('node_modules')) return; const externalSource = getSource(info, def.fileName); @@ -51,22 +51,16 @@ export const getColocatedFragmentNames = ( ); const names = fragmentsForImport.map(fragment => fragment.name.value); - if ( - names.length && - !importSpecifierToFragments[imp.moduleSpecifier.getText()] - ) { - importSpecifierToFragments[imp.moduleSpecifier.getText()] = { + const key = imp.moduleSpecifier.getText(); + let fragmentsEntry = importSpecifierToFragments[key]; + if (names.length && fragmentsEntry) { + fragmentsEntry.fragments = fragmentsEntry.fragments.concat(names); + } else if (names.length && !fragmentsEntry) { + importSpecifierToFragments[key] = fragmentsEntry = { start: imp.moduleSpecifier.getStart(), length: imp.moduleSpecifier.getText().length, fragments: names, }; - } else if (names.length) { - importSpecifierToFragments[ - imp.moduleSpecifier.getText() - ].fragments = - importSpecifierToFragments[ - imp.moduleSpecifier.getText() - ].fragments.concat(names); } } } @@ -79,8 +73,8 @@ export const getColocatedFragmentNames = ( source.fileName, imp.importClause.namedBindings.getStart() ); - if (definitions && definitions.length) { - const [def] = definitions; + const def = definitions && definitions[0]; + if (def) { if (def.fileName.includes('node_modules')) return; const externalSource = getSource(info, def.fileName); @@ -92,22 +86,16 @@ export const getColocatedFragmentNames = ( info ); const names = fragmentsForImport.map(fragment => fragment.name.value); - if ( - names.length && - !importSpecifierToFragments[imp.moduleSpecifier.getText()] - ) { - importSpecifierToFragments[imp.moduleSpecifier.getText()] = { + const key = imp.moduleSpecifier.getText(); + let fragmentsEntry = importSpecifierToFragments[key]; + if (names.length && fragmentsEntry) { + fragmentsEntry.fragments = fragmentsEntry.fragments.concat(names); + } else if (names.length && !fragmentsEntry) { + importSpecifierToFragments[key] = fragmentsEntry = { start: imp.moduleSpecifier.getStart(), length: imp.moduleSpecifier.getText().length, fragments: names, }; - } else if (names.length) { - importSpecifierToFragments[ - imp.moduleSpecifier.getText() - ].fragments = - importSpecifierToFragments[ - imp.moduleSpecifier.getText() - ].fragments.concat(names); } } } else if ( @@ -119,8 +107,8 @@ export const getColocatedFragmentNames = ( source.fileName, el.getStart() ); - if (definitions && definitions.length) { - const [def] = definitions; + const def = definitions && definitions[0]; + if (def) { if (def.fileName.includes('node_modules')) return; const externalSource = getSource(info, def.fileName); @@ -134,22 +122,16 @@ export const getColocatedFragmentNames = ( const names = fragmentsForImport.map( fragment => fragment.name.value ); - if ( - names.length && - !importSpecifierToFragments[imp.moduleSpecifier.getText()] - ) { - importSpecifierToFragments[imp.moduleSpecifier.getText()] = { + const key = imp.moduleSpecifier.getText(); + let fragmentsEntry = importSpecifierToFragments[key]; + if (names.length && fragmentsEntry) { + fragmentsEntry.fragments = fragmentsEntry.fragments.concat(names); + } else if (names.length && !fragmentsEntry) { + importSpecifierToFragments[key] = fragmentsEntry = { start: imp.moduleSpecifier.getStart(), length: imp.moduleSpecifier.getText().length, fragments: names, }; - } else if (names.length) { - importSpecifierToFragments[ - imp.moduleSpecifier.getText() - ].fragments = - importSpecifierToFragments[ - imp.moduleSpecifier.getText() - ].fragments.concat(names); } } }); diff --git a/packages/graphqlsp/src/diagnostics.ts b/packages/graphqlsp/src/diagnostics.ts index 74cd1257..44ca62d3 100644 --- a/packages/graphqlsp/src/diagnostics.ts +++ b/packages/graphqlsp/src/diagnostics.ts @@ -125,7 +125,7 @@ export function getGraphQLDiagnostics( : texts.join('-') + schema.version ); - let tsDiagnostics; + let tsDiagnostics: ts.Diagnostic[]; if (cache.has(cacheKey)) { tsDiagnostics = cache.get(cacheKey)!; } else { @@ -161,8 +161,9 @@ export function getGraphQLDiagnostics( ref, start, length; - if (callExpression.typeArguments) { - const [typeQuery] = callExpression.typeArguments; + const typeQuery = + callExpression.typeArguments && callExpression.typeArguments[0]; + if (typeQuery) { start = typeQuery.getStart(); length = typeQuery.getEnd() - typeQuery.getStart(); @@ -228,6 +229,7 @@ export function getGraphQLDiagnostics( if ( !initializer || !ts.isCallExpression(initializer) || + !initializer.arguments[0] || !ts.isStringLiteralLike(initializer.arguments[0]) ) { // TODO: we can make this check more stringent where we also parse and resolve @@ -261,7 +263,8 @@ export function getGraphQLDiagnostics( info, initializer.arguments[0], foundFilename, - ts.isArrayLiteralExpression(initializer.arguments[1]) + initializer.arguments[1] && + ts.isArrayLiteralExpression(initializer.arguments[1]) ? initializer.arguments[1] : undefined ); @@ -310,7 +313,7 @@ export function getGraphQLDiagnostics( fragments: fragmentNames, start, length, - } = moduleSpecifierToFragments[moduleSpecifier]; + } = moduleSpecifierToFragments[moduleSpecifier]!; const missingFragments = Array.from( new Set(fragmentNames.filter(x => !usedFragments.has(x))) ); @@ -348,7 +351,7 @@ const runDiagnostics = ( }, schema: SchemaRef, info: ts.server.PluginCreateInfo -) => { +): ts.Diagnostic[] => { const filename = source.fileName; const isCallExpression = info.config.templateIsCallExpression ?? true; @@ -429,10 +432,11 @@ const runDiagnostics = ( if (!diag.message.includes('Unknown directive')) return true; const [message] = diag.message.split('('); - const matches = /Unknown directive "@([^)]+)"/g.exec(message); + const matches = + message && /Unknown directive "@([^)]+)"/g.exec(message); if (!matches) return true; - const directiveNmae = matches[1]; - return !clientDirectives.has(directiveNmae); + const directiveName = matches[1]; + return directiveName && !clientDirectives.has(directiveName); }) .map(x => { const { start, end } = x.range; @@ -440,15 +444,15 @@ const runDiagnostics = ( // We add the start.line to account for newline characters which are // split out let startChar = startingPosition + start.line; - for (let i = 0; i <= start.line; i++) { + for (let i = 0; i <= start.line && i < lines.length; i++) { if (i === start.line) startChar += start.character; - else if (lines[i]) startChar += lines[i].length; + else if (lines[i]) startChar += lines[i]!.length; } let endChar = startingPosition + end.line; - for (let i = 0; i <= end.line; i++) { + for (let i = 0; i <= end.line && i < lines.length; i++) { if (i === end.line) endChar += end.character; - else if (lines[i]) endChar += lines[i].length; + else if (lines[i]) endChar += lines[i]!.length; } const locatedInFragment = resolvedSpans.find(x => { @@ -516,22 +520,25 @@ const runDiagnostics = ( .flat() .filter(Boolean) as Array; - const tsDiagnostics = diagnostics.map(diag => ({ - file: source, - length: diag.length, - start: diag.start, - category: - diag.severity === 2 - ? ts.DiagnosticCategory.Warning - : ts.DiagnosticCategory.Error, - code: - typeof diag.code === 'number' - ? diag.code - : diag.severity === 2 - ? USING_DEPRECATED_FIELD_CODE - : SEMANTIC_DIAGNOSTIC_CODE, - messageText: diag.message.split('\n')[0], - })); + const tsDiagnostics = diagnostics.map( + diag => + ({ + file: source, + length: diag.length, + start: diag.start, + category: + diag.severity === 2 + ? ts.DiagnosticCategory.Warning + : ts.DiagnosticCategory.Error, + code: + typeof diag.code === 'number' + ? diag.code + : diag.severity === 2 + ? USING_DEPRECATED_FIELD_CODE + : SEMANTIC_DIAGNOSTIC_CODE, + messageText: diag.message.split('\n')[0], + } as ts.Diagnostic) + ); if (isCallExpression) { const usageDiagnostics = diff --git a/packages/graphqlsp/src/fieldUsage.ts b/packages/graphqlsp/src/fieldUsage.ts index 5ccd8707..27b32a3f 100644 --- a/packages/graphqlsp/src/fieldUsage.ts +++ b/packages/graphqlsp/src/fieldUsage.ts @@ -223,10 +223,10 @@ const crawlScope = ( const isSomeOrEvery = foundRef.name.text === 'every' || foundRef.name.text === 'some'; const callExpression = foundRef.parent; - let func: ts.Expression | ts.FunctionDeclaration = + let func: ts.Expression | ts.FunctionDeclaration | undefined = callExpression.arguments[0]; - if (ts.isIdentifier(func)) { + if (func && ts.isIdentifier(func)) { // TODO: Scope utilities in checkFieldUsageInFile to deduplicate const checker = info.languageService.getProgram()!.getTypeChecker(); @@ -244,36 +244,39 @@ const crawlScope = ( } if ( - ts.isFunctionDeclaration(func) || - ts.isFunctionExpression(func) || - ts.isArrowFunction(func) + func && + (ts.isFunctionDeclaration(func) || + ts.isFunctionExpression(func) || + ts.isArrowFunction(func)) ) { const param = func.parameters[isReduce ? 1 : 0]; - const res = crawlScope( - param.name, - pathParts, - allFields, - source, - info, - true - ); - - if ( - ts.isVariableDeclaration(callExpression.parent) && - !isSomeOrEvery - ) { - const varRes = crawlScope( - callExpression.parent.name, + if (param) { + const res = crawlScope( + param.name, pathParts, allFields, source, info, true ); - res.push(...varRes); - } - return res; + if ( + ts.isVariableDeclaration(callExpression.parent) && + !isSomeOrEvery + ) { + const varRes = crawlScope( + callExpression.parent.name, + pathParts, + allFields, + source, + info, + true + ); + res.push(...varRes); + } + + return res; + } } } else if ( ts.isPropertyAccessExpression(foundRef) && @@ -505,7 +508,7 @@ export const checkFieldUsageInFile = ( if (loc) { aggregatedUnusedFields.add(parentField); if (unusedChildren[parentField]) { - unusedChildren[parentField].add(unusedField); + unusedChildren[parentField]!.add(unusedField); } else { unusedChildren[parentField] = new Set([unusedField]); } diff --git a/packages/graphqlsp/src/graphql/getFragmentSpreadSuggestions.ts b/packages/graphqlsp/src/graphql/getFragmentSpreadSuggestions.ts index 9445ff4b..6d944369 100644 --- a/packages/graphqlsp/src/graphql/getFragmentSpreadSuggestions.ts +++ b/packages/graphqlsp/src/graphql/getFragmentSpreadSuggestions.ts @@ -120,26 +120,26 @@ function lexicalDistance(a: string, b: string): number { } for (j = 1; j <= bLength; j++) { - d[0][j] = j; + d[0]![j] = j; } for (i = 1; i <= aLength; i++) { for (j = 1; j <= bLength; j++) { const cost = a[i - 1] === b[j - 1] ? 0 : 1; - d[i][j] = Math.min( - d[i - 1][j] + 1, - d[i][j - 1] + 1, - d[i - 1][j - 1] + cost + d[i]![j] = Math.min( + d[i - 1]![j]! + 1, + d[i]![j - 1]! + 1, + d[i - 1]![j - 1]! + cost ); if (i > 1 && j > 1 && a[i - 1] === b[j - 2] && a[i - 2] === b[j - 1]) { - d[i][j] = Math.min(d[i][j], d[i - 2][j - 2] + cost); + d[i]![j] = Math.min(d[i]![j]!, d[i - 2]![j - 2]! + cost); } } } - return d[aLength][bLength]; + return d[aLength]![bLength]!; } export type AllTypeInfo = { diff --git a/packages/graphqlsp/src/persisted.ts b/packages/graphqlsp/src/persisted.ts index a036723c..c9ba706f 100644 --- a/packages/graphqlsp/src/persisted.ts +++ b/packages/graphqlsp/src/persisted.ts @@ -89,7 +89,7 @@ export function getPersistedCodeFixAtPosition( foundFilename = filename; if (callExpression.typeArguments) { const [typeQuery] = callExpression.typeArguments; - if (!ts.isTypeQueryNode(typeQuery)) return undefined; + if (!typeQuery || !ts.isTypeQueryNode(typeQuery)) return undefined; const { node: found, filename: fileName } = getDocumentReferenceFromTypeQuery(typeQuery, filename, info); foundNode = found; @@ -116,15 +116,18 @@ export function getPersistedCodeFixAtPosition( if ( !initializer || !ts.isCallExpression(initializer) || + !initializer.arguments[0] || !ts.isStringLiteralLike(initializer.arguments[0]) - ) + ) { return undefined; + } const hash = generateHashForDocument( info, initializer.arguments[0], foundFilename, - ts.isArrayLiteralExpression(initializer.arguments[1]) + initializer.arguments[1] && + ts.isArrayLiteralExpression(initializer.arguments[1]) ? initializer.arguments[1] : undefined ); @@ -183,7 +186,9 @@ export const generateHashForDocument = ( fragments.forEach(fragmentDefinition => { text = `${text}\n\n${print(fragmentDefinition)}`; }); - return createHash('sha256').update(print(parse(text))).digest('hex'); + return createHash('sha256') + .update(print(parse(text))) + .digest('hex'); } else { const externalSource = getSource(info, foundFilename)!; const { fragments } = findAllCallExpressions(externalSource, info); @@ -229,7 +234,9 @@ export const generateHashForDocument = ( resolvedText = `${resolvedText}\n\n${print(fragmentDefinition)}`; } - return createHash('sha256').update(print(parse(resolvedText))).digest('hex'); + return createHash('sha256') + .update(print(parse(resolvedText))) + .digest('hex'); } }; diff --git a/packages/graphqlsp/tsconfig.json b/packages/graphqlsp/tsconfig.json index dd05e015..1c8853c6 100644 --- a/packages/graphqlsp/tsconfig.json +++ b/packages/graphqlsp/tsconfig.json @@ -8,6 +8,7 @@ "forceConsistentCasingInFileNames": true, "allowJs": true, "strict": true, + "noUncheckedIndexedAccess": true, "skipLibCheck": true } } diff --git a/tsconfig.json b/tsconfig.json index 5ae9a393..7caf0637 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -9,6 +9,7 @@ "forceConsistentCasingInFileNames": true /* Ensure that casing is correct in imports. */, /* Type Checking */ "strict": true /* Enable all strict type-checking options. */, + "noUncheckedIndexedAccess": true, "skipLibCheck": true /* Skip type checking all .d.ts files. */ } }