Magic number warning

config/rulesets/solium-all.js

@@ -37,6 +37,7 @@ module.exports = {
         "no-experimental": "warning",
         "max-len": "warning",
         "error-reason": "warning",
+        "magic-to-const": "warning",
 
         // Turn OFF all deprecated rules
         "double-quotes": "off",

config/rulesets/solium-recommended.js

@@ -28,6 +28,7 @@ module.exports = {
         "no-constant": "warning",
         "max-len": "warning",
         "error-reason": "warning",
+        "magic-to-const": "warning",
 
         "lbrace": "off",
         "mixedcase": "off",

config/solium.json

@@ -99,6 +99,13 @@
 			"description": "Ensure that no empty blocks {} exist"
 		},
 
+		"magic-to-const": {
+			"enabled": true,
+			"recommended": true,
+			"type": "warning",
+			"description": "Ensure that magic numbers are extracted to constants"
+		},
+
 		"no-unused-vars": {
 			"enabled": true,
 			"recommended": true,

docs/user-guide.rst

@@ -373,6 +373,9 @@ For eg- your choice of indentation might be Tab or 4 spaces or 2 spaces. What in
 +----------------------------+--------------------------------------------------------------------------------------------------------------+-----------------------------------------------------------------------------------+-------------------------------------+-------+
 | error-reason               | Ensure that error message is provided for revert and require statements                                      |            Object with "revert" and "require" keys with boolean values            | { "revert": true, "require": true } |       |
 +----------------------------+--------------------------------------------------------------------------------------------------------------+-----------------------------------------------------------------------------------+-------------------------------------+-------+
+| magic-to-const             | Ensure that magic numbers are extracted to constants                                                         |                                         -                                         |                                     |       |
++----------------------------+--------------------------------------------------------------------------------------------------------------+-----------------------------------------------------------------------------------+-------------------------------------+-------+
+
 
 .. index:: IDE and Editor integrations
 

lib/rules/magic-to-const.js

@@ -0,0 +1,57 @@
+/**
+ * @fileoverview Ensure that magic numbers are extracted to constants
+ * @author Ivan Mushketyk <[email protected]>
+ */
+
+"use strict";
+
+const allowedMagicNumbers = [0, 1];
+
+module.exports = {
+
+    meta: {
+
+        docs: {
+            recommended: true,
+            type: "warning",
+            description: "Ensure that magic numbers are extracted to constants"
+        },
+
+        schema: []
+    },
+
+    create(context) {
+
+        function inspectLiteral(emitted) {
+            if (emitted.exit) {
+                return;
+            }
+
+            const { node } = emitted,
+                parentNode = context.getSourceCode().getParent(node);
+
+            if (isNumber(node.value) && !isAConstantInitialization(parentNode) && !isAllowed(node.value)) {
+                context.report({
+                    node,
+                    message: `Value '${node.value}' should be extracted to a constant`
+                });
+            }
+        }
+
+        function isNumber(value) {
+            return typeof(value) === "number";
+        }
+
+        function isAConstantInitialization(parentNode) {
+            return (parentNode.type === "StateVariableDeclaration" && parentNode.is_constant === true);
+        }
+
+        function isAllowed(value) {
+            return allowedMagicNumbers.includes(value);
+        }
+
+        return {
+            Literal: inspectLiteral
+        };
+    }
+};

lib/rules/no-constant.js

@@ -9,7 +9,7 @@
 function create(context) {
     function reportIfConstant(emitted) {
         const { node } = emitted, enclosingFunction = context.getSourceCode().getParent(node);
- 
+
         if (emitted.exit || node.name !== "constant" || enclosingFunction.type !== "FunctionDeclaration") {
             return;
         }

test/lib/rules.js

@@ -212,7 +212,7 @@ describe("Checking exported rules object", function() {
         // We extend ALL solium core rules and eliminate a few by setting their severity to 0.
         // The rest of the rules should all be available.
         // The below count will keep changing with every change in the number of core rules that exist in solium.
-        Object.keys(ruleDescriptions).length.should.equal(26);
+        Object.keys(ruleDescriptions).length.should.equal(27);
 
         done();
     });

test/lib/rules/magic-to-const/magic-to-const.js

@@ -0,0 +1,32 @@
+/**
+ * @fileoverview Tests for magic numbers checking rule
+ * @author Ivan Mushketyk <[email protected]>
+ */
+
+"use strict";
+
+let wrappers = require("../../../utils/wrappers");
+let acceptanceCases = wrappers.acceptanceCases;
+let rejectionCases = wrappers.rejectionCases;
+
+let userConfig = {
+    "custom-rules-filename": null,
+    "rules": {
+        "magic-to-const": true
+    }
+};
+
+acceptanceCases("magic-to-const", userConfig,
+    [
+        "uint constant x = 123;",
+        "uint a = 1; uint b = 0; uint c = -1;",
+        "string a = \"abc\";",
+        "string a = \"1\";"
+    ]
+);
+
+rejectionCases("magic-to-const", userConfig,
+    [
+        "uint a = 5;"
+    ]
+);

test/lib/rules/no-constant/no-constant.js

@@ -6,62 +6,36 @@
 "use strict";
 
 const Solium = require("../../../../lib/solium");
+let wrappers = require("../../../utils/wrappers");
+let acceptanceCases = wrappers.acceptanceCases;
+let rejectionCases = wrappers.rejectionCases;
 const userConfig = {
     "rules": {
         "no-constant": "error"
     }
 };
 
-
-describe("[RULE] emit: Acceptances", () => {
-
-    it("should accept function declarations that don't have constant modifier", done => {
-        const declarations = [
-            "function foo() view pure returns(bool);",
-            "function foo() view pure returns(bool) {}",
-            "function foo() {}",
-            "function foo();",
-            "function(){}",
-            "function foo() myModifier(100, \"hello\") boo;"
-        ];
-
-        declarations.forEach(func => {
-            const issues = Solium.lint(`contract Foo { ${func} }`, userConfig);
-
-            issues.should.be.Array();
-            issues.should.be.empty();
-        });
-
-        done();
-    });
-
-});
-
-
-describe("[RULE] emit: Rejections", () => {
-
-    it("should reject function declarations that have constant modifier", done => {
-        const declarations = [
-            "function foo() view pure constant returns(bool);",
-            "function foo() constant pure returns(bool) {}",
-            "function foo() constant {}",
-            "function foo() constant;",
-            "function()constant{}",
-            "function foo() myModifier(100, \"hello\") constant boo;"
-        ];
-
-        declarations.forEach(func => {
-            const issues = Solium.lint(`contract Foo { ${func} }`, userConfig);
-
-            issues.should.be.Array();
-            issues.should.have.size(1);
-        });
-
-        done();
-    });
-
-});
-
+acceptanceCases("no-constant", userConfig,
+    [
+        "function foo() view pure returns(bool);",
+        "function foo() view pure returns(bool) {}",
+        "function foo() {}",
+        "function foo();",
+        "function(){}",
+        "function foo() myModifier(100, \"hello\") boo;"
+    ]
+);
+
+rejectionCases("no-constant", userConfig,
+    [
+        "function foo() view pure constant returns(bool);",
+        "function foo() constant pure returns(bool) {}",
+        "function foo() constant {}",
+        "function foo() constant;",
+        "function()constant{}",
+        "function foo() myModifier(100, \"hello\") constant boo;"
+    ]
+);
 
 describe("[RULE] emit: fixes", () => {
 

test/lib/solium.js

@@ -26,7 +26,7 @@ describe("Checking Exported Solium API", function() {
         Solium.should.be.type("object");
         Solium.should.be.instanceof(EventEmitter);
         Solium.should.have.size(11);
-		
+
         Solium.should.have.ownProperty("reset");
         Solium.reset.should.be.type("function");
         Solium.should.have.ownProperty("lint");
@@ -862,7 +862,7 @@ describe("Solium.lint() comment directives", () => {
         const errors = Solium.lint(code, { "extends": "solium:all" });
 
         errors.should.be.Array();
-        errors.should.have.size(10); // This no. can change if changes are made in any rules from solium:all ruleset
+        errors.should.have.size(12); // This no. can change if changes are made in any rules from solium:all ruleset
 
         Solium.reset();
         done();

test/utils/wrappers.js

@@ -7,6 +7,8 @@
 
 "use strict";
 
+let Solium = require("../../lib/solium");
+
 const { EOL } = require("os");
 
 module.exports = {
@@ -40,5 +42,47 @@ module.exports = {
     addPragma: function(code) {
         let pre = `pragma solidity ^0.4.3;${EOL.repeat(3)}`;
         return pre + code;
+    },
+
+    noErrors: function(code, userConfig) {
+        let errors = Solium.lint(code, userConfig);
+        errors.constructor.name.should.equal("Array");
+        errors.length.should.equal(0);
+    },
+
+    expectError: function(code, userConfig) {
+        let errors = Solium.lint(code, userConfig);
+        errors.constructor.name.should.equal("Array");
+        errors.length.should.equal(1);
+    },
+
+    acceptanceCases: function(ruleName, userConfig, codeSnippets) {
+        describe(`[RULE] ${ruleName}: Acceptances`, function() {
+            codeSnippets.forEach((code) => {
+                it(`should accept - ${code}`, function(done) {
+                    let contractCode = module.exports.toContract(code);
+
+                    module.exports.noErrors(contractCode, userConfig);
+
+                    Solium.reset();
+                    done();
+                });
+            });
+        });
+    },
+
+    rejectionCases: function(ruleName, userConfig, codeSnippets) {
+        describe(`[RULE] ${ruleName}: Rejections`, function() {
+            codeSnippets.forEach((code) => {
+                it(`should reject - ${code}`, function(done) {
+                    let contractCode = module.exports.toContract(code);
+
+                    module.exports.expectError(contractCode, userConfig);
+
+                    Solium.reset();
+                    done();
+                });
+            });
+        });
     }
 };