From 1bdd39afd687cfb1586f1175f84be51979cc9bf8 Mon Sep 17 00:00:00 2001 From: Louis Cruz Date: Sun, 26 Mar 2023 18:01:43 -0700 Subject: [PATCH] Address all unchecked indexed access within source code --- src/execution/execute.ts | 32 +++++++++++------ src/jsutils/formatList.ts | 7 ++-- src/jsutils/mapValue.ts | 10 +++--- src/jsutils/promiseForObject.ts | 7 +++- src/jsutils/suggestionList.ts | 35 ++++++++++++++---- src/language/blockString.ts | 7 ++-- src/language/printLocation.ts | 36 ++++++++++++++----- src/language/printString.ts | 12 ++++++- src/language/visitor.ts | 6 ++-- src/utilities/lexicographicSortSchema.ts | 7 ++-- ...ferStreamDirectiveOnValidOperationsRule.ts | 4 +++ src/validation/rules/KnownDirectivesRule.ts | 2 ++ .../rules/OverlappingFieldsCanBeMergedRule.ts | 32 ++++++++++++----- .../rules/SingleFieldSubscriptionsRule.ts | 4 +-- .../rules/UniqueInputFieldNamesRule.ts | 5 +-- .../rules/custom/NoDeprecatedCustomRule.ts | 5 +-- tsconfig.json | 2 +- 17 files changed, 153 insertions(+), 60 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 1bc6c4267bb..e24f3a70cd2 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -688,7 +688,12 @@ function executeField( asyncPayloadRecord?: AsyncPayloadRecord, ): PromiseOrValue { const errors = asyncPayloadRecord?.errors ?? exeContext.errors; - const fieldName = fieldNodes[0].name.value; + const firstFieldNode = fieldNodes[0]; + invariant(firstFieldNode !== undefined); + + const fieldName = fieldNodes[0]?.name.value; + invariant(fieldName !== undefined); + const fieldDef = exeContext.schema.getField(parentType, fieldName); if (!fieldDef) { return; @@ -712,7 +717,7 @@ function executeField( // TODO: find a way to memoize, in case this field is within a List type. const args = getArgumentValues( fieldDef, - fieldNodes[0], + firstFieldNode, exeContext.variableValues, ); @@ -974,11 +979,14 @@ function getStreamValues( return; } + const firstFieldNode = fieldNodes[0]; + invariant(firstFieldNode !== undefined); + // validation only allows equivalent streams on multiple fields, so it is // safe to only check the first fieldNode for the stream directive const stream = getDirectiveValues( GraphQLStreamDirective, - fieldNodes[0], + firstFieldNode, exeContext.variableValues, ); @@ -1497,16 +1505,19 @@ export const defaultTypeResolver: GraphQLTypeResolver = // Otherwise, test each possible type. const possibleTypes = info.schema.getPossibleTypes(abstractType); - const promisedIsTypeOfResults = []; + const promisedIsTypeOfResults: Array> = []; - for (let i = 0; i < possibleTypes.length; i++) { - const type = possibleTypes[i]; + let type: GraphQLObjectType; + for (type of possibleTypes) { if (type.isTypeOf) { const isTypeOfResult = type.isTypeOf(value, contextValue, info); if (isPromise(isTypeOfResult)) { - promisedIsTypeOfResults[i] = isTypeOfResult; + const possibleTypeName = type.name; + promisedIsTypeOfResults.push( + isTypeOfResult.then((result) => [possibleTypeName, result]), + ); } else if (isTypeOfResult) { return type.name; } @@ -1514,10 +1525,11 @@ export const defaultTypeResolver: GraphQLTypeResolver = } if (promisedIsTypeOfResults.length) { + // QUESTION: Can this be faster if Promise.any or Promise.race is used instead? return Promise.all(promisedIsTypeOfResults).then((isTypeOfResults) => { - for (let i = 0; i < isTypeOfResults.length; i++) { - if (isTypeOfResults[i]) { - return possibleTypes[i].name; + for (const [name, result] of isTypeOfResults) { + if (result) { + return name; } } }); diff --git a/src/jsutils/formatList.ts b/src/jsutils/formatList.ts index b00dd6591a2..97e6a094a01 100644 --- a/src/jsutils/formatList.ts +++ b/src/jsutils/formatList.ts @@ -15,13 +15,14 @@ export function andList(items: ReadonlyArray): string { } function formatList(conjunction: string, items: ReadonlyArray): string { - invariant(items.length !== 0); + const firstItem = items[0]; + invariant(firstItem !== undefined); switch (items.length) { case 1: - return items[0]; + return firstItem; case 2: - return items[0] + ' ' + conjunction + ' ' + items[1]; + return firstItem + ' ' + conjunction + ' ' + items[1]; } const allButLast = items.slice(0, -1); diff --git a/src/jsutils/mapValue.ts b/src/jsutils/mapValue.ts index 8f1299586e2..46edc79d16b 100644 --- a/src/jsutils/mapValue.ts +++ b/src/jsutils/mapValue.ts @@ -8,10 +8,8 @@ export function mapValue( map: ReadOnlyObjMap, fn: (value: T, key: string) => V, ): ObjMap { - const result = Object.create(null); - - for (const key of Object.keys(map)) { - result[key] = fn(map[key], key); - } - return result; + return Object.entries(map).reduce((accumulator, [key, value]) => { + accumulator[key] = fn(value, key); + return accumulator; + }, Object.create(null)); } diff --git a/src/jsutils/promiseForObject.ts b/src/jsutils/promiseForObject.ts index ff48d9f2180..b7314d16d86 100644 --- a/src/jsutils/promiseForObject.ts +++ b/src/jsutils/promiseForObject.ts @@ -1,3 +1,4 @@ +import { invariant } from './invariant.js'; import type { ObjMap } from './ObjMap.js'; /** @@ -15,8 +16,12 @@ export async function promiseForObject( const resolvedValues = await Promise.all(values); const resolvedObject = Object.create(null); + for (let i = 0; i < keys.length; ++i) { - resolvedObject[keys[i]] = resolvedValues[i]; + const key = keys[i]; + invariant(key !== undefined); + + resolvedObject[key] = resolvedValues[i]; } return resolvedObject; } diff --git a/src/jsutils/suggestionList.ts b/src/jsutils/suggestionList.ts index 4114ddad398..8ac8ad66725 100644 --- a/src/jsutils/suggestionList.ts +++ b/src/jsutils/suggestionList.ts @@ -1,3 +1,4 @@ +import { invariant } from './invariant.js'; import { naturalCompare } from './naturalCompare.js'; /** @@ -93,19 +94,34 @@ class LexicalDistance { const upRow = rows[(i - 1) % 3]; const currentRow = rows[i % 3]; + invariant(upRow !== undefined); + invariant(currentRow !== undefined); + let smallestCell = (currentRow[0] = i); for (let j = 1; j <= bLength; j++) { const cost = a[i - 1] === b[j - 1] ? 0 : 1; + const deleteTarget = upRow[j]; + const currentRowTarget = currentRow[j - 1]; + const substituteTarget = upRow[j - 1]; + + invariant(deleteTarget !== undefined); + invariant(currentRowTarget !== undefined); + invariant(substituteTarget !== undefined); + let currentCell = Math.min( - upRow[j] + 1, // delete - currentRow[j - 1] + 1, // insert - upRow[j - 1] + cost, // substitute + deleteTarget + 1, // delete + currentRowTarget + 1, // insert + substituteTarget + cost, // substitute ); if (i > 1 && j > 1 && a[i - 1] === b[j - 2] && a[i - 2] === b[j - 1]) { // transposition - const doubleDiagonalCell = rows[(i - 2) % 3][j - 2]; + const targetedRow = rows[(i - 2) % 3]; + invariant(targetedRow !== undefined); + const doubleDiagonalCell = targetedRow[j - 2]; + invariant(doubleDiagonalCell !== undefined); + currentCell = Math.min(currentCell, doubleDiagonalCell + 1); } @@ -122,8 +138,15 @@ class LexicalDistance { } } - const distance = rows[aLength % 3][bLength]; - return distance <= threshold ? distance : undefined; + const targetedRow = rows[aLength % 3]; + + invariant(targetedRow !== undefined); + + const distance = targetedRow[bLength]; + + return distance !== undefined && distance <= threshold + ? distance + : undefined; } } diff --git a/src/language/blockString.ts b/src/language/blockString.ts index 3d7915b7813..7cb046d4bcd 100644 --- a/src/language/blockString.ts +++ b/src/language/blockString.ts @@ -15,12 +15,11 @@ export function dedentBlockStringLines( let firstNonEmptyLine = null; let lastNonEmptyLine = -1; - for (let i = 0; i < lines.length; ++i) { - const line = lines[i]; + lines.forEach((line, i) => { const indent = leadingWhitespace(line); if (indent === line.length) { - continue; // skip empty lines + return; // skip empty lines } firstNonEmptyLine ??= i; @@ -29,7 +28,7 @@ export function dedentBlockStringLines( if (i !== 0 && indent < commonIndent) { commonIndent = indent; } - } + }); return ( lines diff --git a/src/language/printLocation.ts b/src/language/printLocation.ts index b6f9c784011..578f32b142f 100644 --- a/src/language/printLocation.ts +++ b/src/language/printLocation.ts @@ -1,3 +1,5 @@ +import { invariant } from '../jsutils/invariant.js'; + import type { Location } from './ast.js'; import type { SourceLocation } from './location.js'; import { getLocation } from './location.js'; @@ -35,7 +37,7 @@ export function printSourceLocation( const locationLine = lines[lineIndex]; // Special case for minified documents - if (locationLine.length > 120) { + if (locationLine !== undefined && locationLine.length > 120) { const subLineIndex = Math.floor(columnNum / 80); const subLineColumnNum = columnNum % 80; const subLines: Array = []; @@ -43,27 +45,45 @@ export function printSourceLocation( subLines.push(locationLine.slice(i, i + 80)); } + const firstSubLine = subLines[0]; + const nextSubLines = subLines.slice(1, subLineIndex + 1); + const nextSubLine = subLines[subLineIndex + 1]; + + invariant(firstSubLine !== undefined); + // invariant(nextSubLine !== undefined); + return ( locationStr + printPrefixedLines([ - [`${lineNum} |`, subLines[0]], - ...subLines - .slice(1, subLineIndex + 1) - .map((subLine) => ['|', subLine] as const), + [`${lineNum} |`, firstSubLine], + ...nextSubLines.map<[string, string]>((subLine) => ['|', subLine]), ['|', '^'.padStart(subLineColumnNum)], - ['|', subLines[subLineIndex + 1]], + // TODO: This assertion can be removed if the above invariant is comment in. + ['|', nextSubLine as string], ]) ); } + const previousLine = lines[lineIndex - 1]; + const nextLine = lines[lineIndex + 1]; + + // TODO: With the way the types are set up, we should be able to + // comment these in, but doing so breaks tests. + // + // invariant(previousLine !== undefined); + // invariant(nextLine !== undefined); + invariant(locationLine !== undefined); + return ( locationStr + printPrefixedLines([ // Lines specified like this: ["prefix", "string"], - [`${lineNum - 1} |`, lines[lineIndex - 1]], + // TODO: This assertion can be removed if the above invariant is comment in. + [`${lineNum - 1} |`, previousLine as string], [`${lineNum} |`, locationLine], ['|', '^'.padStart(columnNum)], - [`${lineNum + 1} |`, lines[lineIndex + 1]], + // TODO: This assertion can be removed if the above invariant is comment in. + [`${lineNum + 1} |`, nextLine as string], ]) ); } diff --git a/src/language/printString.ts b/src/language/printString.ts index b091bcc2c13..c5019c4198b 100644 --- a/src/language/printString.ts +++ b/src/language/printString.ts @@ -1,3 +1,5 @@ +import { invariant } from '../jsutils/invariant.js'; + /** * Prints a string as a GraphQL StringValue literal. Replaces control characters * and excluded characters (" U+0022 and \\ U+005C) with escape sequences. @@ -10,7 +12,15 @@ export function printString(str: string): string { const escapedRegExp = /[\x00-\x1f\x22\x5c\x7f-\x9f]/g; function escapedReplacer(str: string): string { - return escapeSequences[str.charCodeAt(0)]; + const firstCharacter = str.charCodeAt(0); + + invariant(firstCharacter !== undefined); + + const replacer = escapeSequences[firstCharacter]; + + invariant(replacer !== undefined); + + return replacer; } // prettier-ignore diff --git a/src/language/visitor.ts b/src/language/visitor.ts index 5639fbceed9..a8a26d24281 100644 --- a/src/language/visitor.ts +++ b/src/language/visitor.ts @@ -319,12 +319,12 @@ export function visitInParallel( const enterList = new Array(visitors.length).fill(undefined); const leaveList = new Array(visitors.length).fill(undefined); - for (let i = 0; i < visitors.length; ++i) { - const { enter, leave } = getEnterLeaveForKind(visitors[i], kind); + visitors.forEach((visitor, i) => { + const { enter, leave } = getEnterLeaveForKind(visitor, kind); hasVisitor ||= enter != null || leave != null; enterList[i] = enter; leaveList[i] = leave; - } + }); if (!hasVisitor) { continue; diff --git a/src/utilities/lexicographicSortSchema.ts b/src/utilities/lexicographicSortSchema.ts index 4675185a4e7..4cba83601d8 100644 --- a/src/utilities/lexicographicSortSchema.ts +++ b/src/utilities/lexicographicSortSchema.ts @@ -166,8 +166,11 @@ function sortObjMap( sortValueFn: (value: T) => R, ): ObjMap { const sortedMap = Object.create(null); - for (const key of Object.keys(map).sort(naturalCompare)) { - sortedMap[key] = sortValueFn(map[key]); + + for (const [key, value] of Object.entries(map).sort( + ([firstKey], [secondKey]) => naturalCompare(firstKey, secondKey), + )) { + sortedMap[key] = sortValueFn(value); } return sortedMap; } diff --git a/src/validation/rules/DeferStreamDirectiveOnValidOperationsRule.ts b/src/validation/rules/DeferStreamDirectiveOnValidOperationsRule.ts index e8e6a292b63..0b91f7c07b7 100644 --- a/src/validation/rules/DeferStreamDirectiveOnValidOperationsRule.ts +++ b/src/validation/rules/DeferStreamDirectiveOnValidOperationsRule.ts @@ -1,3 +1,5 @@ +import { invariant } from '../../jsutils/invariant.js'; + import { GraphQLError } from '../../error/GraphQLError.js'; import type { DirectiveNode } from '../../language/ast.js'; @@ -50,6 +52,8 @@ export function DeferStreamDirectiveOnValidOperationsRule( Directive(node, _key, _parent, _path, ancestors) { const definitionNode = ancestors[2]; + invariant(definitionNode !== undefined); + if ( 'kind' in definitionNode && ((definitionNode.kind === Kind.FRAGMENT_DEFINITION && diff --git a/src/validation/rules/KnownDirectivesRule.ts b/src/validation/rules/KnownDirectivesRule.ts index 57641b91e74..e54d07c719c 100644 --- a/src/validation/rules/KnownDirectivesRule.ts +++ b/src/validation/rules/KnownDirectivesRule.ts @@ -73,6 +73,7 @@ function getDirectiveLocationForASTPath( ancestors: ReadonlyArray>, ): DirectiveLocation | undefined { const appliedTo = ancestors[ancestors.length - 1]; + invariant(appliedTo !== undefined); invariant('kind' in appliedTo); switch (appliedTo.kind) { @@ -115,6 +116,7 @@ function getDirectiveLocationForASTPath( return DirectiveLocation.INPUT_OBJECT; case Kind.INPUT_VALUE_DEFINITION: { const parentNode = ancestors[ancestors.length - 3]; + invariant(parentNode !== undefined); invariant('kind' in parentNode); return parentNode.kind === Kind.INPUT_OBJECT_TYPE_DEFINITION ? DirectiveLocation.INPUT_FIELD_DEFINITION diff --git a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts index e2444047c64..39487528ec8 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts +++ b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts @@ -1,4 +1,5 @@ import { inspect } from '../../jsutils/inspect.js'; +import { invariant } from '../../jsutils/invariant.js'; import type { Maybe } from '../../jsutils/Maybe.js'; import type { ObjMap } from '../../jsutils/ObjMap.js'; @@ -199,6 +200,9 @@ function findConflictsWithinSelectionSet( // (B) Then collect conflicts between these fields and those represented by // each spread fragment name found. for (let i = 0; i < fragmentNames.length; i++) { + const firstFragment = fragmentNames[i]; + invariant(firstFragment !== undefined); + collectConflictsBetweenFieldsAndFragment( context, conflicts, @@ -206,21 +210,25 @@ function findConflictsWithinSelectionSet( comparedFragmentPairs, false, fieldMap, - fragmentNames[i], + firstFragment, ); // (C) Then compare this fragment with all other fragments found in this // selection set to collect conflicts between fragments spread together. // This compares each item in the list of fragment names to every other // item in that same list (except for itself). + for (let j = i + 1; j < fragmentNames.length; j++) { + const secondFragment = fragmentNames[j]; + invariant(secondFragment !== undefined); + collectConflictsBetweenFragments( context, conflicts, cachedFieldsAndFragmentNames, comparedFragmentPairs, false, - fragmentNames[i], - fragmentNames[j], + firstFragment, + secondFragment, ); } } @@ -491,15 +499,21 @@ function collectConflictsWithin( // be compared. if (fields.length > 1) { for (let i = 0; i < fields.length; i++) { + const firstField = fields[i]; + invariant(firstField !== undefined); + for (let j = i + 1; j < fields.length; j++) { + const secondField = fields[j]; + invariant(secondField !== undefined); + const conflict = findConflict( context, cachedFieldsAndFragmentNames, comparedFragmentPairs, false, // within one collection is never mutually exclusive responseName, - fields[i], - fields[j], + firstField, + secondField, ); if (conflict) { conflicts.push(conflict); @@ -784,10 +798,10 @@ function _collectFieldsAndFragmentNames( const responseName = selection.alias ? selection.alias.value : fieldName; - if (!nodeAndDefs[responseName]) { - nodeAndDefs[responseName] = []; - } - nodeAndDefs[responseName].push([parentType, selection, fieldDef]); + + const nodeAndDefsTarget = nodeAndDefs[responseName] ?? []; + nodeAndDefs[responseName] = nodeAndDefsTarget; + nodeAndDefsTarget.push([parentType, selection, fieldDef]); break; } case Kind.FRAGMENT_SPREAD: diff --git a/src/validation/rules/SingleFieldSubscriptionsRule.ts b/src/validation/rules/SingleFieldSubscriptionsRule.ts index 4a3d8341240..03e8e46a968 100644 --- a/src/validation/rules/SingleFieldSubscriptionsRule.ts +++ b/src/validation/rules/SingleFieldSubscriptionsRule.ts @@ -62,8 +62,8 @@ export function SingleFieldSubscriptionsRule( ); } for (const fieldNodes of fields.values()) { - const fieldName = fieldNodes[0].name.value; - if (fieldName.startsWith('__')) { + const fieldName = fieldNodes[0]?.name.value; + if (fieldName?.startsWith('__')) { context.reportError( new GraphQLError( operationName != null diff --git a/src/validation/rules/UniqueInputFieldNamesRule.ts b/src/validation/rules/UniqueInputFieldNamesRule.ts index 7344e9439ba..6c18f122d4b 100644 --- a/src/validation/rules/UniqueInputFieldNamesRule.ts +++ b/src/validation/rules/UniqueInputFieldNamesRule.ts @@ -36,11 +36,12 @@ export function UniqueInputFieldNamesRule( }, ObjectField(node) { const fieldName = node.name.value; - if (knownNames[fieldName]) { + const target = knownNames[fieldName]; + if (target) { context.reportError( new GraphQLError( `There can be only one input field named "${fieldName}".`, - { nodes: [knownNames[fieldName], node.name] }, + { nodes: [target, node.name] }, ), ); } else { diff --git a/src/validation/rules/custom/NoDeprecatedCustomRule.ts b/src/validation/rules/custom/NoDeprecatedCustomRule.ts index 375373eb1d9..f11db26b0f8 100644 --- a/src/validation/rules/custom/NoDeprecatedCustomRule.ts +++ b/src/validation/rules/custom/NoDeprecatedCustomRule.ts @@ -64,10 +64,11 @@ export function NoDeprecatedCustomRule(context: ValidationContext): ASTVisitor { if (isInputObjectType(inputObjectDef)) { const inputFieldDef = inputObjectDef.getFields()[node.name.value]; const deprecationReason = inputFieldDef?.deprecationReason; - if (deprecationReason != null) { + if (inputFieldDef && deprecationReason != null) { + const { name } = inputFieldDef; context.reportError( new GraphQLError( - `The input field ${inputObjectDef.name}.${inputFieldDef.name} is deprecated. ${deprecationReason}`, + `The input field ${inputObjectDef.name}.${name} is deprecated. ${deprecationReason}`, { nodes: node }, ), ); diff --git a/tsconfig.json b/tsconfig.json index 51fd80f4e0c..8e1cae9c402 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -30,7 +30,7 @@ "noImplicitOverride": true, "noImplicitReturns": false, // TODO consider "noPropertyAccessFromIndexSignature": false, // TODO consider - "noUncheckedIndexedAccess": false, // FIXME + "noUncheckedIndexedAccess": true, // FIXME "noUnusedLocals": true, "noUnusedParameters": true, "allowSyntheticDefaultImports": true