Skip to content

Commit

Permalink
fix: correct locations of invalid /* eslint */ comments (eslint#18593)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mdjermanovic
mdjermanovic authored Jun 18, 2024
1 parent 8824aa1 commit f9e95d2
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 46 deletions.
8 changes: 6 additions & 2 deletions lib/languages/js/source-code/source-code.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1147,7 +1147,7 @@ class SourceCode extends TokenStore {
break;

case "eslint": {
const parseResult = commentParser.parseJsonConfig(directiveValue, comment.loc);
const parseResult = commentParser.parseJsonConfig(directiveValue);

if (parseResult.success) {
configs.push({
Expand All @@ -1157,7 +1157,11 @@ class SourceCode extends TokenStore {
node: comment
});
} else {
problems.push(parseResult.error);
problems.push({
ruleId: null,
loc: comment.loc,
message: parseResult.error.message
});
}

break;
Expand Down
19 changes: 3 additions & 16 deletions lib/linter/config-comment-parser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,6 @@ const levn = require("levn"),

const debug = require("debug")("eslint:config-comment-parser");

//------------------------------------------------------------------------------
// Typedefs
//------------------------------------------------------------------------------

/** @typedef {import("../shared/types").LintMessage} LintMessage */

//------------------------------------------------------------------------------
// Public Interface
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -69,10 +63,9 @@ module.exports = class ConfigCommentParser {
/**
* Parses a JSON-like config.
* @param {string} string The string to parse.
* @param {Object} location Start line and column of comments for potential error message.
* @returns {({success: true, config: Object}|{success: false, error: LintMessage})} Result map object
* @returns {({success: true, config: Object}|{success: false, error: {message: string}})} Result map object
*/
parseJsonConfig(string, location) {
parseJsonConfig(string) {
debug("Parsing JSON config");

// Parses a JSON-like comment by the same way as parsing CLI option.
Expand Down Expand Up @@ -115,13 +108,7 @@ module.exports = class ConfigCommentParser {
return {
success: false,
error: {
ruleId: null,
fatal: true,
severity: 2,
message: `Failed to parse JSON from '${normalizedString}': ${ex.message}`,
line: location.start.line,
column: location.start.column + 1,
nodeType: null
message: `Failed to parse JSON from '${normalizedString}': ${ex.message}`
}
};

Expand Down
11 changes: 9 additions & 2 deletions lib/linter/linter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig, config)
break;

case "eslint": {
const parseResult = commentParser.parseJsonConfig(directiveValue, comment.loc);
const parseResult = commentParser.parseJsonConfig(directiveValue);

if (parseResult.success) {
Object.keys(parseResult.config).forEach(name => {
Expand Down Expand Up @@ -557,7 +557,14 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig, config)
configuredRules[name] = ruleOptions;
});
} else {
problems.push(parseResult.error);
const problem = createLintingProblem({
ruleId: null,
loc: comment.loc,
message: parseResult.error.message
});

problem.fatal = true;
problems.push(problem);
}

break;
Expand Down
9 changes: 4 additions & 5 deletions tests/lib/languages/js/source-code/source-code.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3242,13 +3242,12 @@ describe("SourceCode", () => {
const problem = result.problems[0];

// Node.js 19 changes the JSON parsing error format, so we need to check each field separately to use a regex
assert.strictEqual(problem.column, 1);
assert.strictEqual(problem.line, 1);
assert.isTrue(problem.fatal);
assert.strictEqual(problem.loc.start.column, 0);
assert.strictEqual(problem.loc.start.line, 1);
assert.strictEqual(problem.loc.end.column, 24);
assert.strictEqual(problem.loc.end.line, 1);
assert.match(problem.message, /Failed to parse JSON from ' "some-rule"::,': Unexpected token '?:'?/u);
assert.isNull(problem.nodeType);
assert.isNull(problem.ruleId);
assert.strictEqual(problem.severity, 2);
});
});
});
18 changes: 6 additions & 12 deletions tests/lib/linter/config-comment-parser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,6 @@ const assert = require("chai").assert,
describe("ConfigCommentParser", () => {

let commentParser;
const location = {
start: {
line: 1,
column: 0
}
};

beforeEach(() => {
commentParser = new ConfigCommentParser();
Expand All @@ -34,7 +28,7 @@ describe("ConfigCommentParser", () => {

it("should parse JSON config with one item", () => {
const code = "no-alert:0";
const result = commentParser.parseJsonConfig(code, location);
const result = commentParser.parseJsonConfig(code);


assert.deepStrictEqual(result, {
Expand All @@ -47,7 +41,7 @@ describe("ConfigCommentParser", () => {

it("should parse JSON config with two items", () => {
const code = "no-alert:0 semi: 2";
const result = commentParser.parseJsonConfig(code, location);
const result = commentParser.parseJsonConfig(code);


assert.deepStrictEqual(result, {
Expand All @@ -61,7 +55,7 @@ describe("ConfigCommentParser", () => {

it("should parse JSON config with two comma-separated items", () => {
const code = "no-alert:0,semi: 2";
const result = commentParser.parseJsonConfig(code, location);
const result = commentParser.parseJsonConfig(code);


assert.deepStrictEqual(result, {
Expand All @@ -75,7 +69,7 @@ describe("ConfigCommentParser", () => {

it("should parse JSON config with two items and a string severity", () => {
const code = "no-alert:off,semi: 2";
const result = commentParser.parseJsonConfig(code, location);
const result = commentParser.parseJsonConfig(code);


assert.deepStrictEqual(result, {
Expand All @@ -89,7 +83,7 @@ describe("ConfigCommentParser", () => {

it("should parse JSON config with two items and options", () => {
const code = "no-alert:off, semi: [2, always]";
const result = commentParser.parseJsonConfig(code, location);
const result = commentParser.parseJsonConfig(code);


assert.deepStrictEqual(result, {
Expand All @@ -103,7 +97,7 @@ describe("ConfigCommentParser", () => {

it("should parse JSON config with two items and options from plugins", () => {
const code = "plugin/no-alert:off, plugin/semi: [2, always]";
const result = commentParser.parseJsonConfig(code, location);
const result = commentParser.parseJsonConfig(code);

assert.deepStrictEqual(result, {
success: true,
Expand Down
48 changes: 42 additions & 6 deletions tests/lib/linter/linter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3176,6 +3176,12 @@ describe("Linter", () => {
assert.match(messages[0].message, /^Failed to parse JSON from ' "no-alert":'1'':/u);
assert.strictEqual(messages[0].line, 1);
assert.strictEqual(messages[0].column, 1);
assert.strictEqual(messages[0].endLine, 1);
assert.strictEqual(messages[0].endColumn, 24);
assert.strictEqual(messages[0].ruleId, null);
assert.strictEqual(messages[0].fatal, true);
assert.strictEqual(messages[0].severity, 2);
assert.strictEqual(messages[0].nodeType, null);

assert.strictEqual(messages[1].ruleId, "no-alert");
assert.strictEqual(messages[1].message, "Unexpected alert.");
Expand Down Expand Up @@ -3204,6 +3210,12 @@ describe("Linter", () => {
assert.match(messages[0].message, /^Failed to parse JSON from ' "no-alert":abc':/u);
assert.strictEqual(messages[0].line, 1);
assert.strictEqual(messages[0].column, 1);
assert.strictEqual(messages[0].endLine, 1);
assert.strictEqual(messages[0].endColumn, 24);
assert.strictEqual(messages[0].ruleId, null);
assert.strictEqual(messages[0].fatal, true);
assert.strictEqual(messages[0].severity, 2);
assert.strictEqual(messages[0].nodeType, null);

assert.strictEqual(messages[1].ruleId, "no-alert");
assert.strictEqual(messages[1].message, "Unexpected alert.");
Expand All @@ -3213,7 +3225,7 @@ describe("Linter", () => {
});

it("should report a violation", () => {
const code = "/*eslint no-alert:0 2*/ alert('test');";
const code = "\n\n\n /*eslint no-alert:0 2*/ alert('test');";

const config = { rules: { "no-alert": 1 } };

Expand All @@ -3230,8 +3242,14 @@ describe("Linter", () => {
* parseJsonConfig function in lib/eslint.js
*/
assert.match(messages[0].message, /^Failed to parse JSON from ' "no-alert":0 2':/u);
assert.strictEqual(messages[0].line, 1);
assert.strictEqual(messages[0].column, 1);
assert.strictEqual(messages[0].line, 4);
assert.strictEqual(messages[0].column, 5);
assert.strictEqual(messages[0].endLine, 4);
assert.strictEqual(messages[0].endColumn, 28);
assert.strictEqual(messages[0].ruleId, null);
assert.strictEqual(messages[0].fatal, true);
assert.strictEqual(messages[0].severity, 2);
assert.strictEqual(messages[0].nodeType, null);

assert.strictEqual(messages[1].ruleId, "no-alert");
assert.strictEqual(messages[1].message, "Unexpected alert.");
Expand Down Expand Up @@ -11819,6 +11837,12 @@ describe("Linter with FlatConfigArray", () => {
assert.match(messages[0].message, /^Failed to parse JSON from ' "no-alert":'1'':/u);
assert.strictEqual(messages[0].line, 1);
assert.strictEqual(messages[0].column, 1);
assert.strictEqual(messages[0].endLine, 1);
assert.strictEqual(messages[0].endColumn, 24);
assert.strictEqual(messages[0].ruleId, null);
assert.strictEqual(messages[0].fatal, true);
assert.strictEqual(messages[0].severity, 2);
assert.strictEqual(messages[0].nodeType, null);

assert.strictEqual(messages[1].ruleId, "no-alert");
assert.strictEqual(messages[1].message, "Unexpected alert.");
Expand Down Expand Up @@ -11847,6 +11871,12 @@ describe("Linter with FlatConfigArray", () => {
assert.match(messages[0].message, /^Failed to parse JSON from ' "no-alert":abc':/u);
assert.strictEqual(messages[0].line, 1);
assert.strictEqual(messages[0].column, 1);
assert.strictEqual(messages[0].endLine, 1);
assert.strictEqual(messages[0].endColumn, 24);
assert.strictEqual(messages[0].ruleId, null);
assert.strictEqual(messages[0].fatal, true);
assert.strictEqual(messages[0].severity, 2);
assert.strictEqual(messages[0].nodeType, null);

assert.strictEqual(messages[1].ruleId, "no-alert");
assert.strictEqual(messages[1].message, "Unexpected alert.");
Expand All @@ -11856,7 +11886,7 @@ describe("Linter with FlatConfigArray", () => {
});

it("should report a violation", () => {
const code = "/*eslint no-alert:0 2*/ alert('test');";
const code = "\n\n\n /*eslint no-alert:0 2*/ alert('test');";

const config = { rules: { "no-alert": 1 } };

Expand All @@ -11873,8 +11903,14 @@ describe("Linter with FlatConfigArray", () => {
* parseJsonConfig function in lib/eslint.js
*/
assert.match(messages[0].message, /^Failed to parse JSON from ' "no-alert":0 2':/u);
assert.strictEqual(messages[0].line, 1);
assert.strictEqual(messages[0].column, 1);
assert.strictEqual(messages[0].line, 4);
assert.strictEqual(messages[0].column, 5);
assert.strictEqual(messages[0].endLine, 4);
assert.strictEqual(messages[0].endColumn, 28);
assert.strictEqual(messages[0].ruleId, null);
assert.strictEqual(messages[0].fatal, true);
assert.strictEqual(messages[0].severity, 2);
assert.strictEqual(messages[0].nodeType, null);

assert.strictEqual(messages[1].ruleId, "no-alert");
assert.strictEqual(messages[1].message, "Unexpected alert.");
Expand Down
11 changes: 8 additions & 3 deletions tools/check-rule-examples.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,17 @@ async function findProblems(filename) {
continue;
}
const { directiveValue } = commentParser.parseDirective(comment);
const parseResult = commentParser.parseJsonConfig(directiveValue, comment.loc);
const parseResult = commentParser.parseJsonConfig(directiveValue);
const parseError = parseResult.error;

if (parseError) {
parseError.line += codeBlockToken.map[0] + 1;
problems.push(parseError);
problems.push({
fatal: true,
severity: 2,
message: parseError.message,
line: comment.loc.start.line + codeBlockToken.map[0] + 1,
column: comment.loc.start.column + 1
});
} else if (Object.hasOwn(parseResult.config, title)) {
if (hasRuleConfigComment) {
problems.push({
Expand Down

0 comments on commit f9e95d2

Please sign in to comment.