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.

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

Add prefer-optional-catch-binding rule #671

Merged
21 changes: 21 additions & 0 deletions docs/rules/prefer-optional-catch-binding.md
@@ -0,0 +1,21 @@
# Prefer omitted the `catch` binding parameter.

If the `catch` binding parameter is not used, it should be omitted.

This rule is fixable.

## Fail

```js
try {} catch (unused) {}
```

## Pass

```js
try {} catch {}
```

```js
try {} catch (used) {}
```
1 change: 1 addition & 0 deletions index.js
Expand Up @@ -54,6 +54,7 @@ module.exports = {
'unicorn/prefer-node-append': 'error',
'unicorn/prefer-node-remove': 'error',
'unicorn/prefer-number-properties': 'error',
'unicorn/prefer-optional-catch-binding': 'error',
'unicorn/prefer-query-selector': 'error',
'unicorn/prefer-reflect-apply': 'error',
// TODO: Enable this by default when it's shipping in a Node.js LTS version.
Expand Down
2 changes: 2 additions & 0 deletions readme.md
Expand Up @@ -70,6 +70,7 @@ Configure it in `package.json`.
"unicorn/prefer-node-append": "error",
"unicorn/prefer-node-remove": "error",
"unicorn/prefer-number-properties": "error",
"unicorn/prefer-optional-catch-binding": "error",
"unicorn/prefer-query-selector": "error",
"unicorn/prefer-reflect-apply": "error",
"unicorn/prefer-replace-all": "off",
Expand Down Expand Up @@ -126,6 +127,7 @@ Configure it in `package.json`.
- [prefer-node-append](docs/rules/prefer-node-append.md) - Prefer `Node#append()` over `Node#appendChild()`. *(fixable)*
- [prefer-node-remove](docs/rules/prefer-node-remove.md) - Prefer `childNode.remove()` over `parentNode.removeChild(childNode)`. *(fixable)*
- [prefer-number-properties](docs/rules/prefer-number-properties.md) - Prefer `Number` static properties over global ones. *(fixable)*
- [prefer-optional-catch-binding](docs/rules/prefer-optional-catch-binding.md) - Prefer omitted the `catch` binding parameter. *(fixable)*
- [prefer-query-selector](docs/rules/prefer-query-selector.md) - Prefer `.querySelector()` over `.getElementById()`, `.querySelectorAll()` over `.getElementsByClassName()` and `.getElementsByTagName()`. *(partly fixable)*
- [prefer-reflect-apply](docs/rules/prefer-reflect-apply.md) - Prefer `Reflect.apply()` over `Function#apply()`. *(fixable)*
- [prefer-replace-all](docs/rules/prefer-replace-all.md) - Prefer `String#replaceAll()` over regex searches with the global flag. *(fixable)*
Expand Down
61 changes: 61 additions & 0 deletions rules/prefer-optional-catch-binding.js
@@ -0,0 +1,61 @@
'use strict';
const getDocumentationUrl = require('./utils/get-documentation-url');
const {findVariable, isOpeningParenToken, isClosingParenToken} = require('eslint-utils');

const ERROR_MESSAGE_ID = 'error';

const selector = [
'CatchClause',
'>',
'Identifier.param'
].join('');

const create = context => {
return {
[selector]: node => {
const scope = context.getScope();
const variable = findVariable(scope, node);

if (variable.references.length !== 0) {
return;
}

const {name} = node;

context.report({
node,
messageId: ERROR_MESSAGE_ID,
data: {name},
fix: fixer => {
const tokenBefore = context.getTokenBefore(node);
const tokenAfter = context.getTokenAfter(node);

/* istanbul ignore next */
if (!isOpeningParenToken(tokenBefore) || !isClosingParenToken(tokenAfter)) {
throw new Error('Unexpected token.');
}
sindresorhus marked this conversation as resolved.
Show resolved Hide resolved

return [
tokenBefore,
node,
tokenAfter
].map(nodeOrToken => fixer.remove(nodeOrToken));
}
});
}
};
};

module.exports = {
create,
meta: {
type: 'suggestion',
docs: {
url: getDocumentationUrl(__filename)
},
fixable: 'code',
messages: {
[ERROR_MESSAGE_ID]: 'Remove unused catch binding `{{name}}`.'
}
}
};
164 changes: 164 additions & 0 deletions test/prefer-optional-catch-binding.js
@@ -0,0 +1,164 @@
import test from 'ava';
import avaRuleTester from 'eslint-ava-rule-tester';
import {outdent} from 'outdent';
import rule from '../rules/prefer-optional-catch-binding';

const ERROR_MESSAGE_ID = 'error';
const generateError = name => ({messageId: ERROR_MESSAGE_ID, data: {name}});

const ruleTester = avaRuleTester(test, {
parserOptions: {
ecmaVersion: 2020
}
});

ruleTester.run('prefer-optional-catch-binding2', rule, {
valid: [
'try {} catch {}',
outdent`
try {} catch {
error
}
`,
outdent`
try {} catch(used) {
console.error(used);
}
`,
outdent`
try {} catch(usedInADeeperScope) {
function foo() {
function bar() {
console.error(usedInADeeperScope);
}
}
}
`,
// We are not checking destructuring
'try {} catch({message}) {}',
'try {} catch({nonExistsProperty = thisWillExecute()}) {}'
],
invalid: [
{
code: 'try {} catch(_) {}',
output: 'try {} catch {}',
errors: [generateError('_')]
},
{
code: outdent`
try {} catch(foo) {
function bar(foo) {}
}
`,
output: outdent`
try {} catch {
function bar(foo) {}
}
`,
errors: [generateError('foo')]
},
// Many
{
code: outdent`
try {} catch(outer) {
try {} catch(inner) {
}
}
try {
try {} catch(inTry) {
}
} catch(another) {
try {} catch(inCatch) {
}
} finally {
try {} catch(inFinally) {
}
}
`,
output: outdent`
try {} catch {
try {} catch {
}
}
try {
try {} catch {
}
} catch {
try {} catch {
}
} finally {
try {} catch {
}
}
`,
errors: [
generateError('outer'),
generateError('inner'),
generateError('inTry'),
generateError('another'),
generateError('inCatch'),
generateError('inFinally')
]
},
// Actual message
{
code: 'try {} catch(theRealErrorName) {}',
output: 'try {} catch {}',
errors: [{message: 'Remove unused catch binding `theRealErrorName`.'}]
},
// TODO: this `error` should be able to remove
// {
// code: outdent`
// try {
// } catch(foo) {
// var foo = 1;
// }
// `,
// output: outdent`
// try {
// } catch {
// var foo = 1;
// }
// `,
// errors: [generateError('foo')]
// },
// Comments
{
code: outdent`
/* comment */
try {
/* comment */
// comment
} catch (
/* comment */
// comment
unused
/* comment */
// comment
) {
/* comment */
// comment
}
/* comment */
`,
output: outdent`
/* comment */
try {
/* comment */
// comment
} catch{SPACE}
/* comment */
// comment
{TAB}
/* comment */
// comment
{
/* comment */
// comment
}
/* comment */
`.replace('{SPACE}', ' ').replace('{TAB}', '\t'),
errors: [generateError('unused')]
}
]
});