Skip to content

Commit

Permalink
fix: provide unique fix and fix.range objects in lint messages (#…
Browse files Browse the repository at this point in the history 
…17332)

* fix: provide unique `fix` and `fix.range` objects in lint messages

Fixes #16716

* Update lib/linter/report-translator.js

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>

---------

Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com>
  • Loading branch information
@mdjermanovic @nzakas
mdjermanovic and nzakas committed Jul 4, 2023
1 parent 138c096 commit c667055
Show file tree
Hide file tree
Showing 3 changed files with 362 additions and 2 deletions.
20 changes: 18 additions & 2 deletions lib/linter/report-translator.js
View file
Expand Up @@ -100,6 +100,22 @@ function normalizeReportLoc(descriptor) {
return descriptor.node.loc;
}

/**
* Clones the given fix object.
* @param {Fix|null} fix The fix to clone.
* @returns {Fix|null} Deep cloned fix object or `null` if `null` or `undefined` was passed in.
*/
function cloneFix(fix) {
if (!fix) {
return null;
}

return {
range: [fix.range[0], fix.range[1]],
text: fix.text
};
}

/**
* Check that a fix has a valid range.
* @param {Fix|null} fix The fix to validate.
Expand Down Expand Up @@ -137,7 +153,7 @@ function mergeFixes(fixes, sourceCode) {
return null;
}
if (fixes.length === 1) {
return fixes[0];
return cloneFix(fixes[0]);
}

fixes.sort(compareFixesByRange);
Expand Down Expand Up @@ -183,7 +199,7 @@ function normalizeFixes(descriptor, sourceCode) {
}

assertValidFix(fix);
return fix;
return cloneFix(fix);
}

/**
Expand Down
165 changes: 165 additions & 0 deletions tests/lib/linter/linter.js
View file
Expand Up @@ -15917,6 +15917,171 @@ var a = "test2";

assert.strictEqual(suppressedMessages.length, 0);
});

// https://github.com/eslint/eslint/issues/16716
it("should receive unique range arrays in suggestions", () => {
const configs = [
{
plugins: {
"test-processors": {
processors: {
"line-processor": (() => {
const blocksMap = new Map();

return {
preprocess(text, fileName) {
const lines = text.split("\n");

blocksMap.set(fileName, lines);

return lines.map((line, index) => ({
text: line,
filename: `${index}.js`
}));
},

postprocess(messageLists, fileName) {
const lines = blocksMap.get(fileName);
let rangeOffset = 0;

// intentionaly mutates objects and arrays
messageLists.forEach((messages, index) => {
messages.forEach(message => {
message.line += index;
if (typeof message.endLine === "number") {
message.endLine += index;
}
if (message.fix) {
message.fix.range[0] += rangeOffset;
message.fix.range[1] += rangeOffset;
}
if (message.suggestions) {
message.suggestions.forEach(suggestion => {
suggestion.fix.range[0] += rangeOffset;
suggestion.fix.range[1] += rangeOffset;
});
}
});
rangeOffset += lines[index].length + 1;
});

return messageLists.flat();
},

supportsAutofix: true
};
})()
}
},

"test-rules": {
rules: {
"no-foo": {
meta: {
hasSuggestions: true,
messages: {
unexpected: "Don't use 'foo'.",
replaceWithBar: "Replace with 'bar'",
replaceWithBaz: "Replace with 'baz'"
}

},
create(context) {
return {
Identifier(node) {
const { range } = node;

if (node.name === "foo") {
context.report({
node,
messageId: "unexpected",
suggest: [
{
messageId: "replaceWithBar",
fix: () => ({ range, text: "bar" })
},
{
messageId: "replaceWithBaz",
fix: () => ({ range, text: "baz" })
}
]
});
}
}
};
}
}
}
}
}
},
{
files: ["**/*.txt"],
processor: "test-processors/line-processor"
},
{
files: ["**/*.js"],
rules: {
"test-rules/no-foo": 2
}
}
];

const result = linter.verifyAndFix(
"var a = 5;\nvar foo;\nfoo = a;",
configs,
{ filename: "a.txt" }
);

assert.deepStrictEqual(result.messages, [
{
ruleId: "test-rules/no-foo",
severity: 2,
message: "Don't use 'foo'.",
line: 2,
column: 5,
nodeType: "Identifier",
messageId: "unexpected",
endLine: 2,
endColumn: 8,
suggestions: [
{
messageId: "replaceWithBar",
fix: { range: [15, 18], text: "bar" },
desc: "Replace with 'bar'"
},
{
messageId: "replaceWithBaz",
fix: { range: [15, 18], text: "baz" },
desc: "Replace with 'baz'"
}
]
},
{
ruleId: "test-rules/no-foo",
severity: 2,
message: "Don't use 'foo'.",
line: 3,
column: 1,
nodeType: "Identifier",
messageId: "unexpected",
endLine: 3,
endColumn: 4,
suggestions: [
{
messageId: "replaceWithBar",
fix: { range: [20, 23], text: "bar" },
desc: "Replace with 'bar'"
},
{
messageId: "replaceWithBaz",
fix: { range: [20, 23], text: "baz" },
desc: "Replace with 'baz'"
}
]
}
]);
});
});
});

Expand Down
179 changes: 179 additions & 0 deletions tests/lib/linter/report-translator.js
View file
Expand Up @@ -1091,4 +1091,183 @@ describe("createReportTranslator", () => {
}
});
});

