From b1c4099d3e6107f2eacea3e156514953e22a6777 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 25 Oct 2023 11:16:12 +0300 Subject: [PATCH 1/4] chore(typescript-estree): fixed no-unnecessary-condition complaints --- packages/typescript-estree/src/convert.ts | 6 ++-- .../create-program/createDefaultProgram.ts | 2 +- .../create-program/createProjectProgram.ts | 2 +- .../src/create-program/shared.ts | 4 ++- .../src/create-program/useProvidedPrograms.ts | 2 ++ .../typescript-estree/src/getModifiers.ts | 4 +-- packages/typescript-estree/src/node-utils.ts | 17 +++------- .../src/parseSettings/inferSingleRun.ts | 4 +-- .../src/parseSettings/warnAboutTSVersion.ts | 3 +- packages/typescript-estree/src/parser.ts | 33 +++++++++---------- .../typescript-estree/tests/lib/parse.test.ts | 2 +- .../tests/lib/semanticInfo.test.ts | 4 +-- .../typescript-estree/tools/test-utils.ts | 2 +- 13 files changed, 40 insertions(+), 45 deletions(-) diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts index eb4e7c39ea18..cf22e4d1d9c4 100644 --- a/packages/typescript-estree/src/convert.ts +++ b/packages/typescript-estree/src/convert.ts @@ -1,5 +1,7 @@ // There's lots of funny stuff due to the typing of ts.Node /* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-member-access */ +// TODO: deeply check which no-unnecessary-condition reports are valid +/* eslint-disable @typescript-eslint/no-unnecessary-condition */ import * as ts from 'typescript'; import { getDecorators, getModifiers } from './getModifiers'; @@ -257,7 +259,7 @@ export class Converter { result.range ??= getRange(node, this.ast); result.loc ??= getLocFor(result.range, this.ast); - if (result && this.options.shouldPreserveNodeMaps) { + if (this.options.shouldPreserveNodeMaps) { this.esTreeNodeToTSNodeMap.set(result, node); } return result as T; @@ -728,7 +730,7 @@ export class Converter { if ( node.moduleSpecifier && - node.moduleSpecifier?.kind !== SyntaxKind.StringLiteral + node.moduleSpecifier.kind !== SyntaxKind.StringLiteral ) { this.#throwUnlessAllowInvalidAST( node.moduleSpecifier, diff --git a/packages/typescript-estree/src/create-program/createDefaultProgram.ts b/packages/typescript-estree/src/create-program/createDefaultProgram.ts index 76db0b406f89..98e61d6b48e2 100644 --- a/packages/typescript-estree/src/create-program/createDefaultProgram.ts +++ b/packages/typescript-estree/src/create-program/createDefaultProgram.ts @@ -23,7 +23,7 @@ function createDefaultProgram( parseSettings.filePath || 'unnamed file', ); - if (parseSettings.projects?.length !== 1) { + if (parseSettings.projects.length !== 1) { return undefined; } diff --git a/packages/typescript-estree/src/create-program/createProjectProgram.ts b/packages/typescript-estree/src/create-program/createProjectProgram.ts index edfe00992c19..a58097e3cd73 100644 --- a/packages/typescript-estree/src/create-program/createProjectProgram.ts +++ b/packages/typescript-estree/src/create-program/createProjectProgram.ts @@ -58,7 +58,7 @@ function createProjectProgram( ]; let hasMatchedAnError = false; - const extraFileExtensions = parseSettings.extraFileExtensions || []; + const { extraFileExtensions } = parseSettings; extraFileExtensions.forEach(extraExtension => { if (!extraExtension.startsWith('.')) { diff --git a/packages/typescript-estree/src/create-program/shared.ts b/packages/typescript-estree/src/create-program/shared.ts index 8966093372ed..378250291641 100644 --- a/packages/typescript-estree/src/create-program/shared.ts +++ b/packages/typescript-estree/src/create-program/shared.ts @@ -53,8 +53,9 @@ function createDefaultCompilerOptionsFromExtra( // This narrows the type so we can be sure we're passing canonical names in the correct places type CanonicalPath = string & { __brand: unknown }; -// typescript doesn't provide a ts.sys implementation for browser environments const useCaseSensitiveFileNames = + // typescript doesn't provide a ts.sys implementation for browser environments + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition ts.sys !== undefined ? ts.sys.useCaseSensitiveFileNames : true; const correctPathCasing = useCaseSensitiveFileNames ? (filePath: string): string => filePath @@ -118,6 +119,7 @@ function getAstFromProgram( */ function createHash(content: string): string { // No ts.sys in browser environments. + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition if (ts.sys?.createHash) { return ts.sys.createHash(content); } diff --git a/packages/typescript-estree/src/create-program/useProvidedPrograms.ts b/packages/typescript-estree/src/create-program/useProvidedPrograms.ts index c2b67e795750..7a3761baf675 100644 --- a/packages/typescript-estree/src/create-program/useProvidedPrograms.ts +++ b/packages/typescript-estree/src/create-program/useProvidedPrograms.ts @@ -56,6 +56,8 @@ function createProgramFromConfigFile( configFile: string, projectDirectory?: string, ): ts.Program { + // No ts.sys in browser environments. + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition if (ts.sys === undefined) { throw new Error( '`createProgramFromConfigFile` is only supported in a Node-like environment.', diff --git a/packages/typescript-estree/src/getModifiers.ts b/packages/typescript-estree/src/getModifiers.ts index 0ccb3026a901..42609a518c58 100644 --- a/packages/typescript-estree/src/getModifiers.ts +++ b/packages/typescript-estree/src/getModifiers.ts @@ -25,7 +25,7 @@ export function getModifiers( return ( // @ts-expect-error intentional fallback for older TS versions - (node.modifiers as ts.Modifier[])?.filter( + (node.modifiers as ts.Modifier[] | undefined)?.filter( (m): m is ts.Modifier => !ts.isDecorator(m), ) ); @@ -52,6 +52,6 @@ export function getDecorators( return ( // @ts-expect-error intentional fallback for older TS versions - (node.decorators as ts.Node[])?.filter(ts.isDecorator) + (node.decorators as ts.Node[] | undefined)?.filter(ts.isDecorator) ); } diff --git a/packages/typescript-estree/src/node-utils.ts b/packages/typescript-estree/src/node-utils.ts index 8c16b24cee41..fca84aa83373 100644 --- a/packages/typescript-estree/src/node-utils.ts +++ b/packages/typescript-estree/src/node-utils.ts @@ -424,7 +424,7 @@ export function findFirstMatchingAncestor( node: ts.Node, predicate: (node: ts.Node) => boolean, ): ts.Node | undefined { - while (node) { + while (node as ts.Node | undefined) { if (predicate(node)) { return node; } @@ -482,9 +482,7 @@ export function isComputedProperty( export function isOptional(node: { questionToken?: ts.QuestionToken; }): boolean { - return node.questionToken - ? node.questionToken.kind === SyntaxKind.QuestionToken - : false; + return !!node.questionToken; } /** @@ -677,11 +675,7 @@ export function convertTokens(ast: ts.SourceFile): TSESTree.Token[] { } if (isToken(node) && node.kind !== SyntaxKind.EndOfFileToken) { - const converted = convertToken(node, ast); - - if (converted) { - result.push(converted); - } + result.push(convertToken(node, ast)); } else { node.getChildren(ast).forEach(walk); } @@ -943,11 +937,10 @@ export function getNamespaceModifiers( let moduleDeclaration = node; while ( (!modifiers || modifiers.length === 0) && - ts.isModuleDeclaration(moduleDeclaration.parent) && - moduleDeclaration.parent.name + ts.isModuleDeclaration(moduleDeclaration.parent) ) { const parentModifiers = getModifiers(moduleDeclaration.parent); - if (parentModifiers && parentModifiers?.length > 0) { + if (parentModifiers?.length) { modifiers = parentModifiers; } moduleDeclaration = moduleDeclaration.parent; diff --git a/packages/typescript-estree/src/parseSettings/inferSingleRun.ts b/packages/typescript-estree/src/parseSettings/inferSingleRun.ts index 5d765a22ce95..e8a9b020ce86 100644 --- a/packages/typescript-estree/src/parseSettings/inferSingleRun.ts +++ b/packages/typescript-estree/src/parseSettings/inferSingleRun.ts @@ -20,7 +20,7 @@ export function inferSingleRun(options: TSESTreeOptions | undefined): boolean { options?.project == null || // programs passed via options means the user should be managing the programs, so we shouldn't // be creating our own single-run programs accidentally - options?.programs != null + options.programs != null ) { return false; } @@ -34,7 +34,7 @@ export function inferSingleRun(options: TSESTreeOptions | undefined): boolean { } // Currently behind a flag while we gather real-world feedback - if (options?.allowAutomaticSingleRunInference) { + if (options.allowAutomaticSingleRunInference) { if ( // Default to single runs for CI processes. CI=true is set by most CI providers by default. process.env.CI === 'true' || diff --git a/packages/typescript-estree/src/parseSettings/warnAboutTSVersion.ts b/packages/typescript-estree/src/parseSettings/warnAboutTSVersion.ts index a92c690ee6d0..032a5bff6ad3 100644 --- a/packages/typescript-estree/src/parseSettings/warnAboutTSVersion.ts +++ b/packages/typescript-estree/src/parseSettings/warnAboutTSVersion.ts @@ -25,8 +25,7 @@ let warnedAboutTSVersion = false; export function warnAboutTSVersion(parseSettings: ParseSettings): void { if (!isRunningSupportedTypeScriptVersion && !warnedAboutTSVersion) { - const isTTY = - typeof process === 'undefined' ? false : process.stdout?.isTTY; + const isTTY = typeof process === 'undefined' ? false : process.stdout.isTTY; if (isTTY) { const border = '============='; const versionWarning = [ diff --git a/packages/typescript-estree/src/parser.ts b/packages/typescript-estree/src/parser.ts index 887e54e0e1cc..aeef8d1e45ff 100644 --- a/packages/typescript-estree/src/parser.ts +++ b/packages/typescript-estree/src/parser.ts @@ -181,7 +181,7 @@ function parseAndGenerateServices( if ( parseSettings.singleRun && !parseSettings.programs && - parseSettings.projects?.length > 0 + parseSettings.projects.length > 0 ) { parseSettings.programs = { *[Symbol.iterator](): Iterator { @@ -207,25 +207,22 @@ function parseAndGenerateServices( * Generate a full ts.Program or offer provided instances in order to be able to provide parser services, such as type-checking */ const hasFullTypeInformation = - parseSettings.programs != null || parseSettings.projects?.length > 0; + parseSettings.programs != null || parseSettings.projects.length > 0; - if (options !== undefined) { - if ( - typeof options.errorOnTypeScriptSyntacticAndSemanticIssues === - 'boolean' && - options.errorOnTypeScriptSyntacticAndSemanticIssues - ) { - parseSettings.errorOnTypeScriptSyntacticAndSemanticIssues = true; - } + if ( + typeof options.errorOnTypeScriptSyntacticAndSemanticIssues === 'boolean' && + options.errorOnTypeScriptSyntacticAndSemanticIssues + ) { + parseSettings.errorOnTypeScriptSyntacticAndSemanticIssues = true; + } - if ( - parseSettings.errorOnTypeScriptSyntacticAndSemanticIssues && - !hasFullTypeInformation - ) { - throw new Error( - 'Cannot calculate TypeScript semantic issues without a valid project.', - ); - } + if ( + parseSettings.errorOnTypeScriptSyntacticAndSemanticIssues && + !hasFullTypeInformation + ) { + throw new Error( + 'Cannot calculate TypeScript semantic issues without a valid project.', + ); } /** diff --git a/packages/typescript-estree/tests/lib/parse.test.ts b/packages/typescript-estree/tests/lib/parse.test.ts index 1b39713f1557..95ef77de154e 100644 --- a/packages/typescript-estree/tests/lib/parse.test.ts +++ b/packages/typescript-estree/tests/lib/parse.test.ts @@ -135,7 +135,7 @@ describe('parseAndGenerateServices', () => { }); expect( - parseResult.services.esTreeNodeToTSNodeMap?.has( + parseResult.services.esTreeNodeToTSNodeMap.has( parseResult.ast.body[0], ), ).toBe(setting); diff --git a/packages/typescript-estree/tests/lib/semanticInfo.test.ts b/packages/typescript-estree/tests/lib/semanticInfo.test.ts index 83569479085c..8502e70180c4 100644 --- a/packages/typescript-estree/tests/lib/semanticInfo.test.ts +++ b/packages/typescript-estree/tests/lib/semanticInfo.test.ts @@ -228,11 +228,11 @@ describe('semanticInfo', () => { expect(boundName.name).toBe('x'); const tsBoundName = - parseResult.services.esTreeNodeToTSNodeMap?.get(boundName); + parseResult.services.esTreeNodeToTSNodeMap.get(boundName); expectToBeDefined(tsBoundName); expect(tsBoundName).toBeDefined(); - expect(parseResult.services.tsNodeToESTreeNodeMap?.get(tsBoundName)).toBe( + expect(parseResult.services.tsNodeToESTreeNodeMap.get(tsBoundName)).toBe( boundName, ); }); diff --git a/packages/typescript-estree/tools/test-utils.ts b/packages/typescript-estree/tools/test-utils.ts index 81121274b254..3d83fe26e9e8 100644 --- a/packages/typescript-estree/tools/test-utils.ts +++ b/packages/typescript-estree/tools/test-utils.ts @@ -106,7 +106,7 @@ export function omitDeep( > = {}, ): UnknownObject { function shouldOmit(keyName: string, val: unknown): boolean { - if (keysToOmit?.length) { + if (keysToOmit.length) { return keysToOmit.some( keyConfig => keyConfig.key === keyName && keyConfig.predicate(val), ); From c7775df50a8483e6678a261178e6832ac3b8e0c6 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 25 Oct 2023 15:00:28 +0300 Subject: [PATCH 2/4] Removed now-unnecessary disables --- packages/typescript-estree/src/convert.ts | 6 ++---- packages/typescript-estree/src/create-program/shared.ts | 4 +--- .../src/create-program/useProvidedPrograms.ts | 2 -- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts index cf22e4d1d9c4..eb4e7c39ea18 100644 --- a/packages/typescript-estree/src/convert.ts +++ b/packages/typescript-estree/src/convert.ts @@ -1,7 +1,5 @@ // There's lots of funny stuff due to the typing of ts.Node /* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-call, @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-member-access */ -// TODO: deeply check which no-unnecessary-condition reports are valid -/* eslint-disable @typescript-eslint/no-unnecessary-condition */ import * as ts from 'typescript'; import { getDecorators, getModifiers } from './getModifiers'; @@ -259,7 +257,7 @@ export class Converter { result.range ??= getRange(node, this.ast); result.loc ??= getLocFor(result.range, this.ast); - if (this.options.shouldPreserveNodeMaps) { + if (result && this.options.shouldPreserveNodeMaps) { this.esTreeNodeToTSNodeMap.set(result, node); } return result as T; @@ -730,7 +728,7 @@ export class Converter { if ( node.moduleSpecifier && - node.moduleSpecifier.kind !== SyntaxKind.StringLiteral + node.moduleSpecifier?.kind !== SyntaxKind.StringLiteral ) { this.#throwUnlessAllowInvalidAST( node.moduleSpecifier, diff --git a/packages/typescript-estree/src/create-program/shared.ts b/packages/typescript-estree/src/create-program/shared.ts index 378250291641..8966093372ed 100644 --- a/packages/typescript-estree/src/create-program/shared.ts +++ b/packages/typescript-estree/src/create-program/shared.ts @@ -53,9 +53,8 @@ function createDefaultCompilerOptionsFromExtra( // This narrows the type so we can be sure we're passing canonical names in the correct places type CanonicalPath = string & { __brand: unknown }; +// typescript doesn't provide a ts.sys implementation for browser environments const useCaseSensitiveFileNames = - // typescript doesn't provide a ts.sys implementation for browser environments - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition ts.sys !== undefined ? ts.sys.useCaseSensitiveFileNames : true; const correctPathCasing = useCaseSensitiveFileNames ? (filePath: string): string => filePath @@ -119,7 +118,6 @@ function getAstFromProgram( */ function createHash(content: string): string { // No ts.sys in browser environments. - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition if (ts.sys?.createHash) { return ts.sys.createHash(content); } diff --git a/packages/typescript-estree/src/create-program/useProvidedPrograms.ts b/packages/typescript-estree/src/create-program/useProvidedPrograms.ts index 7a3761baf675..c2b67e795750 100644 --- a/packages/typescript-estree/src/create-program/useProvidedPrograms.ts +++ b/packages/typescript-estree/src/create-program/useProvidedPrograms.ts @@ -56,8 +56,6 @@ function createProgramFromConfigFile( configFile: string, projectDirectory?: string, ): ts.Program { - // No ts.sys in browser environments. - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition if (ts.sys === undefined) { throw new Error( '`createProgramFromConfigFile` is only supported in a Node-like environment.', From 1675163919f26f13332571b55fb278ed76fa7f69 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sat, 11 Nov 2023 16:00:47 -0500 Subject: [PATCH 3/4] ease up on process.stdout --- .../typescript-estree/src/parseSettings/warnAboutTSVersion.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/typescript-estree/src/parseSettings/warnAboutTSVersion.ts b/packages/typescript-estree/src/parseSettings/warnAboutTSVersion.ts index 032a5bff6ad3..a92c690ee6d0 100644 --- a/packages/typescript-estree/src/parseSettings/warnAboutTSVersion.ts +++ b/packages/typescript-estree/src/parseSettings/warnAboutTSVersion.ts @@ -25,7 +25,8 @@ let warnedAboutTSVersion = false; export function warnAboutTSVersion(parseSettings: ParseSettings): void { if (!isRunningSupportedTypeScriptVersion && !warnedAboutTSVersion) { - const isTTY = typeof process === 'undefined' ? false : process.stdout.isTTY; + const isTTY = + typeof process === 'undefined' ? false : process.stdout?.isTTY; if (isTTY) { const border = '============='; const versionWarning = [ From 11884da01b553c09abf7aee02dda3344e4d55d1e Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 12 Nov 2023 18:36:46 -0500 Subject: [PATCH 4/4] replace node with a current --- packages/typescript-estree/src/node-utils.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/typescript-estree/src/node-utils.ts b/packages/typescript-estree/src/node-utils.ts index fca84aa83373..0a382248c369 100644 --- a/packages/typescript-estree/src/node-utils.ts +++ b/packages/typescript-estree/src/node-utils.ts @@ -424,11 +424,12 @@ export function findFirstMatchingAncestor( node: ts.Node, predicate: (node: ts.Node) => boolean, ): ts.Node | undefined { - while (node as ts.Node | undefined) { - if (predicate(node)) { - return node; + let current: ts.Node | undefined = node; + while (current) { + if (predicate(current)) { + return current; } - node = node.parent; + current = current.parent; } return undefined; }