Skip to content

Commit

Permalink
Add no-abusive-eslint-disable rule
Browse files Browse the repository at this point in the history
This is a rule proposal, but as I wanted to know whether it was actually possible, I *accidentally* went all the way :p

I saw [this message](https://gitter.im/eslint/eslint?at=5751c0393bdac7ae37b46224) by @alanhussey on the ESLint Gitter, and thought it was a pretty good idea.

The proposal is to enforce specifying the rules to disable when you use `eslint-disable`/`eslint-disable-line`, as you can otherwise hide some useful error reports.

Let me know what you think :)

Closes #33
  • Loading branch information
jfmengels authored and sindresorhus committed Jun 7, 2016
1 parent 8f97d7b commit 5494fbb
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 0 deletions.
49 changes: 49 additions & 0 deletions docs/rules/no-abusive-eslint-disable.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# Enforce specifying rules to disable in `eslint-disable` comments

This rule makes you specify the rules you want to disable when using `eslint-disable` or `eslint-disable-line` comments.

If you want to disable an ESLint rule in a file or on a specific line, you can add a comment.

On a single line:

```js
const message = 'foo';
console.log(message); // eslint-disable-line no-console
```

On the whole (rest of the) file:

```js
/* eslint-disable no-console */
const message = 'foo';
console.log(message);
```

You don't have to specify any rules (like `no-console` in the examples above), but you should, as you might otherwise hide useful errors.

```js
/* eslint-disable */
console.log(message); // `message` is not defined, but it won't be reported
```

This rule enforces specifying the rules to disable. If you want to disable ESLint on a file altogether, you should ignore it through [`.eslintignore`](http://eslint.org/docs/user-guide/configuring#ignoring-files-and-directories) for ESLint or through the [`ignores` property](https://github.com/sindresorhus/xo#ignores) in `package.json` for `XO`.


## Fail

```js
/* eslint-disable */
console.log(message);

console.log(message); // eslint-disable-line
```


## Pass

```js
/* eslint-disable no-console */
console.log(message);

console.log(message); // eslint-disable-line no-console
```
2 changes: 2 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module.exports = {
rules: {
'catch-error-name': require('./rules/catch-error-name'),
'filename-case': require('./rules/filename-case'),
'no-abusive-eslint-disable': require('./rules/no-abusive-eslint-disable'),
'no-process-exit': require('./rules/no-process-exit'),
'throw-new-error': require('./rules/throw-new-error')
},
Expand All @@ -19,6 +20,7 @@ module.exports = {
rules: {
'xo/catch-error-name': ['error', {name: 'err'}],
'xo/filename-case': ['error', {case: 'kebabCase'}],
'xo/no-abusive-eslint-disable': 'error',
'xo/no-process-exit': 'error',
'xo/throw-new-error': 'error'
}
Expand Down
2 changes: 2 additions & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ Configure it in `package.json`.
"rules": {
"xo/catch-error-name": ["error", {"name": "err"}],
"xo/filename-case": ["error", {"case": "kebabCase"}],
"xo/no-abusive-eslint-disable": "error",
"xo/no-process-exit": "error",
"xo/throw-new-error": "error"
}
Expand All @@ -45,6 +46,7 @@ Configure it in `package.json`.

- [catch-error-name](docs/rules/catch-error-name.md) - Enforce a specific parameter name in catch clauses.
- [filename-case](docs/rules/filename-case.md) - Enforce a case style for filenames.
- [no-abusive-eslint-disable](docs/rules/no-abusive-eslint-disable.md) - Enforce specifying rules to disable in `eslint-disable` comments.
- [no-process-exit](docs/rules/no-process-exit.md) - Disallow `process.exit()`.
- [throw-new-error](docs/rules/throw-new-error.md) - Require `new` when throwing an error. *(fixable)*

Expand Down
26 changes: 26 additions & 0 deletions rules/no-abusive-eslint-disable.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
'use strict';

var disableRegex = /^eslint-disable(-line)?(\s+([\w-]+))?/;

module.exports = function (context) {
return {
Program: function (node) {
node.comments.forEach(function (comment) {
var value = comment.value.trim();
var res = disableRegex.exec(value);
if (res && // It is a eslint-disable comment
!res[2] // but it did not specify any rules
) {
context.report({
// Can't set it at the given location as the warning
// will be ignored due to the disable comment
loc: {line: 0, column: 0},
// So specify it in the message
message: 'Specify the rules you want to disable at line {{line}}:{{column}}',
data: comment.loc.start
});
}
});
}
};
};
56 changes: 56 additions & 0 deletions test/no-abusive-eslint-disable.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import test from 'ava';
import {RuleTester} from 'eslint';
import rule from '../rules/no-abusive-eslint-disable';

const ruleTester = new RuleTester({
env: {
es6: true
}
});

const error = (message) => ({
ruleId: 'no-abusive-eslint-disable',
message
});

test(() => {
ruleTester.run('no-abusive-eslint-disable', rule, {
valid: [
`eval();`,
`eval(); // eslint-disable-line no-eval`,
`eval(); // eslint-disable-line no-eval, no-console`,
`eval(); //eslint-disable-line no-eval`,
`eval(); // eslint-disable-line no-eval`,
`eval(); //\teslint-disable-line no-eval`,
`eval(); /* eslint-disable-line no-eval */`,
`eval(); /* eslint-disable-line no-eval */`,
`eval(); // eslint-line-disable`,
`eval(); // some comment`,
`foo();
// eslint-disable-line no-eval
eval();`,
'/* eslint-disable no-eval */',
`foo();
/* eslint-disable no-eval */
eval();`
],
invalid: [
{
code: `eval(); // eslint-disable-line`,
errors: [error('Specify the rules you want to disable at line 1:8')]
},
{
code: 'foo();\neval(); // eslint-disable-line',
errors: [error('Specify the rules you want to disable at line 2:8')]
},
{
code: '/* eslint-disable */',
errors: [error('Specify the rules you want to disable at line 1:0')]
},
{
code: 'foo();\n/* eslint-disable */\neval();',
errors: [error('Specify the rules you want to disable at line 2:0')]
}
]
});
});

0 comments on commit 5494fbb

Please sign in to comment.