Skip to content

Added superClass option. Fixes cartant/eslint-plugin-rxjs-angular#1 #3

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Feb 25, 2025
Merged

Added superClass option. Fixes cartant/eslint-plugin-rxjs-angular#1 #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0bec767
Added superClass option. Fixes cartant/eslint-plugin-rxjs-angular#1
Avasam Feb 20, 2025
cbe5c6f
Lint and docs fixes
Samuel-Therrien-Beslogic Feb 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions docs/rules/prefer-composition.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,18 @@ export class SomeComponent implements OnInit, OnDestroy {

<!-- begin auto-generated rule options list -->

| 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[] |

<!-- end auto-generated rule options list -->

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
{
Expand Down
22 changes: 14 additions & 8 deletions docs/rules/prefer-takeuntil.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,21 +56,26 @@ class SomeComponent implements OnDestroy, OnInit {

<!-- begin auto-generated rule options list -->

| 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[] |

<!-- end auto-generated rule options list -->

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
Expand All @@ -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": []
}
]
}
Expand Down
28 changes: 27 additions & 1 deletion src/rules/prefer-composition.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { ruleCreator } from '../utils';

const defaultOptions: readonly {
checkDecorators?: string[];
superClass?: string[];
}[] = [];

export const preferCompositionRule = ruleCreator({
Expand All @@ -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\`.
`,
},
],
Expand All @@ -47,14 +50,15 @@ 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[];
classDeclaration: es.ClassDeclaration;
propertyDefinitions: es.PropertyDefinition[];
hasDecorator: boolean;
ngOnDestroyDefinition?: es.MethodDefinition;
extendsSuperClassDeclaration?: es.ClassDeclaration;
subscribeCallExpressions: es.CallExpression[];
subscriptions: Set<string>;
unsubscribeCallExpressions: es.CallExpression[];
Expand All @@ -66,6 +70,7 @@ export const preferCompositionRule = ruleCreator({
classDeclaration,
propertyDefinitions,
ngOnDestroyDefinition,
extendsSuperClassDeclaration,
subscribeCallExpressions,
subscriptions,
unsubscribeCallExpressions,
Expand All @@ -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;
Expand All @@ -92,6 +100,9 @@ export const preferCompositionRule = ruleCreator({
});

if (!ngOnDestroyDefinition) {
if (extendsSuperClassDeclaration) {
return;
}
context.report({
messageId: 'notImplemented',
node: classDeclaration.id ?? classDeclaration,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -273,6 +298,7 @@ export const preferCompositionRule = ruleCreator({
entry.propertyDefinitions.push(node);
}
},
...extendsSuperClassDeclaration,
'MethodDefinition[key.name=\'ngOnDestroy\'][kind=\'method\']': (
node: es.MethodDefinition,
) => {
Expand Down
97 changes: 72 additions & 25 deletions src/rules/prefer-takeuntil.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -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`
Expand All @@ -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\`.
`,
},
],
Expand All @@ -73,6 +79,7 @@ export const preferTakeuntilRule = ruleCreator({
checkComplete = false,
checkDecorators = ['Component'],
checkDestroy = alias.length === 0,
superClass = [],
} = config;

interface Entry {
Expand All @@ -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<es.CallExpression, Set<string>>;
}
Expand Down Expand Up @@ -112,13 +121,18 @@ export const preferTakeuntilRule = ruleCreator({
completeCallExpressions,
nextCallExpressions,
ngOnDestroyDefinition,
extendsSuperClassDeclaration,
superNgOnDestroyCallExpression,
subscribeCallExpressionsToNames,
} = entry;
if (subscribeCallExpressionsToNames.size === 0) {
return;
}

if (!ngOnDestroyDefinition) {
if (extendsSuperClassDeclaration) {
return;
}
context.report({
messageId: 'noDestroy',
node: classDeclaration.id ?? classDeclaration,
Expand Down Expand Up @@ -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,
});
}
}
});

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down
38 changes: 38 additions & 0 deletions tests/rules/prefer-composition.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>();
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(
Expand Down
Loading
Loading