https://gitcoin.co/issue/duaraghav8/Solium/169/780

lib/rules/indentation.js

@@ -291,6 +291,76 @@ module.exports = {
 
 
 
+        function inspectAssemblyBlock(emitted) {
+            let node = emitted.node,
+                endingLineNum = sourceCode.getEndingLine(node);
+            let assemblyDeclarationLineText, currentIndent, currentIndentLevel;
+
+            // No need to lint further if entire assembly declaration is on single line
+            if (emitted.exit || sourceCode.getLine(node) === endingLineNum) {
+                return;
+            }
+
+            assemblyDeclarationLineText = sourceCode.getTextOnLine(sourceCode.getLine(node));
+
+            currentIndent = assemblyDeclarationLineText.slice(
+                0,
+                assemblyDeclarationLineText.indexOf(assemblyDeclarationLineText.trim())
+            );
+
+            currentIndentLevel = (currentIndent.match(BASE_INDENTATION_STYLE_REGEXP_GLOBAL) || []).length;
+
+            //ensure that there is only whitespace of correct level on the line containing assembly declaration
+            if (getIndentString(BASE_INDENTATION_STYLE, currentIndentLevel) !== currentIndent) {
+                return;	//exit now, we can' proceed further unless this is fixed
+            }
+
+            // Instead of parsing all kinds of assembly constructs, I will just increase
+            // indent on { and decrease on } (skipping strings)
+            let subLevel = 1;
+            let remainingForLines = 0; // how many lines remain to indent additionally after "for" standing on a separate line
+            let forArgumentSubLevel = 0; // count open/close braces inside a "for" argument
+            for (let line = sourceCode.getLine(node) + 1; line < endingLineNum; ++line) {
+                const codeLineText = sourceCode.getTextOnLine(line);
+
+                if (codeLineText == "") continue;
+                const codeLineWithoutStrings = codeLineText.replace(/"([^"]|\\")*"|'([^']|\\')*'/g, "");
+                const numberOfOpenBraces = (codeLineWithoutStrings.match(/{/g) || []).length;
+                const numberOfClosingBraces = (codeLineWithoutStrings.match(/}/g) || []).length;
+                subLevel += numberOfOpenBraces - numberOfClosingBraces;
+                let effectiveSubLevel = subLevel;
+                if (/^\s*{\s*$/.test(codeLineText)) --effectiveSubLevel; // Don't indent opening brace.
+
+                // maybe, a little slow
+                const assemblyIndent = getIndentString(BASE_INDENTATION_STYLE, currentIndentLevel + effectiveSubLevel);
+                const assemblyIndentDesc = getIndentDescription(BASE_INDENTATION_STYLE, currentIndentLevel + effectiveSubLevel);
+                const indentRegExp = new RegExp("^" + assemblyIndent + "[^\\s(\/\*)]");
+
+                !indentRegExp.test(codeLineText) && context.report({
+                    node: node,
+                    message: `Only use indent of ${assemblyIndentDesc}.`
+                });
+
+                // Special processing for "for" standing on a separate line.
+                // After this we expect 3 indented lines (or more if there are blocks).
+                if(remainingForLines) {
+                    forArgumentSubLevel += numberOfOpenBraces - numberOfClosingBraces;
+                    if (!forArgumentSubLevel) { // A "for" argument ends here.
+                        --remainingForLines;
+                        if (!remainingForLines) --subLevel;
+                    }
+                }
+
+                if (/^\s*for\s*$/.test(codeLineText)) {
+                    // TODO: Support for "for" loop inside a for-initialization or for-increment (needs a stack).
+                    remainingForLines = 3;
+                    ++subLevel;
+                }
+            }
+        }
+
+
+
         function inspectStructDeclaration(emitted) {
             let node = emitted.node,
                 body = node.body || [],
@@ -482,6 +552,7 @@ module.exports = {
             ArrayExpression: inspectArrayExpression,
             StructDeclaration: inspectStructDeclaration,
             BlockStatement: inspectBlockStatement,
+            InlineAssemblyStatement: inspectAssemblyBlock,
             Program: inspectProgram
         };
 

lib/rules/lbrace.js

@@ -324,7 +324,7 @@ module.exports = {
         }
 
 
-        function inspectFunctionDeclaration(emitted) {
+        function inspectFunctionOrConstructorDeclaration(emitted) {
             let node = emitted.node;
 
             if (emitted.exit || node.is_abstract) {
@@ -341,13 +341,17 @@ module.exports = {
                 declarationStartingLine = sourceCode.getLine(node),
                 bodyStartingLine = sourceCode.getLine(node.body);
 
+            // Don't write like this:
+            // if (node.returnParams !== null)
+            // because for contructor they may be undefined.
+
             //If the node just before function's body is on the start line itself, then the opening brace must too be on the start line
-            if (node.returnParams !== null) {
+            if (node.returnParams) {
                 declarationEndingLine = sourceCode.getEndingLine(node.returnParams);
-            } else if (node.modifiers !== null) {
+            } else if (node.modifiers) {
                 //if modifiers is non-null (,i.e., array), compare bodyStartingLine with ending line of the last modifier declaration (last array element)
                 declarationEndingLine = sourceCode.getEndingLine(node.modifiers.slice(-1) [0]);
-            } else if (node.params !== null) {
+            } else if (node.params) {
                 //if params is non-null (,i.e., array), compare bodyStartingLine with ending line of the last param declaration (last array element)
                 declarationEndingLine = sourceCode.getEndingLine(node.params.slice(-1) [0]);
             } else {
@@ -408,7 +412,8 @@ module.exports = {
 
 
         return {
-            FunctionDeclaration: inspectFunctionDeclaration,
+            FunctionDeclaration: inspectFunctionOrConstructorDeclaration,
+            ConstructorDeclaration: inspectFunctionOrConstructorDeclaration,
             LibraryStatement: inspectLibraryStatement,
             ContractStatement: inspectContractStatement,
             StructDeclaration: inspectStructDeclaration,

lib/rules/no-empty-blocks.js

@@ -34,10 +34,6 @@ module.exports = {
             return node.type === "ConstructorDeclaration";
         }
 
-        function isFuncOrConstructor(node) {
-            return isFunc(node) || isConstructor(node);
-        }
-
         function isFallbackFunc(node) {
             return isFunc(node) && node.name === null;
         }
@@ -49,8 +45,11 @@ module.exports = {
             return false;
         }
 
-        function isPayableFuncOrCons(node) {
-            return isFuncOrConstructor(node) && isPayable(node);
+        function hasNonTrivialModifiers(node) {
+            if (!node.modifiers) return false;
+
+            const re = /^(external|public|internal|private|pure|view|payable)$/;
+            return node.modifiers.filter(m => !re.test(m.name)).length !== 0;
         }
 
         function inspectCLI(emitted) {
@@ -70,80 +69,54 @@ module.exports = {
             const { node } = emitted, { body } = node,
                 parent = context.getSourceCode().getParent(node);
 
-            if (emitted.exit || isFallbackFunc(parent) || isPayableFuncOrCons(parent)) {
+            // Historical note: No need to check (as suggested in #264) that a modifier is calling a base constructor,
+            // because ANY modifier in constructor should make its body not required.
+            if (emitted.exit || isConstructor(parent) || isFunc(parent)) {
+                // Use inspectFunctionDeclaration or inspectConstructorDeclaration logic instead,
+                // not to report the error twice.
                 return;
             }
 
             if (Array.isArray(body) && body.length === 0) {
-                if (isConstructor(parent)) {
-                    // If an empty block is a constructor func's body, then it shouldn't
-                    // be reported if the constructor is calling a base constructor.
-                    // The cons. must have a modifier with a name that is also present
-                    // in its enclosing contract's inheritance declaration. Eg-
-                    //
-                    // contract Foo is Bar {
-                    //   constructor() Bar("hello") {}
-                    // }
-                    //
-                    // See issue #264
-                    //
-                    // Since we can't fetch enclosing contract from the BlockStatement
-                    // node or its parent (due to current limitations), we use a
-                    // workaround to not report such a constructor.
-                    constructorsToCheckForBaseCall.push(parent.start);
-                    return;
-                }
-
                 report(node);
             }
         }
 
-        function inspectConstructorDeclaration(emitted) {
+        function inspectFunctionDeclaration(emitted) {
             const { node } = emitted;
 
-            if (!emitted.exit) {
-                // Because parent of a node is not accessible during exit phase,
-                // cache the parents of all constructors during entry so they
-                // can be used during exit.
-                enclosingContractsOfConstructors[node.start] = context.getSourceCode().getParent(node);
+            if (emitted.exit) {
                 return;
             }
 
-            // No need to check the constructor currently being exited if it
-            // isn't even flagged for empty block.
-            if (!constructorsToCheckForBaseCall.includes(node.start)) {
-                return;
+            // If node.modifiers is null, it means no modifiers exist for this
+            // function and it should therefore be reported.
+            if (node.body.body.length === 0 && !hasNonTrivialModifiers(node) && !isFallbackFunc(node) && !isPayable(node)) {
+                report(node.body);
             }
+        }
+
+        function inspectConstructorDeclaration(emitted) {
+            const { node } = emitted;
 
-            // Run constructor inspection while exiting nodes.
-            // By this time, the constructorsToCheckForBaseCall list has been
-            // populated.
-            const enclosingContract = enclosingContractsOfConstructors[node.start];
+            if (emitted.exit) {
+                return;
+            }
 
             // If node.modifiers is null, it means no modifiers exist for this
             // constructor and it should therefore be reported.
-            for (let i = 0; i < (node.modifiers || []).length; i++) {
-                const functionModif = node.modifiers[i].name;
-
-                for (let j = 0; j < enclosingContract.is.length; j++) {
-                    // The constructor is calling a base cons, no need
-                    // to report it.
-                    if (enclosingContract.is[j].name === functionModif) {
-                        return;
-                    }
-                }
+            if (node.body.body.length === 0 && !hasNonTrivialModifiers(node) && !isPayable(node)) {
+                report(node.body);
             }
-
-            report(node.body);
         }
 
 
-        let constructorsToCheckForBaseCall = [], enclosingContractsOfConstructors = {};
         const similarNodeTypes = ["ContractStatement", "LibraryStatement", "InterfaceStatement"];
 
         const response = {
             BlockStatement: inspectBlockStatement,
-            ConstructorDeclaration: inspectConstructorDeclaration
+            ConstructorDeclaration: inspectConstructorDeclaration,
+            FunctionDeclaration: inspectFunctionDeclaration
         };
 
         similarNodeTypes.forEach(function(nodeName) {

package.json

@@ -1,6 +1,6 @@
 {
   "name": "ethlint",
-  "version": "1.2.4",
+  "version": "1.2.5",
   "description": "Linter to identify and fix Style & Security issues in Solidity",
   "main": "./lib/solium.js",
   "bin": {

test/lib/rules/indentation/accept/assembly.sol

@@ -0,0 +1,36 @@
+pragma solidity ^0.4.4;
+
+library GetCode {
+    function at(address _addr) public view returns (bytes memory o_code) {
+        assembly {
+            let len := mload(_data)
+
+            let data := add(_data, 0x20)
+
+            for
+                { let end := add(data, mul(len, 0x20)) }
+                lt(data, end)
+                { data := add(data, 0x20) }
+            {
+                sum := add(sum, mload(data))
+            }
+
+            for
+                {
+                    let end := add(data, mul(len, 0x20))
+                }
+                lt(data, end)
+                {
+                    data := add(data, 0x20)
+                }
+            {
+                sum := add(sum, mload(data))
+            }
+
+            for { let end := add(data, mul(len, 0x20)) } lt(data, end) { data := add(data, 0x20) }
+            {
+                sum := add(sum, mload(data))
+            }
+        }
+    }
+}

test/lib/rules/indentation/accept/one-line-assembly.sol

@@ -0,0 +1,7 @@
+pragma solidity ^0.4.4;
+
+library GetCode {
+    function at(address _addr) public view returns (bytes memory o_code) {
+        assembly { o_code := mload(0x40) }
+    }
+}

test/lib/rules/indentation/indentation.js

@@ -165,6 +165,44 @@ describe("[RULE] indentation: Acceptances", function() {
         done();
     });
 
+    it("should accept a file with an inline assembly", function(done) {
+        let userConfig = {
+            "rules": {
+                "indentation": "error"
+            }
+        };
+
+        let file = "assembly.sol";
+        let code = fs.readFileSync(path.join(acceptDir, file), "utf8");
+
+        let errors = Solium.lint(code, userConfig);
+
+        errors.constructor.name.should.equal("Array");
+        errors.length.should.equal(0);
+
+        Solium.reset();
+        done();
+    });
+
+    it("should accept a file with an assembly in one line", function(done) {
+        let userConfig = {
+            "rules": {
+                "indentation": "error"
+            }
+        };
+
+        let file = "one-line-assembly.sol";
+        let code = fs.readFileSync(path.join(acceptDir, file), "utf8");
+
+        let errors = Solium.lint(code, userConfig);
+
+        errors.constructor.name.should.equal("Array");
+        errors.length.should.equal(0);
+
+        Solium.reset();
+        done();
+    });
+
     it("should accept a file with an array in one line", function(done) {
         let userConfig = {
             "rules": {
@@ -357,6 +395,25 @@ describe("[RULE] indentation: Rejections", function() {
         done();
     });
 
+    it("should reject an invalid file with an inline assembly", function(done) {
+        let userConfig = {
+            "rules": {
+                "indentation": "error"
+            }
+        };
+
+        let file = "assembly.sol";
+        let code = fs.readFileSync(path.join(rejectDir, file), "utf8");
+
+        let errors = Solium.lint(code, userConfig);
+
+        errors.constructor.name.should.equal("Array");
+        errors.length.should.equal(4);
+
+        Solium.reset();
+        done();
+    });
+
     it("should reject top level indentation", function(done) {
         let userConfig = {
             "rules": {