diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts index 3523fd4a477e..6b69d9e381d4 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts @@ -1,3 +1,4 @@ +import type { Scope } from '@typescript-eslint/scope-manager'; import type { TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils'; import * as tsutils from 'ts-api-utils'; @@ -80,33 +81,66 @@ export default createRule({ ) && // ignore class properties as they are compile time guarded // also ignore function arguments as they can't be used before defined - ts.isVariableDeclaration(declaration) && - // is it `const x!: number` - declaration.initializer === undefined && - declaration.exclamationToken === undefined && - declaration.type !== undefined + ts.isVariableDeclaration(declaration) ) { - // check if the defined variable type has changed since assignment - const declarationType = checker.getTypeFromTypeNode(declaration.type); - const type = getConstrainedTypeAtLocation(services, node); + // For var declarations, we need to check whether the node + // is actually in a descendant of its declaration or not. If not, + // it may be used before defined. + + // eg + // if (Math.random() < 0.5) { + // var x: number = 2; + // } else { + // x!.toFixed(); + // } if ( - declarationType === type && - // `declare`s are never narrowed, so never skip them - !( - ts.isVariableDeclarationList(declaration.parent) && - ts.isVariableStatement(declaration.parent.parent) && - tsutils.includesModifier( - getModifiers(declaration.parent.parent), - ts.SyntaxKind.DeclareKeyword, - ) - ) + ts.isVariableDeclarationList(declaration.parent) && + // var + declaration.parent.flags === ts.NodeFlags.None && + // If they are not in the same file it will not exist. + // This situation must not occur using before defined. + services.tsNodeToESTreeNodeMap.has(declaration) ) { - // possibly used before assigned, so just skip it - // better to false negative and skip it, than false positive and fix to compile erroring code - // - // no better way to figure this out right now - // https://github.com/Microsoft/TypeScript/issues/31124 - return true; + const declaratorNode: TSESTree.VariableDeclaration = + services.tsNodeToESTreeNodeMap.get(declaration); + const scope = context.sourceCode.getScope(node); + const declaratorScope = context.sourceCode.getScope(declaratorNode); + let parentScope: Scope | null = declaratorScope; + while ((parentScope = parentScope.upper)) { + if (parentScope === scope) { + return true; + } + } + } + + if ( + // is it `const x!: number` + declaration.initializer === undefined && + declaration.exclamationToken === undefined && + declaration.type !== undefined + ) { + // check if the defined variable type has changed since assignment + const declarationType = checker.getTypeFromTypeNode(declaration.type); + const type = getConstrainedTypeAtLocation(services, node); + if ( + declarationType === type && + // `declare`s are never narrowed, so never skip them + !( + ts.isVariableDeclarationList(declaration.parent) && + ts.isVariableStatement(declaration.parent.parent) && + tsutils.includesModifier( + getModifiers(declaration.parent.parent), + ts.SyntaxKind.DeclareKeyword, + ) + ) + ) { + // possibly used before assigned, so just skip it + // better to false negative and skip it, than false positive and fix to compile erroring code + // + // no better way to figure this out right now + // https://github.com/Microsoft/TypeScript/issues/31124 + return true; + } } } return false; diff --git a/packages/eslint-plugin/tests/fixtures/tsconfig.json b/packages/eslint-plugin/tests/fixtures/tsconfig.json index 6ae5e64730b1..c16815aaf1ac 100644 --- a/packages/eslint-plugin/tests/fixtures/tsconfig.json +++ b/packages/eslint-plugin/tests/fixtures/tsconfig.json @@ -12,6 +12,7 @@ "file.ts", "consistent-type-exports.ts", "mixed-enums-decl.ts", - "react.tsx" + "react.tsx", + "var-declaration.ts" ] } diff --git a/packages/eslint-plugin/tests/fixtures/var-declaration.ts b/packages/eslint-plugin/tests/fixtures/var-declaration.ts new file mode 100644 index 000000000000..5253648a8d2f --- /dev/null +++ b/packages/eslint-plugin/tests/fixtures/var-declaration.ts @@ -0,0 +1 @@ +var varDeclarationFromFixture = 1; diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts index e2a130c39d9b..d292c7bbe19d 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts @@ -348,6 +348,16 @@ const bar = foo.a as string | undefined | bigint; `, languageOptions: { parserOptions: optionsWithExactOptionalPropertyTypes }, }, + { + code: ` +if (Math.random()) { + { + var x = 1; + } +} +x!; + `, + }, ], invalid: [ @@ -1076,5 +1086,55 @@ const bar = foo.a; ], languageOptions: { parserOptions: optionsWithExactOptionalPropertyTypes }, }, + { + code: ` +varDeclarationFromFixture!; + `, + output: ` +varDeclarationFromFixture; + `, + errors: [ + { + messageId: 'unnecessaryAssertion', + line: 2, + }, + ], + }, + { + code: ` +var x = 1; +x!; + `, + output: ` +var x = 1; +x; + `, + errors: [ + { + messageId: 'unnecessaryAssertion', + line: 3, + }, + ], + }, + { + code: ` +var x = 1; +{ + x!; +} + `, + output: ` +var x = 1; +{ + x; +} + `, + errors: [ + { + messageId: 'unnecessaryAssertion', + line: 4, + }, + ], + }, ], });