diff --git a/lib/rules/imports-on-top.js b/lib/rules/imports-on-top.js index a354fb9..6b1ed7c 100644 --- a/lib/rules/imports-on-top.js +++ b/lib/rules/imports-on-top.js @@ -5,6 +5,9 @@ "use strict"; +// Get platform specific newline character for applying fixes +const eol = require("os").EOL; + module.exports = { @@ -16,6 +19,8 @@ module.exports = { description: "Ensure that all import statements are on top of the file" }, + fixable: "code", + schema: [] }, @@ -23,11 +28,13 @@ module.exports = { create: function(context) { /** - * If ImportStatement node is a direct child of Program node 'parent', - * then it is an element of parent.body array. - * The node should precede any other type of node in the array except for the pragma & pragma experimental directives - * and other import statements. - */ + * If ImportStatement node is a direct child of Program node 'parent', + * then it is an element of parent.body array. + * The node should precede any other type of node in the array except for the pragma & pragma experimental directives + * and other import statements. + * The way the fix() algorithm currently works - it only fixes position of 1 import statement in a file in 1 iteration. + * This may be improved in future. + */ function inspectImportStatement(emitted) { if (emitted.exit) { return; @@ -35,23 +42,47 @@ module.exports = { const {node} = emitted, programNode = context.getSourceCode().getParent(node), + indexOfNode = programNode.body.indexOf(node), nodesAllowedAbove = ["ExperimentalPragmaStatement", "PragmaStatement", "ImportStatement"]; - for (let childNode of programNode.body) { - // If we've reached this import while traversing body, it means its position is fine. - if (node.start === childNode.start) { - return; - } + let lastValidNode = programNode.body[0]; // the first node could very well be an unacceptable one, taking care of it below + + // For every node preceding the import node check if it's one of the allowed types. + for (let childNode of programNode.body.slice(0, indexOfNode)) { // The moment we find 1 node not allowed above import, report and exit. - // TODO: write fix() for this issue: // - Remove import from current position // - Place it right before childNode.start - if (nodesAllowedAbove.indexOf(childNode.type) < 0) { + // - The fix will place it right after the last valid import node + if (!nodesAllowedAbove.includes(childNode.type)) { return context.report({ node, + fix(fixer) { + // Add the import statement after the last valid node and remove the import statement + const sourceCode = context.getSourceCode(); + const importStatement = sourceCode.getText(node); + let suffix; + + // If the last valid node is import, we place the current import right below it. + // If its a pragma directive (whether solidity or experimental), add current import after 3 EOLs + if (lastValidNode.type === "ImportStatement") { + suffix = eol; + } else if (nodesAllowedAbove.includes(lastValidNode.type)) { + // Because we've already explicitly checks for import st. above, this one basically + // checks for the remaining, ie, pragma directives + suffix = eol.repeat(3); + } else { + // If lastValidNode is not an allowed one, it means we have to insert the import ABOVE the lvn + return [fixer.insertTextBefore(lastValidNode, importStatement + eol), fixer.remove(node)]; + } + + return [fixer.insertTextAfter(lastValidNode, suffix + importStatement), fixer.remove(node)]; + }, message: "Import Statement must precede everything except pragma directives." }); + } else { + // All nodes allowed above an import statement are considered valid nodes + lastValidNode = childNode; } } } diff --git a/test/lib/rules/imports-on-top/fixes/before-pragma.sol b/test/lib/rules/imports-on-top/fixes/before-pragma.sol new file mode 100644 index 0000000..87ccb72 --- /dev/null +++ b/test/lib/rules/imports-on-top/fixes/before-pragma.sol @@ -0,0 +1,10 @@ +pragma solidity ^0.4.0; + + +contract Halo { + function foo () returns (uint) { + return 0; + } +} + +import "nano.sol"; diff --git a/test/lib/rules/imports-on-top/fixes/only-one-error.sol b/test/lib/rules/imports-on-top/fixes/only-one-error.sol new file mode 100644 index 0000000..97d4660 --- /dev/null +++ b/test/lib/rules/imports-on-top/fixes/only-one-error.sol @@ -0,0 +1,19 @@ +pragma solidity ^0.4.0; + + +import * as symbolName from "filename"; + + +contract Halo { + function foo () returns (uint) { + return 0; + } +} + +import "nano.sol"; + +library Foo { + function bar () returns (uint) { + return 1; + } +} diff --git a/test/lib/rules/imports-on-top/imports-on-top.js b/test/lib/rules/imports-on-top/imports-on-top.js index e5a7dbf..4cd89a5 100644 --- a/test/lib/rules/imports-on-top/imports-on-top.js +++ b/test/lib/rules/imports-on-top/imports-on-top.js @@ -16,6 +16,7 @@ let userConfig = { } }; + describe("[RULE] imports-on-top: Acceptances", function() { it("should accept if all import statements are on top of the file (but below the pragma directive)", function(done) { @@ -85,3 +86,564 @@ describe("[RULE] imports-on-top: Rejections", function() { }); }); + +describe("[RULE] imports-on-top: Fixes", function() { + + it("Should do nothing if source code is empty or has no import-related issues", done => { + // Absolutely empty string results in exception from Solium.lintAndFix() + let codes = [ + "// hello world", + ` + pragma solidity 0.4.0; + pragma experimental ABIEncoderV2; + + import "blah"; + import * as Lola from "blahblah.sol"; + + contract Foo {} + `, + ` + import "foobar"; + library Blah {} + `, + ` + pragma experimental ABIEncoderV2; + import "foobar"; + library Blah {} + `, + ` + contract Drone {} + library Jenny {} + ` + ]; + + codes.forEach(code => { + let { errorMessages: errors, fixedSourceCode, fixesApplied } = Solium.lintAndFix(code, userConfig); + + errors.should.be.Array(); + errors.should.be.size(0); + fixedSourceCode.should.equal(code); + fixesApplied.should.be.Array(); + fixesApplied.should.be.size(0); + }); + + Solium.reset(); + done(); + }); + + it("should place import below pragma solidity", done => { + const codes = [ + ` + pragma solidity ^0.2.3; + library Foo {} + import "wow.sol"; + `, + ` + pragma solidity ^0.2.3; + + + library Foo {} + contract WiggleWiggle{} + import "wow.sol"; + `, + ` + pragma solidity ^0.2.3; + + + library Foo {} + contract WiggleWiggle{} + import "wow.sol"; + import "mickey"; + import "minnie.sol"; + ` + ]; + const fixedCodes = [ + ` + pragma solidity ^0.2.3; + + +import "wow.sol"; + library Foo {} + + `, + ` + pragma solidity ^0.2.3; + + +import "wow.sol"; + + + library Foo {} + contract WiggleWiggle{} + + `, + ` + pragma solidity ^0.2.3; + + +import "wow.sol"; + + + library Foo {} + contract WiggleWiggle{} + + import "mickey"; + import "minnie.sol"; + ` + ]; + + + let result = Solium.lintAndFix(codes[0], userConfig); + result.errorMessages.should.be.size(0); + result.fixesApplied.should.be.size(1); + result.fixedSourceCode.should.equal(fixedCodes[0]); + + result = Solium.lintAndFix(codes[1], userConfig); + result.errorMessages.should.be.size(0); + result.fixesApplied.should.be.size(1); + result.fixedSourceCode.should.equal(fixedCodes[1]); + + result = Solium.lintAndFix(codes[2], userConfig); + result.errorMessages.should.be.size(2); + result.fixesApplied.should.be.size(1); + result.fixedSourceCode.should.equal(fixedCodes[2]); + + Solium.reset(); + done(); + }); + + it("should place import below pragma experimental", done => { + const codes = [ + ` + pragma experimental ABIEncoderV2; + library Foo {} + import "wow.sol"; + `, + ` + pragma experimental ABIEncoderV2; + + + library Foo {} + contract WiggleWiggle{} + import "wow.sol"; + `, + ` + pragma experimental ABIEncoderV2; + + + library Foo {} + contract WiggleWiggle{} + import "wow.sol"; + import "mickey"; + import "minnie.sol"; + `, + ` + pragma solidity ^0.2.3; + pragma experimental ABIEncoderV2; + library Foo {} + import "wow.sol"; + `, + ` + pragma solidity ^0.2.3; + pragma experimental ABIEncoderV2; + + + library Foo {} + contract WiggleWiggle{} + import "wow.sol"; + `, + ` + pragma solidity ^0.2.3; + pragma experimental ABIEncoderV2; + + + library Foo {} + contract WiggleWiggle{} + import "wow.sol"; + import "mickey"; + import "minnie.sol"; + ` + ]; + const fixedCodes = [ + ` + pragma experimental ABIEncoderV2; + + +import "wow.sol"; + library Foo {} + + `, + ` + pragma experimental ABIEncoderV2; + + +import "wow.sol"; + + + library Foo {} + contract WiggleWiggle{} + + `, + ` + pragma experimental ABIEncoderV2; + + +import "wow.sol"; + + + library Foo {} + contract WiggleWiggle{} + + import "mickey"; + import "minnie.sol"; + `, + ` + pragma solidity ^0.2.3; + pragma experimental ABIEncoderV2; + + +import "wow.sol"; + library Foo {} + + `, + ` + pragma solidity ^0.2.3; + pragma experimental ABIEncoderV2; + + +import "wow.sol"; + + + library Foo {} + contract WiggleWiggle{} + + `, + ` + pragma solidity ^0.2.3; + pragma experimental ABIEncoderV2; + + +import "wow.sol"; + + + library Foo {} + contract WiggleWiggle{} + + import "mickey"; + import "minnie.sol"; + ` + ]; + + + let result = Solium.lintAndFix(codes[0], userConfig); + result.errorMessages.should.be.size(0); + result.fixesApplied.should.be.size(1); + result.fixedSourceCode.should.equal(fixedCodes[0]); + + result = Solium.lintAndFix(codes[1], userConfig); + result.errorMessages.should.be.size(0); + result.fixesApplied.should.be.size(1); + result.fixedSourceCode.should.equal(fixedCodes[1]); + + result = Solium.lintAndFix(codes[2], userConfig); + result.errorMessages.should.be.size(2); + result.fixesApplied.should.be.size(1); + result.fixedSourceCode.should.equal(fixedCodes[2]); + + + result = Solium.lintAndFix(codes[3], userConfig); + result.errorMessages.should.be.size(0); + result.fixesApplied.should.be.size(1); + result.fixedSourceCode.should.equal(fixedCodes[3]); + + result = Solium.lintAndFix(codes[4], userConfig); + result.errorMessages.should.be.size(0); + result.fixesApplied.should.be.size(1); + result.fixedSourceCode.should.equal(fixedCodes[4]); + + result = Solium.lintAndFix(codes[5], userConfig); + result.errorMessages.should.be.size(2); + result.fixesApplied.should.be.size(1); + result.fixedSourceCode.should.equal(fixedCodes[5]); + + Solium.reset(); + done(); + }); + + it("should place import below existing import statement", done => { + const codes = [ + ` + pragma experimental ABIEncoderV2; + import "foobar.sol"; + library Foo {} + import "wow.sol"; + `, + ` + pragma experimental ABIEncoderV2; + import "foobar.sol"; + + + library Foo {} + contract WiggleWiggle{} + import "wow.sol"; + `, + ` + pragma experimental ABIEncoderV2; + import "foobar.sol"; + + + library Foo {} + contract WiggleWiggle{} + import "wow.sol"; + import "mickey"; + import "minnie.sol"; + `, + ` + pragma solidity ^0.2.3; + pragma experimental ABIEncoderV2; + import "foobar.sol"; + library Foo {} + import "wow.sol"; + `, + ` + pragma solidity ^0.2.3; + pragma experimental ABIEncoderV2; + import "foobar.sol"; + + + library Foo {} + contract WiggleWiggle{} + import "wow.sol"; + `, + ` + pragma solidity ^0.2.3; + pragma experimental ABIEncoderV2; + import "foobar.sol"; + + + library Foo {} + contract WiggleWiggle{} + import "wow.sol"; + import "mickey"; + import "minnie.sol"; + ` + ]; + const fixedCodes = [ + ` + pragma experimental ABIEncoderV2; + import "foobar.sol"; +import "wow.sol"; + library Foo {} + + `, + ` + pragma experimental ABIEncoderV2; + import "foobar.sol"; +import "wow.sol"; + + + library Foo {} + contract WiggleWiggle{} + + `, + ` + pragma experimental ABIEncoderV2; + import "foobar.sol"; +import "wow.sol"; + + + library Foo {} + contract WiggleWiggle{} + + import "mickey"; + import "minnie.sol"; + `, + ` + pragma solidity ^0.2.3; + pragma experimental ABIEncoderV2; + import "foobar.sol"; +import "wow.sol"; + library Foo {} + + `, + ` + pragma solidity ^0.2.3; + pragma experimental ABIEncoderV2; + import "foobar.sol"; +import "wow.sol"; + + + library Foo {} + contract WiggleWiggle{} + + `, + ` + pragma solidity ^0.2.3; + pragma experimental ABIEncoderV2; + import "foobar.sol"; +import "wow.sol"; + + + library Foo {} + contract WiggleWiggle{} + + import "mickey"; + import "minnie.sol"; + ` + ]; + + + let result = Solium.lintAndFix(codes[0], userConfig); + result.errorMessages.should.be.size(0); + result.fixesApplied.should.be.size(1); + result.fixedSourceCode.should.equal(fixedCodes[0]); + + result = Solium.lintAndFix(codes[1], userConfig); + result.errorMessages.should.be.size(0); + result.fixesApplied.should.be.size(1); + result.fixedSourceCode.should.equal(fixedCodes[1]); + + result = Solium.lintAndFix(codes[2], userConfig); + result.errorMessages.should.be.size(2); + result.fixesApplied.should.be.size(1); + result.fixedSourceCode.should.equal(fixedCodes[2]); + + + result = Solium.lintAndFix(codes[3], userConfig); + result.errorMessages.should.be.size(0); + result.fixesApplied.should.be.size(1); + result.fixedSourceCode.should.equal(fixedCodes[3]); + + result = Solium.lintAndFix(codes[4], userConfig); + result.errorMessages.should.be.size(0); + result.fixesApplied.should.be.size(1); + result.fixedSourceCode.should.equal(fixedCodes[4]); + + result = Solium.lintAndFix(codes[5], userConfig); + result.errorMessages.should.be.size(2); + result.fixesApplied.should.be.size(1); + result.fixedSourceCode.should.equal(fixedCodes[5]); + + Solium.reset(); + done(); + }); + + it("should place import on top", done => { + const codes = [ + ` + contract foo {} + import "coal.sol"; + `, + ` + contract foo {} + library bar {} + import "coal.sol"; + `, + ` + contract foo {} + library bar {} + import "coal.sol"; + import "june.sol"; + ` + ]; + const fixedCodes = [ + ` + import "coal.sol"; +contract foo {} + + `, + ` + import "coal.sol"; +contract foo {} + library bar {} + + `, + ` + import "coal.sol"; +contract foo {} + library bar {} + + import "june.sol"; + ` + ]; + + + let result = Solium.lintAndFix(codes[0], userConfig); + result.errorMessages.should.be.size(0); + result.fixesApplied.should.be.size(1); + result.fixedSourceCode.should.equal(fixedCodes[0]); + + result = Solium.lintAndFix(codes[1], userConfig); + result.errorMessages.should.be.size(0); + result.fixesApplied.should.be.size(1); + result.fixedSourceCode.should.equal(fixedCodes[1]); + + result = Solium.lintAndFix(codes[2], userConfig); + result.errorMessages.should.be.size(1); + result.fixesApplied.should.be.size(1); + result.fixedSourceCode.should.equal(fixedCodes[2]); + + Solium.reset(); + done(); + }); + + it("Should move the import statements below the last valid import node", function(done) { + let code = fs.readFileSync(path.join(__dirname, "./fixes/only-one-error.sol"), "utf8"); + let {errorMessages: errors, fixedSourceCode} = Solium.lintAndFix(code, userConfig); + + // All errors should've been corrected + errors.constructor.name.should.equal("Array"); + errors.length.should.equal(0); + + // Ensure that the bad import is moved to the right place. + const importLine = fixedSourceCode.split("\n")[4].trim(); + importLine.should.equal("import \"nano.sol\";"); + + // If we re-lint the fixedSourceCode with userConfig we should get no errors + errors = Solium.lint(fixedSourceCode, userConfig); + + // Code should've been fixed + errors.constructor.name.should.equal("Array"); + errors.length.should.equal(0); + + Solium.reset(); + done(); + }); + + it("Should move the import statements two lines below the pragma if no valid import exists", function(done) { + let code = fs.readFileSync(path.join(__dirname, "./fixes/before-pragma.sol"), "utf8"); + let {errorMessages: errors, fixedSourceCode} = Solium.lintAndFix(code, userConfig); + + // There should be no errors + errors.constructor.name.should.equal("Array"); + errors.length.should.equal(0); + + // The fixed source code should have two new lines after the first pragma solidity then have all the imports + let lines = fixedSourceCode.split("\n"); + lines[3].should.equal("import \"nano.sol\";"); + + errors = Solium.lint(fixedSourceCode, userConfig); + errors.constructor.name.should.equal("Array"); + errors.length.should.equal(0); + + Solium.reset(); + done(); + }); + + it("Should still fix the file correctly if there's only one invalid import statement", function(done) { + let code = fs.readFileSync(path.join(__dirname, "./fixes/only-one-error.sol"), "utf8"); + let {errorMessages: errors, fixedSourceCode} = Solium.lintAndFix(code, userConfig); + + // There should be no errors + errors.constructor.name.should.equal("Array"); + errors.length.should.equal(0); + + errors = Solium.lint(fixedSourceCode, userConfig); + errors.constructor.name.should.equal("Array"); + errors.length.should.equal(0); + + Solium.reset(); + done(); + }); + +}); diff --git a/test/lib/rules/imports-on-top/reject/intermingled.sol b/test/lib/rules/imports-on-top/reject/intermingled.sol index bd4bd8c..220cdab 100644 --- a/test/lib/rules/imports-on-top/reject/intermingled.sol +++ b/test/lib/rules/imports-on-top/reject/intermingled.sol @@ -1,14 +1,22 @@ +pragma solidity ^0.4.0; + + import "filename" as symbolName; + contract Halo { - function foo () { - if (true) {} - } + function foo () returns (uint) { + return 0; + } } import * as symbolName from "filename"; -library Bar {} +library Foo { + function bar () returns (uint) { + return 1; + } +} import "nano.sol"; \ No newline at end of file