Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
6 changes: 5 additions & 1 deletion docs/rules/prefer-composition.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,11 @@ export class SomeComponent implements OnInit, OnDestroy {

<!-- 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 implements a `Subject`-based `ngOnDestroy`.

```json
{
Expand Down
9 changes: 7 additions & 2 deletions docs/rules/prefer-takeuntil.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,16 @@ class SomeComponent implements OnDestroy, OnInit {

<!-- 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 implements 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 +85,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
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

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

export const preferCompositionRule = ruleCreator({
Expand All @@ -34,11 +35,13 @@
{
properties: {
checkDecorators: { type: 'array', items: { type: 'string' }, description: 'An optional array of decorator names to check.' },
superClass: { type: "array", items: { type: "string" } },

Check failure on line 38 in src/rules/prefer-composition.ts

View workflow job for this annotation

GitHub Actions / check (18)

Schema option is missing an ajv description

Check failure on line 38 in src/rules/prefer-composition.ts

View workflow job for this annotation

GitHub Actions / check (18)

Strings must use singlequote

Check failure on line 38 in src/rules/prefer-composition.ts

View workflow job for this annotation

GitHub Actions / check (18)

Strings must use singlequote

Check failure on line 38 in src/rules/prefer-composition.ts

View workflow job for this annotation

GitHub Actions / check (20)

Schema option is missing an ajv description

Check failure on line 38 in src/rules/prefer-composition.ts

View workflow job for this annotation

GitHub Actions / check (20)

Strings must use singlequote

Check failure on line 38 in src/rules/prefer-composition.ts

View workflow job for this annotation

GitHub Actions / check (20)

Strings must use singlequote

Check failure on line 38 in src/rules/prefer-composition.ts

View workflow job for this annotation

GitHub Actions / check (22)

Schema option is missing an ajv description

Check failure on line 38 in src/rules/prefer-composition.ts

View workflow job for this annotation

GitHub Actions / check (22)

Strings must use singlequote

Check failure on line 38 in src/rules/prefer-composition.ts

View workflow job for this annotation

GitHub Actions / check (22)

Strings must use singlequote
},
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 implements a \`Subject\`-based \`ngOnDestroy\`.
`,
},
],
Expand All @@ -47,14 +50,15 @@
name: 'prefer-composition',
create: (context) => {
const { couldBeObservable, couldBeSubscription } = getTypeServices(context);
const [{ checkDecorators = ['Component'] } = {}] = context.options;
const [{ checkDecorators = ["Component"], superClass = [] } = {}] = context.options;

Check failure on line 53 in src/rules/prefer-composition.ts

View workflow job for this annotation

GitHub Actions / check (18)

Strings must use singlequote

Check failure on line 53 in src/rules/prefer-composition.ts

View workflow job for this annotation

GitHub Actions / check (20)

Strings must use singlequote

Check failure on line 53 in src/rules/prefer-composition.ts

View workflow job for this annotation

GitHub Actions / check (22)

Strings must use singlequote

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 @@
classDeclaration,
propertyDefinitions,
ngOnDestroyDefinition,
extendsSuperClassDeclaration,
subscribeCallExpressions,
subscriptions,
unsubscribeCallExpressions,
Expand All @@ -77,6 +82,9 @@
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 @@
});

