error-reason: add errorMessageMaxLength option

duaraghav8 · Jul 5, 2019 · e11b680 · e11b680
1 parent f48e0b5
commit e11b680
Show file tree

Hide file tree

Showing 4 changed files with 116 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,8 @@
 # Changelog
 
+## 1.2.5 ()
+- Added option `errorMessageMaxLength` for rule `error-reason` to specify a character limit on error message.
+
 ## 1.2.4 (2019-04-08)
 - Added rule `no-trailing-whitespace` to warn the user when code, comment or blank lines contain trailing whitespaces. This rule will supply the `fix` functionality in a future release.
 - Added `getLines()` sourceCode utility function for rule developers. This method returns the source code split into lines.

diff --git a/docs/user-guide.rst b/docs/user-guide.rst
@@ -409,7 +409,7 @@ For eg- your choice of indentation might be Tab or 4 spaces or 2 spaces. What in
 +----------------------------+--------------------------------------------------------------------------------------------------------------+-----------------------------------------------------------------------------------+------------------------------------------------------+-------+
 | max-len                    | Ensure that a line of code doesn't exceed the specified number of characters                                 |  Single integer representing the number of characters to allow per line of code   | 145                                                  |       |
 +----------------------------+--------------------------------------------------------------------------------------------------------------+-----------------------------------------------------------------------------------+------------------------------------------------------+-------+
-| 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 }                  |       |
+| error-reason               | Ensure that error message is provided for revert and require statements                                      |Object with "revert" and "require" booleans & "errorMessageMaxLength" unsigned int | { "revert": true, "require": true, "errorMessageMaxLength": 76 }|       |
 +----------------------------+--------------------------------------------------------------------------------------------------------------+-----------------------------------------------------------------------------------+------------------------------------------------------+-------+
 | visibility-first           | Ensure that the visibility modifier for a function should come before any custom modifiers                   |                                         -                                         |                                                      |       |
 +----------------------------+--------------------------------------------------------------------------------------------------------------+-----------------------------------------------------------------------------------+------------------------------------------------------+-------+

diff --git a/lib/rules/error-reason.js b/lib/rules/error-reason.js
@@ -19,9 +19,9 @@ module.exports = {
             type: "object",
             properties: {
                 revert: { type: "boolean" },
-                require: { type: "boolean" }
+                require: { type: "boolean" },
+                errorMessageMaxLength: { type: "integer", minimum: 1 }
             },
-            required: ["revert", "require"],
             additionalProperties: false
         }]
 
@@ -37,20 +37,47 @@ module.exports = {
             }
 
             const { name } = node.callee, callArgs = node.arguments;
-            const options = context.options && context.options[0] || { revert: true, require: true };
+            const options = { revert: true, require: true, errorMessageMaxLength: 76 };
+            let callArgsErrorMessageIndex = -1;
 
-            if (options.revert && name === "revert" && callArgs.length < 1) {
-                return context.report({
-                    node,
-                    message: "Provide an error message for revert()."
-                });
+            if (context.options) {
+                const { revert: rev, require: req, errorMessageMaxLength: max } = context.options[0];
+
+                if (rev !== undefined) {
+                    options.revert = rev;
+                }
+                if (req !== undefined) {
+                    options.require = req;
+                }
+                if (max !== undefined) {
+                    options.errorMessageMaxLength = max;
+                }
+            }
+
+            if (options.revert && name === "revert") {
+                if (callArgs.length < 1) {
+                    return context.report({ node, message: "Provide an error message for revert()" });
+                }
+                callArgsErrorMessageIndex = 0; // Access the first call arg to get error message
             }
 
-            if (options.require && name === "require" && callArgs.length < 2) {
-                context.report({
-                    node,
-                    message: "Provide an error message for require()."
-                });
+            if (options.require && name === "require") {
+                if (callArgs.length < 2) {
+                    return context.report({ node, message: "Provide an error message for require()" });
+                }
+                callArgsErrorMessageIndex = 1; // Access the second call arg to get error message
+            }
+
+            // If the function does contain the error message, validate it for max length
+            if (callArgsErrorMessageIndex > -1) {
+                const { value: errMessage } = callArgs[callArgsErrorMessageIndex];
+
+                if (typeof errMessage === "string" && errMessage.length > options.errorMessageMaxLength) {
+                    return context.report({
+                        node,
+                        message: `Error message exceeds max length of ${options.errorMessageMaxLength} characters`
+                    });
+                }
             }
         }
 

diff --git a/test/lib/rules/error-reason/error-reason.js b/test/lib/rules/error-reason/error-reason.js
@@ -72,6 +72,54 @@ describe("[RULE] error-reason: Acceptances", function() {
         done();
     });
 
+    it("should accept error messages that don't breach max character limit", done => {
+        const userConfig = {
+            "rules": {
+                "error-reason": ["warning", { "errorMessageMaxLength": 10 }]
+            }
+        };
+
+        const snippets = [
+            toFunction("require(1 == 1, \"123456789\");"),
+            toFunction("revert(\"123456789\");"),
+            toFunction("require(1 == 1, \"0123456789\");"),
+            toFunction("revert(\"0123456789\");"),
+            toFunction("require(1 == 1, \"\");"),
+            toFunction("revert(\"\");")
+        ];
+
+        snippets.forEach(code => {
+            const errors = Solium.lint(code, userConfig);
+            errors.should.be.Array();
+            errors.should.be.empty();
+        });
+
+        Solium.reset();
+        done();
+    });
+
+    it("should accept error messages that breach max character limit when function is disabled", done => {
+        const userConfig = {
+            "rules": {
+                "error-reason": ["warning", { "revert": false, "require": false, "errorMessageMaxLength": 10 }]
+            }
+        };
+
+        const snippets = [
+            toFunction("require(1 == 1, \"12345shdh782728617626789\");"),
+            toFunction("revert(\"12310-2908sjbjsb456789\");")
+        ];
+
+        snippets.forEach(code => {
+            const errors = Solium.lint(code, userConfig);
+            errors.should.be.Array();
+            errors.should.be.empty();
+        });
+
+        Solium.reset();
+        done();
+    });
+
 });
 
 describe("[RULE] error-reason: Rejections", function() {
@@ -120,5 +168,29 @@ describe("[RULE] error-reason: Rejections", function() {
         done();
     });
 
+    it("should reject error messages that breach max character limit", done => {
+        const userConfig = {
+            "rules": {
+                "error-reason": ["warning", { "errorMessageMaxLength": 10 }]
+            }
+        };
+
+        const snippets = [
+            toFunction("require(1 == 1, \"0123456789-\");"),
+            toFunction("revert(\"0123456789-\");"),
+            toFunction("require(1 == 1, \"0123456789-----------\");"),
+            toFunction("revert(\"0123456789----------\");")
+        ];
+
+        snippets.forEach(code => {
+            const errors = Solium.lint(code, userConfig);
+            errors.should.be.Array();
+            errors.should.have.size(1);
+        });
+
+        Solium.reset();
+        done();
+    });
+
 });