Skip to content

Commit

Permalink
Update: support multiple fixes in a report (refs #7348)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mysticatea
mysticatea committed Feb 16, 2017
1 parent c7e64f3 commit c7f878e
Show file tree
Hide file tree
Showing 4 changed files with 228 additions and 20 deletions.
78 changes: 71 additions & 7 deletions lib/rule-context.js
View file
Expand Up @@ -8,7 +8,8 @@
// Requirements
//------------------------------------------------------------------------------

const ruleFixer = require("./util/rule-fixer");
const assert = require("assert");
const RuleFixer = require("./util/rule-fixer");

//------------------------------------------------------------------------------
// Constants
Expand Down Expand Up @@ -60,6 +61,74 @@ const PASSTHROUGHS = [
// Rule Definition
//------------------------------------------------------------------------------

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

/**
* Merges the given fixes array into one.
* @param {Fix[]} fixes The fixes to merge.
* @param {SourceCode} sourceCode The source code object to get the text between fixes.
* @returns {void}
*/
function mergeFixes(fixes, sourceCode) {
if (fixes.length === 0) {
return null;
}
if (fixes.length === 1) {
return fixes[0];
}

fixes.sort(compareFixesByRange);

const originalText = sourceCode.text;
const start = fixes[0].range[0];
let text = "";
let lastPos = start;

for (let i = 0; i < fixes.length; ++i) {
const fix = fixes[i];

assert(fix.range[0] >= lastPos, "Fix objects must not be overlapped in a report.");

text += originalText.slice(lastPos, fix.range[0]);
text += fix.text;
lastPos = fix.range[1];
}

return { range: [start, lastPos], text };
}

/**
* Gets one fix object from the given descriptor.
* If the descriptor retrieves multiple fixes, this merges those to one.
* @param {Object} descriptor The report descriptor.
* @param {SourceCode} sourceCode The source code object to get text between fixes.
* @returns {Fix} The got fix object.
*/
function getFix(descriptor, sourceCode) {
if (typeof descriptor.fix !== "function") {
return null;
}

// @type {null | Fix | Fix[] | IterableIterator<Fix>}
let fix = descriptor.fix(new RuleFixer(sourceCode));

// Merge to one.
if (fix && Symbol.iterator in fix) {
fix = mergeFixes(Array.from(fix), sourceCode);
}

return fix;
}

/**
* Rule context class
* Acts as an abstraction layer between rules and the main eslint object.
Expand Down Expand Up @@ -120,12 +189,7 @@ class RuleContext {
// check to see if it's a new style call
if (arguments.length === 1) {
const descriptor = nodeOrDescriptor;
let fix = null;

// if there's a fix specified, get it
if (typeof descriptor.fix === "function") {
fix = descriptor.fix(ruleFixer);
}
const fix = getFix(descriptor, this.getSourceCode());

this.eslint.report(
this.id,
Expand Down
52 changes: 40 additions & 12 deletions lib/util/rule-fixer.js
View file
Expand Up @@ -33,10 +33,12 @@ function insertTextAt(index, text) {
//------------------------------------------------------------------------------

/**
* Creates code fixing commands for rules.
* The class to create code fixing commands for rules.
*/

const ruleFixer = Object.freeze({
class RuleFixer {
constructor(sourceCode) {
this.sourceCode = sourceCode;
}

/**
* Creates a fix command that inserts text after the given node or token.
Expand All @@ -47,7 +49,7 @@ const ruleFixer = Object.freeze({
*/
insertTextAfter(nodeOrToken, text) {
return this.insertTextAfterRange(nodeOrToken.range, text);
},
}

/**
* Creates a fix command that inserts text after the specified range in the source text.
Expand All @@ -57,9 +59,10 @@ const ruleFixer = Object.freeze({
* @param {string} text The text to insert.
* @returns {Object} The fix command.
*/
// eslint-disable-next-line class-methods-use-this
insertTextAfterRange(range, text) {
return insertTextAt(range[1], text);
},
}

/**
* Creates a fix command that inserts text before the given node or token.
Expand All @@ -70,7 +73,7 @@ const ruleFixer = Object.freeze({
*/
insertTextBefore(nodeOrToken, text) {
return this.insertTextBeforeRange(nodeOrToken.range, text);
},
}

/**
* Creates a fix command that inserts text before the specified range in the source text.
Expand All @@ -80,9 +83,10 @@ const ruleFixer = Object.freeze({
* @param {string} text The text to insert.
* @returns {Object} The fix command.
*/
// eslint-disable-next-line class-methods-use-this
insertTextBeforeRange(range, text) {
return insertTextAt(range[0], text);
},
}

/**
* Creates a fix command that replaces text at the node or token.
Expand All @@ -93,7 +97,7 @@ const ruleFixer = Object.freeze({
*/
replaceText(nodeOrToken, text) {
return this.replaceTextRange(nodeOrToken.range, text);
},
}

