diff --git a/README.md b/README.md index 7b819887..28373293 100644 --- a/README.md +++ b/README.md @@ -106,6 +106,6 @@ The package includes the following rules. | [no-unsafe-takeuntil](docs/rules/no-unsafe-takeuntil.md) | Disallow applying operators after `takeUntil`. | ✅ | | | 💭 | | | [prefer-observer](docs/rules/prefer-observer.md) | Disallow passing separate handlers to `subscribe` and `tap`. | | 🔧 | 💡 | 💭 | | | [suffix-subjects](docs/rules/suffix-subjects.md) | Enforce the use of a suffix in subject identifiers. | | | | 💭 | | -| [throw-error](docs/rules/throw-error.md) | Enforce passing only `Error` values to error notifications. | | | | 💭 | | +| [throw-error](docs/rules/throw-error.md) | Enforce passing only `Error` values to `throwError`. | | | | 💭 | | diff --git a/docs/rules/throw-error.md b/docs/rules/throw-error.md index 2764f87c..924b74f3 100644 --- a/docs/rules/throw-error.md +++ b/docs/rules/throw-error.md @@ -1,49 +1,38 @@ -# Enforce passing only `Error` values to error notifications (`rxjs-x/throw-error`) +# Enforce passing only `Error` values to `throwError` (`rxjs-x/throw-error`) 💭 This rule requires [type information](https://typescript-eslint.io/linting/typed-linting). -This rule forbids throwing values that are neither `Error` nor `DOMException` instances. +This rule forbids passing values that are not `Error` objects to `throwError`. +It's similar to the typescript-eslint [`only-throw-error`](https://typescript-eslint.io/rules/only-throw-error/) rule, +but is for the `throwError` Observable creation function - not `throw` statements. ## Rule details Examples of **incorrect** code for this rule: -```ts -throw "Kaboom!"; -``` - ```ts import { throwError } from "rxjs"; -throwError("Kaboom!"); -``` -```ts -import { throwError } from "rxjs"; throwError(() => "Kaboom!"); ``` Examples of **correct** code for this rule: ```ts -throw new Error("Kaboom!"); -``` +import { throwError } from "rxjs"; -```ts -throw new RangeError("Kaboom!"); +throwError(() => new Error("Kaboom!")); ``` -```ts -throw new DOMException("Kaboom!"); -``` +## Options -```ts -import { throwError } from "rxjs"; -throwError(new Error("Kaboom!")); -``` + -```ts -import { throwError } from "rxjs"; -throwError(() => new Error("Kaboom!")); -``` +| Name | Description | Type | Default | +| :--------------------- | :---------------------------------------------------------- | :------ | :------ | +| `allowThrowingAny` | Whether to always allow throwing values typed as `any`. | Boolean | `true` | +| `allowThrowingUnknown` | Whether to always allow throwing values typed as `unknown`. | Boolean | `true` | + + diff --git a/src/rules/throw-error.ts b/src/rules/throw-error.ts index beb6cfae..75c63b6d 100644 --- a/src/rules/throw-error.ts +++ b/src/rules/throw-error.ts @@ -4,54 +4,86 @@ import ts from 'typescript'; import { couldBeFunction, couldBeType, getTypeServices } from '../etc'; import { ruleCreator } from '../utils'; +const defaultOptions: readonly { + allowThrowingAny?: boolean; + allowThrowingUnknown?: boolean; +}[] = []; + export const throwErrorRule = ruleCreator({ - defaultOptions: [], + defaultOptions, meta: { docs: { description: - 'Enforce passing only `Error` values to error notifications.', + 'Enforce passing only `Error` values to `throwError`.', requiresTypeChecking: true, }, messages: { - forbidden: 'Passing non-Error values are forbidden.', + forbidden: 'Passing non-Error values is forbidden.', }, - schema: [], + schema: [ + { + properties: { + allowThrowingAny: { type: 'boolean', default: true, description: 'Whether to always allow throwing values typed as `any`.' }, + allowThrowingUnknown: { type: 'boolean', default: true, description: 'Whether to always allow throwing values typed as `unknown`.' }, + }, + type: 'object', + }, + ], type: 'problem', }, name: 'throw-error', create: (context) => { const { esTreeNodeToTSNodeMap, program, getTypeAtLocation } = ESLintUtils.getParserServices(context); const { couldBeObservable } = getTypeServices(context); + const [config = {}] = context.options; + const { allowThrowingAny = true, allowThrowingUnknown = true } = config; - function checkNode(node: es.Node) { + function checkThrowArgument(node: es.Node) { let type = getTypeAtLocation(node); + let reportNode = node; + if (couldBeFunction(type)) { + reportNode = (node as es.ArrowFunctionExpression).body ?? node; + const tsNode = esTreeNodeToTSNodeMap.get(node); const annotation = (tsNode as ts.ArrowFunction).type; const body = (tsNode as ts.ArrowFunction).body; type = program.getTypeChecker().getTypeAtLocation(annotation ?? body); } - if ( - !tsutils.isIntrinsicAnyType(type) - && !tsutils.isIntrinsicUnknownType(type) - && !couldBeType(type, /^(Error|DOMException)$/) - ) { - context.report({ - messageId: 'forbidden', - node, - }); + + if (allowThrowingAny && tsutils.isIntrinsicAnyType(type)) { + return; + } + + if (allowThrowingUnknown && tsutils.isIntrinsicUnknownType(type)) { + return; + } + + if (couldBeType(type, /^Error$/)) { + return; + } + + context.report({ + messageId: 'forbidden', + node: reportNode, + }); + } + + function checkNode(node: es.CallExpression) { + if (couldBeObservable(node)) { + const [arg] = node.arguments; + if (arg) { + checkThrowArgument(arg); + } } } return { - 'ThrowStatement > *': checkNode, 'CallExpression[callee.name=\'throwError\']': (node: es.CallExpression) => { - if (couldBeObservable(node)) { - const [arg] = node.arguments; - if (arg) { - checkNode(arg); - } - } + checkNode(node); + }, + 'CallExpression[callee.property.name=\'throwError\']': (node: es.CallExpression) => { + checkNode(node); }, }; }, diff --git a/tests/rules/throw-error.test.ts b/tests/rules/throw-error.test.ts index 590821cd..6c51c954 100644 --- a/tests/rules/throw-error.test.ts +++ b/tests/rules/throw-error.test.ts @@ -6,97 +6,130 @@ import { ruleTester } from '../rule-tester'; ruleTester({ types: true }).run('throw-error', throwErrorRule, { valid: [ stripIndent` - // throw Error - const a = () => { throw new Error("error"); }; + // Error + import { throwError } from "rxjs"; + + const ob1 = throwError(() => new Error("Boom!")); `, stripIndent` - // throw DOMException - const a = () => { throw new DOMException("error"); }; + // RangeError + import { throwError } from "rxjs"; + + const ob1 = throwError(() => new RangeError("Boom!")); `, stripIndent` - // throw any - const a = () => { throw "error" as any }; + // DOMException + /// + import { throwError } from "rxjs"; + + const ob1 = throwError(() => new DOMException("Boom!")); `, stripIndent` - // throw returned any - const a = () => { throw errorMessage(); }; + // custom Error + import { throwError } from "rxjs"; - function errorMessage(): any { - return "error"; - } + class MyFailure extends Error {} + + const ob1 = throwError(() => new MyFailure("Boom!")); `, stripIndent` - // throwError Error + // arrow function return import { throwError } from "rxjs"; - const ob1 = throwError(new Error("Boom!")); + throwError(() => { + return new Error("Boom!"); + }); `, stripIndent` - // throwError DOMException + // function return import { throwError } from "rxjs"; - const ob1 = throwError(new DOMException("Boom!")); + throwError(function () { + return new Error("Boom!"); + }); `, stripIndent` - // throwError any + // any import { throwError } from "rxjs"; - const ob1 = throwError("Boom!" as any); + const ob1 = throwError(() => "Boom!" as any); `, stripIndent` - // throwError returned any + // returned any import { throwError } from "rxjs"; - const ob1 = throwError(errorMessage()); + const ob1 = throwError(() => errorMessage()); function errorMessage(): any { return "error"; } `, stripIndent` - // throwError unknown + // unknown import { throwError } from "rxjs"; - const ob1 = throwError("Boom!" as unknown); + const ob1 = throwError(() => "Boom!" as unknown); `, stripIndent` - // throwError returned unknown + // returned unknown import { throwError } from "rxjs"; - const ob1 = throwError(errorMessage()); + const ob1 = throwError(() => errorMessage()); function errorMessage(): unknown { return "error"; } `, stripIndent` - // throwError Error with factory + // Error without factory (deprecated) import { throwError } from "rxjs"; - const ob1 = throwError(() => new Error("Boom!")); + const ob1 = throwError(new Error("Boom!")); `, stripIndent` - // throwError DOMException with factory + // DOMException without factory (deprecated) + /// import { throwError } from "rxjs"; - const ob1 = throwError(() => new DOMException("Boom!")); + const ob1 = throwError(new DOMException("Boom!")); `, stripIndent` - // throwError any with factory + // any without factory (deprecated) import { throwError } from "rxjs"; - const ob1 = throwError(() => "Boom!" as any); + const ob1 = throwError("Boom!" as any); `, stripIndent` - // throwError returned any with factory + // returned any without factory (deprecated) import { throwError } from "rxjs"; - const ob1 = throwError(() => errorMessage()); + const ob1 = throwError(errorMessage()); function errorMessage(): any { return "error"; } `, + stripIndent` + // Object.assign + // https://github.com/cartant/rxjs-tslint-rules/issues/86 + import { throwError } from "rxjs"; + + throwError(() => Object.assign( + new Error("Not Found"), + { code: "NOT_FOUND" } + )); + `, + stripIndent` + // Object.assign arrow function return + import { throwError } from "rxjs"; + + throwError(() => { + return Object.assign( + new Error("Not Found"), + { code: "NOT_FOUND" } + ); + }); + `, stripIndent` // no signature // There will be no signature for callback and @@ -105,6 +138,26 @@ ruleTester({ types: true }).run('throw-error', throwErrorRule, { callback(); `, stripIndent` + // unrelated throw statements (use @typescript-eslint/only-throw-error instead). + const a = () => { throw "error"; }; + const b = () => { throw new Error("error"); }; + + const errorMessage = "Boom!"; + const c = () => { throw errorMessage; }; + + const d = () => { throw errorMessage(); }; + function errorMessage() { + return "error"; + } + + const e = () => { throw new DOMException("error"); }; + const f = () => { throw "error" as any }; + + const g = () => { throw errorMessageAny(); }; + function errorMessageAny(): any { + return "error"; + } + https://github.com/cartant/rxjs-tslint-rules/issues/85 try { throw new Error("error"); @@ -116,34 +169,29 @@ ruleTester({ types: true }).run('throw-error', throwErrorRule, { invalid: [ fromFixture( stripIndent` - // throw string - const a = () => { throw "error"; }; - ~~~~~~~ [forbidden] + // string + import { throwError } from "rxjs"; + + const ob1 = throwError(() => "Boom!"); + ~~~~~~~ [forbidden] `, ), fromFixture( stripIndent` - // throw returned string - const a = () => { throw errorMessage(); }; - ~~~~~~~~~~~~~~ [forbidden] + // returned string + import { throwError } from "rxjs"; + + const ob1 = throwError(() => errorMessage()); + ~~~~~~~~~~~~~~ [forbidden] function errorMessage() { - return "error"; + return "Boom!"; } `, ), fromFixture( stripIndent` - // throw string variable - const errorMessage = "Boom!"; - - const a = () => { throw errorMessage; }; - ~~~~~~~~~~~~ [forbidden] - `, - ), - fromFixture( - stripIndent` - // throwError string + // string without factory (deprecated) import { throwError } from "rxjs"; const ob1 = throwError("Boom!"); @@ -152,7 +200,7 @@ ruleTester({ types: true }).run('throw-error', throwErrorRule, { ), fromFixture( stripIndent` - // throwError returned string + // returned string without factory (deprecated) import { throwError } from "rxjs"; const ob1 = throwError(errorMessage()); @@ -165,38 +213,55 @@ ruleTester({ types: true }).run('throw-error', throwErrorRule, { ), fromFixture( stripIndent` - https://github.com/cartant/rxjs-tslint-rules/issues/86 - const a = () => { throw "error"; }; - ~~~~~~~ [forbidden] - const b = () => { throw new Error("error"); }; - const c = () => { - throw Object.assign( - new Error("Not Found"), - { code: "NOT_FOUND" } - ); - }; + // any not allowed + import { throwError } from "rxjs"; + + throwError(() => "Boom!" as any); + ~~~~~~~~~~~~~~ [forbidden] `, + { options: [{ allowThrowingAny: false }] }, ), fromFixture( stripIndent` - // throwError string with factory + // unknown not allowed import { throwError } from "rxjs"; - const ob1 = throwError(() => "Boom!"); - ~~~~~~~~~~~~~ [forbidden] + throwError(() => "Boom!" as unknown); + ~~~~~~~~~~~~~~~~~~ [forbidden] `, + { options: [{ allowThrowingUnknown: false }] }, ), fromFixture( stripIndent` - // throwError returned string with factory + // falsy import { throwError } from "rxjs"; - const ob1 = throwError(() => errorMessage()); - ~~~~~~~~~~~~~~~~~~~~ [forbidden] + const ob1 = throwError(() => 0); + ~ [forbidden] + const ob2 = throwError(() => false); + ~~~~~ [forbidden] + const ob3 = throwError(() => null); + ~~~~ [forbidden] + const ob4 = throwError(() => undefined); + ~~~~~~~~~ [forbidden] + `, + ), + fromFixture( + stripIndent` + // Object.assign with non-Error + import { throwError } from "rxjs"; - function errorMessage() { - return "Boom!"; - } + throwError(() => Object.assign({ message: "Not Found" }, { code: "NOT_FOUND" })); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [forbidden] + `, + ), + fromFixture( + stripIndent` + // namespace import + import * as Rx from "rxjs"; + + Rx.throwError(() => "Boom!"); + ~~~~~~~ [forbidden] `, ), ],