diff --git a/lib/util/source-code-fixer.js b/lib/util/source-code-fixer.js index 064a0a1f902..5890b209535 100644 --- a/lib/util/source-code-fixer.js +++ b/lib/util/source-code-fixer.js @@ -66,63 +66,71 @@ SourceCodeFixer.applyFixes = function(sourceCode, messages) { } // clone the array - const remainingMessages = [], - fixes = [], - text = sourceCode.text; - let lastFixPos = text.length + 1, - prefix = (sourceCode.hasBOM ? BOM : ""); + const remainingMessages = []; + const problems = []; + let text = sourceCode.text; + let prefix = (sourceCode.hasBOM ? BOM : ""); messages.forEach(function(problem) { if (problem.hasOwnProperty("fix")) { - fixes.push(problem); + problems.push(problem); } else { remainingMessages.push(problem); } }); - if (fixes.length) { + if (problems.length) { debug("Found fixes to apply"); - // sort in reverse order of occurrence - fixes.sort(function(a, b) { - return b.fix.range[1] - a.fix.range[1] || b.fix.range[0] - a.fix.range[0]; - }); + const allFixes = problems.reduce((fixes, problem) => fixes.concat(problem.fix), []).sort((a, b) => a.range[1] - b.range[1] || a.range[0] - b.range[0]); + let lastFixPos = Infinity; + const fixesToApply = allFixes.reduceRight((fixes, fix) => { + + // If a fix conflicts with a previous fix, don't apply it. + if (fix.range[1] < lastFixPos) { + fixes.add(fix); + lastFixPos = fix.range[0]; + } + return fixes; + }, new Set()); + + problems.forEach(problem => { + const fixes = [].concat(problem.fix); - // split into array of characters for easier manipulation - const chars = text.split(""); + if (!fixes.every(fix => fixesToApply.has(fix))) { - fixes.forEach(function(problem) { - const fix = problem.fix; + // If not every fix for a given problem can be applied, don't apply any of the fixes. + fixes.filter(fix => fixesToApply.has(fix)).forEach(fix => fixesToApply.delete(fix)); + remainingMessages.push(problem); + } + }); + + allFixes.filter(fix => fixesToApply.has(fix)).reverse().forEach(fix => { let start = fix.range[0]; const end = fix.range[1]; let insertionText = fix.text; - if (end < lastFixPos) { - if (start < 0) { - - // Remove BOM. - prefix = ""; - start = 0; - } + if (start < 0) { - if (start === 0 && insertionText[0] === BOM) { + // Remove BOM. + prefix = ""; + start = 0; + } - // Set BOM. - prefix = BOM; - insertionText = insertionText.slice(1); - } + if (start === 0 && insertionText[0] === BOM) { - chars.splice(start, end - start, insertionText); - lastFixPos = start; - } else { - remainingMessages.push(problem); + // Set BOM. + prefix = BOM; + insertionText = insertionText.slice(1); } + + text = text.slice(0, start) + insertionText + text.slice(end); }); return { fixed: true, messages: remainingMessages.sort(compareMessagesByLocation), - output: prefix + chars.join("") + output: prefix + text }; } else { debug("No fixes to apply"); diff --git a/tests/lib/util/source-code-fixer.js b/tests/lib/util/source-code-fixer.js index 50f7b275a0b..ed4be99d336 100644 --- a/tests/lib/util/source-code-fixer.js +++ b/tests/lib/util/source-code-fixer.js @@ -122,6 +122,19 @@ const INSERT_AT_END = { message: "nofix2", line: 1, column: 7 + }, + ADD_PARENS = { + message: "add-parens", + fix: [ + { + range: [13, 13], + text: "(" + }, + { + range: [18, 18], + text: ")" + } + ] }; //------------------------------------------------------------------------------ @@ -303,6 +316,14 @@ describe("SourceCodeFixer", function() { assert.equal(result1.output, result2.output); }); + + it("should not apply fixes from the same problem unless all of them can be applied", function() { + const result1 = SourceCodeFixer.applyFixes(sourceCode, [ ADD_PARENS, INSERT_IN_MIDDLE ]); + + assert.equal(result1.output, TEST_CODE.replace("6 * 7;", "5 *6 * 7;")); + }); + + // TODO: (not-an-aardvark) Add more atomicity tests }); describe("No Fixes", function() {