rules: add the no-modifiers-without-revert rule

README.md

@@ -22,6 +22,9 @@ In the .soliumrc.json file, add:
         "zeppelin/no-arithmetic-operations": [
           "warning"
         ],
+        "zeppelin/no-modifiers-without-revert": [
+          "warning"
+        ],
         "zeppelin/no-unchecked-send": [
           "warning"
         ],

index.js

@@ -13,6 +13,7 @@ module.exports = {
     rules: {
         "constant-candidates": require("./rules/constant-candidates"),
         "no-arithmetic-operations": require("./rules/no-arithmetic-operations"),
+        "no-modifiers-without-revert": require("./rules/no-modifiers-without-revert"),
         "no-unchecked-send": require("./rules/no-unchecked-send"),
         "no-unused-imports": require("./rules/no-unused-imports")
     }

rules/no-modifiers-without-revert.js

@@ -0,0 +1,57 @@
+/**
+ * @fileoverview Disallow modifiers without revert
+ */
+
+"use strict";
+
+module.exports = {
+    meta: {
+        docs: {
+            recommended: true,
+            type: "warning",
+            description: "Warn on modifiers without revert"
+        },
+        schema: []
+    },
+
+    create: function(context) {
+        function inspectModifierDeclaration(emitted) {
+            if (emitted.exit) {
+                return;
+            }
+            const node = emitted.node;
+            if (!hasRevertChild(node)) {
+                context.report({
+                    node: node,
+                    message: `Modifier '${node.name}' without revert.`
+                });
+            }
+        }
+
+        function hasRevertChild(node) {
+            // XXX Recursive function.
+            // --elopio - 20180105
+            if (Array.isArray(node)) {
+                return node.some(element => hasRevertChild(element));
+            } else if (node !== null && typeof node === "object") {
+                return isRevertExpression(node) ||
+                    Object.keys(node).some(key => {
+                        if (key !== "parent") {
+                            return hasRevertChild(node[key]);
+                        }
+                    });
+            }
+        }
+
+        function isRevertExpression(statement) {
+            return statement.type === "ExpressionStatement" &&
+                statement.expression.type === "CallExpression" &&
+                (statement.expression.callee.name === "require" ||
+                 statement.expression.callee.name === "revert");
+        }
+
+        return {
+            ModifierDeclaration: inspectModifierDeclaration
+        };
+    }
+};

test/no-modifiers-without-revert.js

@@ -0,0 +1,72 @@
+/**
+ * @fileoverview Tests for no-unused-imports rule
+ */
+
+"use strict";
+
+let Solium = require("solium");
+let wrappers = require("./utils/wrappers");
+let addPragma = wrappers.addPragma;
+
+let userConfig = {
+    rules: {
+        "zeppelin/no-modifiers-without-revert": 1
+    }
+};
+
+describe("[RULE] no-modifiers-without-revert: Acceptances", function() {
+
+    afterEach(function(done) {
+        Solium.reset();
+        done();
+    });
+
+    it("should accept modifier with require call", function(done) {
+        let code = "contract Dummy {\n" +
+            "modifier testModifier {\n" +
+            "    require('dummy' == 'dummy');\n" +
+            "    _;\n" +
+            "}}",
+            errors = Solium.lint(addPragma(code), userConfig);
+
+        errors.constructor.name.should.equal("Array");
+        errors.length.should.equal(0);
+
+        done();
+    });
+
+    it("should accept modifier with revert call", function(done) {
+        let code = "contract Dummy {\n" +
+            "modifier testModifier {\n" +
+            "    if ('dummy' == 'dummy') {\n" +
+            "        revert();\n" +
+            "    }\n" +
+            "    _;\n" +
+            "}}",
+            errors = Solium.lint(addPragma(code), userConfig);
+
+        errors.constructor.name.should.equal("Array");
+        errors.length.should.equal(0);
+
+        done();
+    });
+});
+
+describe("[RULE] no-modifiers-without-revert: Rejections", function() {
+    it("should reject modifiers without revert", function(done) {
+        let code = "contract Dummy {\n" +
+            "modifier testModifier {\n" +
+            "    _;\n" +
+            "}}",
+            errors = Solium.lint(addPragma(code), userConfig);
+
+        errors.constructor.name.should.equal("Array");
+        errors.length.should.equal(1);
+        errors[0].message.should.equal(
+            "Modifier 'testModifier' without revert.");
+
+        Solium.reset();
+
+        done();
+    });
+});