/**
* Creates a fix command that replaces text at the specified range in the source text.
Expand All @@ -103,12 +107,13 @@ const ruleFixer = Object.freeze({
* @param {string} text The text to insert.
* @returns {Object} The fix command.
*/
// eslint-disable-next-line class-methods-use-this
replaceTextRange(range, text) {
return {
range,
text
};
},
}

/**
* Creates a fix command that removes the node or token from the source.
Expand All @@ -118,7 +123,7 @@ const ruleFixer = Object.freeze({
*/
remove(nodeOrToken) {
return this.removeRange(nodeOrToken.range);
},
}

/**
* Creates a fix command that removes the specified range of text from the source.
Expand All @@ -127,14 +132,37 @@ const ruleFixer = Object.freeze({
* is end of range.
* @returns {Object} The fix command.
*/
// eslint-disable-next-line class-methods-use-this
removeRange(range) {
return {
range,
text: ""
};
}

});
/**
* Creates a fix command that keeps the node or token in the source.
* This fix is used to check overlapping with other fixes in applyFixes().
* @param {ASTNode|Token} nodeOrToken The node or token to keep.
* @returns {Object} The fix command.
*/
keep(nodeOrToken) {
return this.keepRange(nodeOrToken.range);
}

/**
* Creates a fix command that keeps the specified range of text in the source.
* This fix is used to check overlapping with other fixes in applyFixes().
* @param {int[]} range The range to remove, first item is start of range, second
* is end of range.
* @returns {Object} The fix command.
*/
keepRange(range) {
return {
range,
text: this.sourceCode.text.slice(range[0], range[1])
};
}
}

module.exports = ruleFixer;
module.exports = RuleFixer;
68 changes: 68 additions & 0 deletions tests/lib/rule-context.js
View file
Expand Up @@ -81,6 +81,9 @@ describe("RuleContext", () => {
mockESLint.expects("report")
.once()
.withArgs("fake-rule", 2, node, location, message, messageOpts, fixerObj);
mockESLint.expects("getSourceCode")
.once()
.returns(null);

ruleContext.report({
node,
Expand All @@ -93,6 +96,71 @@ describe("RuleContext", () => {
fix.verify();
mockESLint.verify();
});

it("should merge fixes to one if 'fix' function returns an array of fixes.", () => {
const mockESLint = sandbox.mock(eslint);

mockESLint.expects("getSourceCode")
.returns({ text: "var foo = 100;" });
mockESLint.expects("report")
.once()
.withArgs(
sinon.match.any,
sinon.match.any,
sinon.match.any,
sinon.match.any,
sinon.match.any,
sinon.match.any,
sinon.match({
range: [4, 13],
text: "bar = 234"
})
);

ruleContext.report({
node: {},
loc: {},
message: "Message",
fix(fixer) {
return [
fixer.replaceTextRange([10, 13], "234"),
fixer.replaceTextRange([4, 7], "bar")
];
}
});

mockESLint.verify();
});

it("should merge fixes to one if 'fix' function returns an iterator of fixes.", () => {
const mockESLint = sandbox.mock(eslint);

mockESLint.expects("getSourceCode").returns({ text: "var foo = 100;" });
mockESLint.expects("report").once().withArgs(
sinon.match.any,
sinon.match.any,
sinon.match.any,
sinon.match.any,
sinon.match.any,
sinon.match.any,
sinon.match({
range: [4, 13],
text: "bar = 234"
})
);

ruleContext.report({
node: {},
loc: {},
message: "Message",
*fix(fixer) {
yield fixer.replaceTextRange([10, 13], "234");
yield fixer.replaceTextRange([4, 7], "bar");
}
});

mockESLint.verify();
});
});
});

Expand Down
50 changes: 49 additions & 1 deletion tests/lib/util/rule-fixer.js
View file
Expand Up @@ -9,13 +9,31 @@
//------------------------------------------------------------------------------

const assert = require("chai").assert,
ruleFixer = require("../../../lib/util/rule-fixer");
espree = require("espree"),
SourceCode = require("../../../lib/util/source-code"),
RuleFixer = require("../../../lib/util/rule-fixer");

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------

const DEFAULT_CONFIG = {
ecmaVersion: 6,
comment: true,
tokens: true,
range: true,
loc: true
};
const TEXT = "let foo = bar;";
const AST = espree.parse(TEXT, DEFAULT_CONFIG);
const SOURCE_CODE = new SourceCode(TEXT, AST);

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------

describe("RuleFixer", () => {
const ruleFixer = new RuleFixer(SOURCE_CODE);

describe("insertTextBefore", () => {

Expand Down Expand Up @@ -138,4 +156,34 @@ describe("RuleFixer", () => {

});

describe("keep", () => {

it("should return an object with the correct information when called", () => {

const result = ruleFixer.keep(AST.tokens[1]);

assert.deepEqual(result, {
range: [4, 7],
text: "foo"
});

});

});

describe("keepRange", () => {

it("should return an object with the correct information when called", () => {

const result = ruleFixer.keepRange([0, 7]);

assert.deepEqual(result, {
range: [0, 7],
text: "let foo"
});

});

});

});

0 comments on commit c7f878e

Please sign in to comment.