Merge dc68039 into 4e04449

lib/config/configuration.js

@@ -1135,6 +1135,8 @@ Configuration.prototype.registerDefaultRules = function() {
     this.registerRule(require('../rules/disallow-tabs'));
 
     this.registerRule(require('../rules/require-aligned-multiline-params'));
+
+    this.registerRule(require('../rules/require-early-return'));
 };
 
 /**

lib/rules/require-early-return.js

@@ -0,0 +1,111 @@
+/**
+ * Requires to return early in a function.
+ *
+ * Types: `String`
+ *
+ * Values:
+ *  - `noElse`: disallow to use of else if the corrisponding `if` block contain a return.
+ *
+ * #### Example
+ *
+ * ```js
+ * "requireEarlyReturn": "noElse"
+ * ```
+ *
+ * ##### Valid
+ *
+ * ```js
+ * function test() {
+ *     if (x) {
+ *         return x;
+ *     }
+ *     return y;
+ * }
+ * ```
+ *
+ * ##### Invalid
+ *
+ * ```js
+ * function test() {
+ *     if (x) {
+ *         return x;
+ *     } else {
+ *         return y;
+ *     }
+ * }
+ * ```
+ */
+
+var assert = require('assert');
+
+module.exports = function() {};
+
+module.exports.prototype = {
+    configure: function(options) {
+        assert(
+            options === 'noElse',
+            this.getOptionName() + ' option allow only the `noElse` value'
+        );
+    },
+
+    getOptionName: function() {
+        return 'requireEarlyReturn';
+    },
+
+    check: function(file, errors) {
+        function addError(entity) {
+            errors.add(
+                'Use of else after return',
+                entity.loc.start.line,
+                entity.loc.start.column
+            );
+        }
+
+        // Check if the IfStatement node contain a ReturnStatement.
+        // If the node has a block, check all the statements in backward order to see if there is one.
+        // This is to ensure that code like this will still return true:
+        //
+        // if (true) {
+        //    return;
+        //    eval();
+        // }
+        function hasNodeReturn(node) {
+            if (node.type === 'BlockStatement') {
+                for (var i = node.body.length - 1; i >= 0; i--) {
+                    if (node.body[i].type === 'ReturnStatement') {
+                        return true;
+                    }
+                }
+                return false;
+            }
+            return node.type === 'ReturnStatement';
+        }
+
+        file.iterateNodesByType('IfStatement', function(node) {
+            if (!node.alternate) {
+                return;
+            }
+
+            // Check if all the parents have a return statement, if not continue to the following IfStatement node.
+            //
+            // Example:
+            //
+            // if (foo) {
+            //     return;
+            // } else if (bar) {  <-- error
+            //     bar();
+            // } else if (baz) {  <-- safe
+            //     return baz();
+            // } else {           <-- safe
+            //     bas();
+            // }
+            for (var nodeIf = node; nodeIf && nodeIf.type === 'IfStatement'; nodeIf = nodeIf.parentNode) {
+                if (nodeIf.alternate && !hasNodeReturn(nodeIf.consequent)) {
+                    return;
+                }
+            }
+
+            return addError(node.alternate);
+        });
+    }
+};

test/specs/rules/require-early-return.js

@@ -0,0 +1,105 @@
+var expect = require('chai').expect;
+var Checker = require('../../../lib/checker');
+
+describe('rules/require-early-return', function() {
+    var checker;
+
+    beforeEach(function() {
+        checker = new Checker();
+        checker.registerDefaultRules();
+    });
+
+    describe('with option value noElse - ', function() {
+        beforeEach(function() {
+            checker.configure({ requireEarlyReturn: 'noElse' });
+        });
+
+        var str;
+
+        describe('errors with simple if-else - ', function() {
+            it('should report the use of else after return on an if-else block', function() {
+                str = 'if (true) { return } else { }';
+                expect(checker.checkString(str)).to.have.one.validation.error.from('requireEarlyReturn');
+            });
+
+            it('should report the use of else after return', function() {
+                str = 'function foo() { if (x) { return y; } else { return z; } }';
+                expect(checker.checkString(str)).to.have.one.validation.error.from('requireEarlyReturn');
+            });
+
+            it('should report the use of else after return with small blocks', function() {
+                str = 'function foo() { if (x) { var a; return y; } else { return z; } }';
+                expect(checker.checkString(str)).to.have.one.validation.error.from('requireEarlyReturn');
+            });
+
+            it('should report the use of else after return without braces', function() {
+                str = 'function foo() { if (x) return y; else return z; }';
+                expect(checker.checkString(str)).to.have.one.validation.error.from('requireEarlyReturn');
+            });
+
+            it('should report the use of else after return when the if block contain a return', function() {
+                str = 'function test() { if (true) { return; eval() } else {} }';
+                expect(checker.checkString(str)).to.have.one.validation.error.from('requireEarlyReturn');
+            });
+        });
+
+        describe('errors with complex if-else - ', function() {
+            it('should report the use of else after return in a chain of if-else', function() {
+                str = 'function foo() { if (x) { return y; } else if (z) { return w; } else { return t; } }';
+                expect(checker.checkString(str)).to.have.errors.from('requireEarlyReturn');
+            });
+
+            it('should report the use of else after return in an if-else', function() {
+                str = 'function foo() { if (x) { return y; } else { var t = "foo"; } return t; }';
+                expect(checker.checkString(str)).to.have.one.validation.error.from('requireEarlyReturn');
+            });
+
+            it('should report the use of else after return in a nested chain of if-else', function() {
+                str = 'function foo() { if (x) { if (y) { return y; } else { return x; } } else { return z; } }';
+                expect(checker.checkString(str)).to.have.errors.from('requireEarlyReturn');
+            });
+
+            it('should report the use of else after return in a nested chain of if-else without braces', function() {
+                str = 'function foo() { if (x) if (y) return; else var b }';
+                expect(checker.checkString(str)).to.have.one.validation.error.from('requireEarlyReturn');
+            });
+
+            it('should report the use of else after return in a nested chain of if-else mixed', function() {
+                str = 'function foo() { if (x) { } else { if (y) return; else var b } }';
+                expect(checker.checkString(str)).to.have.one.validation.error.from('requireEarlyReturn');
+            });
+        });
+
+        describe('safe if-else - ', function() {
+            it('should not report on an empty if', function() {
+                str = 'function test() { if (true) { } }';
+                expect(checker.checkString(str)).to.have.no.errors();
+            });
+
+            it('should not report the use of else after return', function() {
+                str = 'function foo() { if (x) { return y; } return z; }';
+                expect(checker.checkString(str)).to.have.no.errors();
+            });
+
+            it('should not report the use of else after return with blocks 1', function() {
+                str = 'function foo() { if (x) { foo(); } else if (z) { var t = "foo"; } else { return w; } }';
+                expect(checker.checkString(str)).to.have.no.errors();
+            });
+
+            it('should not report the use of else after return with blocks 2', function() {
+                str = 'function foo() { if (x) { bar(); } else if (z) { return "foo"; } else { baz(); } }';
+                expect(checker.checkString(str)).to.have.no.errors();
+            });
+
+            it('should not report the use of else after return with blocks without braces', function() {
+                str = 'function foo() { if (x) bar(); else if (z) return "foo"; else baz(); }';
+                expect(checker.checkString(str)).to.have.no.errors();
+            });
+
+            it('should not report the use of else after return with nested if', function() {
+                str = 'function foo() { if (x) { if (z) { return y; } } else { return z; } }';
+                expect(checker.checkString(str)).to.have.no.errors();
+            });
+        });
+    });
+});