From ec857a90c8d4fe5a1574e1790bd62f6875179805 Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Sat, 3 Sep 2022 19:23:24 +0300 Subject: [PATCH 1/3] feat(49962): disallow comparison against NaN --- src/compiler/checker.ts | 18 ++++ src/compiler/diagnosticMessages.json | 13 ++- .../codefixes/fixAddMissingConstraint.ts | 9 -- src/services/codefixes/fixNaNEquality.ts | 65 +++++++++++++ src/services/codefixes/helpers.ts | 9 ++ src/services/tsconfig.json | 1 + .../reference/nanEquality.errors.txt | 89 ++++++++++++++++++ tests/baselines/reference/nanEquality.js | 45 +++++++++ tests/baselines/reference/nanEquality.symbols | 68 ++++++++++++++ tests/baselines/reference/nanEquality.types | 92 +++++++++++++++++++ tests/cases/compiler/nanEquality.ts | 25 +++++ tests/cases/fourslash/fixNaNEquality1.ts | 9 ++ tests/cases/fourslash/fixNaNEquality2.ts | 9 ++ tests/cases/fourslash/fixNaNEquality3.ts | 9 ++ tests/cases/fourslash/fixNaNEquality4.ts | 9 ++ tests/cases/fourslash/fixNaNEquality_all.ts | 16 ++++ 16 files changed, 476 insertions(+), 10 deletions(-) create mode 100644 src/services/codefixes/fixNaNEquality.ts create mode 100644 tests/baselines/reference/nanEquality.errors.txt create mode 100644 tests/baselines/reference/nanEquality.js create mode 100644 tests/baselines/reference/nanEquality.symbols create mode 100644 tests/baselines/reference/nanEquality.types create mode 100644 tests/cases/compiler/nanEquality.ts create mode 100644 tests/cases/fourslash/fixNaNEquality1.ts create mode 100644 tests/cases/fourslash/fixNaNEquality2.ts create mode 100644 tests/cases/fourslash/fixNaNEquality3.ts create mode 100644 tests/cases/fourslash/fixNaNEquality4.ts create mode 100644 tests/cases/fourslash/fixNaNEquality_all.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index a457944e027a7..6e7c99a04de19 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -34386,6 +34386,7 @@ namespace ts { const eqType = operator === SyntaxKind.EqualsEqualsToken || operator === SyntaxKind.EqualsEqualsEqualsToken; error(errorNode, Diagnostics.This_condition_will_always_return_0_since_JavaScript_compares_objects_by_reference_not_value, eqType ? "false" : "true"); } + checkNaNEquality(errorNode, operator, left, right, leftType, rightType); reportOperatorErrorUnless((left, right) => isTypeEqualityComparableTo(left, right) || isTypeEqualityComparableTo(right, left)); return booleanType; @@ -34618,6 +34619,23 @@ namespace ts { return undefined; } } + + function checkNaNEquality(errorNode: Node | undefined, operator: SyntaxKind, left: Expression, right: Expression, leftType: Type, rightType: Type) { + const isLeftNaN = leftType.flags & TypeFlags.Number && isNaN(skipParentheses(left)); + const isRightNaN = rightType.flags & TypeFlags.Number && isNaN(skipParentheses(right)); + if (isLeftNaN || isRightNaN) { + const err = error(errorNode, Diagnostics.This_condition_will_always_return_0, + tokenToString(operator === SyntaxKind.EqualsEqualsEqualsToken || operator === SyntaxKind.EqualsEqualsToken ? SyntaxKind.FalseKeyword : SyntaxKind.TrueKeyword)); + if (isLeftNaN && isRightNaN) return; + const location = isLeftNaN ? right : left; + addRelatedInfo(err, createDiagnosticForNode(location, Diagnostics.Did_you_mean_0, + `${operator === SyntaxKind.ExclamationEqualsEqualsToken || operator === SyntaxKind.ExclamationEqualsToken ? tokenToString(SyntaxKind.ExclamationToken) : ""}Number.isNaN(${getTextOfNode(location)})`)); + } + } + + function isNaN(expr: Expression): boolean { + return isIdentifier(expr) && expr.escapedText === "NaN"; + } } function getBaseTypesIfUnrelated(leftType: Type, rightType: Type, isRelated: (left: Type, right: Type) => boolean): [Type, Type] { diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 6cec02b9e1a41..c4cf974c0d59b 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -3559,6 +3559,10 @@ "category": "Error", "code": 2844 }, + "This condition will always return '{0}'.": { + "category": "Error", + "code": 2845 + }, "Import declaration '{0}' is using private name '{1}'.": { "category": "Error", @@ -7352,7 +7356,14 @@ "category": "Message", "code": 95173 }, - + "Use `{0}`.": { + "category": "Message", + "code": 95174 + }, + "Use `Number.isNaN` in all conditions.": { + "category": "Message", + "code": 95175 + }, "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": { "category": "Error", diff --git a/src/services/codefixes/fixAddMissingConstraint.ts b/src/services/codefixes/fixAddMissingConstraint.ts index 2d3b6c51a346c..c8cbce9ee02af 100644 --- a/src/services/codefixes/fixAddMissingConstraint.ts +++ b/src/services/codefixes/fixAddMissingConstraint.ts @@ -94,15 +94,6 @@ namespace ts.codefix { } } - function findAncestorMatchingSpan(sourceFile: SourceFile, span: TextSpan): Node { - const end = textSpanEnd(span); - let token = getTokenAtPosition(sourceFile, span.start); - while (token.end < end) { - token = token.parent; - } - return token; - } - function tryGetConstraintFromDiagnosticMessage(messageText: string | DiagnosticMessageChain) { const [_, constraint] = flattenDiagnosticMessageText(messageText, "\n", 0).match(/`extends (.*)`/) || []; return constraint; diff --git a/src/services/codefixes/fixNaNEquality.ts b/src/services/codefixes/fixNaNEquality.ts new file mode 100644 index 0000000000000..7448aa52be725 --- /dev/null +++ b/src/services/codefixes/fixNaNEquality.ts @@ -0,0 +1,65 @@ +/* @internal */ +namespace ts.codefix { + const fixId = "fixNaNEquality"; + const errorCodes = [ + Diagnostics.This_condition_will_always_return_0.code, + ]; + + registerCodeFix({ + errorCodes, + getCodeActions(context) { + const { sourceFile, span, program } = context; + const info = getInfo(program, sourceFile, span); + if (info === undefined) return; + + const { suggestion, expression, arg } = info; + const changes = textChanges.ChangeTracker.with(context, t => doChange(t, sourceFile, arg, expression)); + return [createCodeFixAction(fixId, changes, [Diagnostics.Use_0, suggestion], fixId, Diagnostics.Use_Number_isNaN_in_all_conditions)]; + }, + fixIds: [fixId], + getAllCodeActions: context => { + return codeFixAll(context, errorCodes, (changes, diag) => { + const info = getInfo(context.program, diag.file, createTextSpan(diag.start, diag.length)); + if (info) { + doChange(changes, diag.file, info.arg, info.expression); + } + }); + } + }); + + interface Info { + suggestion: string; + expression: BinaryExpression; + arg: Expression; + } + + function getInfo(program: Program, sourceFile: SourceFile, span: TextSpan): Info | undefined { + const diag = find(program.getSemanticDiagnostics(sourceFile), diag => diag.start === span.start && diag.length === span.length); + if (diag === undefined || diag.relatedInformation === undefined) return; + + const related = find(diag.relatedInformation, related => related.code === Diagnostics.Did_you_mean_0.code); + if (related === undefined || related.file === undefined || related.start === undefined || related.length === undefined) return; + + const token = findAncestorMatchingSpan(related.file, createTextSpan(related.start, related.length)); + if (token === undefined) return; + + if (isExpression(token) && isBinaryExpression(token.parent)) { + return { suggestion: getSuggestion(related.messageText), expression: token.parent, arg: token }; + } + return undefined; + } + + function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, arg: Expression, expression: BinaryExpression) { + const callExpression = factory.createCallExpression( + factory.createPropertyAccessExpression(factory.createIdentifier("Number"), factory.createIdentifier("isNaN")), /*typeArguments*/ undefined, [arg]); + const operator = expression.operatorToken.kind ; + changes.replaceNode(sourceFile, expression, + operator === SyntaxKind.ExclamationEqualsEqualsToken || operator === SyntaxKind.ExclamationEqualsToken + ? factory.createPrefixUnaryExpression(SyntaxKind.ExclamationToken, callExpression) : callExpression); + } + + function getSuggestion(messageText: string | DiagnosticMessageChain) { + const [_, suggestion] = flattenDiagnosticMessageText(messageText, "\n", 0).match(/\'(.*)\'/) || []; + return suggestion; + } +} diff --git a/src/services/codefixes/helpers.ts b/src/services/codefixes/helpers.ts index ba8bd0453dd09..9d578b5b0da1c 100644 --- a/src/services/codefixes/helpers.ts +++ b/src/services/codefixes/helpers.ts @@ -743,4 +743,13 @@ namespace ts.codefix { export function importSymbols(importAdder: ImportAdder, symbols: readonly Symbol[]) { symbols.forEach(s => importAdder.addImportFromExportedSymbol(s, /*isValidTypeOnlyUseSite*/ true)); } + + export function findAncestorMatchingSpan(sourceFile: SourceFile, span: TextSpan): Node { + const end = textSpanEnd(span); + let token = getTokenAtPosition(sourceFile, span.start); + while (token.end < end) { + token = token.parent; + } + return token; + } } diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index 949d5c8a964a7..2237163ebb20d 100644 --- a/src/services/tsconfig.json +++ b/src/services/tsconfig.json @@ -82,6 +82,7 @@ "codefixes/fixConstructorForDerivedNeedSuperCall.ts", "codefixes/fixEnableExperimentalDecorators.ts", "codefixes/fixEnableJsxFlag.ts", + "codefixes/fixNaNEquality.ts", "codefixes/fixModuleAndTargetOptions.ts", "codefixes/fixPropertyAssignment.ts", "codefixes/fixExtendsInterfaceBecomesImplements.ts", diff --git a/tests/baselines/reference/nanEquality.errors.txt b/tests/baselines/reference/nanEquality.errors.txt new file mode 100644 index 0000000000000..2c4b78ee66de0 --- /dev/null +++ b/tests/baselines/reference/nanEquality.errors.txt @@ -0,0 +1,89 @@ +tests/cases/compiler/nanEquality.ts(3,5): error TS2845: This condition will always return 'false'. +tests/cases/compiler/nanEquality.ts(4,5): error TS2845: This condition will always return 'false'. +tests/cases/compiler/nanEquality.ts(6,5): error TS2845: This condition will always return 'false'. +tests/cases/compiler/nanEquality.ts(7,5): error TS2845: This condition will always return 'false'. +tests/cases/compiler/nanEquality.ts(9,5): error TS2845: This condition will always return 'true'. +tests/cases/compiler/nanEquality.ts(10,5): error TS2845: This condition will always return 'true'. +tests/cases/compiler/nanEquality.ts(12,5): error TS2845: This condition will always return 'true'. +tests/cases/compiler/nanEquality.ts(13,5): error TS2845: This condition will always return 'true'. +tests/cases/compiler/nanEquality.ts(15,5): error TS2845: This condition will always return 'false'. +tests/cases/compiler/nanEquality.ts(16,5): error TS2845: This condition will always return 'false'. +tests/cases/compiler/nanEquality.ts(18,5): error TS2845: This condition will always return 'true'. +tests/cases/compiler/nanEquality.ts(19,5): error TS2845: This condition will always return 'true'. +tests/cases/compiler/nanEquality.ts(21,5): error TS2845: This condition will always return 'false'. +tests/cases/compiler/nanEquality.ts(22,5): error TS2845: This condition will always return 'true'. +tests/cases/compiler/nanEquality.ts(24,5): error TS2845: This condition will always return 'false'. +tests/cases/compiler/nanEquality.ts(25,5): error TS2845: This condition will always return 'true'. + + +==== tests/cases/compiler/nanEquality.ts (16 errors) ==== + declare const x: number; + + if (x === NaN) {} + ~~~~~~~~~ +!!! error TS2845: This condition will always return 'false'. +!!! related TS1369 tests/cases/compiler/nanEquality.ts:3:5: Did you mean 'Number.isNaN(x)'? + if (NaN === x) {} + ~~~~~~~~~ +!!! error TS2845: This condition will always return 'false'. +!!! related TS1369 tests/cases/compiler/nanEquality.ts:4:13: Did you mean 'Number.isNaN(x)'? + + if (x == NaN) {} + ~~~~~~~~ +!!! error TS2845: This condition will always return 'false'. +!!! related TS1369 tests/cases/compiler/nanEquality.ts:6:5: Did you mean 'Number.isNaN(x)'? + if (NaN == x) {} + ~~~~~~~~ +!!! error TS2845: This condition will always return 'false'. +!!! related TS1369 tests/cases/compiler/nanEquality.ts:7:12: Did you mean 'Number.isNaN(x)'? + + if (x !== NaN) {} + ~~~~~~~~~ +!!! error TS2845: This condition will always return 'true'. +!!! related TS1369 tests/cases/compiler/nanEquality.ts:9:5: Did you mean '!Number.isNaN(x)'? + if (NaN !== x) {} + ~~~~~~~~~ +!!! error TS2845: This condition will always return 'true'. +!!! related TS1369 tests/cases/compiler/nanEquality.ts:10:13: Did you mean '!Number.isNaN(x)'? + + if (x != NaN) {} + ~~~~~~~~ +!!! error TS2845: This condition will always return 'true'. +!!! related TS1369 tests/cases/compiler/nanEquality.ts:12:5: Did you mean '!Number.isNaN(x)'? + if (NaN != x) {} + ~~~~~~~~ +!!! error TS2845: This condition will always return 'true'. +!!! related TS1369 tests/cases/compiler/nanEquality.ts:13:12: Did you mean '!Number.isNaN(x)'? + + if (x === ((NaN))) {} + ~~~~~~~~~~~~~ +!!! error TS2845: This condition will always return 'false'. +!!! related TS1369 tests/cases/compiler/nanEquality.ts:15:5: Did you mean 'Number.isNaN(x)'? + if (((NaN)) === x) {} + ~~~~~~~~~~~~~ +!!! error TS2845: This condition will always return 'false'. +!!! related TS1369 tests/cases/compiler/nanEquality.ts:16:17: Did you mean 'Number.isNaN(x)'? + + if (x !== ((NaN))) {} + ~~~~~~~~~~~~~ +!!! error TS2845: This condition will always return 'true'. +!!! related TS1369 tests/cases/compiler/nanEquality.ts:18:5: Did you mean '!Number.isNaN(x)'? + if (((NaN)) !== x) {} + ~~~~~~~~~~~~~ +!!! error TS2845: This condition will always return 'true'. +!!! related TS1369 tests/cases/compiler/nanEquality.ts:19:17: Did you mean '!Number.isNaN(x)'? + + if (NaN === NaN) {} + ~~~~~~~~~~~ +!!! error TS2845: This condition will always return 'false'. + if (NaN !== NaN) {} + ~~~~~~~~~~~ +!!! error TS2845: This condition will always return 'true'. + + if (NaN == NaN) {} + ~~~~~~~~~~ +!!! error TS2845: This condition will always return 'false'. + if (NaN != NaN) {} + ~~~~~~~~~~ +!!! error TS2845: This condition will always return 'true'. + \ No newline at end of file diff --git a/tests/baselines/reference/nanEquality.js b/tests/baselines/reference/nanEquality.js new file mode 100644 index 0000000000000..fd6ebcb39eb0f --- /dev/null +++ b/tests/baselines/reference/nanEquality.js @@ -0,0 +1,45 @@ +//// [nanEquality.ts] +declare const x: number; + +if (x === NaN) {} +if (NaN === x) {} + +if (x == NaN) {} +if (NaN == x) {} + +if (x !== NaN) {} +if (NaN !== x) {} + +if (x != NaN) {} +if (NaN != x) {} + +if (x === ((NaN))) {} +if (((NaN)) === x) {} + +if (x !== ((NaN))) {} +if (((NaN)) !== x) {} + +if (NaN === NaN) {} +if (NaN !== NaN) {} + +if (NaN == NaN) {} +if (NaN != NaN) {} + + +//// [nanEquality.js] +if (x === NaN) { } +if (NaN === x) { } +if (x == NaN) { } +if (NaN == x) { } +if (x !== NaN) { } +if (NaN !== x) { } +if (x != NaN) { } +if (NaN != x) { } +if (x === ((NaN))) { } +if (((NaN)) === x) { } +if (x !== ((NaN))) { } +if (((NaN)) !== x) { } +if (NaN === NaN) { } +if (NaN !== NaN) { } +if (NaN == NaN) { } +if (NaN != NaN) { } diff --git a/tests/baselines/reference/nanEquality.symbols b/tests/baselines/reference/nanEquality.symbols new file mode 100644 index 0000000000000..e15fbcd7f3cfb --- /dev/null +++ b/tests/baselines/reference/nanEquality.symbols @@ -0,0 +1,68 @@ +=== tests/cases/compiler/nanEquality.ts === +declare const x: number; +>x : Symbol(x, Decl(nanEquality.ts, 0, 13)) + +if (x === NaN) {} +>x : Symbol(x, Decl(nanEquality.ts, 0, 13)) +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) + +if (NaN === x) {} +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) +>x : Symbol(x, Decl(nanEquality.ts, 0, 13)) + +if (x == NaN) {} +>x : Symbol(x, Decl(nanEquality.ts, 0, 13)) +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) + +if (NaN == x) {} +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) +>x : Symbol(x, Decl(nanEquality.ts, 0, 13)) + +if (x !== NaN) {} +>x : Symbol(x, Decl(nanEquality.ts, 0, 13)) +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) + +if (NaN !== x) {} +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) +>x : Symbol(x, Decl(nanEquality.ts, 0, 13)) + +if (x != NaN) {} +>x : Symbol(x, Decl(nanEquality.ts, 0, 13)) +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) + +if (NaN != x) {} +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) +>x : Symbol(x, Decl(nanEquality.ts, 0, 13)) + +if (x === ((NaN))) {} +>x : Symbol(x, Decl(nanEquality.ts, 0, 13)) +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) + +if (((NaN)) === x) {} +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) +>x : Symbol(x, Decl(nanEquality.ts, 0, 13)) + +if (x !== ((NaN))) {} +>x : Symbol(x, Decl(nanEquality.ts, 0, 13)) +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) + +if (((NaN)) !== x) {} +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) +>x : Symbol(x, Decl(nanEquality.ts, 0, 13)) + +if (NaN === NaN) {} +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) + +if (NaN !== NaN) {} +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) + +if (NaN == NaN) {} +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) + +if (NaN != NaN) {} +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) + diff --git a/tests/baselines/reference/nanEquality.types b/tests/baselines/reference/nanEquality.types new file mode 100644 index 0000000000000..dc648ec437c65 --- /dev/null +++ b/tests/baselines/reference/nanEquality.types @@ -0,0 +1,92 @@ +=== tests/cases/compiler/nanEquality.ts === +declare const x: number; +>x : number + +if (x === NaN) {} +>x === NaN : boolean +>x : number +>NaN : number + +if (NaN === x) {} +>NaN === x : boolean +>NaN : number +>x : number + +if (x == NaN) {} +>x == NaN : boolean +>x : number +>NaN : number + +if (NaN == x) {} +>NaN == x : boolean +>NaN : number +>x : number + +if (x !== NaN) {} +>x !== NaN : boolean +>x : number +>NaN : number + +if (NaN !== x) {} +>NaN !== x : boolean +>NaN : number +>x : number + +if (x != NaN) {} +>x != NaN : boolean +>x : number +>NaN : number + +if (NaN != x) {} +>NaN != x : boolean +>NaN : number +>x : number + +if (x === ((NaN))) {} +>x === ((NaN)) : boolean +>x : number +>((NaN)) : number +>(NaN) : number +>NaN : number + +if (((NaN)) === x) {} +>((NaN)) === x : boolean +>((NaN)) : number +>(NaN) : number +>NaN : number +>x : number + +if (x !== ((NaN))) {} +>x !== ((NaN)) : boolean +>x : number +>((NaN)) : number +>(NaN) : number +>NaN : number + +if (((NaN)) !== x) {} +>((NaN)) !== x : boolean +>((NaN)) : number +>(NaN) : number +>NaN : number +>x : number + +if (NaN === NaN) {} +>NaN === NaN : boolean +>NaN : number +>NaN : number + +if (NaN !== NaN) {} +>NaN !== NaN : boolean +>NaN : number +>NaN : number + +if (NaN == NaN) {} +>NaN == NaN : boolean +>NaN : number +>NaN : number + +if (NaN != NaN) {} +>NaN != NaN : boolean +>NaN : number +>NaN : number + diff --git a/tests/cases/compiler/nanEquality.ts b/tests/cases/compiler/nanEquality.ts new file mode 100644 index 0000000000000..d8f66d61d9dda --- /dev/null +++ b/tests/cases/compiler/nanEquality.ts @@ -0,0 +1,25 @@ +declare const x: number; + +if (x === NaN) {} +if (NaN === x) {} + +if (x == NaN) {} +if (NaN == x) {} + +if (x !== NaN) {} +if (NaN !== x) {} + +if (x != NaN) {} +if (NaN != x) {} + +if (x === ((NaN))) {} +if (((NaN)) === x) {} + +if (x !== ((NaN))) {} +if (((NaN)) !== x) {} + +if (NaN === NaN) {} +if (NaN !== NaN) {} + +if (NaN == NaN) {} +if (NaN != NaN) {} diff --git a/tests/cases/fourslash/fixNaNEquality1.ts b/tests/cases/fourslash/fixNaNEquality1.ts new file mode 100644 index 0000000000000..1177793c9d307 --- /dev/null +++ b/tests/cases/fourslash/fixNaNEquality1.ts @@ -0,0 +1,9 @@ +/// + +////if (x === NaN) {} + +verify.codeFix({ + index: 0, + description: "Use `Number.isNaN(x)`.", + newFileContent: "if (Number.isNaN(x)) {}", +}); diff --git a/tests/cases/fourslash/fixNaNEquality2.ts b/tests/cases/fourslash/fixNaNEquality2.ts new file mode 100644 index 0000000000000..d93ada01867b3 --- /dev/null +++ b/tests/cases/fourslash/fixNaNEquality2.ts @@ -0,0 +1,9 @@ +/// + +////if (NaN === x) {} + +verify.codeFix({ + index: 0, + description: "Use `Number.isNaN(x)`.", + newFileContent: "if (Number.isNaN(x)) {}", +}); diff --git a/tests/cases/fourslash/fixNaNEquality3.ts b/tests/cases/fourslash/fixNaNEquality3.ts new file mode 100644 index 0000000000000..434408b55dae6 --- /dev/null +++ b/tests/cases/fourslash/fixNaNEquality3.ts @@ -0,0 +1,9 @@ +/// + +////if (x !== NaN) {} + +verify.codeFix({ + index: 0, + description: "Use `!Number.isNaN(x)`.", + newFileContent: "if (!Number.isNaN(x)) {}", +}); diff --git a/tests/cases/fourslash/fixNaNEquality4.ts b/tests/cases/fourslash/fixNaNEquality4.ts new file mode 100644 index 0000000000000..689305dbf640b --- /dev/null +++ b/tests/cases/fourslash/fixNaNEquality4.ts @@ -0,0 +1,9 @@ +/// + +////if (NaN !== x) {} + +verify.codeFix({ + index: 0, + description: "Use `!Number.isNaN(x)`.", + newFileContent: "if (!Number.isNaN(x)) {}", +}); diff --git a/tests/cases/fourslash/fixNaNEquality_all.ts b/tests/cases/fourslash/fixNaNEquality_all.ts new file mode 100644 index 0000000000000..c49a3ce20ebca --- /dev/null +++ b/tests/cases/fourslash/fixNaNEquality_all.ts @@ -0,0 +1,16 @@ +/// + +////if (x === NaN) {} +////if (NaN === x) {} +////if (x !== NaN) {} +////if (NaN !== x) {} + +verify.codeFixAll({ + fixId: "fixNaNEquality", + fixAllDescription: ts.Diagnostics.Use_Number_isNaN_in_all_conditions.message, + newFileContent: +`if (Number.isNaN(x)) {} +if (Number.isNaN(x)) {} +if (!Number.isNaN(x)) {} +if (!Number.isNaN(x)) {}` +}); From fb44a1dd812b742fd542c28816d7640b60b8db4d Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Wed, 14 Sep 2022 10:27:47 +0300 Subject: [PATCH 2/3] change diagnostic message --- src/compiler/checker.ts | 8 +++++--- tests/baselines/reference/nanEquality.errors.txt | 10 +++++++++- tests/baselines/reference/nanEquality.js | 5 +++++ tests/baselines/reference/nanEquality.symbols | 8 ++++++++ tests/baselines/reference/nanEquality.types | 13 +++++++++++++ tests/cases/compiler/nanEquality.ts | 4 ++++ tests/cases/fourslash/fixNaNEquality1.ts | 5 +++-- tests/cases/fourslash/fixNaNEquality2.ts | 5 +++-- tests/cases/fourslash/fixNaNEquality3.ts | 5 +++-- tests/cases/fourslash/fixNaNEquality4.ts | 5 +++-- tests/cases/fourslash/fixNaNEquality5.ts | 10 ++++++++++ tests/cases/fourslash/fixNaNEquality_all.ts | 10 ++++++++-- 12 files changed, 74 insertions(+), 14 deletions(-) create mode 100644 tests/cases/fourslash/fixNaNEquality5.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 6e7c99a04de19..8e47a5ea3fbc4 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -34621,15 +34621,17 @@ namespace ts { } function checkNaNEquality(errorNode: Node | undefined, operator: SyntaxKind, left: Expression, right: Expression, leftType: Type, rightType: Type) { - const isLeftNaN = leftType.flags & TypeFlags.Number && isNaN(skipParentheses(left)); - const isRightNaN = rightType.flags & TypeFlags.Number && isNaN(skipParentheses(right)); + const isLeftNaN = (leftType.flags & TypeFlags.Number) && isNaN(skipParentheses(left)); + const isRightNaN = (rightType.flags & TypeFlags.Number) && isNaN(skipParentheses(right)); if (isLeftNaN || isRightNaN) { const err = error(errorNode, Diagnostics.This_condition_will_always_return_0, tokenToString(operator === SyntaxKind.EqualsEqualsEqualsToken || operator === SyntaxKind.EqualsEqualsToken ? SyntaxKind.FalseKeyword : SyntaxKind.TrueKeyword)); if (isLeftNaN && isRightNaN) return; + const operatorString = operator === SyntaxKind.ExclamationEqualsEqualsToken || operator === SyntaxKind.ExclamationEqualsToken ? tokenToString(SyntaxKind.ExclamationToken) : ""; const location = isLeftNaN ? right : left; + const expression = skipParentheses(location); addRelatedInfo(err, createDiagnosticForNode(location, Diagnostics.Did_you_mean_0, - `${operator === SyntaxKind.ExclamationEqualsEqualsToken || operator === SyntaxKind.ExclamationEqualsToken ? tokenToString(SyntaxKind.ExclamationToken) : ""}Number.isNaN(${getTextOfNode(location)})`)); + `${operatorString}Number.isNaN(${isEntityNameExpression(expression) ? entityNameToString(expression) : "..."})`)); } } diff --git a/tests/baselines/reference/nanEquality.errors.txt b/tests/baselines/reference/nanEquality.errors.txt index 2c4b78ee66de0..ee2ad1f1bac16 100644 --- a/tests/baselines/reference/nanEquality.errors.txt +++ b/tests/baselines/reference/nanEquality.errors.txt @@ -14,9 +14,10 @@ tests/cases/compiler/nanEquality.ts(21,5): error TS2845: This condition will alw tests/cases/compiler/nanEquality.ts(22,5): error TS2845: This condition will always return 'true'. tests/cases/compiler/nanEquality.ts(24,5): error TS2845: This condition will always return 'false'. tests/cases/compiler/nanEquality.ts(25,5): error TS2845: This condition will always return 'true'. +tests/cases/compiler/nanEquality.ts(29,5): error TS2845: This condition will always return 'false'. -==== tests/cases/compiler/nanEquality.ts (16 errors) ==== +==== tests/cases/compiler/nanEquality.ts (17 errors) ==== declare const x: number; if (x === NaN) {} @@ -86,4 +87,11 @@ tests/cases/compiler/nanEquality.ts(25,5): error TS2845: This condition will alw if (NaN != NaN) {} ~~~~~~~~~~ !!! error TS2845: This condition will always return 'true'. + + // ... + declare let y: any; + if (NaN === y[0][1]) {} + ~~~~~~~~~~~~~~~ +!!! error TS2845: This condition will always return 'false'. +!!! related TS1369 tests/cases/compiler/nanEquality.ts:29:13: Did you mean 'Number.isNaN(...)'? \ No newline at end of file diff --git a/tests/baselines/reference/nanEquality.js b/tests/baselines/reference/nanEquality.js index fd6ebcb39eb0f..4c4a115530bc5 100644 --- a/tests/baselines/reference/nanEquality.js +++ b/tests/baselines/reference/nanEquality.js @@ -24,6 +24,10 @@ if (NaN !== NaN) {} if (NaN == NaN) {} if (NaN != NaN) {} + +// ... +declare let y: any; +if (NaN === y[0][1]) {} //// [nanEquality.js] @@ -43,3 +47,4 @@ if (NaN === NaN) { } if (NaN !== NaN) { } if (NaN == NaN) { } if (NaN != NaN) { } +if (NaN === y[0][1]) { } diff --git a/tests/baselines/reference/nanEquality.symbols b/tests/baselines/reference/nanEquality.symbols index e15fbcd7f3cfb..47e9430f603de 100644 --- a/tests/baselines/reference/nanEquality.symbols +++ b/tests/baselines/reference/nanEquality.symbols @@ -66,3 +66,11 @@ if (NaN != NaN) {} >NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) >NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) +// ... +declare let y: any; +>y : Symbol(y, Decl(nanEquality.ts, 27, 11)) + +if (NaN === y[0][1]) {} +>NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) +>y : Symbol(y, Decl(nanEquality.ts, 27, 11)) + diff --git a/tests/baselines/reference/nanEquality.types b/tests/baselines/reference/nanEquality.types index dc648ec437c65..5bb060d8de03d 100644 --- a/tests/baselines/reference/nanEquality.types +++ b/tests/baselines/reference/nanEquality.types @@ -90,3 +90,16 @@ if (NaN != NaN) {} >NaN : number >NaN : number +// ... +declare let y: any; +>y : any + +if (NaN === y[0][1]) {} +>NaN === y[0][1] : boolean +>NaN : number +>y[0][1] : any +>y[0] : any +>y : any +>0 : 0 +>1 : 1 + diff --git a/tests/cases/compiler/nanEquality.ts b/tests/cases/compiler/nanEquality.ts index d8f66d61d9dda..db2aa5ff3e9ee 100644 --- a/tests/cases/compiler/nanEquality.ts +++ b/tests/cases/compiler/nanEquality.ts @@ -23,3 +23,7 @@ if (NaN !== NaN) {} if (NaN == NaN) {} if (NaN != NaN) {} + +// ... +declare let y: any; +if (NaN === y[0][1]) {} diff --git a/tests/cases/fourslash/fixNaNEquality1.ts b/tests/cases/fourslash/fixNaNEquality1.ts index 1177793c9d307..52004b122fa2d 100644 --- a/tests/cases/fourslash/fixNaNEquality1.ts +++ b/tests/cases/fourslash/fixNaNEquality1.ts @@ -1,9 +1,10 @@ /// -////if (x === NaN) {} +////declare const x: number; +////[|if (x === NaN) {}|] verify.codeFix({ index: 0, description: "Use `Number.isNaN(x)`.", - newFileContent: "if (Number.isNaN(x)) {}", + newRangeContent: "if (Number.isNaN(x)) {}", }); diff --git a/tests/cases/fourslash/fixNaNEquality2.ts b/tests/cases/fourslash/fixNaNEquality2.ts index d93ada01867b3..e941ac7bd4f61 100644 --- a/tests/cases/fourslash/fixNaNEquality2.ts +++ b/tests/cases/fourslash/fixNaNEquality2.ts @@ -1,9 +1,10 @@ /// -////if (NaN === x) {} +////declare const x: number; +////[|if (NaN === x) {}|] verify.codeFix({ index: 0, description: "Use `Number.isNaN(x)`.", - newFileContent: "if (Number.isNaN(x)) {}", + newRangeContent: "if (Number.isNaN(x)) {}", }); diff --git a/tests/cases/fourslash/fixNaNEquality3.ts b/tests/cases/fourslash/fixNaNEquality3.ts index 434408b55dae6..6ae682a83c9a5 100644 --- a/tests/cases/fourslash/fixNaNEquality3.ts +++ b/tests/cases/fourslash/fixNaNEquality3.ts @@ -1,9 +1,10 @@ /// -////if (x !== NaN) {} +////declare const x: number; +////[|if (x !== NaN) {}|] verify.codeFix({ index: 0, description: "Use `!Number.isNaN(x)`.", - newFileContent: "if (!Number.isNaN(x)) {}", + newRangeContent: "if (!Number.isNaN(x)) {}", }); diff --git a/tests/cases/fourslash/fixNaNEquality4.ts b/tests/cases/fourslash/fixNaNEquality4.ts index 689305dbf640b..6f4128317b977 100644 --- a/tests/cases/fourslash/fixNaNEquality4.ts +++ b/tests/cases/fourslash/fixNaNEquality4.ts @@ -1,9 +1,10 @@ /// -////if (NaN !== x) {} +////declare const x: number; +////[|if (NaN !== x) {}|] verify.codeFix({ index: 0, description: "Use `!Number.isNaN(x)`.", - newFileContent: "if (!Number.isNaN(x)) {}", + newRangeContent: "if (!Number.isNaN(x)) {}", }); diff --git a/tests/cases/fourslash/fixNaNEquality5.ts b/tests/cases/fourslash/fixNaNEquality5.ts new file mode 100644 index 0000000000000..00b954f33bd59 --- /dev/null +++ b/tests/cases/fourslash/fixNaNEquality5.ts @@ -0,0 +1,10 @@ +/// + +////declare const x: any; +////[|if (NaN !== x[0][1]) {}|] + +verify.codeFix({ + index: 0, + description: "Use `!Number.isNaN(...)`.", + newRangeContent: "if (!Number.isNaN(x[0][1])) {}", +}); diff --git a/tests/cases/fourslash/fixNaNEquality_all.ts b/tests/cases/fourslash/fixNaNEquality_all.ts index c49a3ce20ebca..e5536ec8f041b 100644 --- a/tests/cases/fourslash/fixNaNEquality_all.ts +++ b/tests/cases/fourslash/fixNaNEquality_all.ts @@ -1,16 +1,22 @@ /// +////declare const x: number; +////declare const y: any; ////if (x === NaN) {} ////if (NaN === x) {} ////if (x !== NaN) {} ////if (NaN !== x) {} +////if (NaN === y[0][1]) {} verify.codeFixAll({ fixId: "fixNaNEquality", fixAllDescription: ts.Diagnostics.Use_Number_isNaN_in_all_conditions.message, newFileContent: -`if (Number.isNaN(x)) {} +`declare const x: number; +declare const y: any; if (Number.isNaN(x)) {} +if (Number.isNaN(x)) {} +if (!Number.isNaN(x)) {} if (!Number.isNaN(x)) {} -if (!Number.isNaN(x)) {}` +if (Number.isNaN(y[0][1])) {}` }); From 5ca49bb073659e8b6394f77c2e8a703848d29458 Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Thu, 15 Sep 2022 09:07:34 +0300 Subject: [PATCH 3/3] use global NaN symbol for NaN equality comparisons --- src/compiler/checker.ts | 21 ++++++++---- .../reference/nanEquality.errors.txt | 12 +++++++ tests/baselines/reference/nanEquality.js | 21 ++++++++++++ tests/baselines/reference/nanEquality.symbols | 29 +++++++++++++++++ tests/baselines/reference/nanEquality.types | 32 +++++++++++++++++++ tests/cases/compiler/nanEquality.ts | 12 +++++++ 6 files changed, 121 insertions(+), 6 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 8e47a5ea3fbc4..4c0188f7fa53b 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -996,6 +996,7 @@ namespace ts { let deferredGlobalOmitSymbol: Symbol | undefined; let deferredGlobalAwaitedSymbol: Symbol | undefined; let deferredGlobalBigIntType: ObjectType | undefined; + let deferredGlobalNaNSymbol: Symbol | undefined; const allPotentiallyUnusedIdentifiers = new Map(); // key is file name @@ -14287,6 +14288,10 @@ namespace ts { return (deferredGlobalBigIntType ||= getGlobalType("BigInt" as __String, /*arity*/ 0, /*reportErrors*/ false)) || emptyObjectType; } + function getGlobalNaNSymbol(): Symbol | undefined { + return (deferredGlobalNaNSymbol ||= getGlobalValueSymbol("NaN" as __String, /*reportErrors*/ false)); + } + /** * Instantiates a global type that is generic with some element type, and returns that instantiation. */ @@ -34386,7 +34391,7 @@ namespace ts { const eqType = operator === SyntaxKind.EqualsEqualsToken || operator === SyntaxKind.EqualsEqualsEqualsToken; error(errorNode, Diagnostics.This_condition_will_always_return_0_since_JavaScript_compares_objects_by_reference_not_value, eqType ? "false" : "true"); } - checkNaNEquality(errorNode, operator, left, right, leftType, rightType); + checkNaNEquality(errorNode, operator, left, right); reportOperatorErrorUnless((left, right) => isTypeEqualityComparableTo(left, right) || isTypeEqualityComparableTo(right, left)); return booleanType; @@ -34620,9 +34625,9 @@ namespace ts { } } - function checkNaNEquality(errorNode: Node | undefined, operator: SyntaxKind, left: Expression, right: Expression, leftType: Type, rightType: Type) { - const isLeftNaN = (leftType.flags & TypeFlags.Number) && isNaN(skipParentheses(left)); - const isRightNaN = (rightType.flags & TypeFlags.Number) && isNaN(skipParentheses(right)); + function checkNaNEquality(errorNode: Node | undefined, operator: SyntaxKind, left: Expression, right: Expression) { + const isLeftNaN = isGlobalNaN(skipParentheses(left)); + const isRightNaN = isGlobalNaN(skipParentheses(right)); if (isLeftNaN || isRightNaN) { const err = error(errorNode, Diagnostics.This_condition_will_always_return_0, tokenToString(operator === SyntaxKind.EqualsEqualsEqualsToken || operator === SyntaxKind.EqualsEqualsToken ? SyntaxKind.FalseKeyword : SyntaxKind.TrueKeyword)); @@ -34635,8 +34640,12 @@ namespace ts { } } - function isNaN(expr: Expression): boolean { - return isIdentifier(expr) && expr.escapedText === "NaN"; + function isGlobalNaN(expr: Expression): boolean { + if (isIdentifier(expr) && expr.escapedText === "NaN") { + const globalNaNSymbol = getGlobalNaNSymbol(); + return !!globalNaNSymbol && globalNaNSymbol === getResolvedSymbol(expr); + } + return false; } } diff --git a/tests/baselines/reference/nanEquality.errors.txt b/tests/baselines/reference/nanEquality.errors.txt index ee2ad1f1bac16..410d782d4b16c 100644 --- a/tests/baselines/reference/nanEquality.errors.txt +++ b/tests/baselines/reference/nanEquality.errors.txt @@ -94,4 +94,16 @@ tests/cases/compiler/nanEquality.ts(29,5): error TS2845: This condition will alw ~~~~~~~~~~~~~~~ !!! error TS2845: This condition will always return 'false'. !!! related TS1369 tests/cases/compiler/nanEquality.ts:29:13: Did you mean 'Number.isNaN(...)'? + + function t1(value: number, NaN: number) { + return value === NaN; // ok + } + + function t2(value: number, NaN: number) { + return NaN == value; // ok + } + + function t3(NaN: number) { + return NaN === NaN; // ok + } \ No newline at end of file diff --git a/tests/baselines/reference/nanEquality.js b/tests/baselines/reference/nanEquality.js index 4c4a115530bc5..21a10e7d0fb91 100644 --- a/tests/baselines/reference/nanEquality.js +++ b/tests/baselines/reference/nanEquality.js @@ -28,6 +28,18 @@ if (NaN != NaN) {} // ... declare let y: any; if (NaN === y[0][1]) {} + +function t1(value: number, NaN: number) { + return value === NaN; // ok +} + +function t2(value: number, NaN: number) { + return NaN == value; // ok +} + +function t3(NaN: number) { + return NaN === NaN; // ok +} //// [nanEquality.js] @@ -48,3 +60,12 @@ if (NaN !== NaN) { } if (NaN == NaN) { } if (NaN != NaN) { } if (NaN === y[0][1]) { } +function t1(value, NaN) { + return value === NaN; // ok +} +function t2(value, NaN) { + return NaN == value; // ok +} +function t3(NaN) { + return NaN === NaN; // ok +} diff --git a/tests/baselines/reference/nanEquality.symbols b/tests/baselines/reference/nanEquality.symbols index 47e9430f603de..4398b9c32f649 100644 --- a/tests/baselines/reference/nanEquality.symbols +++ b/tests/baselines/reference/nanEquality.symbols @@ -74,3 +74,32 @@ if (NaN === y[0][1]) {} >NaN : Symbol(NaN, Decl(lib.es5.d.ts, --, --)) >y : Symbol(y, Decl(nanEquality.ts, 27, 11)) +function t1(value: number, NaN: number) { +>t1 : Symbol(t1, Decl(nanEquality.ts, 28, 23)) +>value : Symbol(value, Decl(nanEquality.ts, 30, 12)) +>NaN : Symbol(NaN, Decl(nanEquality.ts, 30, 26)) + + return value === NaN; // ok +>value : Symbol(value, Decl(nanEquality.ts, 30, 12)) +>NaN : Symbol(NaN, Decl(nanEquality.ts, 30, 26)) +} + +function t2(value: number, NaN: number) { +>t2 : Symbol(t2, Decl(nanEquality.ts, 32, 1)) +>value : Symbol(value, Decl(nanEquality.ts, 34, 12)) +>NaN : Symbol(NaN, Decl(nanEquality.ts, 34, 26)) + + return NaN == value; // ok +>NaN : Symbol(NaN, Decl(nanEquality.ts, 34, 26)) +>value : Symbol(value, Decl(nanEquality.ts, 34, 12)) +} + +function t3(NaN: number) { +>t3 : Symbol(t3, Decl(nanEquality.ts, 36, 1)) +>NaN : Symbol(NaN, Decl(nanEquality.ts, 38, 12)) + + return NaN === NaN; // ok +>NaN : Symbol(NaN, Decl(nanEquality.ts, 38, 12)) +>NaN : Symbol(NaN, Decl(nanEquality.ts, 38, 12)) +} + diff --git a/tests/baselines/reference/nanEquality.types b/tests/baselines/reference/nanEquality.types index 5bb060d8de03d..ce64d93017405 100644 --- a/tests/baselines/reference/nanEquality.types +++ b/tests/baselines/reference/nanEquality.types @@ -103,3 +103,35 @@ if (NaN === y[0][1]) {} >0 : 0 >1 : 1 +function t1(value: number, NaN: number) { +>t1 : (value: number, NaN: number) => boolean +>value : number +>NaN : number + + return value === NaN; // ok +>value === NaN : boolean +>value : number +>NaN : number +} + +function t2(value: number, NaN: number) { +>t2 : (value: number, NaN: number) => boolean +>value : number +>NaN : number + + return NaN == value; // ok +>NaN == value : boolean +>NaN : number +>value : number +} + +function t3(NaN: number) { +>t3 : (NaN: number) => boolean +>NaN : number + + return NaN === NaN; // ok +>NaN === NaN : boolean +>NaN : number +>NaN : number +} + diff --git a/tests/cases/compiler/nanEquality.ts b/tests/cases/compiler/nanEquality.ts index db2aa5ff3e9ee..88d949c8eaa9d 100644 --- a/tests/cases/compiler/nanEquality.ts +++ b/tests/cases/compiler/nanEquality.ts @@ -27,3 +27,15 @@ if (NaN != NaN) {} // ... declare let y: any; if (NaN === y[0][1]) {} + +function t1(value: number, NaN: number) { + return value === NaN; // ok +} + +function t2(value: number, NaN: number) { + return NaN == value; // ok +} + +function t3(NaN: number) { + return NaN === NaN; // ok +}