Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update: support eslint-disable-* block comments (fixes #8781) #9745

Merged
merged 3 commits into from
Feb 10, 2018
Merged

Update: support eslint-disable-* block comments (fixes #8781) #9745

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dc22098
Update: support eslint-disable-* block comments (fixes #8781)
Dec 20, 2017
7e14e53
updating tests
Feb 7, 2018
d5f435e
more cleaning up tests
Feb 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 17 additions & 1 deletion docs/user-guide/configuring.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,13 +394,18 @@ You can also disable or enable specific rules for an entire file:
alert('foo');
```

To disable all rules on a specific line, use a line comment in one of the following formats:
To disable all rules on a specific line, use a line or block comment in one of the following formats:

```js
alert('foo'); // eslint-disable-line

// eslint-disable-next-line
alert('foo');

/* eslint-disable-next-line */
alert('foo');

alert('foo'); /* eslint-disable-line */
```

To disable a specific rule on a specific line:
Expand All @@ -410,6 +415,11 @@ alert('foo'); // eslint-disable-line no-alert

// eslint-disable-next-line no-alert
alert('foo');

alert('foo'); /* eslint-disable-line no-alert */

/* eslint-disable-next-line no-alert */
alert('foo');
```

To disable multiple rules on a specific line:
Expand All @@ -419,12 +429,18 @@ alert('foo'); // eslint-disable-line no-alert, quotes, semi

// eslint-disable-next-line no-alert, quotes, semi
alert('foo');

alert('foo'); /* eslint-disable-line no-alert, quotes, semi */

/* eslint-disable-next-line no-alert, quotes, semi */
alert('foo');
```

All of the above methods also work for plugin rules. For example, to disable `eslint-plugin-example`'s `rule-name` rule, combine the plugin's name (`example`) and the rule's name (`rule-name`) into `example/rule-name`:

```js
foo(); // eslint-disable-line example/rule-name
foo(); /* eslint-disable-line example/rule-name */
```

**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
13 changes: 12 additions & 1 deletion lib/linter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ function modifyConfigsFromComments(filename, ast, config, ruleMapper) {

if (match) {
value = value.slice(match.index + match[1].length);

if (comment.type === "Block") {
switch (match[1]) {
case "exported":
Expand All @@ -323,6 +322,18 @@ function modifyConfigsFromComments(filename, ast, config, ruleMapper) {
[].push.apply(disableDirectives, createDisableDirectives("disable", comment.loc.start, value));
break;

case "eslint-disable-line":
if (comment.loc.start.line === comment.loc.end.line) {
[].push.apply(disableDirectives, createDisableDirectives("disable-line", comment.loc.start, value));
}
break;

case "eslint-disable-next-line":
if (comment.loc.start.line === comment.loc.end.line) {
[].push.apply(disableDirectives, createDisableDirectives("disable-next-line", comment.loc.start, value));
}
break;

case "eslint-enable":
[].push.apply(disableDirectives, createDisableDirectives("enable", comment.loc.start, value));
break;
Expand Down
174 changes: 165 additions & 9 deletions tests/lib/linter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1889,7 +1889,7 @@ describe("Linter", () => {
});
});

describe("when evaluating code with comments to ignore reporting on of specific rules on a specific line", () => {
describe("when evaluating code with comments to ignore reporting on specific rules on a specific line", () => {

describe("eslint-disable-line", () => {
it("should report a violation", () => {
Expand Down Expand Up @@ -1931,22 +1931,60 @@ describe("Linter", () => {
assert.strictEqual(messages[0].ruleId, "no-alert");
});

it("should report a violation", () => {
it("should report a violation if eslint-disable-line in a block comment is not on a single line", () => {
const code = [
"alert('test'); // eslint-disable-line no-alert",
"alert('test'); /*eslint-disable-line no-alert*/" // here
"/* eslint-disable-line",
"*",
"*/ console.log('test');" // here
].join("\n");
const config = {
rules: {
"no-alert": 1
"no-console": 1
}
};

const messages = linter.verify(code, config, filename);

assert.strictEqual(messages.length, 1);

assert.strictEqual(messages[0].ruleId, "no-console");
});

it("should report a violation if eslint-disable-line in a block comment is not on a single line", () => {
const code = [
"alert('test'); /* eslint-disable-line ",
"no-alert */"
].join("\n");
const config = {
rules: {
"no-alert": 1,
quotes: [1, "double"],
semi: [1, "always"]
}
};

const messages = linter.verify(code, config, filename);

assert.strictEqual(messages.length, 2);

assert.strictEqual(messages[0].ruleId, "no-alert");
assert.strictEqual(messages[1].ruleId, "quotes");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear which line in the fixture (quotes in alert or quotes in console.log) this message points to. Could we either add a line check, or change one of the examples so this is unambiguous? (My understanding is the quotes in alert should be warned, but the quotes in console.log should not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I just removed the console.log since it wasn't really necessary for this example anyway

});

it("should not report a violation for eslint-disable-line in block comment", () => {
const code = [
"alert('test'); // eslint-disable-line no-alert",
"alert('test'); /*eslint-disable-line no-alert*/"
].join("\n");
const config = {
rules: {
"no-alert": 1
}
};

const messages = linter.verify(code, config, filename);

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

it("should not report a violation", () => {
Expand Down Expand Up @@ -1984,6 +2022,40 @@ describe("Linter", () => {

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

it("should not report a violation", () => {
const code = [
"alert('test') /* eslint-disable-line no-alert, quotes, semi */",
"console.log('test'); /* eslint-disable-line */"
].join("\n");
const config = {
rules: {
"no-alert": 1,
quotes: [1, "double"],
semi: [1, "always"],
"no-console": 1
}
};

const messages = linter.verify(code, config, filename);

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

it("should ignore violations of multiple rules when specified in mixed comments", () => {
const code = [
" alert(\"test\"); /* eslint-disable-line no-alert */ // eslint-disable-line quotes"
].join("\n");
const config = {
rules: {
"no-alert": 1,
quotes: [1, "single"]
}
};
const messages = linter.verify(code, config, filename);

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

describe("eslint-disable-next-line", () => {
Expand All @@ -2005,6 +2077,54 @@ describe("Linter", () => {
assert.strictEqual(messages[0].ruleId, "no-console");
});

it("should ignore violation of specified rule if eslint-disable-next-line is a block comment", () => {
const code = [
"/* eslint-disable-next-line no-alert */",
"alert('test');",
"console.log('test');"
].join("\n");
const config = {
rules: {
"no-alert": 1,
"no-console": 1
}
};
const messages = linter.verify(code, config, filename);

assert.strictEqual(messages.length, 1);
assert.strictEqual(messages[0].ruleId, "no-console");
});
it("should ignore violation of specified rule if eslint-disable-next-line is a block comment", () => {
const code = [
"/* eslint-disable-next-line no-alert */",
"alert('test');"
].join("\n");
const config = {
rules: {
"no-alert": 1
}
};
const messages = linter.verify(code, config, filename);

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

it("should not ignore violation if block comment is not on a single line", () => {
const code = [
"/* eslint-disable-next-line",
"no-alert */alert('test');"
].join("\n");
const config = {
rules: {
"no-alert": 1
}
};
const messages = linter.verify(code, config, filename);

assert.strictEqual(messages.length, 1);
assert.strictEqual(messages[0].ruleId, "no-alert");
});

it("should ignore violations only of specified rule", () => {
const code = [
"// eslint-disable-next-line no-console",
Expand Down Expand Up @@ -2043,6 +2163,22 @@ describe("Linter", () => {
assert.strictEqual(messages[0].ruleId, "no-console");
});

it("should ignore violations of multiple rules when specified in mixed comments", () => {
const code = [
"/* eslint-disable-next-line no-alert */ // eslint-disable-next-line quotes",
"alert(\"test\");"
].join("\n");
const config = {
rules: {
"no-alert": 1,
quotes: [1, "single"]
}
};
const messages = linter.verify(code, config, filename);

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

it("should ignore violations of only the specified rule on next line", () => {
const code = [
"// eslint-disable-next-line quotes",
Expand Down Expand Up @@ -2103,7 +2239,7 @@ describe("Linter", () => {
assert.strictEqual(messages[0].ruleId, "no-console");
});

it("should not ignore violations if comment is in block quotes", () => {
it("should ignore violations if eslint-disable-next-line is a block comment", () => {
const code = [
"alert('test');",
"/* eslint-disable-next-line no-alert */",
Expand All @@ -2118,10 +2254,30 @@ describe("Linter", () => {
};
const messages = linter.verify(code, config, filename);

assert.strictEqual(messages.length, 3);
assert.strictEqual(messages.length, 2);
assert.strictEqual(messages[0].ruleId, "no-alert");
assert.strictEqual(messages[1].ruleId, "no-alert");
assert.strictEqual(messages[2].ruleId, "no-console");
assert.strictEqual(messages[1].ruleId, "no-console");
});

it("should report a violation", () => {
const code = [
"/* eslint-disable-next-line",
"*",
"*/",
"console.log('test');" // here
].join("\n");
const config = {
rules: {
"no-alert": 1,
"no-console": 1
}
};

const messages = linter.verify(code, config, filename);

assert.strictEqual(messages.length, 1);

assert.strictEqual(messages[0].ruleId, "no-console");
});

it("should not ignore violations if comment is of the type Shebang", () => {
Expand Down