8000 fix(eslint-plugin): [no-unnecessary-type-assertion] conflict with TS … · jakebailey/typescript-eslint@3ca8477 · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit 3ca8477

Browse files
liuxingbaoyuliuxingbaoyu
authored
fix(eslint-plugin): [no-unnecessary-type-assertion] conflict with TS for variables used before assignment (typescript-eslint#9209)
* fix * review * review
1 parent 75a09a8 commit 3ca8477

File tree

4 files changed

+121
-25
lines changed

4 files changed

+121
-25
lines changed

‎packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts

Lines changed: 58 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import type { Scope } from '@typescript-eslint/scope-manager';
12
import type { TSESTree } from '@typescript-eslint/utils';
23
import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils';
34
import * as tsutils from 'ts-api-utils';
@@ -80,33 +81,66 @@ export default createRule<Options, MessageIds>({
8081
) &&
8182
// ignore class properties as they are compile time guarded
8283
// also ignore function arguments as they can't be used before defined
83-
ts.isVariableDeclaration(declaration) &&
84-
// is it `const x!: number`
85-
declaration.initializer === undefined &&
86-
declaration.exclamationToken === undefined &&
87-
declaration.type !== undefined
84+
ts.isVariableDeclaration(declaration)
8885
) {
89-
// check if the defined variable type has changed since assignment
90-
const declarationType = checker.getTypeFromTypeNode(declaration.type);
91-
const type = getConstrainedTypeAtLocation(services, node);
86+
// For var declarations, we need to check whether the node
87+
// is actually in a descendant of its declaration or not. If not,
88+
// it may be used before defined.
89+
90+
// eg
91+
// if (Math.random() < 0.5) {
92+
// var x: number = 2;
93+
// } else {
94+
// x!.toFixed();
95+
// }
9296
if (
93-
declarationType === type &&
94-
// `declare`s are never narrowed, so never skip them
95-
!(
96-
ts.isVariableDeclarationList(declaration.parent) &&
97-
ts.isVariableStatement(declaration.parent.parent) &&
98-
tsutils.includesModifier(
99-
getModifiers(declaration.parent.parent),
100-
ts.SyntaxKind.DeclareKeyword,
101-
)
102-
)
97+
ts.isVariableDeclarationList(declaration.parent) &&
98+
// var
99+
declaration.parent.flags === ts.NodeFlags.None &&
100+
// If they are not in the same file it will not exist.
101+
// This situation must not occur using before defined.
102+
services.tsNodeToESTreeNodeMap.has(declaration)
103103
) {
104-
// possibly used before assigned, so just skip it
105-
// better to false negative and skip it, than false positive and fix to compile erroring code
106-
//
107-
// no better way to figure this out right now
108-
// https://github.com/Microsoft/TypeScript/issues/31124
109-
return true;
104+
const declaratorNode: TSESTree.VariableDeclaration =
105+
services.tsNodeToESTreeNodeMap.get(declaration);
106+
const scope = context.sourceCode.getScope(node);
107+
const declaratorScope = context.sourceCode.getScope(declaratorNode);
108+
let parentScope: Scope | null = declaratorScope;
109+
while ((parentScope = parentScope.upper)) {
110+
if (parentScope === scope) {
111+
return true;
112+
}
113+
}
114+
}
115 8000 +
116+
if (
117+
// is it `const x!: number`
118+
declaration.initializer === undefined &&
119+
declaration.exclamationToken === undefined &&
120+
declaration.type !== undefined
121+
) {
122+
// check if the defined variable type has changed since assignment
123+
const declarationType = checker.getTypeFromTypeNode(declaration.type);
124+
const type = getConstrainedTypeAtLocation(services, node);
125+
if (
126+
declarationType === type &&
127+
// `declare`s are never narrowed, so never skip them
128+
!(
129+
ts.isVariableDeclarationList(declaration.parent) &&
130+
ts.isVariableStatement(declaration.parent.parent) &&
131+
tsutils.includesModifier(
132+
getModifiers(declaration.parent.parent),
133+
ts.SyntaxKind.DeclareKeyword,
134+
)
135+
)
136+
) {
137+
// possibly used before assigned, so just skip it
138+
// better to false negative and skip it, than false positive and fix to compile erroring code
139+
//
140+
// no better way to figure this out right now
141+
// https://github.com/Microsoft/TypeScript/issues/31124
142+
return true;
143+
}
110144
}
111145
}
112146
return false;

‎packages/eslint-plugin/tests/fixtures/tsconfig.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
"file.ts",
1313
"consistent-type-exports.ts",
1414
"mixed-enums-decl.ts",
15-
"react.tsx"
15+
"react.tsx",
16+
"var-declaration.ts"
1617
]
1718
}

‎packages/eslint-plugin/tests/fixtures/var-declaration.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
var varDeclarationFromFixture = 1;

‎packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,16 @@ const bar = foo.a as string | undefined | bigint;
349349
`,
350350
languageOptions: { parserOptions: optionsWithExactOptionalPropertyTypes },
351351
},
352+
{
353+
code: `
354+
if (Math.random()) {
355+
{
356+
var x = 1;
357+
}
358+
}
359+
x!;
360+
`,
361+
},
352362
],
353363

354364
invalid: [
@@ -1077,5 +1087,55 @@ const bar = foo.a;
10771087
],
10781088
languageOptions: { parserOptions: optionsWithExactOptionalPropertyTypes },
10791089
},
1090+
{
1091+
code: `
1092+
varDeclarationFromFixture!;
1093+
`,
1094+
output: `
1095+
varDeclarationFromFixture;
1096+
`,
1097+
errors: [
1098+
{
1099+
messageId: 'unnecessaryAssertion',
1100+
line: 2,
1101+
},
1102+
],
1103+
},
1104+
{
1105+
code: `
1106+
var x = 1;
1107+
x!;
1108+
`,
1109+
output: `
1110+
var x = 1;
1111+
x;
1112+
`,
1113+
errors: [
1114+
{
1115+
messageId: 'unnecessaryAssertion',
1116+
line: 3,
1117+
},
1118+
],
1119+
},
1120+
{
1121+
code: `
1122+
var x = 1;
1123+
{
1124+
x!;
1125+
}
1126+
`,
1127+
output: `
1128+
var x = 1;
1129+
{
1130+
x;
1131+
}
1132+
`,
1133+
errors: [
1134+
{
1135+
messageId: 'unnecessaryAssertion',
1136+
line: 4,
1137+
},
1138+
],
1139+
},
10801140
],
10811141
});

0 commit comments

Comments
 (0)
0