From df8eff561d687116b55e6e072bc6a9121724cdc1 Mon Sep 17 00:00:00 2001 From: Leo Arias Date: Fri, 5 Jan 2018 07:53:10 +0000 Subject: [PATCH 1/3] rules: add the no-modifiers-without-revert rule --- index.js | 1 + rules/no-modifiers-without-revert.js | 57 ++++++++++++++++++++++ test/no-modifiers-without-revert.js | 72 ++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+) create mode 100644 rules/no-modifiers-without-revert.js create mode 100644 test/no-modifiers-without-revert.js diff --git a/index.js b/index.js index 2176cd4..f6682cf 100644 --- a/index.js +++ b/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") } diff --git a/rules/no-modifiers-without-revert.js b/rules/no-modifiers-without-revert.js new file mode 100644 index 0000000..42252ca --- /dev/null +++ b/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: "Disallow 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 + }; + } +}; diff --git a/test/no-modifiers-without-revert.js b/test/no-modifiers-without-revert.js new file mode 100644 index 0000000..f9435f0 --- /dev/null +++ b/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(); + }); +}); From b352bfe834916d9b7b4fa735f3fce50c1906385a Mon Sep 17 00:00:00 2001 From: Leo Arias Date: Wed, 17 Jan 2018 06:33:27 +0000 Subject: [PATCH 2/3] Update the readme --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index e726248..2542768 100644 --- a/README.md +++ b/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" ], From cbf107080631ab8d745a01a308da08bee40a7869 Mon Sep 17 00:00:00 2001 From: Leo Arias Date: Wed, 17 Jan 2018 06:34:12 +0000 Subject: [PATCH 3/3] update the description as suggested by facu --- rules/no-modifiers-without-revert.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/no-modifiers-without-revert.js b/rules/no-modifiers-without-revert.js index 42252ca..2b973a4 100644 --- a/rules/no-modifiers-without-revert.js +++ b/rules/no-modifiers-without-revert.js @@ -9,7 +9,7 @@ module.exports = { docs: { recommended: true, type: "warning", - description: "Disallow modifiers without revert" + description: "Warn on modifiers without revert" }, schema: [] },