diff --git a/README.md b/README.md index c2e88286..4c0f50d9 100644 --- a/README.md +++ b/README.md @@ -98,7 +98,7 @@ The package includes the following rules. | [no-subject-value](docs/rules/no-subject-value.md) | Disallow accessing the `value` property of a `BehaviorSubject` instance. | | | | 💭 | | | [no-subscribe-handlers](docs/rules/no-subscribe-handlers.md) | Disallow passing handlers to `subscribe`. | | | | 💭 | | | [no-tap](docs/rules/no-tap.md) | Disallow the `tap` operator. | | | | | ❌ | -| [no-topromise](docs/rules/no-topromise.md) | Disallow use of the `toPromise` method. | | | | 💭 | | +| [no-topromise](docs/rules/no-topromise.md) | Disallow use of the `toPromise` method. | | | 💡 | 💭 | | | [no-unbound-methods](docs/rules/no-unbound-methods.md) | Disallow passing unbound methods. | ✅ | | | 💭 | | | [no-unsafe-catch](docs/rules/no-unsafe-catch.md) | Disallow unsafe `catchError` usage in effects and epics. | | | | 💭 | | | [no-unsafe-first](docs/rules/no-unsafe-first.md) | Disallow unsafe `first`/`take` usage in effects and epics. | | | | 💭 | | diff --git a/docs/rules/no-topromise.md b/docs/rules/no-topromise.md index 1b1d2c60..f88c565a 100644 --- a/docs/rules/no-topromise.md +++ b/docs/rules/no-topromise.md @@ -1,7 +1,13 @@ # Disallow use of the `toPromise` method (`rxjs-x/no-topromise`) +💡 This rule is manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions). + 💭 This rule requires [type information](https://typescript-eslint.io/linting/typed-linting). This rule effects failures if the `toPromise` method is used. + +## Further reading + +- [Conversion to Promises](https://rxjs.dev/deprecations/to-promise) diff --git a/src/etc/is.ts b/src/etc/is.ts index 57e648dc..7e3f4d6b 100644 --- a/src/etc/is.ts +++ b/src/etc/is.ts @@ -62,6 +62,18 @@ export function isIdentifier(node: TSESTree.Node): node is TSESTree.Identifier { return node.type === AST_NODE_TYPES.Identifier; } +export function isImportDeclaration(node: TSESTree.Node): node is TSESTree.ImportDeclaration { + return node.type === AST_NODE_TYPES.ImportDeclaration; +} + +export function isImportNamespaceSpecifier(node: TSESTree.Node): node is TSESTree.ImportNamespaceSpecifier { + return node.type === AST_NODE_TYPES.ImportNamespaceSpecifier; +} + +export function isImportSpecifier(node: TSESTree.Node): node is TSESTree.ImportSpecifier { + return node.type === AST_NODE_TYPES.ImportSpecifier; +} + export function isLiteral(node: TSESTree.Node): node is TSESTree.Literal { return node.type === AST_NODE_TYPES.Literal; } diff --git a/src/rules/no-topromise.ts b/src/rules/no-topromise.ts index 7b2e4071..c71acedf 100644 --- a/src/rules/no-topromise.ts +++ b/src/rules/no-topromise.ts @@ -1,5 +1,5 @@ -import { TSESTree as es } from '@typescript-eslint/utils'; -import { getTypeServices } from '../etc'; +import { TSESTree as es, TSESLint } from '@typescript-eslint/utils'; +import { getTypeServices, isIdentifier, isImportDeclaration, isImportNamespaceSpecifier, isImportSpecifier } from '../etc'; import { ruleCreator } from '../utils'; export const noTopromiseRule = ruleCreator({ @@ -9,8 +9,11 @@ export const noTopromiseRule = ruleCreator({ description: 'Disallow use of the `toPromise` method.', requiresTypeChecking: true, }, + hasSuggestions: true, messages: { forbidden: 'The toPromise method is forbidden.', + suggestLastValueFrom: 'Use lastValueFrom instead.', + suggestFirstValueFrom: 'Use firstValueFrom instead.', }, schema: [], type: 'problem', @@ -18,16 +21,93 @@ export const noTopromiseRule = ruleCreator({ name: 'no-topromise', create: (context) => { const { couldBeObservable } = getTypeServices(context); + + function getQuote(raw: string) { + const match = /^\s*('|")/.exec(raw); + if (!match) { + return undefined; + } + const [, quote] = match; + return quote; + } + + function createFix( + conversion: 'lastValueFrom' | 'firstValueFrom', + callExpression: es.CallExpression, + observableNode: es.Node, + importDeclarations: es.ImportDeclaration[], + ) { + return function* fix(fixer: TSESLint.RuleFixer) { + let namespace = ''; + let functionName: string = conversion; + + const rxjsImportDeclaration = importDeclarations.find(node => node.source.value === 'rxjs'); + + if (rxjsImportDeclaration?.specifiers?.every(isImportNamespaceSpecifier)) { + // Existing rxjs namespace import. Use alias. + namespace = rxjsImportDeclaration.specifiers[0].local.name + '.'; + } else if (rxjsImportDeclaration?.specifiers?.every(isImportSpecifier)) { + // Existing rxjs named import. + const { specifiers } = rxjsImportDeclaration; + const existingSpecifier = specifiers.find(node => (isIdentifier(node.imported) ? node.imported.name : node.imported.value) === functionName); + if (existingSpecifier) { + // Function already imported. Use its alias, if any. + functionName = existingSpecifier.local.name; + } else { + // Function not already imported. Add it. + const lastSpecifier = specifiers[specifiers.length - 1]; + yield fixer.insertTextAfter(lastSpecifier, `, ${functionName}`); + } + } else if (importDeclarations.length) { + // No rxjs import. Add to end of imports, respecting quotes. + const lastImport = importDeclarations[importDeclarations.length - 1]; + const quote = getQuote(lastImport.source.raw) ?? '"'; + yield fixer.insertTextAfter( + importDeclarations[importDeclarations.length - 1], + `\nimport { ${functionName} } from ${quote}rxjs${quote};`, + ); + } else { + console.warn('No import declarations found. Unable to suggest a fix.'); + return; + } + + yield fixer.replaceText( + callExpression, + `${namespace}${functionName}(${context.sourceCode.getText(observableNode)})`, + ); + }; + } + return { - [`MemberExpression[property.name="toPromise"]`]: ( - node: es.MemberExpression, + [`CallExpression[callee.property.name="toPromise"]`]: ( + node: es.CallExpression, ) => { - if (couldBeObservable(node.object)) { - context.report({ - messageId: 'forbidden', - node: node.property, - }); + const memberExpression = node.callee as es.MemberExpression; + if (!couldBeObservable(memberExpression.object)) { + return; } + + const { body } = context.sourceCode.ast; + const importDeclarations = body.filter(isImportDeclaration); + if (!importDeclarations.length) { + // couldBeObservable yet no imports? Skip. + return; + } + + context.report({ + messageId: 'forbidden', + node: memberExpression.property, + suggest: [ + { + messageId: 'suggestLastValueFrom', + fix: createFix('lastValueFrom', node, memberExpression.object, importDeclarations), + }, + { + messageId: 'suggestFirstValueFrom', + fix: createFix('firstValueFrom', node, memberExpression.object, importDeclarations), + }, + ], + }); }, }; }, diff --git a/tests/rules/no-topromise.test.ts b/tests/rules/no-topromise.test.ts index fabb684a..79e21249 100644 --- a/tests/rules/no-topromise.test.ts +++ b/tests/rules/no-topromise.test.ts @@ -20,6 +20,16 @@ ruleTester({ types: true }).run('no-topromise', noTopromiseRule, { }; a.toPromise().then(value => console.log(value)); `, + stripIndent` + // no imports + class Observable { + toPromise() { + return Promise.resolve("a"); + } + } + const a = new Observable(); + a.toPromise().then(value => console.log(value)); + `, ], invalid: [ fromFixture( @@ -28,8 +38,30 @@ ruleTester({ types: true }).run('no-topromise', noTopromiseRule, { import { of } from "rxjs"; const a = of("a"); a.toPromise().then(value => console.log(value)); - ~~~~~~~~~ [forbidden] + ~~~~~~~~~ [forbidden suggest 0 1] `, + { + suggestions: [ + { + messageId: 'suggestLastValueFrom', + output: stripIndent` + // observable toPromise + import { of, lastValueFrom } from "rxjs"; + const a = of("a"); + lastValueFrom(a).then(value => console.log(value)); + `, + }, + { + messageId: 'suggestFirstValueFrom', + output: stripIndent` + // observable toPromise + import { of, firstValueFrom } from "rxjs"; + const a = of("a"); + firstValueFrom(a).then(value => console.log(value)); + `, + }, + ], + }, ), fromFixture( stripIndent` @@ -37,8 +69,166 @@ ruleTester({ types: true }).run('no-topromise', noTopromiseRule, { import { Subject } from "rxjs"; const a = new Subject(); a.toPromise().then(value => console.log(value)); - ~~~~~~~~~ [forbidden] + ~~~~~~~~~ [forbidden suggest 0 1] + `, + { + suggestions: [ + { + messageId: 'suggestLastValueFrom', + output: stripIndent` + // subject toPromise + import { Subject, lastValueFrom } from "rxjs"; + const a = new Subject(); + lastValueFrom(a).then(value => console.log(value)); + `, + }, + { + messageId: 'suggestFirstValueFrom', + output: stripIndent` + // subject toPromise + import { Subject, firstValueFrom } from "rxjs"; + const a = new Subject(); + firstValueFrom(a).then(value => console.log(value)); + `, + }, + ], + }, + ), + fromFixture( + stripIndent` + // weird whitespace + import { of } from "rxjs"; + const a = { foo$: of("a") }; + a + .foo$ + .toPromise().then(value => console.log(value)) + ~~~~~~~~~ [forbidden suggest 0 1] + .catch(error => console.error(error)); + `, + { + suggestions: [ + { + messageId: 'suggestLastValueFrom', + output: stripIndent` + // weird whitespace + import { of, lastValueFrom } from "rxjs"; + const a = { foo$: of("a") }; + lastValueFrom(a + .foo$).then(value => console.log(value)) + .catch(error => console.error(error)); + `, + }, + { + messageId: 'suggestFirstValueFrom', + output: stripIndent` + // weird whitespace + import { of, firstValueFrom } from "rxjs"; + const a = { foo$: of("a") }; + firstValueFrom(a + .foo$).then(value => console.log(value)) + .catch(error => console.error(error)); + `, + }, + ], + }, + ), + fromFixture( + stripIndent` + // lastValueFrom already imported + import { lastValueFrom as lvf, of } from "rxjs"; + const a = of("a"); + a.toPromise().then(value => console.log(value)); + ~~~~~~~~~ [forbidden suggest 0 1] + `, + { + suggestions: [ + { + messageId: 'suggestLastValueFrom', + output: stripIndent` + // lastValueFrom already imported + import { lastValueFrom as lvf, of } from "rxjs"; + const a = of("a"); + lvf(a).then(value => console.log(value)); + `, + }, + { + messageId: 'suggestFirstValueFrom', + output: stripIndent` + // lastValueFrom already imported + import { lastValueFrom as lvf, of, firstValueFrom } from "rxjs"; + const a = of("a"); + firstValueFrom(a).then(value => console.log(value)); + `, + }, + ], + }, + ), + fromFixture( + stripIndent` + // rxjs not already imported + import { fromFetch } from "rxjs/fetch"; + + const a = fromFetch("https://api.some.com"); + a.toPromise().then(value => console.log(value)); + ~~~~~~~~~ [forbidden suggest 0 1] + `, + { + suggestions: [ + { + messageId: 'suggestLastValueFrom', + output: stripIndent` + // rxjs not already imported + import { fromFetch } from "rxjs/fetch"; + import { lastValueFrom } from "rxjs"; + + const a = fromFetch("https://api.some.com"); + lastValueFrom(a).then(value => console.log(value)); + `, + }, + { + messageId: 'suggestFirstValueFrom', + output: stripIndent` + // rxjs not already imported + import { fromFetch } from "rxjs/fetch"; + import { firstValueFrom } from "rxjs"; + + const a = fromFetch("https://api.some.com"); + firstValueFrom(a).then(value => console.log(value)); + `, + }, + ], + }, + ), + fromFixture( + stripIndent` + // namespace import + import * as Rx from "rxjs"; + const a = Rx.of("a"); + a.toPromise().then(value => console.log(value)); + ~~~~~~~~~ [forbidden suggest 0 1] `, + { + suggestions: [ + { + messageId: 'suggestLastValueFrom', + output: stripIndent` + // namespace import + import * as Rx from "rxjs"; + const a = Rx.of("a"); + Rx.lastValueFrom(a).then(value => console.log(value)); + `, + }, + { + messageId: 'suggestFirstValueFrom', + output: stripIndent` + // namespace import + import * as Rx from "rxjs"; + const a = Rx.of("a"); + Rx.firstValueFrom(a).then(value => console.log(value)); + `, + }, + ], + }, ), ], });