Skip to content

Commit

Permalink
Breaking: description in directive comments (refs eslint/rfcs#33)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mysticatea
mysticatea committed Dec 22, 2019
1 parent 827259e commit f466894
Show file tree
Hide file tree
Showing 3 changed files with 343 additions and 3 deletions.
20 changes: 20 additions & 0 deletions docs/user-guide/configuring.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,19 @@ If a rule has additional options, you can specify them using array literal synta

This comment specifies the "double" option for the [`quotes`](../rules/quotes) rule. The first item in the array is always the rule severity (number or string).

If the part preceded by `--` exists in the configuration comment, ESLint ignores the part. The `--` can be longer than 2. For examples:

```js
/* eslint eqeqeq: "off", curly: "error" -- You can clarify the reason of this configuration. */
```

```js
/* eslint eqeqeq: "off", curly: "error"
--------
You can clarify the reason of this configuration.
But mind that '*' at beginning lines may cause syntax error. */
```

### Using Configuration Files

To configure rules inside of a configuration file, use the `rules` key along with an error level and any options you want to use. For example:
Expand Down Expand Up @@ -574,6 +587,13 @@ foo(); // eslint-disable-line example/rule-name
foo(); /* eslint-disable-line example/rule-name */
```

If the part preceded by `--` exists in the inline comments, ESLint ignores the part. The `--` can be longer than 2. For examples:

```js
// eslint-disable-next-line no-console -- this console.log makes fine the program for some reason.
console.log('hello');
```

**Note:** Comments that disable warnings for a portion of a file tell ESLint not to report rule violations for the disabled code. ESLint still parses the entire file, however, so disabled code still needs to be syntactically valid JavaScript.


Expand Down
18 changes: 15 additions & 3 deletions lib/linter/linter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,15 @@ function createDisableDirectives(options) {
return result;
}

/**
* Remove the ignored part from a given directive comment and trim it.
* @param {string} value The comment text to strip.
* @returns {string} The stripped text.
*/
function stripDirectiveComment(value) {
return value.split(/\s-{2,}\s/u)[0].trim();
}

/**
* Parses comments in file to extract file-specific config of rules, globals
* and environments and merges them with global config; also code blocks
Expand All @@ -286,7 +295,7 @@ function getDirectiveComments(filename, ast, ruleMapper, warnInlineConfig) {
const disableDirectives = [];

ast.comments.filter(token => token.type !== "Shebang").forEach(comment => {
const trimmedCommentText = comment.value.trim();
const trimmedCommentText = stripDirectiveComment(comment.value);
const match = /^(eslint(?:-env|-enable|-disable(?:(?:-next)?-line)?)?|exported|globals?)(?:\s|$)/u.exec(trimmedCommentText);

if (!match) {
Expand Down Expand Up @@ -444,8 +453,11 @@ function findEslintEnv(text) {

eslintEnvPattern.lastIndex = 0;

while ((match = eslintEnvPattern.exec(text))) {
retv = Object.assign(retv || {}, commentParser.parseListConfig(match[1]));
while ((match = eslintEnvPattern.exec(text)) !== null) {
retv = Object.assign(
retv || {},
commentParser.parseListConfig(stripDirectiveComment(match[1]))
);
}

return retv;
Expand Down
308 changes: 308 additions & 0 deletions tests/lib/linter/linter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3759,6 +3759,314 @@ describe("Linter", () => {
linter.verify("x", { rules: { "foo-bar-baz": "error" } });
assert(spy.calledOnce);
});

describe("comments in directive comments", () => {
it("should ignore the part preceded by '--' in '/*eslint*/'.", () => {
const aaa = sinon.stub().returns({});
const bbb = sinon.stub().returns({});

linter.defineRule("aaa", { create: aaa });
linter.defineRule("bbb", { create: bbb });
const messages = linter.verify(`
/*eslint aaa:error -- bbb:error */
console.log("hello")
`, {});

// Don't include syntax error of the comment.
assert.deepStrictEqual(messages, []);

// Use only `aaa`.
assert.strictEqual(aaa.callCount, 1);
assert.strictEqual(bbb.callCount, 0);
});

it("should ignore the part preceded by '--' in '/*eslint-env*/'.", () => {
const messages = linter.verify(`
/*eslint-env es2015 -- es2017 */
var Promise = {}
var Atomics = {}
`, { rules: { "no-redeclare": "error" } });

// Don't include `Atomics`
assert.deepStrictEqual(
messages,
[{
column: 25,
endColumn: 32,
endLine: 3,
line: 3,
message: "'Promise' is already defined as a built-in global variable.",
messageId: "redeclaredAsBuiltin",
nodeType: "Identifier",
ruleId: "no-redeclare",
severity: 2
}]
);
});

it("should ignore the part preceded by '--' in '/*global*/'.", () => {
const messages = linter.verify(`
/*global aaa -- bbb */
var aaa = {}
var bbb = {}
`, { rules: { "no-redeclare": "error" } });

// Don't include `bbb`
assert.deepStrictEqual(
messages,
[{
column: 30,
line: 2,
message: "'aaa' is already defined by a variable declaration.",
messageId: "redeclaredBySyntax",
nodeType: "Block",
ruleId: "no-redeclare",
severity: 2
}]
);
});

it("should ignore the part preceded by '--' in '/*globals*/'.", () => {
const messages = linter.verify(`
/*globals aaa -- bbb */
var aaa = {}
var bbb = {}
`, { rules: { "no-redeclare": "error" } });

// Don't include `bbb`
assert.deepStrictEqual(
messages,
[{
column: 31,
line: 2,
message: "'aaa' is already defined by a variable declaration.",
messageId: "redeclaredBySyntax",
nodeType: "Block",
ruleId: "no-redeclare",
severity: 2
}]
);
});

it("should ignore the part preceded by '--' in '/*exported*/'.", () => {
const messages = linter.verify(`
/*exported aaa -- bbb */
var aaa = {}
var bbb = {}
`, { rules: { "no-unused-vars": "error" } });

// Don't include `aaa`
assert.deepStrictEqual(
messages,
[{
column: 25,
endColumn: 28,
endLine: 4,
line: 4,
message: "'bbb' is assigned a value but never used.",
nodeType: "Identifier",
ruleId: "no-unused-vars",
severity: 2
}]
);
});

it("should ignore the part preceded by '--' in '/*eslint-disable*/'.", () => {
const messages = linter.verify(`
/*eslint-disable no-redeclare -- no-unused-vars */
var aaa = {}
var aaa = {}
`, { rules: { "no-redeclare": "error", "no-unused-vars": "error" } });

// Do include `no-unused-vars` but not `no-redeclare`
assert.deepStrictEqual(
messages,
[{
column: 25,
endLine: 3,
endColumn: 28,
line: 3,
message: "'aaa' is assigned a value but never used.",
nodeType: "Identifier",
ruleId: "no-unused-vars",
severity: 2
}]
);
});

it("should ignore the part preceded by '--' in '/*eslint-enable*/'.", () => {
const messages = linter.verify(`
/*eslint-disable no-redeclare, no-unused-vars */
/*eslint-enable no-redeclare -- no-unused-vars */
var aaa = {}
var aaa = {}
`, { rules: { "no-redeclare": "error", "no-unused-vars": "error" } });

// Do include `no-redeclare` but not `no-unused-vars`
assert.deepStrictEqual(
messages,
[{
column: 25,
endLine: 5,
endColumn: 28,
line: 5,
message: "'aaa' is already defined.",
messageId: "redeclared",
nodeType: "Identifier",
ruleId: "no-redeclare",
severity: 2
}]
);
});

it("should ignore the part preceded by '--' in '//eslint-disable-line'.", () => {
const messages = linter.verify(`
var aaa = {} //eslint-disable-line no-redeclare -- no-unused-vars
var aaa = {} //eslint-disable-line no-redeclare -- no-unused-vars
`, { rules: { "no-redeclare": "error", "no-unused-vars": "error" } });

// Do include `no-unused-vars` but not `no-redeclare`
assert.deepStrictEqual(
messages,
[{
column: 25,
endLine: 2,
endColumn: 28,
line: 2,
message: "'aaa' is assigned a value but never used.",
nodeType: "Identifier",
ruleId: "no-unused-vars",
severity: 2
}]
);
});

it("should ignore the part preceded by '--' in '/*eslint-disable-line*/'.", () => {
const messages = linter.verify(`
var aaa = {} /*eslint-disable-line no-redeclare -- no-unused-vars */
var aaa = {} /*eslint-disable-line no-redeclare -- no-unused-vars */
`, { rules: { "no-redeclare": "error", "no-unused-vars": "error" } });

// Do include `no-unused-vars` but not `no-redeclare`
assert.deepStrictEqual(
messages,
[{
column: 25,
endLine: 2,
endColumn: 28,
line: 2,
message: "'aaa' is assigned a value but never used.",
nodeType: "Identifier",
ruleId: "no-unused-vars",
severity: 2
}]
);
});

it("should ignore the part preceded by '--' in '//eslint-disable-next-line'.", () => {
const messages = linter.verify(`
//eslint-disable-next-line no-redeclare -- no-unused-vars
var aaa = {}
//eslint-disable-next-line no-redeclare -- no-unused-vars
var aaa = {}
`, { rules: { "no-redeclare": "error", "no-unused-vars": "error" } });

// Do include `no-unused-vars` but not `no-redeclare`
assert.deepStrictEqual(
messages,
[{
column: 25,
endLine: 3,
endColumn: 28,
line: 3,
message: "'aaa' is assigned a value but never used.",
nodeType: "Identifier",
ruleId: "no-unused-vars",
severity: 2
}]
);
});

it("should ignore the part preceded by '--' in '/*eslint-disable-next-line*/'.", () => {
const messages = linter.verify(`
/*eslint-disable-next-line no-redeclare -- no-unused-vars */
var aaa = {}
/*eslint-disable-next-line no-redeclare -- no-unused-vars */
var aaa = {}
`, { rules: { "no-redeclare": "error", "no-unused-vars": "error" } });

// Do include `no-unused-vars` but not `no-redeclare`
assert.deepStrictEqual(
messages,
[{
column: 25,
endLine: 3,
endColumn: 28,
line: 3,
message: "'aaa' is assigned a value but never used.",
nodeType: "Identifier",
ruleId: "no-unused-vars",
severity: 2
}]
);
});

it("should not ignore the part preceded by '--' if the '--' is not surrounded by whitespaces.", () => {
const rule = sinon.stub().returns({});

linter.defineRule("a--rule", { create: rule });
const messages = linter.verify(`
/*eslint a--rule:error */
console.log("hello")
`, {});

// Don't include syntax error of the comment.
assert.deepStrictEqual(messages, []);

// Use `a--rule`.
assert.strictEqual(rule.callCount, 1);
});

it("should ignore the part preceded by '--' even if the '--' is longer than 2.", () => {
const aaa = sinon.stub().returns({});
const bbb = sinon.stub().returns({});

linter.defineRule("aaa", { create: aaa });
linter.defineRule("bbb", { create: bbb });
const messages = linter.verify(`
/*eslint aaa:error -------- bbb:error */
console.log("hello")
`, {});

// Don't include syntax error of the comment.
assert.deepStrictEqual(messages, []);

// Use only `aaa`.
assert.strictEqual(aaa.callCount, 1);
assert.strictEqual(bbb.callCount, 0);
});

it("should ignore the part preceded by '--' with line breaks.", () => {
const aaa = sinon.stub().returns({});
const bbb = sinon.stub().returns({});

linter.defineRule("aaa", { create: aaa });
linter.defineRule("bbb", { create: bbb });
const messages = linter.verify(`
/*eslint aaa:error
--------
bbb:error */
console.log("hello")
`, {});

// Don't include syntax error of the comment.
assert.deepStrictEqual(messages, []);

// Use only `aaa`.
assert.strictEqual(aaa.callCount, 1);
assert.strictEqual(bbb.callCount, 0);
});
});
});

describe("context.getScope()", () => {
Expand Down

0 comments on commit f466894

Please sign in to comment.