Skip to content

Commit

Permalink
Chore: simplify and improve performance for autofix (#8035)
Browse files Browse the repository at this point in the history 
* Chore: refactor fix

* Fix: first overlap check
  • Loading branch information
@mysticatea @ilyavolodin
mysticatea authored and ilyavolodin committed Feb 11, 2017
1 parent b04fde7 commit fcc38db
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 51 deletions.
74 changes: 35 additions & 39 deletions lib/util/source-code-fixer.js
View file
Expand Up @@ -16,6 +16,17 @@ const debug = require("debug")("eslint:text-fixer");

const BOM = "\uFEFF";

/**
* Compares items in a messages array by range.
* @param {Message} a The first message.
* @param {Message} b The second message.
* @returns {int} -1 if a comes before b, 1 if a comes after b, 0 if equal.
* @private
*/
function compareMessagesByFixRange(a, b) {
return a.fix.range[0] - b.fix.range[0] || a.fix.range[1] - b.fix.range[1];
}

/**
* Compares items in a messages array by line and column.
* @param {Message} a The first message.
Expand All @@ -24,13 +35,7 @@ const BOM = "\uFEFF";
* @private
*/
function compareMessagesByLocation(a, b) {
const lineDiff = a.line - b.line;

if (lineDiff === 0) {
return a.column - b.column;
}
return lineDiff;

return a.line - b.line || a.column - b.column;
}

//------------------------------------------------------------------------------
Expand Down Expand Up @@ -68,9 +73,10 @@ SourceCodeFixer.applyFixes = function(sourceCode, messages) {
// clone the array
const remainingMessages = [],
fixes = [],
bom = (sourceCode.hasBOM ? BOM : ""),
text = sourceCode.text;
let lastFixPos = text.length + 1,
prefix = (sourceCode.hasBOM ? BOM : "");
let lastPos = Number.NEGATIVE_INFINITY,
output = bom;

messages.forEach(problem => {
if (problem.hasOwnProperty("fix")) {
Expand All @@ -83,51 +89,41 @@ SourceCodeFixer.applyFixes = function(sourceCode, messages) {
if (fixes.length) {
debug("Found fixes to apply");

// sort in reverse order of occurrence
fixes.sort((a, b) => b.fix.range[1] - a.fix.range[1] || b.fix.range[0] - a.fix.range[0]);

// split into array of characters for easier manipulation
const chars = text.split("");

fixes.forEach(problem => {
for (const problem of fixes.sort(compareMessagesByFixRange)) {
const fix = problem.fix;
let start = fix.range[0];
const 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 && insertionText[0] === BOM) {

// Set BOM.
prefix = BOM;
insertionText = insertionText.slice(1);
}

chars.splice(start, end - start, insertionText);
lastFixPos = start;
} else {
// Remain it as a problem if it's overlapped.
if (lastPos >= start) {
remainingMessages.push(problem);
continue;
}
});

// Remove BOM.
if ((start < 0 && end >= 0) || (start === 0 && fix.text.startsWith(BOM))) {
output = "";
}

// Make output to this fix.
output += text.slice(Math.max(0, lastPos), Math.max(0, start));
output += fix.text;
lastPos = end;
}
output += text.slice(Math.max(0, lastPos));

return {
fixed: true,
messages: remainingMessages.sort(compareMessagesByLocation),
output: prefix + chars.join("")
output
};
}

debug("No fixes to apply");
return {
fixed: false,
messages,
output: prefix + text
output: bom + text
};

};
Expand Down
24 changes: 12 additions & 12 deletions tests/lib/util/source-code-fixer.js
View file
Expand Up @@ -272,28 +272,28 @@ describe("SourceCodeFixer", () => {
it("should only apply one fix when ranges overlap", () => {
const result = SourceCodeFixer.applyFixes(sourceCode, [REMOVE_MIDDLE, REPLACE_ID]);

assert.equal(result.output, TEST_CODE.replace("answer", "a"));
assert.equal(result.output, TEST_CODE.replace("answer", "foo"));
assert.equal(result.messages.length, 1);
assert.equal(result.messages[0].message, "foo");
assert.equal(result.messages[0].message, "removemiddle");
assert.isTrue(result.fixed);
});

it("should apply one fix when the end of one range is the same as the start of a previous range overlap", () => {
const result = SourceCodeFixer.applyFixes(sourceCode, [REMOVE_START, REPLACE_ID]);

assert.equal(result.output, TEST_CODE.replace("answer", "foo"));
assert.equal(result.output, TEST_CODE.replace("var ", ""));
assert.equal(result.messages.length, 1);
assert.equal(result.messages[0].message, "removestart");
assert.equal(result.messages[0].message, "foo");
assert.isTrue(result.fixed);
});

it("should only apply one fix when ranges overlap and one message has no fix", () => {
const result = SourceCodeFixer.applyFixes(sourceCode, [REMOVE_MIDDLE, REPLACE_ID, NO_FIX]);

assert.equal(result.output, TEST_CODE.replace("answer", "a"));
assert.equal(result.output, TEST_CODE.replace("answer", "foo"));
assert.equal(result.messages.length, 2);
assert.equal(result.messages[0].message, "nofix");
assert.equal(result.messages[1].message, "foo");
assert.equal(result.messages[1].message, "removemiddle");
assert.isTrue(result.fixed);
});

Expand Down Expand Up @@ -493,28 +493,28 @@ describe("SourceCodeFixer", () => {
it("should only apply one fix when ranges overlap", () => {
const result = SourceCodeFixer.applyFixes(sourceCode, [REMOVE_MIDDLE, REPLACE_ID]);

assert.equal(result.output, `\uFEFF${TEST_CODE.replace("answer", "a")}`);
assert.equal(result.output, `\uFEFF${TEST_CODE.replace("answer", "foo")}`);
assert.equal(result.messages.length, 1);
assert.equal(result.messages[0].message, "foo");
assert.equal(result.messages[0].message, "removemiddle");
assert.isTrue(result.fixed);
});

it("should apply one fix when the end of one range is the same as the start of a previous range overlap", () => {
const result = SourceCodeFixer.applyFixes(sourceCode, [REMOVE_START, REPLACE_ID]);

assert.equal(result.output, `\uFEFF${TEST_CODE.replace("answer", "foo")}`);
assert.equal(result.output, `\uFEFF${TEST_CODE.replace("var ", "")}`);
assert.equal(result.messages.length, 1);
assert.equal(result.messages[0].message, "removestart");
assert.equal(result.messages[0].message, "foo");
assert.isTrue(result.fixed);
});

it("should only apply one fix when ranges overlap and one message has no fix", () => {
const result = SourceCodeFixer.applyFixes(sourceCode, [REMOVE_MIDDLE, REPLACE_ID, NO_FIX]);

assert.equal(result.output, `\uFEFF${TEST_CODE.replace("answer", "a")}`);
assert.equal(result.output, `\uFEFF${TEST_CODE.replace("answer", "foo")}`);
assert.equal(result.messages.length, 2);
assert.equal(result.messages[0].message, "nofix");
assert.equal(result.messages[1].message, "foo");
assert.equal(result.messages[1].message, "removemiddle");
assert.isTrue(result.fixed);
});

Expand Down

0 comments on commit fcc38db

Please sign in to comment.