Implements "magic-number" rule

config/rulesets/solium-all.js

@@ -22,6 +22,7 @@ module.exports = {
         "indentation": "warning",
         "arg-overflow": "warning",
         "whitespace": "warning",
+        "magic-number": "warning",
         "function-whitespace": "error",
         "semicolon-whitespace": "error",
         "comma-whitespace": "error",

config/rulesets/solium-recommended.js

@@ -25,6 +25,7 @@ module.exports = {
         "operator-whitespace": "warning",
         "emit": "warning",
         "no-constant": "warning",
+        "magic-number": "warning",
 
         "lbrace": "off",
         "mixedcase": "off",

config/solium.json

@@ -99,6 +99,13 @@
 			"description": "Ensure that no empty blocks {} exist"
 		},
 
+		"magic-number": {
+			"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

@@ -347,6 +347,8 @@ For eg- your choice of indentation might be Tab or 4 spaces or 2 spaces. What in
 +----------------------------+--------------------------------------------------------------------------------------------------------------+-----------------------------------------------------------------------------------+-----------------+-------+
 | quotes                     | Ensure that all strings use only 1 style - either double quotes or single quotes                             |                    Single option - either "double" or "single"                    | double          | YES   |
 +----------------------------+--------------------------------------------------------------------------------------------------------------+-----------------------------------------------------------------------------------+-----------------+-------+
+| magic-number               | Ensure that magic numbers are extracted to constants                                                         |                                         -                                         |                 |       |
++----------------------------+--------------------------------------------------------------------------------------------------------------+-----------------------------------------------------------------------------------+-----------------+-------+
 | blank-lines                | Ensure that there is exactly a 2-line gap between Contract and Funtion declarations                          |                                         -                                         |                 | YES   |
 +----------------------------+--------------------------------------------------------------------------------------------------------------+-----------------------------------------------------------------------------------+-----------------+-------+
 | indentation                | Ensure consistent indentation of 4 spaces per level                                                          |            either "tab" or an integer representing the number of spaces           | 4 spaces        |       |

lib/rules/double-quotes.js

@@ -55,7 +55,7 @@ module.exports = {
             if (!doubleQuotesLiteralRegExp.test(nodeText)) {
                 context.report({
                     node: node,
-                    message: "'" + node.value + "': String Literals must be quoted with \"double quotes\" only."
+                    message: `'${node.value}': String Literals must be quoted with "double quotes" only.`
                 });
             }
         }

lib/rules/magic-number.js

@@ -0,0 +1,60 @@
+/**
+ * @fileoverview Ensure that magic numbers are extracted to constants
+ * @author Ivan Mushketyk <[email protected]>
+ */
+
+"use strict";
+
+module.exports = {
+
+    meta: {
+
+        docs: {
+            recommended: true,
+            type: "warning",
+            description: "Ensure that magic numbers are extracted to constants",
+            replacedBy: ["quotes"]
+        },
+
+        schema: [],
+        deprecated: true
+
+    },
+
+    create: function(context) {
+
+        function inspectLiteral(emitted) {
+            if (emitted.exit) {
+                return;
+            }
+
+            let parentNode = context.getSourceCode().getParent(emitted.node);
+            if (_isNumber(emitted.node.value) &&
+                !_constantInitialization(parentNode) &&
+                !_allowedMagicNumber(emitted.node.value)) {
+
+                context.report({
+                    node: emitted.node,
+                    message: `Value '${emitted.node.value}' should be extracted to a constant`
+                });
+            }
+        }
+
+        function _isNumber(value) {
+            return typeof(value) === "number";
+        }
+
+        function _constantInitialization(parentNode) {
+            return parentNode.type === "StateVariableDeclaration" &&
+                    parentNode.is_constant === true;
+        }
+
+        function _allowedMagicNumber(value) {
+            return value === 0 || value === 1;
+        }
+
+        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(22);
+        Object.keys(ruleDescriptions).length.should.equal(23);
 
         done();
     });

test/lib/rules/magic-number/magic-number.js

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