Skip to content

Commit

Permalink
New: Allow passing a function as fix option (fixes #8039)
Browse files Browse the repository at this point in the history
  • Loading branch information
IanVS committed Jun 15, 2017
1 parent b5a70b4 commit c2f0916
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 27 deletions.
2 changes: 1 addition & 1 deletion docs/developer-guide/nodejs-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ The `CLIEngine` is a constructor, and you can create a new instance by passing i
* `cwd` - Path to a directory that should be considered as the current working directory.
* `envs` - An array of environments to load (default: empty array). Corresponds to `--env`.
* `extensions` - An array of filename extensions that should be checked for code. The default is an array containing just `".js"`. Corresponds to `--ext`. It is only used in conjunction with directories, not with filenames or glob patterns.
* `fix` - True indicates that fixes should be included with the output report, and that errors and warnings should not be listed if they can be fixed. However, the files on disk will not be changed. To persist changes to disk, call [`outputFixes()`](#outputfixes).
* `fix` - This can be a boolean or a function which will be provided each linting message and should return a boolean. True indicates that fixes should be included with the output report, and that errors and warnings should not be listed if they can be fixed. However, the files on disk will not be changed. To persist changes to disk, call [`outputFixes()`](#outputfixes).
* `globals` - An array of global variables to declare (default: empty array). Corresponds to `--global`.
* `ignore` - False disables use of `.eslintignore`, `ignorePath` and `ignorePattern` (default: true). Corresponds to `--no-ignore`.
* `ignorePath` - The ignore file to use instead of `.eslintignore` (default: null). Corresponds to `--ignore-path`.
Expand Down
6 changes: 3 additions & 3 deletions lib/cli-engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ const debug = require("debug")("eslint:cli-engine");
* @property {string} cwd The value to use for the current working directory.
* @property {string[]} envs An array of environments to load.
* @property {string[]} extensions An array of file extensions to check.
* @property {boolean} fix Execute in autofix mode.
* @property {boolean|Function} fix Execute in autofix mode. If a function, should return a boolean.
* @property {string[]} globals An array of global variables to declare.
* @property {boolean} ignore False disables use of .eslintignore.
* @property {string} ignorePath The ignore file to use instead of .eslintignore.
Expand Down Expand Up @@ -135,7 +135,7 @@ function calculateStatsPerRun(results) {
* @param {string} text The source code to check.
* @param {Object} configHelper The configuration options for ESLint.
* @param {string} filename An optional string representing the texts filename.
* @param {boolean} fix Indicates if fixes should be processed.
* @param {boolean|Function} fix Indicates if fixes should be processed.
* @param {boolean} allowInlineConfig Allow/ignore comments that change config.
* @param {Linter} linter Linter context
* @returns {LintResult} The results for linting on this text.
Expand Down Expand Up @@ -196,7 +196,7 @@ function processText(text, configHelper, filename, fix, allowInlineConfig, linte
fixedResult = linter.verifyAndFix(text, config, {
filename,
allowInlineConfig
});
}, fix);
messages = fixedResult.messages;
} else {
messages = linter.verify(text, config, {
Expand Down
5 changes: 3 additions & 2 deletions lib/linter.js
Original file line number Diff line number Diff line change
Expand Up @@ -1197,10 +1197,11 @@ class Linter extends EventEmitter {
* @param {string} options.filename The filename from which the text was read.
* @param {boolean} options.allowInlineConfig Flag indicating if inline comments
* should be allowed.
* @param {boolean|Function} shouldFix Determines whether fixes should be applied
* @returns {Object} The result of the fix operation as returned from the
* SourceCodeFixer.
*/
verifyAndFix(text, config, options) {
verifyAndFix(text, config, options, shouldFix) {
let messages = [],
fixedResult,
fixed = false,
Expand All @@ -1222,7 +1223,7 @@ class Linter extends EventEmitter {
messages = this.verify(text, config, options);

debug(`Generating fixed text for ${options.filename} (pass ${passNumber})`);
fixedResult = SourceCodeFixer.applyFixes(this.getSourceCode(), messages);
fixedResult = SourceCodeFixer.applyFixes(this.getSourceCode(), messages, shouldFix);

// stop if there are any syntax errors.
// 'fixedResult.output' is a empty string.
Expand Down
70 changes: 49 additions & 21 deletions lib/util/source-code-fixer.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ function SourceCodeFixer() {
* smart about the fixes and won't apply fixes over the same area in the text.
* @param {SourceCode} sourceCode The source code to apply the changes to.
* @param {Message[]} messages The array of messages reported by ESLint.
* @param {boolean|Function} [shouldFix=true] Determines whether each message should be fixed
* @returns {Object} An object containing the fixed text and any unfixed messages.
*/
SourceCodeFixer.applyFixes = function(sourceCode, messages) {

SourceCodeFixer.applyFixes = function(sourceCode, messages, shouldFix) {
debug("Applying fixes");

if (!sourceCode) {
Expand All @@ -70,6 +70,15 @@ SourceCodeFixer.applyFixes = function(sourceCode, messages) {
};
}

if (shouldFix === false) {
debug("shouldFix parameter was false, not attempting fixes");
return {
fixed: false,
messages,
output: ""
};
}

// clone the array
const remainingMessages = [],
fixes = [],
Expand All @@ -78,6 +87,34 @@ SourceCodeFixer.applyFixes = function(sourceCode, messages) {
let lastPos = Number.NEGATIVE_INFINITY,
output = bom;

/**
* Try to use the 'fix' from a problem.
* @param {Message} problem The message object to apply fixes from
* @returns {boolean} Whether fix was successfully applied
*/
function attemptFix(problem) {
const fix = problem.fix;
const start = fix.range[0];
const end = fix.range[1];

// Remain it as a problem if it's overlapped or it's a negative range
if (lastPos >= start || start > end) {
remainingMessages.push(problem);
return false;
}

// 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;
return true;
}

messages.forEach(problem => {
if (problem.hasOwnProperty("fix")) {
fixes.push(problem);
Expand All @@ -88,32 +125,23 @@ SourceCodeFixer.applyFixes = function(sourceCode, messages) {

if (fixes.length) {
debug("Found fixes to apply");
let fixesWereApplied = false;

for (const problem of fixes.sort(compareMessagesByFixRange)) {
const fix = problem.fix;
const start = fix.range[0];
const end = fix.range[1];

// Remain it as a problem if it's overlapped or it's a negative range
if (lastPos >= start || start > end) {
remainingMessages.push(problem);
continue;
}

// Remove BOM.
if ((start < 0 && end >= 0) || (start === 0 && fix.text.startsWith(BOM))) {
output = "";
if (typeof shouldFix === "function") {
if (shouldFix(problem)) {
fixesWereApplied = attemptFix(problem) || fixesWereApplied;
} else {
remainingMessages.push(problem);
}
} else {
fixesWereApplied = attemptFix(problem) || fixesWereApplied;
}

// 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,
fixed: fixesWereApplied,
messages: remainingMessages.sort(compareMessagesByLocation),
output
};
Expand Down
43 changes: 43 additions & 0 deletions tests/lib/util/source-code-fixer.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

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

Expand Down Expand Up @@ -162,6 +163,48 @@ describe("SourceCodeFixer", () => {
assert.equal(result.output.length, 0);
});

describe("shouldFix parameter", () => {

beforeEach(() => {
sourceCode = new SourceCode(TEST_CODE, TEST_AST);
});

it("Should not perform any fixes if 'shouldFix' is false", () => {
const result = SourceCodeFixer.applyFixes(sourceCode, [INSERT_AT_END], false);

assert.isFalse(result.fixed);
assert.equal(result.output.length, 0);
});

it("should call a function provided as 'shouldFix' for each message", () => {
const shouldFixSpy = sinon.spy();

SourceCodeFixer.applyFixes(sourceCode, [INSERT_IN_MIDDLE, INSERT_AT_START, INSERT_AT_END], shouldFixSpy);
assert.isTrue(shouldFixSpy.calledThrice);
});

it("should provide a message object as an argument to 'shouldFix'", () => {
const shouldFixSpy = sinon.spy();

SourceCodeFixer.applyFixes(sourceCode, [INSERT_AT_START], shouldFixSpy);
assert.equal(shouldFixSpy.firstCall.args[0], INSERT_AT_START);
});

it("should not perform fixes if 'shouldFix' function returns false", () => {
const shouldFixSpy = sinon.spy(() => false);
const result = SourceCodeFixer.applyFixes(sourceCode, [INSERT_AT_START], shouldFixSpy);

assert.isFalse(result.fixed);
});

it("should return original text as output if 'shouldFix' function prevents all fixes", () => {
const shouldFixSpy = sinon.spy(() => false);
const result = SourceCodeFixer.applyFixes(sourceCode, [INSERT_AT_START], shouldFixSpy);

assert.equal(result.output, TEST_CODE);
});
});

describe("Text Insertion", () => {

it("should insert text at the end of the code", () => {
Expand Down

0 comments on commit c2f0916

Please sign in to comment.