Skip to content

Commit

Permalink
Breaking: description in directive comments (refs eslint/rfcs#33) (#1…
Browse files Browse the repository at this point in the history 
…2699)

* Breaking: description in directive comments (refs eslint/rfcs#33)

* Update docs/user-guide/configuring.md

Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>

* Update docs/user-guide/configuring.md

Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>

* Update docs/user-guide/configuring.md

Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>

* Update docs/user-guide/configuring.md

Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>

* Update docs/user-guide/configuring.md

Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>

* Update tests/lib/linter/linter.js

Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>

* Update docs/user-guide/configuring.md

Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>

* update configuring.md

Co-authored-by: Kai Cataldo <kai@kaicataldo.com>
  • Loading branch information
@mysticatea @kaicataldo @btmills
2 people authored and btmills committed Jan 17, 2020
1 parent 7350589 commit cf46df7
Show file tree
Hide file tree
Showing 3 changed files with 349 additions and 3 deletions.
26 changes: 26 additions & 0 deletions docs/user-guide/configuring.md
View file
Expand Up @@ -420,6 +420,25 @@ 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).

Configuration comments can include descriptions to explain why the comment is necessary. The description must occur after the configuration and is separated from the configuration by two or more consecutive `-` characters. For example:

```js
/* eslint eqeqeq: "off", curly: "error" -- Here's a description about why this configuration is necessary. */
```

```js
/* eslint eqeqeq: "off", curly: "error"
--------
Here's a description about why this configuration is necessary. */
```

```js
/* eslint eqeqeq: "off", curly: "error"
* --------
* This will not work due to the line above starting with a '*' character.
*/
```

### 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 +593,13 @@ foo(); // eslint-disable-line example/rule-name
foo(); /* eslint-disable-line example/rule-name */
```

Configuration comments can include descriptions to explain why the comment is necessary. The description must occur after the configuration and is separated from the configuration by two or more consecutive `-` characters. For example:

```js
// eslint-disable-next-line no-console -- Here's a description about why this configuration is necessary.
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
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
Expand Up @@ -3759,6 +3759,314 @@ describe("Linter", () => {
linter.verify("x", { rules: { "foo-bar-baz": "error" } });
assert(spy.calledOnce);
});

describe("descriptions 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 cf46df7

Please sign in to comment.