diff --git a/docs/rules/prefer-composition.md b/docs/rules/prefer-composition.md index 8ffd890..6ab4dd7 100644 --- a/docs/rules/prefer-composition.md +++ b/docs/rules/prefer-composition.md @@ -54,13 +54,18 @@ export class SomeComponent implements OnInit, OnDestroy { -| Name | Description | Type | -| :---------------- | :--------------------------------------------- | :------- | -| `checkDecorators` | An optional array of decorator names to check. | String[] | +| Name | Description | Type | +| :---------------- | :------------------------------------------------------------------------------------------- | :------- | +| `checkDecorators` | An optional array of decorator names to check. | String[] | +| `superClass` | An optional array of superclass names that already implement a `Subject`-based `ngOnDestroy` | String[] | -This rule accepts a single option which is an object with a `checkDecorators` property which is an array containing the names of the decorators that determine whether or not a class is checked. By default, `checkDecorators` is `["Component"]`. +This rule accepts a single option which is an object with a `checkDecorators` and `superClass` properties. + +The `checkDecorators` property is an array containing the names of the decorators that determine whether or not a class is checked. By default, `checkDecorators` is `["Component"]`. + +The `superClass` property is an array containing the names of classes to extend from that already implement a `Subject`-based `ngOnDestroy`. ```json { diff --git a/docs/rules/prefer-takeuntil.md b/docs/rules/prefer-takeuntil.md index a89dbdc..c473054 100644 --- a/docs/rules/prefer-takeuntil.md +++ b/docs/rules/prefer-takeuntil.md @@ -56,21 +56,26 @@ class SomeComponent implements OnDestroy, OnInit { -| Name | Description | Type | -| :---------------- | :-------------------------------------------------------------- | :------- | -| `alias` | An optional array of operator names that alias for `takeUntil`. | String[] | -| `checkComplete` | Check for `complete` calls. | Boolean | -| `checkDecorators` | An optional array of decorator names to check. | String[] | -| `checkDestroy` | Check for `Subject`-based `ngOnDestroy`. | Boolean | +| Name | Description | Type | +| :---------------- | :------------------------------------------------------------------------------------------- | :------- | +| `alias` | An optional array of operator names that alias for `takeUntil`. | String[] | +| `checkComplete` | Check for `complete` calls. | Boolean | +| `checkDecorators` | An optional array of decorator names to check. | String[] | +| `checkDestroy` | Check for `Subject`-based `ngOnDestroy`. | Boolean | +| `superClass` | An optional array of superclass names that already implement a `Subject`-based `ngOnDestroy` | String[] | -This rule accepts a single option which is an object with `checkComplete`, `checkDecorators`, `checkDestroy` and `alias` properties. +This rule accepts a single option which is an object with `checkComplete`, `checkDecorators`, `checkDestroy`, `superClass` and `alias` properties. The `checkComplete` property is a boolean that determines whether or not `complete` must be called after `next` and the `checkDestroy` property is a boolean that determines whether or not a `Subject`-based `ngOnDestroy` must be implemented. The `checkDecorators` property is an array containing the names of the decorators that determine whether or not a class is checked. By default, `checkDecorators` is `["Component"]`. +The `checkDestroy` property is a boolean that determines whether or not a `Subject`-based `ngOnDestroy` must be implemented. + +The `superClass` property is an array containing the names of classes to extend from that already implement a `Subject`-based `ngOnDestroy`. + The `alias` property is an array of names of operators that should be treated similarly to `takeUntil`. ```json @@ -81,7 +86,8 @@ The `alias` property is an array of names of operators that should be treated si "alias": ["untilDestroyed"], "checkComplete": true, "checkDecorators": ["Component"], - "checkDestroy": true + "checkDestroy": true, + "superClass": [] } ] } diff --git a/src/rules/prefer-composition.ts b/src/rules/prefer-composition.ts index 680d8e8..abe82a7 100644 --- a/src/rules/prefer-composition.ts +++ b/src/rules/prefer-composition.ts @@ -12,6 +12,7 @@ import { ruleCreator } from '../utils'; const defaultOptions: readonly { checkDecorators?: string[]; + superClass?: string[]; }[] = []; export const preferCompositionRule = ruleCreator({ @@ -34,11 +35,13 @@ export const preferCompositionRule = ruleCreator({ { properties: { checkDecorators: { type: 'array', items: { type: 'string' }, description: 'An optional array of decorator names to check.' }, + superClass: { type: 'array', items: { type: 'string' }, description: 'An optional array of superclass names that already implement a `Subject`-based `ngOnDestroy`' }, }, type: 'object', description: stripIndent` An optional object with an optional \`checkDecorators\` property. The \`checkDecorators\` property is an array containing the names of the decorators that determine whether or not a class is checked. + The \`superClass\` property is an array containing the names of classes to extend from that already implement a \`Subject\`-based \`ngOnDestroy\`. `, }, ], @@ -47,7 +50,7 @@ export const preferCompositionRule = ruleCreator({ name: 'prefer-composition', create: (context) => { const { couldBeObservable, couldBeSubscription } = getTypeServices(context); - const [{ checkDecorators = ['Component'] } = {}] = context.options; + const [{ checkDecorators = ['Component'], superClass = [] } = {}] = context.options; interface Entry { addCallExpressions: es.CallExpression[]; @@ -55,6 +58,7 @@ export const preferCompositionRule = ruleCreator({ propertyDefinitions: es.PropertyDefinition[]; hasDecorator: boolean; ngOnDestroyDefinition?: es.MethodDefinition; + extendsSuperClassDeclaration?: es.ClassDeclaration; subscribeCallExpressions: es.CallExpression[]; subscriptions: Set; unsubscribeCallExpressions: es.CallExpression[]; @@ -66,6 +70,7 @@ export const preferCompositionRule = ruleCreator({ classDeclaration, propertyDefinitions, ngOnDestroyDefinition, + extendsSuperClassDeclaration, subscribeCallExpressions, subscriptions, unsubscribeCallExpressions, @@ -77,6 +82,9 @@ export const preferCompositionRule = ruleCreator({ subscribeCallExpressions.forEach((callExpression) => { const { callee } = callExpression; if (isMemberExpression(callee)) { + if (extendsSuperClassDeclaration) { + return; + } const { object, property } = callee; if (!couldBeObservable(object)) { return; @@ -92,6 +100,9 @@ export const preferCompositionRule = ruleCreator({ }); if (!ngOnDestroyDefinition) { + if (extendsSuperClassDeclaration) { + return; + } context.report({ messageId: 'notImplemented', node: classDeclaration.id ?? classDeclaration, @@ -233,6 +244,20 @@ export const preferCompositionRule = ruleCreator({ return true; } + const extendsSuperClassDeclaration + = superClass.length === 0 + ? {} + : { + [`ClassDeclaration:matches(${superClass + .map((className) => `[superClass.name="${className}"]`) + .join()})`]: (node: es.ClassDeclaration) => { + const entry = getEntry(); + if (entry.hasDecorator) { + entry.extendsSuperClassDeclaration = node; + } + }, + }; + return { 'CallExpression[callee.property.name=\'add\']': ( node: es.CallExpression, @@ -273,6 +298,7 @@ export const preferCompositionRule = ruleCreator({ entry.propertyDefinitions.push(node); } }, + ...extendsSuperClassDeclaration, 'MethodDefinition[key.name=\'ngOnDestroy\'][kind=\'method\']': ( node: es.MethodDefinition, ) => { diff --git a/src/rules/prefer-takeuntil.ts b/src/rules/prefer-takeuntil.ts index 23c5eac..cf137e2 100644 --- a/src/rules/prefer-takeuntil.ts +++ b/src/rules/prefer-takeuntil.ts @@ -21,11 +21,15 @@ const messages = { } as const; type MessageIds = keyof typeof messages; +const ngOnDestroyMethodSelector + = 'MethodDefinition[key.name=\'ngOnDestroy\'][kind=\'method\']'; + const defaultOptions: readonly { alias?: string[]; checkComplete?: boolean; checkDecorators?: string[]; checkDestroy?: boolean; + superClass?: string[]; }[] = []; export const preferTakeuntilRule = ruleCreator({ @@ -46,6 +50,7 @@ export const preferTakeuntilRule = ruleCreator({ checkComplete: { type: 'boolean', description: 'Check for `complete` calls.' }, checkDecorators: { type: 'array', items: { type: 'string' }, description: 'An optional array of decorator names to check.' }, checkDestroy: { type: 'boolean', description: 'Check for `Subject`-based `ngOnDestroy`.' }, + superClass: { type: 'array', items: { type: 'string' }, description: 'An optional array of superclass names that already implement a `Subject`-based `ngOnDestroy`' }, }, type: 'object', description: stripIndent` @@ -54,6 +59,7 @@ export const preferTakeuntilRule = ruleCreator({ The \`checkComplete\` property is a boolean that determines whether or not \`complete\` must be called after \`next\`. The \`checkDecorators\` property is an array containing the names of the decorators that determine whether or not a class is checked. The \`checkDestroy\` property is a boolean that determines whether or not a \`Subject\`-based \`ngOnDestroy\` must be implemented. + The \`superClass\` property is an array containing the names of classes to extend from that already implement a \`Subject\`-based \`ngOnDestroy\`. `, }, ], @@ -73,6 +79,7 @@ export const preferTakeuntilRule = ruleCreator({ checkComplete = false, checkDecorators = ['Component'], checkDestroy = alias.length === 0, + superClass = [], } = config; interface Entry { @@ -82,6 +89,8 @@ export const preferTakeuntilRule = ruleCreator({ hasDecorator: boolean; nextCallExpressions: es.CallExpression[]; ngOnDestroyDefinition?: es.MethodDefinition; + extendsSuperClassDeclaration?: es.ClassDeclaration; + superNgOnDestroyCallExpression?: es.CallExpression; subscribeCallExpressions: es.CallExpression[]; subscribeCallExpressionsToNames: Map>; } @@ -112,6 +121,8 @@ export const preferTakeuntilRule = ruleCreator({ completeCallExpressions, nextCallExpressions, ngOnDestroyDefinition, + extendsSuperClassDeclaration, + superNgOnDestroyCallExpression, subscribeCallExpressionsToNames, } = entry; if (subscribeCallExpressionsToNames.size === 0) { @@ -119,6 +130,9 @@ export const preferTakeuntilRule = ruleCreator({ } if (!ngOnDestroyDefinition) { + if (extendsSuperClassDeclaration) { + return; + } context.report({ messageId: 'noDestroy', node: classDeclaration.id ?? classDeclaration, @@ -149,26 +163,39 @@ export const preferTakeuntilRule = ruleCreator({ }; namesToChecks.set(name, check); - if (!checkSubjectProperty(name, entry)) { - check.descriptors.push({ - data: { name }, - messageId: 'notDeclared', - node: classDeclaration.id ?? classDeclaration, - }); - } - if (!checkSubjectCall(name, nextCallExpressions)) { - check.descriptors.push({ - data: { method: 'next', name }, - messageId: 'notCalled', - node: ngOnDestroyDefinition.key, - }); - } - if (checkComplete && !checkSubjectCall(name, completeCallExpressions)) { - check.descriptors.push({ - data: { method: 'complete', name }, - messageId: 'notCalled', - node: ngOnDestroyDefinition.key, - }); + if (extendsSuperClassDeclaration) { + if (!superNgOnDestroyCallExpression) { + check.descriptors.push({ + data: { method: 'ngOnDestroy', name: 'super' }, + messageId: 'notCalled', + node: ngOnDestroyDefinition.key, + }); + } + } else { + if (!checkSubjectProperty(name, entry)) { + check.descriptors.push({ + data: { name }, + messageId: 'notDeclared', + node: classDeclaration.id ?? classDeclaration, + }); + } + if (!checkSubjectCall(name, nextCallExpressions)) { + check.descriptors.push({ + data: { method: 'next', name }, + messageId: 'notCalled', + node: ngOnDestroyDefinition.key, + }); + } + if ( + checkComplete + && !checkSubjectCall(name, completeCallExpressions) + ) { + check.descriptors.push({ + data: { method: 'complete', name }, + messageId: 'notCalled', + node: ngOnDestroyDefinition.key, + }); + } } }); @@ -304,6 +331,20 @@ export const preferTakeuntilRule = ruleCreator({ ); } + const extendsSuperClassDeclaration + = superClass.length === 0 + ? {} + : { + [`ClassDeclaration:matches(${superClass + .map((className) => `[superClass.name="${className}"]`) + .join()})`]: (node: es.ClassDeclaration) => { + const entry = getEntry(); + if (entry.hasDecorator) { + entry.extendsSuperClassDeclaration = node; + } + }, + }; + return { 'CallExpression[callee.property.name=\'subscribe\']': ( node: es.CallExpression, @@ -340,22 +381,28 @@ export const preferTakeuntilRule = ruleCreator({ entry.propertyDefinitions.push(node); } }, - 'MethodDefinition[key.name=\'ngOnDestroy\'][kind=\'method\']': ( - node: es.MethodDefinition, - ) => { + [ngOnDestroyMethodSelector]: (node: es.MethodDefinition) => { const entry = getEntry(); if (entry?.hasDecorator) { entry.ngOnDestroyDefinition = node; } }, - 'MethodDefinition[key.name=\'ngOnDestroy\'][kind=\'method\'] CallExpression[callee.property.name=\'next\']': + ...extendsSuperClassDeclaration, + [`${ngOnDestroyMethodSelector} CallExpression[callee.object.type='Super'][callee.property.name='ngOnDestroy']`]: + (node: es.CallExpression) => { + const entry = getEntry(); + if (entry.hasDecorator) { + entry.superNgOnDestroyCallExpression = node; + } + }, + [`${ngOnDestroyMethodSelector} CallExpression[callee.property.name='next']`]: (node: es.CallExpression) => { const entry = getEntry(); if (entry?.hasDecorator) { entry.nextCallExpressions.push(node); } }, - 'MethodDefinition[key.name=\'ngOnDestroy\'][kind=\'method\'] CallExpression[callee.property.name=\'complete\']': + [`${ngOnDestroyMethodSelector} CallExpression[callee.property.name='complete']`]: (node: es.CallExpression) => { const entry = getEntry(); if (entry?.hasDecorator) { diff --git a/tests/rules/prefer-composition.test.ts b/tests/rules/prefer-composition.test.ts index 96dc2df..3175ba9 100644 --- a/tests/rules/prefer-composition.test.ts +++ b/tests/rules/prefer-composition.test.ts @@ -93,6 +93,44 @@ ruleTester({ types: true }).run('prefer-composition', preferCompositionRule, { } `, }, + { + code: stripIndent` + // extends superClass + // https://github.com/cartant/eslint-plugin-rxjs-angular/issues/1 + import { Component, Directive, OnDestroy } from "@angular/core"; + import { of, Subject } from "rxjs"; + import { switchMap, takeUntil } from "rxjs/operators"; + + const o = of("o"); + + @Directive() + abstract class BaseComponent implements OnDestroy { + private readonly destroySubject = new Subject(); + protected readonly destroy = this.destroySubject.asObservable(); + ngOnDestroy() { + this.destroySubject.next(); + this.destroySubject.complete(); + } + } + + @Component({ + selector: "component-with-super-class" + }) + class CorrectComponent extends BaseComponent { + someMethod() { + o.pipe( + switchMap(_ => o), + takeUntil(this.destroy) + ).subscribe(); + } + } + `, + options: [ + { + superClass: ['BaseComponent'], + }, + ], + }, ], invalid: [ fromFixture( diff --git a/tests/rules/prefer-takeuntil.test.ts b/tests/rules/prefer-takeuntil.test.ts index 4431568..a221773 100644 --- a/tests/rules/prefer-takeuntil.test.ts +++ b/tests/rules/prefer-takeuntil.test.ts @@ -364,6 +364,90 @@ ruleTester({ types: true }).run('prefer-takeuntil', preferTakeuntilRule, { }, ], }, + { + code: stripIndent` + // extends superClass + // https://github.com/cartant/eslint-plugin-rxjs-angular/issues/1 + import { Component, Directive, OnDestroy } from "@angular/core"; + import { of, Subject } from "rxjs"; + import { switchMap, takeUntil } from "rxjs/operators"; + + const o = of("o"); + + @Directive() + abstract class BaseComponent implements OnDestroy { + private readonly destroySubject = new Subject(); + protected readonly destroy = this.destroySubject.asObservable(); + + ngOnDestroy() { + this.destroySubject.next(); + this.destroySubject.complete(); + } + } + + @Component({ + selector: "component-with-super-class" + }) + class CorrectComponent extends BaseComponent { + someMethod() { + o.pipe( + switchMap(_ => o), + takeUntil(this.destroy) + ).subscribe(); + } + } + `, + options: [ + { + superClass: ['BaseComponent'], + checkComplete: true, + }, + ], + }, + { + code: stripIndent` + // extends superClass and implements OnDestroy + // https://github.com/cartant/eslint-plugin-rxjs-angular/issues/1 + import { Component, Directive, OnDestroy } from "@angular/core"; + import { of, Subject } from "rxjs"; + import { switchMap, takeUntil } from "rxjs/operators"; + + const o = of("o"); + + @Directive() + abstract class BaseComponent implements OnDestroy { + private readonly destroySubject = new Subject(); + protected readonly destroy = this.destroySubject.asObservable(); + + ngOnDestroy() { + this.destroySubject.next(); + this.destroySubject.complete(); + } + } + + @Component({ + selector: "component-with-super-class-and-destroy" + }) + class CorrectDestroyComponent extends BaseComponent implements OnDestroy { + someMethod() { + o.pipe( + switchMap(_ => o), + takeUntil(this.destroy) + ).subscribe(); + } + + override ngOnDestroy(): void { + super.ngOnDestroy() + } + } + `, + options: [ + { + superClass: ['BaseComponent'], + checkComplete: true, + }, + ], + }, ], invalid: [ fromFixture( @@ -652,5 +736,89 @@ ruleTester({ types: true }).run('prefer-takeuntil', preferTakeuntilRule, { ], }, ), + fromFixture( + stripIndent` + // extends superClass and implements OnDestroy, missing super.ngOnDestroy() + // https://github.com/cartant/eslint-plugin-rxjs-angular/issues/1 + import { Component, Directive, OnDestroy } from "@angular/core"; + import { of, Subject } from "rxjs"; + import { switchMap, takeUntil } from "rxjs/operators"; + + const o = of("o"); + + @Directive() + abstract class BaseComponent implements OnDestroy { + private readonly destroySubject = new Subject(); + protected readonly destroy = this.destroySubject.asObservable(); + + ngOnDestroy() { + this.destroySubject.next(); + this.destroySubject.complete(); + } + } + + @Component({ + selector: "missing-super-call" + }) + class MissingSuperCallComponent extends BaseComponent implements OnDestroy { + someMethod() { + o.pipe( + switchMap(_ => o), + takeUntil(this.destroy) + ).subscribe(); + } + + override ngOnDestroy(): void { + ~~~~~~~~~~~ [notCalled { "method": "ngOnDestroy", "name": "super" }] + } + } + `, + { + options: [ + { + superClass: ['BaseComponent'], + checkComplete: true, + }, + ], + }, + ), + fromFixture( + stripIndent` + // Calls super.ngOnDestroy() w/o extending base class + // https://github.com/cartant/eslint-plugin-rxjs-angular/issues/1 + import { Component, OnDestroy } from "@angular/core"; + import { of } from "rxjs"; + import { switchMap, takeUntil } from "rxjs/operators"; + + const o = of("o"); + + @Component({ + selector: "missing-base" + }) + class MissingBaseComponent extends SomeClass implements OnDestroy { + ~~~~~~~~~~~~~~~~~~~~ [notDeclared { "name": "destroy" }] + someMethod() { + o.pipe( + switchMap(_ => o), + takeUntil(this.destroy) + ).subscribe(); + } + + override ngOnDestroy(): void { + ~~~~~~~~~~~ [notCalled { "method": "next", "name": "destroy" }] + ~~~~~~~~~~~ [notCalled { "method": "complete", "name": "destroy" }] + super.ngOnDestroy() + } + } + `, + { + options: [ + { + superClass: ['BaseComponent'], + checkComplete: true, + }, + ], + }, + ), ], });