Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Issue #1054 - Ported the use-isnan rule from tslint-microsoft-contrib… #1140

Merged
merged 1 commit into from
Apr 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ A sample configuration file with all options is available [here](https://github.
* `"parameter"` checks function parameters.
* `"property-declaration"` checks object property declarations.
* `"variable-declaration"` checks variable declaration.
* `use-isnan` enforces that you use the isNaN() function to check for NaN references instead of a comparison to the NaN constant. Similar to the [use-isnan ESLint rule](http://eslint.org/docs/rules/use-isnan).
* `use-strict` enforces ECMAScript 5's strict mode.
* `check-module` checks that all top-level modules are using strict mode.
* `check-function` checks that all top-level functions are using strict mode.
Expand Down
44 changes: 44 additions & 0 deletions src/rules/useIsnanRule.ts
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";

/**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can delete this comment

Copy link
Contributor Author

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?

Copy link
Contributor

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 👍

* 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 {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blank line is uneeded

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which blank line is that? This looks properly spaced to me.

Copy link
Contributor

Choose a reason for hiding this comment

The 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";
}
}
1 change: 1 addition & 0 deletions src/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@
"rules/tripleEqualsRule.ts",
"rules/typedefRule.ts",
"rules/typedefWhitespaceRule.ts",
"rules/useIsnanRule.ts",
"rules/useStrictRule.ts",
"rules/variableNameRule.ts",
"rules/whitespaceRule.ts",
Expand Down
28 changes: 28 additions & 0 deletions test/rules/use-isnan/test.ts.lint
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]
5 changes: 5 additions & 0 deletions test/rules/use-isnan/tslint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"rules": {
"use-isnan": true
}
}
3 changes: 2 additions & 1 deletion test/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -142,4 +143,4 @@
"rule-tester/testData.ts",
"rule-tester/utilsTests.ts"
]
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The 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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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:

core.autocrlf=input
core.eol=lf

Copy link
Contributor

Choose a reason for hiding this comment

The 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 grunt-ts always removes this when rewriting tsconfig.json files. We might as well remove it and leave it removed.