-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New: Allow passing a function as fix
option (fixes #8039)
#8730
Changes from all commits
6edc672
24d62a2
d8fa64e
e0c3035
e7b5f4a
4966811
d06f84a
bc94f81
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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"); | ||
|
||
|
@@ -162,6 +163,69 @@ 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, sourceCode.text); | ||
}); | ||
|
||
it("Should perform fixes if 'shouldFix' is not provided", () => { | ||
const result = SourceCodeFixer.applyFixes(sourceCode, [INSERT_AT_END]); | ||
|
||
assert.isTrue(result.fixed); | ||
}); | ||
|
||
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); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a test to assert that the fix filter is called as a pure function (without a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a test that asserts that only the fixes for which the fix filter returns |
||
|
||
it("should only apply fixes for which the 'shouldFix' function returns true", () => { | ||
const shouldFixSpy = sinon.spy(problem => problem.message === "foo"); | ||
const result = SourceCodeFixer.applyFixes(sourceCode, [INSERT_AT_START, REPLACE_ID], shouldFixSpy); | ||
|
||
assert.equal(result.output, "var foo = 6 * 7;"); | ||
}); | ||
|
||
it("is called without access to internal eslint state", () => { | ||
const shouldFixSpy = sinon.spy(); | ||
|
||
SourceCodeFixer.applyFixes(sourceCode, [INSERT_AT_START], shouldFixSpy); | ||
|
||
assert.isUndefined(shouldFixSpy.thisValues[0]); | ||
}); | ||
}); | ||
|
||
describe("Text Insertion", () => { | ||
|
||
it("should insert text at the end of the code", () => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note, we are only checking "truthiness" here. I think that's okay, but the other option would be to check for an explicit
true
return value.