if (!ngOnDestroyDefinition) {
if (extendsSuperClassDeclaration) {
return;
}
context.report({
messageId: 'notImplemented',
node: classDeclaration.id ?? classDeclaration,
Expand Down Expand Up @@ -233,6 +244,20 @@
return true;
}

const extendsSuperClassDeclaration =

Check failure on line 247 in src/rules/prefer-composition.ts

View workflow job for this annotation

GitHub Actions / check (18)

'=' should be placed at the beginning of the line

Check failure on line 247 in src/rules/prefer-composition.ts

View workflow job for this annotation

GitHub Actions / check (20)

'=' should be placed at the beginning of the line

Check failure on line 247 in src/rules/prefer-composition.ts

View workflow job for this annotation

GitHub Actions / check (22)

'=' should be placed at the beginning of the line
superClass.length === 0
? {}
: {
[`ClassDeclaration:matches(${superClass
.map((className) => `[superClass.name="${className}"]`)
.join()})`]: (node: es.ClassDeclaration) => {
const entry = getEntry();
if (entry && entry.hasDecorator) {

Check failure on line 255 in src/rules/prefer-composition.ts

View workflow job for this annotation

GitHub Actions / check (18)

Prefer using an optional chain expression instead, as it's more concise and easier to read

Check failure on line 255 in src/rules/prefer-composition.ts

View workflow job for this annotation

GitHub Actions / check (20)

Prefer using an optional chain expression instead, as it's more concise and easier to read

Check failure on line 255 in src/rules/prefer-composition.ts

View workflow job for this annotation

GitHub Actions / check (22)

Prefer using an optional chain expression instead, as it's more concise and easier to read
entry.extendsSuperClassDeclaration = node;
}
},
};

return {
'CallExpression[callee.property.name=\'add\']': (
node: es.CallExpression,
Expand Down Expand Up @@ -273,6 +298,7 @@
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
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,15 @@
} as const;
type MessageIds = keyof typeof messages;

const ngOnDestroyMethodSelector =

Check failure on line 24 in src/rules/prefer-takeuntil.ts

View workflow job for this annotation

GitHub Actions / check (18)

'=' should be placed at the beginning of the line

Check failure on line 24 in src/rules/prefer-takeuntil.ts

View workflow job for this annotation

GitHub Actions / check (20)

'=' should be placed at the beginning of the line

Check failure on line 24 in src/rules/prefer-takeuntil.ts

View workflow job for this annotation

GitHub Actions / check (22)

'=' should be placed at the beginning of the line
"MethodDefinition[key.name='ngOnDestroy'][kind='method']";

Check failure on line 25 in src/rules/prefer-takeuntil.ts

View workflow job for this annotation

GitHub Actions / check (18)

Strings must use singlequote

Check failure on line 25 in src/rules/prefer-takeuntil.ts

View workflow job for this annotation

GitHub Actions / check (20)

Strings must use singlequote

Check failure on line 25 in src/rules/prefer-takeuntil.ts

View workflow job for this annotation

GitHub Actions / check (22)

Strings must use singlequote

const defaultOptions: readonly {
alias?: string[];
checkComplete?: boolean;
checkDecorators?: string[];
checkDestroy?: boolean;
superClass?: string[];
}[] = [];

export const preferTakeuntilRule = ruleCreator({
Expand All @@ -46,6 +50,7 @@
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" } },

Check failure on line 53 in src/rules/prefer-takeuntil.ts

View workflow job for this annotation

GitHub Actions / check (18)

Schema option is missing an ajv description

Check failure on line 53 in src/rules/prefer-takeuntil.ts

View workflow job for this annotation

GitHub Actions / check (18)

Strings must use singlequote

Check failure on line 53 in src/rules/prefer-takeuntil.ts

View workflow job for this annotation

GitHub Actions / check (20)

Schema option is missing an ajv description

Check failure on line 53 in src/rules/prefer-takeuntil.ts

View workflow job for this annotation

GitHub Actions / check (20)

Strings must use singlequote

Check failure on line 53 in src/rules/prefer-takeuntil.ts

View workflow job for this annotation

GitHub Actions / check (22)

Schema option is missing an ajv description

Check failure on line 53 in src/rules/prefer-takeuntil.ts

View workflow job for this annotation

GitHub Actions / check (22)

Strings must use singlequote
},
type: 'object',
description: stripIndent`
Expand All @@ -54,6 +59,7 @@
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 implements a \`Subject\`-based \`ngOnDestroy\`.
`,
},
],
Expand All @@ -73,6 +79,7 @@
checkComplete = false,
checkDecorators = ['Component'],
checkDestroy = alias.length === 0,
superClass = [],
} = config;

interface Entry {
Expand All @@ -82,6 +89,8 @@
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 @@
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 @@
};
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 @@
);
}

const extendsSuperClassDeclaration =
superClass.length === 0
? {}
: {
[`ClassDeclaration:matches(${superClass
.map((className) => `[superClass.name="${className}"]`)
.join()})`]: (node: es.ClassDeclaration) => {
const entry = getEntry();
if (entry && entry.hasDecorator) {
entry.extendsSuperClassDeclaration = node;
}
},
};

return {
'CallExpression[callee.property.name=\'subscribe\']': (
node: es.CallExpression,
Expand Down Expand Up @@ -340,22 +381,28 @@
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 && 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
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