-
Notifications
You must be signed in to change notification settings - Fork 885
Issue #1054 - Ported the use-isnan rule from tslint-microsoft-contrib… #1140
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
/** | ||
* @license | ||
* Copyright 2013 Palantir Technologies, Inc. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
import * as ts from "typescript"; | ||
import * as Lint from "../lint"; | ||
|
||
/** | ||
* Implementation of the use-isnan rule. | ||
*/ | ||
export class Rule extends Lint.Rules.AbstractRule { | ||
public static FAILURE_STRING = "Found an invalid comparison for NaN: "; | ||
|
||
public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { | ||
return this.applyWithWalker(new UseIsnanRuleWalker(sourceFile, this.getOptions())); | ||
} | ||
} | ||
|
||
class UseIsnanRuleWalker extends Lint.RuleWalker { | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. blank line is uneeded There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which blank line is that? This looks properly spaced to me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Line 33, but no matter, not important enough at all to make you revise for. Merged! |
||
protected visitBinaryExpression(node: ts.BinaryExpression): void { | ||
if (this.isExpressionNaN(node.left) || this.isExpressionNaN(node.right)) { | ||
this.addFailure(this.createFailure(node.getStart(), node.getWidth(), Rule.FAILURE_STRING + node.getText())); | ||
} | ||
super.visitBinaryExpression(node); | ||
} | ||
|
||
private isExpressionNaN(node: ts.Node) { | ||
return node.kind === ts.SyntaxKind.Identifier && node.getText() === "NaN"; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
|
||
// no violation for comparing NaN using isNaN | ||
if (isNaN(NaN)) { | ||
} | ||
|
||
// no violation for correctly checking for isNaN | ||
if (isNaN(something)) { } | ||
|
||
|
||
// do not use equality operators to compare for NaN | ||
if (foo == NaN) { } | ||
~~~~~~~~~~ [Found an invalid comparison for NaN: foo == NaN] | ||
if (NaN === foo) { } | ||
~~~~~~~~~~~ [Found an invalid comparison for NaN: NaN === foo] | ||
if (foo != NaN) { } | ||
~~~~~~~~~~ [Found an invalid comparison for NaN: foo != NaN] | ||
if (NaN !== foo) { } | ||
~~~~~~~~~~~ [Found an invalid comparison for NaN: NaN !== foo] | ||
|
||
// do not use any binary operators to compare for NaN | ||
if (foo > NaN) { } | ||
~~~~~~~~~ [Found an invalid comparison for NaN: foo > NaN] | ||
if (NaN >= foo) { } | ||
~~~~~~~~~~ [Found an invalid comparison for NaN: NaN >= foo] | ||
if (foo < NaN) { } | ||
~~~~~~~~~ [Found an invalid comparison for NaN: foo < NaN] | ||
if (NaN <= foo) { } | ||
~~~~~~~~~~ [Found an invalid comparison for NaN: NaN <= foo] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
{ | ||
"rules": { | ||
"use-isnan": true | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -111,6 +111,7 @@ | |
"../src/rules/tripleEqualsRule.ts", | ||
"../src/rules/typedefRule.ts", | ||
"../src/rules/typedefWhitespaceRule.ts", | ||
"../src/rules/useIsnanRule.ts", | ||
"../src/rules/useStrictRule.ts", | ||
"../src/rules/variableNameRule.ts", | ||
"../src/rules/whitespaceRule.ts", | ||
|
@@ -142,4 +143,4 @@ | |
"rule-tester/testData.ts", | ||
"rule-tester/utilsTests.ts" | ||
] | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you re-add the newline at the end of this file? (So we prevent merge conflicts and unnecessary diffs) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. However, this was added automatically by the grunt build. So you may want to change the build somehow. I am on Windows using git-bash. My .git/config file has:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, just ignore my previous comment in that case, it seems as if |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can delete this comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean the block comment and copyright notice? Or the JSDoc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant the JSDoc, but actually it doesn't hurt, you can just leave it 👍