// https://github.com/eslint/eslint/issues/16716
describe("unique `fix` and `fix.range` objects", () => {
const range = [0, 3];
const fix = { range, text: "baz" };
const additionalRange = [4, 7];
const additionalFix = { range: additionalRange, text: "qux" };

it("should deep clone returned fix object", () => {
const translatedReport = translateReport({
node,
messageId: "testMessage",
fix: () => fix
});

assert.deepStrictEqual(translatedReport.fix, fix);
assert.notStrictEqual(translatedReport.fix, fix);
assert.notStrictEqual(translatedReport.fix.range, fix.range);
});

it("should create a new fix object with a new range array when `fix()` returns an array with a single item", () => {
const translatedReport = translateReport({
node,
messageId: "testMessage",
fix: () => [fix]
});

assert.deepStrictEqual(translatedReport.fix, fix);
assert.notStrictEqual(translatedReport.fix, fix);
assert.notStrictEqual(translatedReport.fix.range, fix.range);
});

it("should create a new fix object with a new range array when `fix()` returns an array with multiple items", () => {
const translatedReport = translateReport({
node,
messageId: "testMessage",
fix: () => [fix, additionalFix]
});

assert.notStrictEqual(translatedReport.fix, fix);
assert.notStrictEqual(translatedReport.fix.range, fix.range);
assert.notStrictEqual(translatedReport.fix, additionalFix);
assert.notStrictEqual(translatedReport.fix.range, additionalFix.range);
});

it("should create a new fix object with a new range array when `fix()` generator yields a single item", () => {
const translatedReport = translateReport({
node,
messageId: "testMessage",
*fix() {
yield fix;
}
});

assert.deepStrictEqual(translatedReport.fix, fix);
assert.notStrictEqual(translatedReport.fix, fix);
assert.notStrictEqual(translatedReport.fix.range, fix.range);
});

it("should create a new fix object with a new range array when `fix()` generator yields multiple items", () => {
const translatedReport = translateReport({
node,
messageId: "testMessage",
*fix() {
yield fix;
yield additionalFix;
}
});

assert.notStrictEqual(translatedReport.fix, fix);
assert.notStrictEqual(translatedReport.fix.range, fix.range);
assert.notStrictEqual(translatedReport.fix, additionalFix);
assert.notStrictEqual(translatedReport.fix.range, additionalFix.range);
});

it("should deep clone returned suggestion fix object", () => {
const translatedReport = translateReport({
node,
messageId: "testMessage",
suggest: [{
messageId: "suggestion1",
fix: () => fix
}]
});

assert.deepStrictEqual(translatedReport.suggestions[0].fix, fix);
assert.notStrictEqual(translatedReport.suggestions[0].fix, fix);
assert.notStrictEqual(translatedReport.suggestions[0].fix.range, fix.range);
});

it("should create a new fix object with a new range array when suggestion `fix()` returns an array with a single item", () => {
const translatedReport = translateReport({
node,
messageId: "testMessage",
suggest: [{
messageId: "suggestion1",
fix: () => [fix]
}]
});

assert.deepStrictEqual(translatedReport.suggestions[0].fix, fix);
assert.notStrictEqual(translatedReport.suggestions[0].fix, fix);
assert.notStrictEqual(translatedReport.suggestions[0].fix.range, fix.range);
});

it("should create a new fix object with a new range array when suggestion `fix()` returns an array with multiple items", () => {
const translatedReport = translateReport({
node,
messageId: "testMessage",
suggest: [{
messageId: "suggestion1",
fix: () => [fix, additionalFix]
}]
});

assert.notStrictEqual(translatedReport.suggestions[0].fix, fix);
assert.notStrictEqual(translatedReport.suggestions[0].fix.range, fix.range);
assert.notStrictEqual(translatedReport.suggestions[0].fix, additionalFix);
assert.notStrictEqual(translatedReport.suggestions[0].fix.range, additionalFix.range);
});

it("should create a new fix object with a new range array when suggestion `fix()` generator yields a single item", () => {
const translatedReport = translateReport({
node,
messageId: "testMessage",
suggest: [{
messageId: "suggestion1",
*fix() {
yield fix;
}
}]
});

assert.deepStrictEqual(translatedReport.suggestions[0].fix, fix);
assert.notStrictEqual(translatedReport.suggestions[0].fix, fix);
assert.notStrictEqual(translatedReport.suggestions[0].fix.range, fix.range);
});

it("should create a new fix object with a new range array when suggestion `fix()` generator yields multiple items", () => {
const translatedReport = translateReport({
node,
messageId: "testMessage",
suggest: [{
messageId: "suggestion1",
*fix() {
yield fix;
yield additionalFix;
}
}]
});

assert.notStrictEqual(translatedReport.suggestions[0].fix, fix);
assert.notStrictEqual(translatedReport.suggestions[0].fix.range, fix.range);
assert.notStrictEqual(translatedReport.suggestions[0].fix, additionalFix);
assert.notStrictEqual(translatedReport.suggestions[0].fix.range, additionalFix.range);
});

it("should create different instances of range arrays when suggestions reuse the same instance", () => {
const translatedReport = translateReport({
node,
messageId: "testMessage",
suggest: [
{
messageId: "suggestion1",
fix: () => ({ range, text: "baz" })
},
{
messageId: "suggestion2",
data: { interpolated: "'interpolated value'" },
fix: () => ({ range, text: "qux" })
}
]
});

assert.deepStrictEqual(translatedReport.suggestions[0].fix.range, range);
assert.deepStrictEqual(translatedReport.suggestions[1].fix.range, range);
assert.notStrictEqual(translatedReport.suggestions[0].fix.range, translatedReport.suggestions[1].fix.range);
});
});
});

0 comments on commit c667055

Please sign in to comment.