Skip to content

Commit ec857a9

Browse files
a-tarasyuka-tarasyuk
committed
feat(49962): disallow comparison against NaN
1 parent 7910c50 commit ec857a9

16 files changed

+476
-10
lines changed

Diff for: ‎src/compiler/checker.ts

+18
Original file line numberDiff line numberDiff line change
@@ -34386,6 +34386,7 @@ namespace ts {
3438634386
const eqType = operator === SyntaxKind.EqualsEqualsToken || operator === SyntaxKind.EqualsEqualsEqualsToken;
3438734387
error(errorNode, Diagnostics.This_condition_will_always_return_0_since_JavaScript_compares_objects_by_reference_not_value, eqType ? "false" : "true");
3438834388
}
34389+
checkNaNEquality(errorNode, operator, left, right, leftType, rightType);
3438934390
reportOperatorErrorUnless((left, right) => isTypeEqualityComparableTo(left, right) || isTypeEqualityComparableTo(right, left));
3439034391
return booleanType;
3439134392

@@ -34618,6 +34619,23 @@ namespace ts {
3461834619
return undefined;
3461934620
}
3462034621
}
34622+
34623+
function checkNaNEquality(errorNode: Node | undefined, operator: SyntaxKind, left: Expression, right: Expression, leftType: Type, rightType: Type) {
34624+
const isLeftNaN = leftType.flags & TypeFlags.Number && isNaN(skipParentheses(left));
34625+
const isRightNaN = rightType.flags & TypeFlags.Number && isNaN(skipParentheses(right));
34626+
if (isLeftNaN || isRightNaN) {
34627+
const err = error(errorNode, Diagnostics.This_condition_will_always_return_0,
34628+
tokenToString(operator === SyntaxKind.EqualsEqualsEqualsToken || operator === SyntaxKind.EqualsEqualsToken ? SyntaxKind.FalseKeyword : SyntaxKind.TrueKeyword));
34629+
if (isLeftNaN && isRightNaN) return;
34630+
const location = isLeftNaN ? right : left;
34631+
addRelatedInfo(err, createDiagnosticForNode(location, Diagnostics.Did_you_mean_0,
34632+
`${operator === SyntaxKind.ExclamationEqualsEqualsToken || operator === SyntaxKind.ExclamationEqualsToken ? tokenToString(SyntaxKind.ExclamationToken) : ""}Number.isNaN(${getTextOfNode(location)})`));
34633+
}
34634+
}
34635+
34636+
function isNaN(expr: Expression): boolean {
34637+
return isIdentifier(expr) && expr.escapedText === "NaN";
34638+
}
3462134639
}
3462234640

3462334641
function getBaseTypesIfUnrelated(leftType: Type, rightType: Type, isRelated: (left: Type, right: Type) => boolean): [Type, Type] {

Diff for: ‎src/compiler/diagnosticMessages.json

+12-1
Original file line numberDiff line numberDiff line change
@@ -3559,6 +3559,10 @@
35593559
"category": "Error",
35603560
"code": 2844
35613561
},
3562+
"This condition will always return '{0}'.": {
3563+
"category": "Error",
3564+
"code": 2845
3565+
},
35623566

35633567
"Import declaration '{0}' is using private name '{1}'.": {
35643568
"category": "Error",
@@ -7352,7 +7356,14 @@
73527356
"category": "Message",
73537357
"code": 95173
73547358
},
7355-
7359+
"Use `{0}`.": {
7360+
"category": "Message",
7361+
"code": 95174
7362+
},
7363+
"Use `Number.isNaN` in all conditions.": {
7364+
"category": "Message",
7365+
"code": 95175
7366+
},
73567367

73577368
"No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
73587369
"category": "Error",

Diff for: ‎src/services/codefixes/fixAddMissingConstraint.ts

-9
Original file line numberDiff line numberDiff line change
@@ -94,15 +94,6 @@ namespace ts.codefix {
9494
}
9595
}
9696

