Merge pull request #624 from makepanic/621-no-constant-condition

New Rule: no-constant-condition (fixes #621)
eslint · Feb 23, 2014 · 71a8cdd · 71a8cdd
2 parents 0b30abb + ba12826
commit 71a8cdd
Show file tree

Hide file tree

Showing 5 changed files with 179 additions and 0 deletions.
diff --git a/conf/eslint.json b/conf/eslint.json
@@ -14,6 +14,7 @@
         "no-comma-dangle": 2,
         "no-cond-assign": 2,
         "no-console": 2,
+        "no-constant-condition": 2,
         "no-control-regex": 2,
         "no-debugger": 2,
         "no-delete-var": 2,

diff --git a/docs/rules/README.md b/docs/rules/README.md
@@ -8,6 +8,7 @@ The following rules point out areas where you might have made mistakes.
 
 * [no-cond-assign](no-cond-assign.md) - disallow assignment in conditional expressions
 * [no-console](no-console.md) - disallow use of `console`
+* [no-constant-condition](no-constant-condition.md) - disallow use of constant expressions in conditions
 * [no-comma-dangle](no-comma-dangle.md) - disallow trailing commas in object literals
 * [no-control-regex](no-control-regex.md) - disallow control characters in regular expressions
 * [no-debugger](no-debugger.md) - disallow use of `debugger`

diff --git a/docs/rules/no-constant-condition.md b/docs/rules/no-constant-condition.md
@@ -0,0 +1,60 @@
+# Disallow use of constant expressions in conditions
+
+Comparing a literal expression in a condition is usually a typo or development trigger for a specific behavior.
+
+```js
+if (false) {
+    doSomethingUnfinished();
+}
+```
+
+This pattern is most likely an error and should be avoided.
+
+## Rule Details
+
+The rule is aimed at preventing the use of a constant expression in a condition.
+As such, it warns whenever it sees a constant expression inside a condition expression.
+
+The following patterns are considered warnings:
+
+```js
+if (true) {
+    doSomething();
+}
+```
+
+```js
+var result = 0 ? a : b;
+```
+
+```js
+while (-2) {
+    doSomething();
+}
+```
+
+```js
+for (;true;) {
+    doSomething();
+}
+```
+
+```js
+do{
+    something();
+} while (x = -1)
+```
+
+The following patterns are not warnings:
+
+```js
+if (x === 0) {
+    doSomething();
+}
+```
+
+```js
+do {
+    something();
+} while (x)
+```
diff --git a/lib/rules/no-constant-condition.js b/lib/rules/no-constant-condition.js
@@ -0,0 +1,75 @@
+/**
+ * @fileoverview Rule to flag use constant conditions
+ * @author Christian Schulz <http://rndm.de>
+ */
+
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = function(context) {
+
+    "use strict";
+
+    // array of types that should always trigger this rule if used inside a condition
+    var alwaysSimpleConstantCondition = ["Literal", "FunctionExpression", "ObjectExpression", "ArrayExpression"],
+        // map of functions that should be called to determine if it is a constant condition
+        sometimesSimpleConstantCondition = {
+            /**
+             * Checks assignment expression for literal argument.
+             * @example returns true for: if (t = +2) { doSomething() }
+             * @param {ASTNode} field The AST node to check.
+             * @returns {Boolean} true if assignment has right side literal or identifier.
+             */
+            "AssignmentExpression": function(field) {
+                return (field.right.type === "Literal" ||
+                    (field.right.type === "UnaryExpression" && field.right.argument.type === "Literal"));
+            },
+            /**
+             * Checks unary expression for literal argument.
+             * @example returns true for: if (+2) { doSomething(); }
+             * @param {ASTNode} field The AST node to check.
+             * @returns {Boolean} true if field has literal argument type.
+             */
+            "UnaryExpression": function(field) {
+                return field.argument.type === "Literal";
+            }
+        };
+
+    //--------------------------------------------------------------------------
+    // Helpers
+    //--------------------------------------------------------------------------
+
+    /**
+     * Checks if a node contains a constant condition.
+     * @param {ASTNode} node The AST node to check.
+     * @returns {void}
+     * @private
+     */
+    function checkConstantCondition(node) {
+        var field = node.test;
+        // check if type exists in simpleConditions array
+        if (alwaysSimpleConstantCondition.indexOf(field.type) !== -1 ||
+            // check if type exists in sometimes simple conditions
+            (sometimesSimpleConstantCondition.hasOwnProperty(field.type) &&
+                // check if type is a simple condition
+                sometimesSimpleConstantCondition[field.type](field))) {
+
+            context.report(node, "Unexpected constant condition.");
+        }
+    }
+
+    //--------------------------------------------------------------------------
+    // Public
+    //--------------------------------------------------------------------------
+
+    return {
+        "ConditionalExpression": checkConstantCondition,
+        "IfStatement": checkConstantCondition,
+        "WhileStatement": checkConstantCondition,
+        "DoWhileStatement": checkConstantCondition,
+        "ForStatement": checkConstantCondition
+    };
+
+};
diff --git a/tests/lib/rules/no-constant-condition.js b/tests/lib/rules/no-constant-condition.js
@@ -0,0 +1,42 @@
+/**
+ * @fileoverview Tests for no-constant-condition rule.
+ * @author Christian Schulz <http://rndm.de>
+ */
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+var eslintTester = require("eslint-tester");
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+eslintTester.addRuleTest("lib/rules/no-constant-condition", {
+    valid: [
+        "if(x){}",
+        "var r = q > 0 ? 1 : 2;",
+        "if(x == 0){ doSomething(); }",
+        "if(x = readBuf()){ doSomething(); }",
+        "while(isTrue()){}",
+        "while(x = y){}",
+        "for(;x < 10;){ doSomething(); }",
+        "do{ doSomething(); }while(x)"
+    ],
+    invalid: [
+        { code: "for(;true;){}", errors: [{ message: "Unexpected constant condition.", type: "ForStatement"}] },
+        { code: "do{}while(true)", errors: [{ message: "Unexpected constant condition.", type: "DoWhileStatement"}] },
+        { code: "do{}while(t = -2)", errors: [{ message: "Unexpected constant condition.", type: "DoWhileStatement"}] },
+        { code: "var r = true ? 1 : 2;", errors: [{ message: "Unexpected constant condition.", type: "ConditionalExpression"}] },
+        { code: "var r = q = 0 ? 1 : 2;", errors: [{ message: "Unexpected constant condition.", type: "ConditionalExpression"}] },
+        { code: "var r = (q = 0) ? 1 : 2;", errors: [{ message: "Unexpected constant condition.", type: "ConditionalExpression"}] },
+        { code: "if(-2){}", errors: [{ message: "Unexpected constant condition.", type: "IfStatement"}] },
+        { code: "if(true){}", errors: [{ message: "Unexpected constant condition.", type: "IfStatement"}] },
+        { code: "if({}){}", errors: [{ message: "Unexpected constant condition.", type: "IfStatement"}] },
+        { code: "while([]){}", errors: [{ message: "Unexpected constant condition.", type: "WhileStatement"}] },
+        { code: "while(42){}", errors: [{ message: "Unexpected constant condition.", type: "WhileStatement"}] },
+        { code: "while(x = 1){}", errors: [{ message: "Unexpected constant condition.", type: "WhileStatement"}] },
+        { code: "while(function(){}){}", errors: [{ message: "Unexpected constant condition.", type: "WhileStatement"}] }
+    ]
+});