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

feat(eslint-plugin): add no-implicit-any-catch rule #2202

Merged
merged 18 commits into from Aug 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -122,6 +122,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int
| [`@typescript-eslint/no-extraneous-class`](./docs/rules/no-extraneous-class.md) | Forbids the use of classes as namespaces | | | |
| [`@typescript-eslint/no-floating-promises`](./docs/rules/no-floating-promises.md) | Requires Promise-like values to be handled appropriately | :heavy_check_mark: | | :thought_balloon: |
| [`@typescript-eslint/no-for-in-array`](./docs/rules/no-for-in-array.md) | Disallow iterating over an array with a for-in loop | :heavy_check_mark: | | :thought_balloon: |
| [`@typescript-eslint/no-implicit-any-catch`](./docs/rules/no-implicit-any-catch.md) | Disallow usage of the implicit `any` type in catch clauses | | :wrench: | |
| [`@typescript-eslint/no-implied-eval`](./docs/rules/no-implied-eval.md) | Disallow the use of `eval()`-like methods | :heavy_check_mark: | | :thought_balloon: |
| [`@typescript-eslint/no-inferrable-types`](./docs/rules/no-inferrable-types.md) | Disallows explicit type declarations for variables or parameters initialized to a number, string, or boolean | :heavy_check_mark: | :wrench: | |
| [`@typescript-eslint/no-invalid-void-type`](./docs/rules/no-invalid-void-type.md) | Disallows usage of `void` type outside of generic or return types | | | |
Expand Down
76 changes: 76 additions & 0 deletions packages/eslint-plugin/docs/rules/no-implicit-any-catch.md
@@ -0,0 +1,76 @@
# Disallow usage of the implicit `any` type in catch clauses (`no-implicit-any-catch`)

TypeScript 4.0 added support for adding an explicit `any` or `unknown` type annotation on a catch clause variable.

By default, TypeScript will type a catch clause variable as `any`, so explicitly annotating it as `unknown` can add a lot of safety to your codebase.

The `noImplicitAny` flag in TypeScript does not cover this for backwards compatibility reasons.

## Rule Details

This rule requires an explicit type to be declared on a catch clause variable.

The following pattern is considered a warning:

```ts
try {
// ...
} catch (e) {
// ...
}
```

The following pattern is **_not_** considered a warning:

<!-- TODO: prettier currently removes the type annotations, re-enable this once prettier is updated -->
<!-- prettier-ignore-start -->

```ts
try {
// ...
} catch (e: unknown) {
// ...
}
```

<!-- prettier-ignore-end -->

## Options

The rule accepts an options object with the following properties:

```ts
type Options = {
// if false, disallow specifying `: any` as the error type as well. See also `no-explicit-any`
allowExplicitAny: boolean;
};

const defaults = {
allowExplicitAny: false,
};
```

### `allowExplicitAny`

The follow is is **_not_** considered a warning with `{ allowExplicitAny: true }`

<!-- TODO: prettier currently removes the type annotations, re-enable this once prettier is updated -->
<!-- prettier-ignore-start -->

```ts
try {
// ...
} catch (e: any) {
// ...
}
```

<!-- prettier-ignore-end -->

## When Not To Use It

If you are not using TypeScript 4.0 (or greater), then you will not be able to use this rule, annotations on catch clauses is not supported.

## Further Reading

