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
Update: support multiple fixes in a report (fixes #7348) #8101
Changes from all commits
926b00a
7271a5e
5dba973
cc5de10
c888f09
3847673
a4c3372
f85aee0
11a9043
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 |
---|---|---|
|
@@ -8,6 +8,7 @@ | |
// Requirements | ||
//------------------------------------------------------------------------------ | ||
|
||
const assert = require("assert"); | ||
const ruleFixer = require("./util/rule-fixer"); | ||
|
||
//------------------------------------------------------------------------------ | ||
|
@@ -60,6 +61,75 @@ 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) { | ||
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. Worth noting that 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. As public API, this is simple enhancement; the There are 2 topics here.
As I mentioned in this post, currently we have a pain to remain comments. The purpose of the In addition, we can address locked ranges in the same mechanism. In fact, this is the 1st step of my plan. Because the report of linting has However, I have a plan (though it's not accepted yet) to change the |
||
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]; | ||
const end = fixes[fixes.length - 1].range[1]; | ||
let text = ""; | ||
let lastPos = Number.MIN_SAFE_INTEGER; | ||
|
||
for (const fix of fixes) { | ||
assert(fix.range[0] >= lastPos, "Fix objects must not be overlapped in a report."); | ||
|
||
if (fix.range[0] >= 0) { | ||
text += originalText.slice(Math.max(0, start, lastPos), fix.range[0]); | ||
} | ||
text += fix.text; | ||
lastPos = fix.range[1]; | ||
} | ||
text += originalText.slice(Math.max(0, start, lastPos), end); | ||
|
||
return { range: [start, end], 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>} | ||
const fix = descriptor.fix(ruleFixer); | ||
|
||
// Merge to one. | ||
if (fix && Symbol.iterator in fix) { | ||
return mergeFixes(Array.from(fix), sourceCode); | ||
} | ||
return fix; | ||
} | ||
|
||
/** | ||
* Rule context class | ||
* Acts as an abstraction layer between rules and the main eslint object. | ||
|
@@ -120,12 +190,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, | ||
|
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.
Should be "The first fix" and "The second fix"
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.
Oops, it's my copy/paste mistake. Thank you!