8000 feat(eslint-plugin): Add better non-null handling [no-unnecessary-typ… · lcombs15/typescript-eslint@4cd5590 · GitHub < 8000 link rel="manifest" href="/manifest.json" crossOrigin="use-credentials">
[go: up one dir, main page]
Skip to content
This repository was archived by the owner on Sep 25, 2020. It is now read-only.

Commit 4cd5590

Browse files
bradzacherbradzacher
authored
feat(eslint-plugin): Add better non-null handling [no-unnecessary-type-assertion] (typescript-eslint#478)
1 parent 298d66c commit 4cd5590

File tree

6 files changed

+381
-77
lines changed

6 files changed

+381
-77
lines changed

‎.eslintrc.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
"extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended"],
99
"rules": {
1010
"comma-dangle": ["error", "always-multiline"],
11+
"curly": ["error", "all"],
1112
"no-mixed-operators": "error",
1213
"no-console": "off",
1314
"no-undef": "off",

‎packages/eslint-plugin/src/rules/member-naming.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,13 @@ export default util.createRule<Options, MessageIds>({
7676
const convention = conventions[accessibility];
7777

7878
const method = node as TSESTree.MethodDefinition;
79-
if (method.kind === 'constructor') return;
79+
if (method.kind === 'constructor') {
80+
return;
81+
}
8082

81-
if (!convention || convention.test(name)) return;
83+
if (!convention || convention.test(name)) {
84+
return;
85+
}
8286

8387
context.report({
8488
node: node.key,

‎packages/eslint-plugin/src/rules/no-extraneous-class.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,9 @@ export default util.createRule<Options, MessageIds>({
9696
onlyStatic = false;
9797
}
9898
}
99-
if (!(onlyStatic || onlyConstructor)) break;
99+
if (!(onlyStatic || onlyConstructor)) {
100+
break;
101+
}
100102
}
101103

102104
if (onlyConstructor) {

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

Lines changed: 163 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,15 @@
11
import { TSESTree } from '@typescript-eslint/typescript-estree';
2-
import * as tsutils from 'tsutils';
2+
import {
3+
isCallExpression,
4+
isNewExpression,
5+
isObjectType,
6+
isObjectFlagSet,
7+
isParameterDeclaration,
8+
isPropertyDeclaration,
9+
isStrictCompilerOptionEnabled,
10+
isTypeFlagSet,
11+
isVariableDeclaration,
12+
} from 'tsutils';
313
import ts from 'typescript';
414
import * as util from '../util';
515

@@ -8,7 +18,7 @@ type Options = [
818
typesToIgnore?: string[];
919
}
1020
];
11-
type MessageIds = 'unnecessaryAssertion';
21+
type MessageIds = 'contextuallyUnnecessary' | 'unnecessaryAssertion';
1222

1323
export default util.createRule<Options, MessageIds>({
1424
name: 'no-unnecessary-type-assertion',
@@ -24,6 +34,8 @@ export default util.createRule<Options, MessageIds>({
2434
messages: {
2535
unnecessaryAssertion:
2636
'This assertion is unnecessary since it does not change the type of the expression.',
37+
contextuallyUnnecessary:
38+
'This assertion is unnecessary since the receiver accepts the original type of the expression.',
2739
},
2840
schema: [
2941
{
@@ -44,6 +56,8 @@ export default util.createRule<Options, MessageIds>({
4456
create(context, [options]) {
4557
const sourceCode = context.getSourceCode();
4658
const parserServices = util.getParserServices(context);
59+
const checker = parserServices.program.getTypeChecker();
60+
const compilerOptions = parserServices.program.getCompilerOptions();
4761

4862
/**
4963
* Sometimes tuple types don't have ObjectFlags.Tuple set, like when they're being matched against an inferred type.
@@ -76,91 +90,170 @@ export default util.createRule<Options, MessageIds>({
7690
return true;
7791
}
7892

79-
function checkNonNullAssertion(
80-
node: TSESTree.Node,
93+
/**
94+
* Returns the contextual type of a given node.
95+
* Contextual type is the type of the target the node is going into.
96+
* i.e. the type of a called function's parameter, or the defined type of a variable declaration
97+
*/
98+
function getContextualType(
8199
checker: ts.TypeChecker,
82-
): void {
83-
const originalNode = parserServices.esTreeNodeToTSNodeMap.get<
84-
ts.NonNullExpression
85-
>(node);
86-
const type = checker.getTypeAtLocation(originalNode.expression);
87-
88-
if (type === checker.getNonNullableType(type)) {
89-
context.report({
90-
node,
91-
messageId: 'unnecessaryAssertion',
92-
fix(fixer) {
93-
return fixer.removeRange([
94-
originalNode.expression.end,
95-
originalNode.end,
96-
]);
97-
},
98-
});
100+
node: ts.Expression,
101+
): ts.Type | undefined {
102+
const parent = node.parent;
103+
if (!parent) {
104+
return;
99105
}
100-
}
101106

102-
function verifyCast(
103-
node: TSESTree.TSTypeAssertion | TSESTree.TSAsExpression,
104-
checker: ts.TypeChecker,
105-
): void {
106-
if (
107-
options &&
108-
options.typesToIgnore &&
109-
options.typesToIgnore.indexOf(
110-
sourceCode.getText(node.typeAnnotation),
111-
) !== -1
107+
if (isCallExpression(parent) || isNewExpression(parent)) {
108+
if (node === parent.expression) {
109+
// is the callee, so has no contextual type
110+
return;
111+
}
112+
} else if (
113+
isVariableDeclaration(parent) ||
114+
isPropertyDeclaration(parent) ||
115+
isParameterDeclaration(parent)
116+
) {
117+
return parent.type
118+
? checker.getTypeFromTypeNode(parent.type)
119+
: undefined;
120+
} else if (
121+
![ts.SyntaxKind.TemplateSpan, ts.SyntaxKind.JsxExpression].includes(
122+
parent.kind,
123+
)
112124
) {
125+
// parent is not something we know we can get the contextual type of
113126
return;
114127
}
128+
// TODO - support return statement checking
115129

116-
const originalNode = parserServices.esTreeNodeToTSNodeMap.get<
117-
ts.AssertionExpression
118-
>(node);
119-
const castType = checker.getTypeAtLocation(originalNode);
130+
return checker.getContextualType(node);
131+
}
120132

133+
/**
134+
* Returns true if there's a chance the variable has been used before a value has been assigned to it
135+
*/
136+
function isPossiblyUsedBeforeAssigned(node: ts.Expression): boolean {
137+
const declaration = util.getDeclaration(checker, node);
121138
if (
122-
tsutils.isTypeFlagSet(castType, ts.TypeFlags.Literal) ||
123-
(tsutils.isObjectType(castType) &&
124-
(tsutils.isObjectFlagSet(castType, ts.ObjectFlags.Tuple) ||
125-
couldBeTupleType(castType)))
139+
// non-strict mode doesn't care about used before assigned errors
140+
isStrictCompilerOptionEnabled(compilerOptions, 'strictNullChecks') &&
141+
// ignore class properties as they are compile time guarded
142+
// also ignore function arguments as they can't be used before defined
143+
isVariableDeclaration(declaration) &&
144+
// is it `const x!: number`
145+
declaration.initializer === undefined &&
146+
declaration.exclamationToken === undefined &&
147+
declaration.type !== undefined
126148
) {
127-
// It's not always safe to remove a cast to a literal type or tuple
128-
// type, as those types are sometimes widened without the cast.
129-
return;
149+
// check if the defined variable type has changed since assignment
150+
const declarationType = checker.getTypeFromTypeNode(declaration.type);
151+
const type = util.getConstrainedTypeAtLocation(checker, node);
152+
if (declarationType === type) {
153+
// possibly used before assigned, so just skip it
154+
// better to false negative and skip it, than false postiive and fix to compile erroring code
155+
//
156+
// no better way to figure this out right now
157+
// https://github.com/Microsoft/TypeScript/issues/31124
158+
return true;
159+
}
130160
}
161+
return false;
162+
}
163+
164+
return {
165+
TSNonNullExpression(node) {
166+
const originalNode = parserServices.esTreeNodeToTSNodeMap.get<
167+
ts.NonNullExpression
168+
>(node);
169+
const type = util.getConstrainedTypeAtLocation(
170+
checker,
171+
originalNode.expression,
172+
);
173+
174+
if (!util.isNullableType(type)) {
175+
if (isPossiblyUsedBeforeAssigned(originalNode.expression)) {
176+
return;
177+
}
131178

132-
const uncastType = checker.getTypeAtLocation(originalNode.expression);
133-
134-
if (uncastType === castType) {
135-
context.report({
136-
node,
137-
messageId: 'unnecessaryAssertion',
138-
fix(fixer) {
139-
return originalNode.kind === ts.SyntaxKind.TypeAssertionExpression
140-
? fixer.removeRange([
141-
originalNode.getStart(),
142-
originalNode.expression.getStart(),
143-
])
144-
: fixer.removeRange([
179+
context.report({
180+
node,
181+
messageId: 'unnecessaryAssertion',
182+
fix(fixer) {
183+
return fixer.removeRange([
184+
originalNode.expression.end,
185+
originalNode.end,
186+
]);
187+
},
188+
});
189+
} else {
190+
// we know it's a nullable type
191+
// so figure out if the variable is used in a place that accepts nullable types
192+
const contextualType = getContextualType(checker, originalNode);
193+
if (contextualType && util.isNullableType(contextualType)) {
194+
context.report({
195+
node,
196+
messageId: 'contextuallyUnnecessary',
197+
fix(fixer) {
198+
return fixer.removeRange([
145199
originalNode.expression.end,
146200
originalNode.end,
147201
]);
148-
},
149-
});
150-
}
151-
}
202+
},
203+
});
204+
}
205+
}
206+
},
207+
'TSAsExpression, TSTypeAssertion'(
208+
node: TSESTree.TSTypeAssertion | TSESTree.TSAsExpression,
209+
): void {
210+
if (
211+
options &&
212+
options.typesToIgnore &&
213+
options.typesToIgnore.indexOf(
214+
sourceCode.getText(node.typeAnnotation),
215+
) !== -1
216+
) {
217+
return;
218+
}
152219

153-
const checker = parserServices.program.getTypeChecker();
220+
const originalNode = parserServices.esTreeNodeToTSNodeMap.get<
221+
ts.AssertionExpression
222+
>(node);
223+
const castType = checker.getTypeAtLocation(originalNode);
154224

155-
return {
156-
TSNonNullExpression(node) {
157-
checkNonNullAssertion(node, checker);
158-
},
159-
TSTypeAssertion(node) {
160-
verifyCast(node, checker);
161-
},
162-
TSAsExpression(node) {
163-
verifyCast(node, checker);
225+
if (
226+
isTypeFlagSet(castType, ts.TypeFlags.Literal) ||
227+
(isObjectType(castType) &&
228+
(isObjectFlagSet(castType, ts.ObjectFlags.Tuple) ||
229+
couldBeTupleType(castType)))
230+
) {
231+
// It's not always safe to remove a cast to a literal type or tuple
232+
// type, as those types are sometimes widened without the cast.
233+
return;
234+
}
235+
236+
const uncastType = checker.getTypeAtLocation(originalNode.expression);
237+
238+
if (uncastType === castType) {
239+
context.report({
240+
node,
241+
messageId: 'unnecessaryAssertion',
242+
fix(fixer) {
243+
return originalNode.kind === ts.SyntaxKind.TypeAssertionExpression
244+
? fixer.removeRange([
245+
originalNode.getStart(),
246+
originalNode.expression.getStart(),
247+
])
248+
: fixer.removeRange([
249+
originalNode.expression.end,
250+
originalNode.end,
251+
]);
252+
},
253+
});
254+
}
255+
256+
// TODO - add contextually unnecessary check for this
164257
},
165258
};
166259
},

‎packages/eslint-plugin/src/util/types.ts

Lines changed: 50 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
import * as tsutils from 'tsutils';
1+
import {
2+
isTypeFlagSet,
3+
isTypeReference,
4+
isUnionOrIntersectionType,
5+
unionTypeParts,
6+
} from 'tsutils';
27
import ts from 'typescript';
38

49
/**
@@ -10,11 +15,11 @@ export function containsTypeByName(
1015
type: ts.Type,
1116
allowedNames: Set<string>,
1217
): boolean {
13-
if (tsutils.isTypeFlagSet(type, ts.TypeFlags.Any | ts.TypeFlags.Unknown)) {
18+
if (isTypeFlagSet(type, ts.TypeFlags.Any | ts.TypeFlags.Unknown)) {
1419
return true;
1520
}
1621

17-
if (tsutils.isTypeReference(type)) {
22+
if (isTypeReference(type)) {
1823
type = type.target;
1924
}
2025

@@ -25,7 +30,7 @@ export function containsTypeByName(
2530
return true;
2631
}
2732

28-
if (tsutils.isUnionOrIntersectionType(type)) {
33+
if (isUnionOrIntersectionType(type)) {
2934
return type.types.some(t => containsTypeByName(t, allowedNames));
3035
}
3136

@@ -35,3 +40,44 @@ export function containsTypeByName(
3540
bases.some(t => containsTypeByName(t, allowedNames))
3641
);
3742
}
43+
44+
/**
45+
* Resolves the given node's type. Will resolve to the type's generic constraint, if it has one.
46+
*/
47+
export function getConstrainedTypeAtLocation(
48+
checker: ts.TypeChecker,
49+
node: ts.Node,
50+
): ts.Type {
51+
const nodeType = checker.getTypeAtLocation(node);
52+
const constrained = checker.getBaseConstraintOfType(nodeType);
53+
54+
return constrained || nodeType;
55+
}
56+
57+
/**
58+
* Checks if the given type is (or accepts) nullable
59+
* @param isReceiver true if the type is a receiving type (i.e. the type of a called function's parameter)
60+
*/
61+
export function isNullableType(type: ts.Type, isReceiver?: boolean): boolean {
62+
let flags: ts.TypeFlags = 0;
63+
for (const t of unionTypeParts(type)) {
64+
flags |= t.flags;
65+
}
66+
67+
flags =
68+
isReceiver && flags & (ts.TypeFlags.Any | ts.TypeFlags.Unknown)
69+
? -1
70+
: flags;
71+
72+
return (flags & (ts.TypeFlags.Null | ts.TypeFlags.Undefined)) !== 0;
73+
}
74+
75+
/**
76+
* Gets the declaration for the given variable
77+
*/
78+
export function getDeclaration(
79+
checker: ts.TypeChecker,
80+
node: ts.Expression,
81+
): ts.Declaration {
82+
return checker.getSymbolAtLocation(node)!.declarations![0];
83+
}

0 commit comments

Comments
 (0)
0