- [TypeScript 4.0 Beta Release Notes](https://devblogs.microsoft.com/typescript/announcing-typescript-4-0-beta/#unknown-on-catch)
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/all.ts
Expand Up @@ -58,6 +58,7 @@ export = {
'@typescript-eslint/no-extraneous-class': 'error',
'@typescript-eslint/no-floating-promises': 'error',
'@typescript-eslint/no-for-in-array': 'error',
'@typescript-eslint/no-implicit-any-catch': 'error',
'@typescript-eslint/no-implied-eval': 'error',
'@typescript-eslint/no-inferrable-types': 'error',
'no-invalid-this': 'off',
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -31,6 +31,7 @@ import noDynamicDelete from './no-dynamic-delete';
import noEmptyFunction from './no-empty-function';
import noEmptyInterface from './no-empty-interface';
import noExplicitAny from './no-explicit-any';
import noImplicitAnyCatch from './no-implicit-any-catch';
import noExtraneousClass from './no-extraneous-class';
import noExtraNonNullAssertion from './no-extra-non-null-assertion';
import noExtraParens from './no-extra-parens';
Expand Down Expand Up @@ -132,6 +133,7 @@ export default {
'no-empty-function': noEmptyFunction,
'no-empty-interface': noEmptyInterface,
'no-explicit-any': noExplicitAny,
'no-implicit-any-catch': noImplicitAnyCatch,
'no-extra-non-null-assertion': noExtraNonNullAssertion,
'no-extra-parens': noExtraParens,
'no-extra-semi': noExtraSemi,
Expand Down
95 changes: 95 additions & 0 deletions packages/eslint-plugin/src/rules/no-implicit-any-catch.ts
@@ -0,0 +1,95 @@
import * as util from '../util';
import {
TSESLint,
AST_NODE_TYPES,
} from '@typescript-eslint/experimental-utils';

export type Options = [
{
allowExplicitAny: boolean;
},
];
export type MessageIds =
| 'implicitAnyInCatch'
| 'explicitAnyInCatch'
| 'suggestExplicitUnknown';

export default util.createRule<Options, MessageIds>({
name: 'no-implicit-any-catch',
meta: {
type: 'suggestion',
docs: {
description: 'Disallow usage of the implicit `any` type in catch clauses',
category: 'Best Practices',
recommended: false,
suggestion: true,
},
fixable: 'code',
messages: {
implicitAnyInCatch: 'Implicit any in catch clause',
explicitAnyInCatch: 'Explicit any in catch clause',
suggestExplicitUnknown:
'Use `unknown` instead, this will force you to explicitly, and safely assert the type is correct.',
},
schema: [
{
type: 'object',
additionalProperties: false,
properties: {
allowExplicitAny: {
type: 'boolean',
},
},
},
],
},
defaultOptions: [
{
allowExplicitAny: false,
},
],
create(context, [{ allowExplicitAny }]) {
return {
CatchClause(node): void {
if (!node.param) {
return; // ignore catch without variable
}

if (!node.param.typeAnnotation) {
context.report({
node,
messageId: 'implicitAnyInCatch',
suggest: [
{
messageId: 'suggestExplicitUnknown',
fix(fixer): TSESLint.RuleFix {
return fixer.insertTextAfter(node.param!, ': unknown');
},
},
],
});
} else if (
!allowExplicitAny &&
node.param.typeAnnotation.typeAnnotation.type ===
AST_NODE_TYPES.TSAnyKeyword
) {
context.report({
node,
messageId: 'explicitAnyInCatch',
suggest: [
{
messageId: 'suggestExplicitUnknown',
fix(fixer): TSESLint.RuleFix {
return fixer.replaceText(
node.param!.typeAnnotation!,
': unknown',
);
},
},
],
});
}
},
};
},
});
78 changes: 78 additions & 0 deletions packages/eslint-plugin/tests/rules/no-implicit-any-catch.test.ts
@@ -0,0 +1,78 @@
/* eslint-disable eslint-comments/no-use */
// TODO - prettier currently removes the type annotations, re-enable this once prettier is updated
/* eslint "@typescript-eslint/internal/plugin-test-formatting": ["error", { formatWithPrettier: false }] */
/* eslint-enable eslint-comments/no-use */

import rule from '../../src/rules/no-implicit-any-catch';
import { RuleTester } from '../RuleTester';

const ruleTester = new RuleTester({
parser: '@typescript-eslint/parser',
});

ruleTester.run('no-implicit-any-catch', rule, {
valid: [
`
try {
} catch (e1: unknown) {}
`,
{
code: `
try {
} catch (e2: any) {}
`,
options: [{ allowExplicitAny: true }],
},
],
invalid: [
{
code: `
try {
} catch (e3) {}
`.trim(),
errors: [
{
line: 2,
column: 3,
messageId: 'implicitAnyInCatch',
endLine: 2,
endColumn: 16,
suggestions: [
{
messageId: 'suggestExplicitUnknown',
output: `
try {
} catch (e3: unknown) {}
`.trim(),
},
],
},
],
},
{
code: `
try {
} catch (e4: any) {}
`.trim(),
options: [{ allowExplicitAny: false }],
errors: [
{
line: 2,
column: 3,
messageId: 'explicitAnyInCatch',
endLine: 2,
endColumn: 21,
suggestions: [
{
messageId: 'suggestExplicitUnknown',
output: `
try {
} catch (e4: unknown) {}
`.trim(),
},
],
},
],
},
],
});