From 19f6fff6d5f9a576ac956d3aee3a5e084873398e Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Thu, 14 Apr 2016 14:37:09 -0700 Subject: [PATCH] Update: Autofixing does multiple passes (refs #5329) --- lib/cli-engine.js | 89 +++++++++++++++++++++++++++++++++++++---- tests/lib/cli-engine.js | 26 +++--------- 2 files changed, 87 insertions(+), 28 deletions(-) diff --git a/lib/cli-engine.js b/lib/cli-engine.js index 2cc62f955eac..e54bf7bf02f1 100644 --- a/lib/cli-engine.js +++ b/lib/cli-engine.js @@ -115,6 +115,79 @@ function calculateStatsPerRun(results) { }); } +/** + * Performs multiple autofix passes over the text until as many fixes as possible + * have been applied. + * @param {string} text The source text to apply fixes to. + * @param {Object} config The ESLint config object to use. + * @param {Object} options The ESLint options object to use. + * @param {string} options.filename The filename from which the text was read. + * @param {boolean} options.allowInlineConfig Flag indicating if inline comments + * should be allowed. + * @returns {Object} The result of the fix operation as returned from the + * SourceCodeFixer. + * @private + */ +function multipassFix(text, config, options) { + + var messages = [], + fixedResult, + fixed = false, + passNumber = 0, + lastMessageCount, + MAX_PASSES = 10; + + /** + * This loop continues until one of the following is true: + * + * 1. No more fixes have been applied. + * 2. There are no more linting errors reported. + * 3. The number of linting errors is no different between two passes. + * 4. Ten passes have been made. + * + * That means anytime a fix is successfully applied, there will be another pass. + * Essentially, guaranteeing a minimum of two passes. + */ + do { + passNumber++; + lastMessageCount = messages.length; + + debug("Linting code for " + options.filename + " (pass " + passNumber + ")"); + messages = eslint.verify(text, config, options); + + debug("Generating fixed text for " + options.filename + " (pass " + passNumber + ")"); + fixedResult = SourceCodeFixer.applyFixes(eslint.getSourceCode(), messages); + + // keep track if any fixes were ever applied - important for return value + fixed = fixed || fixedResult.fixed; + + // update to use the fixed output instead of the original text + text = fixedResult.output; + + } while ( + fixedResult.fixed && fixedResult.messages.length > 0 && + fixedResult.messages.length !== lastMessageCount && + passNumber < MAX_PASSES + ); + + + /* + * If the last result had fixes, we need to lint again to me sure we have + * the most up-to-date information. + */ + if (fixedResult.fixed) { + fixedResult.messages = eslint.verify(text, config, options); + } + + + // ensure the last result properly reflects if fixes were done + fixedResult.fixed = fixed; + fixedResult.output = text; + + return fixedResult; + +} + /** * Processes an source code using ESLint. * @param {string} text The source code to check. @@ -179,15 +252,17 @@ function processText(text, configHelper, filename, fix, allowInlineConfig) { } else { - messages = eslint.verify(text, config, { - filename: filename, - allowInlineConfig: allowInlineConfig - }); - if (fix) { - debug("Generating fixed text for " + filename); - fixedResult = SourceCodeFixer.applyFixes(eslint.getSourceCode(), messages); + fixedResult = multipassFix(text, config, { + filename: filename, + allowInlineConfig: allowInlineConfig + }); messages = fixedResult.messages; + } else { + messages = eslint.verify(text, config, { + filename: filename, + allowInlineConfig: allowInlineConfig + }); } } diff --git a/tests/lib/cli-engine.js b/tests/lib/cli-engine.js index 967d93e53da0..f2a59b186a8b 100644 --- a/tests/lib/cli-engine.js +++ b/tests/lib/cli-engine.js @@ -914,22 +914,6 @@ describe("CLIEngine", function() { { "filePath": fs.realpathSync(path.resolve(fixtureDir, "fixmode/quotes-semi-eqeqeq.js")), "messages": [ - { - "column": 11, - "fix": { - "range": [ - 10, - 14 - ], - "text": "\"hi\"" - }, - "line": 1, - "message": "Strings must use doublequote.", - "nodeType": "Literal", - "ruleId": "quotes", - "severity": 2, - "source": "var msg = 'hi'" - }, { "column": 9, "line": 2, @@ -937,12 +921,12 @@ describe("CLIEngine", function() { "nodeType": "BinaryExpression", "ruleId": "eqeqeq", "severity": 2, - "source": "if (msg == 'hi') {" + "source": "if (msg == \"hi\") {" } ], - "errorCount": 2, + "errorCount": 1, "warningCount": 0, - "output": "var msg = 'hi';\nif (msg == \"hi\") {\n\n}\n" + "output": "var msg = \"hi\";\nif (msg == \"hi\") {\n\n}\n" }, { "filePath": fs.realpathSync(path.resolve(fixtureDir, "fixmode/quotes.js")), @@ -954,7 +938,7 @@ describe("CLIEngine", function() { nodeType: "Identifier", ruleId: "no-undef", severity: 2, - source: "var msg = 'hi' + foo;" + source: "var msg = \"hi\" + foo;" } ], "errorCount": 1, @@ -962,7 +946,7 @@ describe("CLIEngine", function() { "output": "var msg = \"hi\" + foo;\n" } ], - "errorCount": 3, + "errorCount": 2, "warningCount": 0 }); });