97-
function findAncestorMatchingSpan(sourceFile: SourceFile, span: TextSpan): Node {
98-
const end = textSpanEnd(span);
99-
let token = getTokenAtPosition(sourceFile, span.start);
100-
while (token.end < end) {
101-
token = token.parent;
102-
}
103-
return token;
104-
}
105-
10697
function tryGetConstraintFromDiagnosticMessage(messageText: string | DiagnosticMessageChain) {
10798
const [_, constraint] = flattenDiagnosticMessageText(messageText, "\n", 0).match(/`extends (.*)`/) || [];
10899
return constraint;

Diff for: ‎src/services/codefixes/fixNaNEquality.ts

+65
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/* @internal */
2+
namespace ts.codefix {
3+
const fixId = "fixNaNEquality";
4+
const errorCodes = [
5+
Diagnostics.This_condition_will_always_return_0.code,
6+
];
7+
8+
registerCodeFix({
9+
errorCodes,
10+
getCodeActions(context) {
11+
const { sourceFile, span, program } = context;
12+
const info = getInfo(program, sourceFile, span);
13+
if (info === undefined) return;
14+
15+
const { suggestion, expression, arg } = info;
16+
const changes = textChanges.ChangeTracker.with(context, t => doChange(t, sourceFile, arg, expression));
17+
return [createCodeFixAction(fixId, changes, [Diagnostics.Use_0, suggestion], fixId, Diagnostics.Use_Number_isNaN_in_all_conditions)];
18+
},
19+
fixIds: [fixId],
20+
getAllCodeActions: context => {
21+
return codeFixAll(context, errorCodes, (changes, diag) => {
22+
const info = getInfo(context.program, diag.file, createTextSpan(diag.start, diag.length));
23+
if (info) {
24+
doChange(changes, diag.file, info.arg, info.expression);
25+
}
26+
});
27+
}
28+
});
29+
30+
interface Info {
31+
suggestion: string;
32+
expression: BinaryExpression;
33+
arg: Expression;
34+
}
35+
36+
function getInfo(program: Program, sourceFile: SourceFile, span: TextSpan): Info | undefined {
37+
const diag = find(program.getSemanticDiagnostics(sourceFile), diag => diag.start === span.start && diag.length === span.length);
38+
if (diag === undefined || diag.relatedInformation === undefined) return;
39+
40+
const related = find(diag.relatedInformation, related => related.code === Diagnostics.Did_you_mean_0.code);
41+
if (related === undefined || related.file === undefined || related.start === undefined || related.length === undefined) return;
42+
43+
const token = findAncestorMatchingSpan(related.file, createTextSpan(related.start, related.length));
44+
if (token === undefined) return;
45+
46+
if (isExpression(token) && isBinaryExpression(token.parent)) {
47+
return { suggestion: getSuggestion(related.messageText), expression: token.parent, arg: token };
48+
}
49+
return undefined;
50+
}
51+
52+
function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, arg: Expression, expression: BinaryExpression) {
53+
const callExpression = factory.createCallExpression(
54+
factory.createPropertyAccessExpression(factory.createIdentifier("Number"), factory.createIdentifier("isNaN")), /*typeArguments*/ undefined, [arg]);
55+
const operator = expression.operatorToken.kind ;
56+
changes.replaceNode(sourceFile, expression,
57+
operator === SyntaxKind.ExclamationEqualsEqualsToken || operator === SyntaxKind.ExclamationEqualsToken
58+
? factory.createPrefixUnaryExpression(SyntaxKind.ExclamationToken, callExpression) : callExpression);
59+
}
60+
61+
function getSuggestion(messageText: string | DiagnosticMessageChain) {
62+
const [_, suggestion] = flattenDiagnosticMessageText(messageText, "\n", 0).match(/\'(.*)\'/) || [];
63+
return suggestion;
64+
}
65+
}

Diff for: ‎src/services/codefixes/helpers.ts

+9
Original file line numberDiff line numberDiff line change
@@ -743,4 +743,13 @@ namespace ts.codefix {
743743
export function importSymbols(importAdder: ImportAdder, symbols: readonly Symbol[]) {
744744
symbols.forEach(s => importAdder.addImportFromExportedSymbol(s, /*isValidTypeOnlyUseSite*/ true));
745745
}
746+
747+
export function findAncestorMatchingSpan(sourceFile: SourceFile, span: TextSpan): Node {
748+
const end = textSpanEnd(span);
749+
let token = getTokenAtPosition(sourceFile, span.start);
750+
while (token.end < end) {
751+
token = token.parent;
752+
}
753+
return token;
754+
}
746755
}

Diff for: ‎src/services/tsconfig.json

+1
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
"codefixes/fixConstructorForDerivedNeedSuperCall.ts",
8383
"codefixes/fixEnableExperimentalDecorators.ts",
8484
"codefixes/fixEnableJsxFlag.ts",
85+
"codefixes/fixNaNEquality.ts",
8586
"codefixes/fixModuleAndTargetOptions.ts",
8687
"codefixes/fixPropertyAssignment.ts",
8788
"codefixes/fixExtendsInterfaceBecomesImplements.ts",

Diff for: ‎tests/baselines/reference/nanEquality.errors.txt

+89
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
tests/cases/compiler/nanEquality.ts(3,5): error TS2845: This condition will always return 'false'.
2+
tests/cases/compiler/nanEquality.ts(4,5): error TS2845: This condition will always return 'false'.
3+
tests/cases/compiler/nanEquality.ts(6,5): error TS2845: This condition will always return 'false'.
4+
tests/cases/compiler/nanEquality.ts(7,5): error TS2845: This condition will always return 'false'.
5+
tests/cases/compiler/nanEquality.ts(9,5): error TS2845: This condition will always return 'true'.
6+
tests/cases/compiler/nanEquality.ts(10,5): error TS2845: This condition will always return 'true'.
7+
tests/cases/compiler/nanEquality.ts(12,5): error TS2845: This condition will always return 'true'.
8+
tests/cases/compiler/nanEquality.ts(13,5): error TS2845: This condition will always return 'true'.
9+
tests/cases/compiler/nanEquality.ts(15,5): error TS2845: This condition will always return 'false'.
10+
tests/cases/compiler/nanEquality.ts(16,5): error TS2845: This condition will always return 'false'.
11+
tests/cases/compiler/nanEquality.ts(18,5): error TS2845: This condition will always return 'true'.
12+
tests/cases/compiler/nanEquality.ts(19,5): error TS2845: This condition will always return 'true'.
13+
tests/cases/compiler/nanEquality.ts(21,5): error TS2845: This condition will always return 'false'.
14+
tests/cases/compiler/nanEquality.ts(22,5): error TS2845: This condition will always return 'true'.
15+
tests/cases/compiler/nanEquality.ts(24,5): error TS2845: This condition will always return 'false'.
16+
tests/cases/compiler/nanEquality.ts(25,5): error TS2845: This condition will always return 'true'.
17+
18+
19+
==== tests/cases/compiler/nanEquality.ts (16 errors) ====
20+
declare const x: number;
21+
22+
if (x === NaN) {}
23+
~~~~~~~~~
24+
!!! error TS2845: This condition will always return 'false'.
25+
!!! related TS1369 tests/cases/compiler/nanEquality.ts:3:5: Did you mean 'Number.isNaN(x)'?
26+
if (NaN === x) {}
27+
~~~~~~~~~
28+
!!! error TS2845: This condition will always return 'false'.
29+
!!! related TS1369 tests/cases/compiler/nanEquality.ts:4:13: Did you mean 'Number.isNaN(x)'?
30+
31+
if (x == NaN) {}
32+
~~~~~~~~
33+
!!! error TS2845: This condition will always return 'false'.
34+
!!! related TS1369 tests/cases/compiler/nanEquality.ts:6:5: Did you mean 'Number.isNaN(x)'?
35+
if (NaN == x) {}
36+
~~~~~~~~
37+
!!! error TS2845: This condition will always return 'false'.
38+
!!! related TS1369 tests/cases/compiler/nanEquality.ts:7:12: Did you mean 'Number.isNaN(x)'?
39+
40+
if (x !== NaN) {}
41+
~~~~~~~~~
42+
!!! error TS2845: This condition will always return 'true'.
43+
!!! related TS1369 tests/cases/compiler/nanEquality.ts:9:5: Did you mean '!Number.isNaN(x)'?
44+
if (NaN !== x) {}
45+
~~~~~~~~~
46+
!!! error TS2845: This condition will always return 'true'.
47+
!!! related TS1369 tests/cases/compiler/nanEquality.ts:10:13: Did you mean '!Number.isNaN(x)'?
48+
49+
if (x != NaN) {}
50+
~~~~~~~~
51+
!!! error TS2845: This condition will always return 'true'.
52+
!!! related TS1369 tests/cases/compiler/nanEquality.ts:12:5: Did you mean '!Number.isNaN(x)'?
53+
if (NaN != x) {}
54+
~~~~~~~~
55+
!!! error TS2845: This condition will always return 'true'.
56+
!!! related TS1369 tests/cases/compiler/nanEquality.ts:13:12: Did you mean '!Number.isNaN(x)'?
57+
58+
if (x === ((NaN))) {}
59+
~~~~~~~~~~~~~
60+
!!! error TS2845: This condition will always return 'false'.
61+
!!! related TS1369 tests/cases/compiler/nanEquality.ts:15:5: Did you mean 'Number.isNaN(x)'?
62+
if (((NaN)) === x) {}
63+
~~~~~~~~~~~~~
64+
!!! error TS2845: This condition will always return 'false'.
65+
!!! related TS1369 tests/cases/compiler/nanEquality.ts:16:17: Did you mean 'Number.isNaN(x)'?
66+
67+
if (x !== ((NaN))) {}
68+
~~~~~~~~~~~~~
69+
!!! error TS2845: This condition will always return 'true'.
70+
!!! related TS1369 tests/cases/compiler/nanEquality.ts:18:5: Did you mean '!Number.isNaN(x)'?
71+
if (((NaN)) !== x) {}
72+
~~~~~~~~~~~~~
73+
!!! error TS2845: This condition will always return 'true'.
74+
!!! related TS1369 tests/cases/compiler/nanEquality.ts:19:17: Did you mean '!Number.isNaN(x)'?
75+
76+
if (NaN === NaN) {}
77+
~~~~~~~~~~~
78+
!!! error TS2845: This condition will always return 'false'.
79+
if (NaN !== NaN) {}
80+
~~~~~~~~~~~
81+
!!! error TS2845: This condition will always return 'true'.
82+
83+
if (NaN == NaN) {}
84+
~~~~~~~~~~
85+
!!! error TS2845: This condition will always return 'false'.
86+
if (NaN != NaN) {}
87+
~~~~~~~~~~
88+
!!! error TS2845: This condition will always return 'true'.
89+

Diff for: ‎tests/baselines/reference/nanEquality.js

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
//// [nanEquality.ts]
2+
declare const x: number;
3+
4+
if (x === NaN) {}
5+
if (NaN === x) {}
6+
7+
if (x == NaN) {}
8+
if (NaN == x) {}
9+
10+
if (x !== NaN) {}
11+
if (NaN !== x) {}
12+
13+
if (x != NaN) {}
14+
if (NaN != x) {}
15+
16+
if (x === ((NaN))) {}
17+
if (((NaN)) === x) {}
18+
19+
if (x !== ((NaN))) {}
20+
if (((NaN)) !== x) {}
21+
22+
if (NaN === NaN) {}
23+
if (NaN !== NaN) {}
24+
25+
if (NaN == NaN) {}
26+
if (NaN != NaN) {}
27+
28+
29+
//// [nanEquality.js]
30+
if (x === NaN) { }
31+
if (NaN === x) { }
32+
if (x == NaN) { }
33+
if (NaN == x) { }
34+
if (x !== NaN) { }
35+
if (NaN !== x) { }
36+
if (x != NaN) { }
37+
if (NaN != x) { }
38+
if (x === ((NaN))) { }
39+
if (((NaN)) === x) { }
40+
if (x !== ((NaN))) { }
41+
if (((NaN)) !== x) { }
42+
if (NaN === NaN) { }
43+
if (NaN !== NaN) { }
44+
if (NaN == NaN) { }
45+
if (NaN != NaN) { }

Diff for: ‎tests/baselines/reference/nanEquality.symbols

+68
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
=== tests/cases/compiler/nanEquality.ts ===
2+
declare const x: number;
3+
>x : Symbol(x, Decl(nanEquality.ts, 0, 13))
4+
5+
if (x === NaN) {}
6+
>x : Symbol(x, Decl(nanEquality.ts, 0, 13))
7+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
8+
9+
if (NaN === x) {}
10+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
11+
>x : Symbol(x, Decl(nanEquality.ts, 0, 13))
12+
13+
if (x == NaN) {}
14+
>x : Symbol(x, Decl(nanEquality.ts, 0, 13))
15+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
16+
17+
if (NaN == x) {}
18+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
19+
>x : Symbol(x, Decl(nanEquality.ts, 0, 13))
20+
21+
if (x !== NaN) {}
22+
>x : Symbol(x, Decl(nanEquality.ts, 0, 13))
23+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
24+
25+
if (NaN !== x) {}
26+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
27+
>x : Symbol(x, Decl(nanEquality.ts, 0, 13))
28+
29+
if (x != NaN) {}
30+
>x : Symbol(x, Decl(nanEquality.ts, 0, 13))
31+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
32+
33+
if (NaN != x) {}
34+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
35+
>x : Symbol(x, Decl(nanEquality.ts, 0, 13))
36+
37+
if (x === ((NaN))) {}
38+
>x : Symbol(x, Decl(nanEquality.ts, 0, 13))
39+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
40+
41+
if (((NaN)) === x) {}
42+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
43+
>x : Symbol(x, Decl(nanEquality.ts, 0, 13))
44+
45+
if (x !== ((NaN))) {}
46+
>x : Symbol(x, Decl(nanEquality.ts, 0, 13))
47+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
48+
49+
if (((NaN)) !== x) {}
50+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
51+
>x : Symbol(x, Decl(nanEquality.ts, 0, 13))
52+
53+
if (NaN === NaN) {}
54+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
55+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
56+
57+
if (NaN !== NaN) {}
58+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
59+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
60+
61+
if (NaN == NaN) {}
62+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
63+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
64+
65+
if (NaN != NaN) {}
66+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
67+
>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --))
68+

0 commit comments

Comments
 (0)