From 4ce701343c3e17445aad411c7f24f4fd641f3209 Mon Sep 17 00:00:00 2001 From: GabrielAlacchi Date: Tue, 26 Dec 2017 22:21:57 -0500 Subject: [PATCH 1/3] Added autofix functions for blank-lines rule + test cases for blank-lines --- lib/rules/blank-lines.js | 100 +++++++++++++-- test/lib/rules/blank-lines/blank-lines.js | 147 ++++++++++++++++++++++ 2 files changed, 234 insertions(+), 13 deletions(-) diff --git a/lib/rules/blank-lines.js b/lib/rules/blank-lines.js index 45dbfd5..6efd584 100644 --- a/lib/rules/blank-lines.js +++ b/lib/rules/blank-lines.js @@ -6,9 +6,22 @@ "use strict"; -// TODO: generalize this method for N consec. blank lines in future -function has2ConsecutiveBlankLines(string) { - return /\n[ \t]*\n[ \t]*\n/.test(string); +let eol = require("os").EOL; + + +function hasNConsecutiveBlankLines(string, n) { + let pattern = `(\r?\n[ \t]*){${n},}\r?\n`; + return new RegExp(pattern).test(string); +} + + +function getCorrectionString(string, n) { + let matches = string.match(/(\r?\n[ \t]*)/g); + if (!matches) { + return eol.repeat(n + 1); + } + let number = Math.max(0, n + 1 - matches.length); + return eol.repeat(number); } @@ -19,9 +32,11 @@ module.exports = { docs: { recommended: true, type: "warning", - description: "Ensure that there is exactly a 2-line gap between Contract and Funtion declarations" + description: "Ensure that there is exactly a 2-line gap between Contract and Function declarations" }, + fixable: "whitespace", + schema: [] }, @@ -29,8 +44,9 @@ module.exports = { create: function(context) { let sourceCode = context.getSourceCode(), - noCodeRegExp = /^[ \t]*$/, - isCommentRegExp = /\s*(\/\/*)|(\*\\*)|(\/\**)$/; + noCodeRegExp = /^\s*$/, + isCommentRegExp = /\s*(\/\/*)|(\*\\*)|(\/\**)$/, + strictCommentRegExp = /^\s*(\/\/*)|(\*\\*)|(\/\**)$/; // Tests whether the entire line is a comment let topLevelDeclarations = [ "ContractStatement", "LibraryStatement" ]; function inspectProgram(emitted) { @@ -38,11 +54,63 @@ module.exports = { return; } - function report(topLevelNode) { + function report(topLevelNode, spacing) { const message = `${topLevelNode.type.replace("Statement", "")} '${topLevelNode.name}' ` + "must be preceded by 2 blank lines."; - context.report({ node: topLevelNode, message }); + context.report({ + node: topLevelNode, message, + fix(fixer) { + // The fix strategy is as follows: + // - count lines going upwards (without going beyond the bounds of the spacing between this node and the previous one) + // - Skip over any accompanying comments of the top level node + // - Count the number of blank lines directly above the top-most accompanying comments iteratively + // - Insert the desired number of line break characters right before the comment + + let line = sourceCode.getLine(topLevelNode) - 1, // The line number + maxLines = spacing.split("\n").length, + bottomLine = sourceCode.getLine(topLevelNode), + multipleLines = spacing.indexOf("\n") > -1, // Whether there are multiple lines + insertAt = topLevelNode.start, // The point to insert the new line characters at + numberOfBreaks = multipleLines ? 1 : 0; // The number of line breaks there are currently (default to 1 if there are multiple lines) + + // If there are multiple lines, manually count the number of lines (after skipping the accompanying comments) + if (multipleLines) { + // Subtract by the column number to preserve any indentation when inserting the newlines during the fix + insertAt -= sourceCode.getColumn(topLevelNode); + + // Skip over all the node's accompanying comments + while (strictCommentRegExp.test(sourceCode.getTextOnLine(line))) { + insertAt -= sourceCode.getTextOnLine(line).length + 1; + + let text = sourceCode.getTextOnLine(line); + + // Multi line block comment, jump over the entire thing + if (text.indexOf("*/") > -1 && text.indexOf("/*") === -1) { + let rest = sourceCode.getText().slice(0, insertAt); + let blockCommentStart = rest.lastIndexOf("/*"); + + // Decrement the line number by the number of lines the block comment takes up + line -= sourceCode.getText().slice(blockCommentStart, insertAt).split("\n").length; + // Set the endpoint to the end of the line above the block comment + insertAt = sourceCode.getText().slice(0, blockCommentStart).lastIndexOf("\n") + 1; + } else { + // If it's just a regular comment, decrement the line number + line--; + } + } + + // Count how many blank lines there are currently + while (bottomLine - line < maxLines && line > 0 && noCodeRegExp.test(sourceCode.getTextOnLine(line--))) { + numberOfBreaks++; + } + } + + // Repeat the end-of-line string to ensure there are 2 blank lines + const correction = Math.max(0, 3 - numberOfBreaks); + return [fixer.insertTextAt(insertAt, eol.repeat(correction))]; + } + }); } const {node} = emitted, programBody = node.body; @@ -54,8 +122,9 @@ module.exports = { continue; } - if (!has2ConsecutiveBlankLines(sourceCode.getStringBetweenNodes(prevNode, currNode))) { - report(currNode); + const spacing = sourceCode.getStringBetweenNodes(prevNode, currNode); + if (!hasNConsecutiveBlankLines(spacing, 2)) { + report(currNode, spacing); } } } @@ -64,10 +133,14 @@ module.exports = { function inspectChild(emitted) { let node = emitted.node, body = node.body || []; - function report(node) { + function report(node, spacing) { context.report({ node: node, - message: node.type + " must be succeeded by 1 blank line" + message: node.type + " must be succeeded by 1 blank line", + fix(fixer) { + // Add the correct number of line breaks at the after the node + return [fixer.insertTextAfter(node, getCorrectionString(spacing, 1))]; + } }); } @@ -91,7 +164,8 @@ module.exports = { !isCommentRegExp.test(c) && ((!noCodeRegExp.test(a)) || noCodeRegExp.test(b) || endingLineNumber === sourceCode.getLine(body [i+1])) ) { - report(body [i]); + const spacing = sourceCode.getStringBetweenNodes(body[i], body[i + 1]); + report(body [i], spacing); } } } diff --git a/test/lib/rules/blank-lines/blank-lines.js b/test/lib/rules/blank-lines/blank-lines.js index 8e8b6df..fc35ebc 100644 --- a/test/lib/rules/blank-lines/blank-lines.js +++ b/test/lib/rules/blank-lines/blank-lines.js @@ -330,3 +330,150 @@ describe("[RULE] blank-lines: Rejections", function() { }); }); + + +describe("[RULE] blank-lines: Fixes", function() { + + it("Should correct spacing between top level declarations with < 2 lines of gap between them", function(done) { + const code = fs.readFileSync(path.join(__dirname, "./reject/contract.sol"), "utf8"); + let {errorMessages: errors, fixedSourceCode} = Solium.lintAndFix(addPragma(code), userConfig); + + errors.constructor.name.should.equal("Array"); + errors.length.should.equal(0); + + // The fixes should have fixed all linting errors with respect to this rule + errors = Solium.lint(fixedSourceCode, userConfig); + + errors.constructor.name.should.equal("Array"); + errors.length.should.equal(0); + + Solium.reset(); + done(); + }); + + it("Should correct multiline functions not followed by a blank line", function(done) { + let code = fs.readFileSync(path.join(__dirname, "./reject/function.sol"), "utf8"), + {errorMessages: errors, fixedSourceCode} = Solium.lintAndFix(addPragma(code), userConfig); + + errors.constructor.name.should.equal("Array"); + errors.length.should.equal(0); + + // The fixes should have fixed all linting errors with respect to this rule + errors = Solium.lint(fixedSourceCode, userConfig); + + errors.constructor.name.should.equal("Array"); + errors.length.should.equal(0); + + Solium.reset(); + done(); + }); + + it("should correct top level declarations accompanied by comments but not gapped properly", function(done) { + let snippets = [ + ` + pragma solidity ^0.4.17; + // import 'zeppelin-solidity/contracts/token/StandardToken.sol'; + + contract Test { + + function Test () { + } + + } + `, + ` + pragma solidity ^0.4.17; + + // import 'zeppelin-solidity/contracts/token/StandardToken.sol'; + contract Test { + + function Test () { + } + + } + `, + ` + pragma solidity ^0.4.17; + // import 'zeppelin-solidity/contracts/token/StandardToken.sol'; + + // import 'zeppelin-solidity/contracts/token/StandardToken.sol'; + contract Test { + + function Test () { + } + + } + `, + ` + pragma solidity ^0.4.17; + + /* import 'zeppelin-solidity/contracts/token/StandardToken.sol'; */ + + contract Test { + + function Test () { + } + + } + `, + ` + pragma solidity ^0.4.17; + + /* import 'zeppelin-solidity/contracts/token/StandardToken.sol'; */ + + contract Test { + + function Test () { + } + + } + `, + ` + pragma solidity ^0.4.17; + + /* import 'zeppelin-solidity/contracts/token/StandardToken.sol'; */ + + /* import 'zeppelin-solidity/contracts/token/StandardToken.sol'; */ + + contract Test { + + function Test () { + } + + } + `, + ` + pragma solidity ^0.4.17; + + /* + This is a test for block comments + that extend across multiple lines + */ + contract Test { + + } + `, + ` + pragma solidity ^0.4.17; + + + contract T {} // Test + contract S {} + ` + ]; + + snippets.forEach(code => { + let {errorMessages: errors, fixedSourceCode} = Solium.lintAndFix(code, userConfig); + errors.should.be.Array(); + errors.should.have.size(0); + + errors = Solium.lint(fixedSourceCode, userConfig); + errors.should.be.Array(); + errors.should.have.size(0); + }); + + Solium.reset(); + done(); + }); + +}); From d367da148992d4a36788e0777fa3f6aa306ede0b Mon Sep 17 00:00:00 2001 From: GabrielAlacchi Date: Sat, 6 Jan 2018 16:43:53 -0500 Subject: [PATCH 2/3] Refactoring blank-lines rule and test to use ES6 --- lib/rules/blank-lines.js | 14 ++++----- test/lib/rules/blank-lines/blank-lines.js | 36 +++++++++++------------ 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/lib/rules/blank-lines.js b/lib/rules/blank-lines.js index 6efd584..e77de53 100644 --- a/lib/rules/blank-lines.js +++ b/lib/rules/blank-lines.js @@ -6,7 +6,7 @@ "use strict"; -let eol = require("os").EOL; +const eol = require("os").EOL; function hasNConsecutiveBlankLines(string, n) { @@ -41,13 +41,13 @@ module.exports = { }, - create: function(context) { + create(context) { - let sourceCode = context.getSourceCode(), + const sourceCode = context.getSourceCode(), noCodeRegExp = /^\s*$/, isCommentRegExp = /\s*(\/\/*)|(\*\\*)|(\/\**)$/, strictCommentRegExp = /^\s*(\/\/*)|(\*\\*)|(\/\**)$/; // Tests whether the entire line is a comment - let topLevelDeclarations = [ "ContractStatement", "LibraryStatement" ]; + const topLevelDeclarations = [ "ContractStatement", "LibraryStatement" ]; function inspectProgram(emitted) { if (emitted.exit) { @@ -70,7 +70,7 @@ module.exports = { let line = sourceCode.getLine(topLevelNode) - 1, // The line number maxLines = spacing.split("\n").length, bottomLine = sourceCode.getLine(topLevelNode), - multipleLines = spacing.indexOf("\n") > -1, // Whether there are multiple lines + multipleLines = spacing.includes('\n'), // Whether there are multiple lines insertAt = topLevelNode.start, // The point to insert the new line characters at numberOfBreaks = multipleLines ? 1 : 0; // The number of line breaks there are currently (default to 1 if there are multiple lines) @@ -86,7 +86,7 @@ module.exports = { let text = sourceCode.getTextOnLine(line); // Multi line block comment, jump over the entire thing - if (text.indexOf("*/") > -1 && text.indexOf("/*") === -1) { + if (text.includes("*/") && !text.includes("/*")) { let rest = sourceCode.getText().slice(0, insertAt); let blockCommentStart = rest.lastIndexOf("/*"); @@ -118,7 +118,7 @@ module.exports = { for (let i = 1; i < programBody.length; i++) { const currNode = programBody [i], prevNode = programBody [i-1]; - if (topLevelDeclarations.indexOf(currNode.type) < 0) { + if (!topLevelDeclarations.includes(currNode.type)) { continue; } diff --git a/test/lib/rules/blank-lines/blank-lines.js b/test/lib/rules/blank-lines/blank-lines.js index fc35ebc..b10c282 100644 --- a/test/lib/rules/blank-lines/blank-lines.js +++ b/test/lib/rules/blank-lines/blank-lines.js @@ -5,23 +5,23 @@ "use strict"; -let Solium = require("../../../../lib/solium"), +const Solium = require("../../../../lib/solium"), wrappers = require("../../../utils/wrappers"), fs = require("fs"), path = require("path"); -let userConfig = { +const userConfig = { "custom-rules-filename": null, "rules": { "blank-lines": true } }; -let addPragma = wrappers.addPragma; +const addPragma = wrappers.addPragma; -describe("[RULE] blank-lines: Acceptances", function() { +describe("[RULE] blank-lines: Acceptances", () => { - it("should accept contract declarations succeeded by 2 blank lines (all declarations except for last)", function(done) { + it("should accept contract declarations succeeded by 2 blank lines (all declarations except for last)", done => { let code = fs.readFileSync(path.join(__dirname, "./accept/contract.sol"), "utf8"), errors = Solium.lint(addPragma(code), userConfig); @@ -32,7 +32,7 @@ describe("[RULE] blank-lines: Acceptances", function() { done(); }); - it("should accept library declarations succeeded by 2 blank lines (all declarations except for last)", function(done) { + it("should accept library declarations succeeded by 2 blank lines (all declarations except for last)", done => { let code = fs.readFileSync(path.join(__dirname, "./accept/library.sol"), "utf8"), errors = Solium.lint(addPragma(code), userConfig); @@ -43,7 +43,7 @@ describe("[RULE] blank-lines: Acceptances", function() { done(); }); - it("should accept single contract declaration", function(done) { + it("should accept single contract declaration", done => { let code = fs.readFileSync(path.join(__dirname, "./accept/contract-single.sol"), "utf8"), errors = Solium.lint(addPragma(code), userConfig); @@ -54,7 +54,7 @@ describe("[RULE] blank-lines: Acceptances", function() { done(); }); - it("should accept single library declaration", function(done) { + it("should accept single library declaration", done => { let code = fs.readFileSync(path.join(__dirname, "./accept/library-single.sol"), "utf8"), errors = Solium.lint(addPragma(code), userConfig); @@ -173,7 +173,7 @@ describe("[RULE] blank-lines: Acceptances", function() { done(); }); - it("should accept single-line functions without blank lines between them & multiline functions WITH them", function(done) { + it("should accept single-line functions without blank lines between them & multiline functions WITH them", done => { let code = fs.readFileSync(path.join(__dirname, "./accept/function.sol"), "utf8"), errors = Solium.lint(addPragma(code), userConfig); @@ -184,7 +184,7 @@ describe("[RULE] blank-lines: Acceptances", function() { done(); }); - it("should not enforce blank line rules on top level declarations other than contract & library declarations", function(done) { + it("should not enforce blank line rules on top level declarations other than contract & library declarations", done => { let code = "import * as x from \"y\";\nimport * as x from \"y\";\nimport * as x from \"y\";\n\n\ncontract Yoda {} import * as foo from \"bar.sol\";", errors = Solium.lint(addPragma(code), userConfig); @@ -198,9 +198,9 @@ describe("[RULE] blank-lines: Acceptances", function() { }); -describe("[RULE] blank-lines: Rejections", function() { +describe("[RULE] blank-lines: Rejections", () => { - it("should reject contract declarations with < 2 lines of gap between them", function(done) { + it("should reject contract declarations with < 2 lines of gap between them", done => { let code = fs.readFileSync(path.join(__dirname, "./reject/contract.sol"), "utf8"), errors = Solium.lint(addPragma(code), userConfig); @@ -215,7 +215,7 @@ describe("[RULE] blank-lines: Rejections", function() { done(); }); - it("should reject library declarations with < 2 lines of gap between them", function(done) { + it("should reject library declarations with < 2 lines of gap between them", done => { let code = fs.readFileSync(path.join(__dirname, "./reject/library.sol"), "utf8"), errors = Solium.lint(addPragma(code), userConfig); @@ -230,7 +230,7 @@ describe("[RULE] blank-lines: Rejections", function() { done(); }); - it("should reject a multiline function that is not followed by a blank line", function(done) { + it("should reject a multiline function that is not followed by a blank line", done => { let code = fs.readFileSync(path.join(__dirname, "./reject/function.sol"), "utf8"), errors = Solium.lint(addPragma(code), userConfig); @@ -332,9 +332,9 @@ describe("[RULE] blank-lines: Rejections", function() { }); -describe("[RULE] blank-lines: Fixes", function() { +describe("[RULE] blank-lines: Fixes", () => { - it("Should correct spacing between top level declarations with < 2 lines of gap between them", function(done) { + it("Should correct spacing between top level declarations with < 2 lines of gap between them", done => { const code = fs.readFileSync(path.join(__dirname, "./reject/contract.sol"), "utf8"); let {errorMessages: errors, fixedSourceCode} = Solium.lintAndFix(addPragma(code), userConfig); @@ -351,7 +351,7 @@ describe("[RULE] blank-lines: Fixes", function() { done(); }); - it("Should correct multiline functions not followed by a blank line", function(done) { + it("Should correct multiline functions not followed by a blank line", done => { let code = fs.readFileSync(path.join(__dirname, "./reject/function.sol"), "utf8"), {errorMessages: errors, fixedSourceCode} = Solium.lintAndFix(addPragma(code), userConfig); @@ -368,7 +368,7 @@ describe("[RULE] blank-lines: Fixes", function() { done(); }); - it("should correct top level declarations accompanied by comments but not gapped properly", function(done) { + it("should correct top level declarations accompanied by comments but not gapped properly", done => { let snippets = [ ` pragma solidity ^0.4.17; From d5d7dae84190b98ab119de21c545111610d6ebf7 Mon Sep 17 00:00:00 2001 From: GabrielAlacchi Date: Sun, 7 Jan 2018 11:40:30 -0500 Subject: [PATCH 3/3] Replaced \n with eol --- lib/rules/blank-lines.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/rules/blank-lines.js b/lib/rules/blank-lines.js index e77de53..ee4ea6c 100644 --- a/lib/rules/blank-lines.js +++ b/lib/rules/blank-lines.js @@ -68,9 +68,9 @@ module.exports = { // - Insert the desired number of line break characters right before the comment let line = sourceCode.getLine(topLevelNode) - 1, // The line number - maxLines = spacing.split("\n").length, + maxLines = spacing.split(eol).length, bottomLine = sourceCode.getLine(topLevelNode), - multipleLines = spacing.includes('\n'), // Whether there are multiple lines + multipleLines = spacing.includes(eol), // Whether there are multiple lines insertAt = topLevelNode.start, // The point to insert the new line characters at numberOfBreaks = multipleLines ? 1 : 0; // The number of line breaks there are currently (default to 1 if there are multiple lines) @@ -91,9 +91,9 @@ module.exports = { let blockCommentStart = rest.lastIndexOf("/*"); // Decrement the line number by the number of lines the block comment takes up - line -= sourceCode.getText().slice(blockCommentStart, insertAt).split("\n").length; + line -= sourceCode.getText().slice(blockCommentStart, insertAt).split(eol).length; // Set the endpoint to the end of the line above the block comment - insertAt = sourceCode.getText().slice(0, blockCommentStart).lastIndexOf("\n") + 1; + insertAt = sourceCode.getText().slice(0, blockCommentStart).lastIndexOf(eol) + 1; } else { // If it's just a regular comment, decrement the line number line--;