Skip to content

Commit 2e9b0cf

Browse files
Added superClass option. Fixes #1
Co-authored-by: Samuel-Therrien-Beslogic <[email protected]> Co-authored-by: Avasam <[email protected]>
1 parent 4b3e20f commit 2e9b0cf

File tree

6 files changed

+320
-31
lines changed

6 files changed

+320
-31
lines changed

docs/rules/prefer-composition.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,11 @@ export class SomeComponent implements OnInit, OnDestroy {
4848

4949
## Options
5050

51-
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"]`.
51+
This rule accepts a single option which is an object with a `checkDecorators` and `superClass` properties.
52+
53+
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"]`.
54+
55+
The `superClass` property is an array containing the names of classes to extend from that already implements a `Subject`-based `ngOnDestroy`.
5256

5357
```json
5458
{
@@ -61,4 +65,4 @@ This rule accepts a single option which is an object with a `checkDecorators` pr
6165

6266
## Further reading
6367

64-
- [Composing Subscriptions](https://ncjamieson.com/composing-subscriptions/)
68+
- [Composing Subscriptions](https://ncjamieson.com/composing-subscriptions/)

docs/rules/prefer-takeuntil.md

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,16 @@ class SomeComponent implements OnDestroy, OnInit {
5050

5151
## Options
5252

53-
This rule accepts a single option which is an object with `checkComplete`, `checkDecorators`, `checkDestroy` and `alias` properties.
53+
This rule accepts a single option which is an object with `checkComplete`, `checkDecorators`, `checkDestroy`, `superClass` and `alias` properties.
5454

5555
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.
5656

5757
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"]`.
5858

59+
The `checkDestroy` property is a boolean that determines whether or not a `Subject`-based `ngOnDestroy` must be implemented.
60+
61+
The `superClass` property is an array containing the names of classes to extend from that already implements a `Subject`-based `ngOnDestroy`.
62+
5963
The `alias` property is an array of names of operators that should be treated similarly to `takeUntil`.
6064

6165
```json
@@ -66,12 +70,13 @@ The `alias` property is an array of names of operators that should be treated si
6670
"alias": ["untilDestroyed"],
6771
"checkComplete": true,
6872
"checkDecorators": ["Component"],
69-
"checkDestroy": true
73+
"checkDestroy": true,
74+
"superClass": []
7075
}
7176
]
7277
}
7378
```
7479

7580
## Further reading
7681

77-
- [Avoiding takeUntil leaks](https://ncjamieson.com/avoiding-takeuntil-leaks/)
82+
- [Avoiding takeUntil leaks](https://ncjamieson.com/avoiding-takeuntil-leaks/)

source/rules/prefer-composition.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { ruleCreator } from "../utils";
1818

1919
const defaultOptions: readonly {
2020
checkDecorators?: string[];
21+
superClass?: string[];
2122
}[] = [];
2223

2324
const rule = ruleCreator({
@@ -40,11 +41,13 @@ const rule = ruleCreator({
4041
{
4142
properties: {
4243
checkDecorators: { type: "array", items: { type: "string" } },
44+
superClass: { type: "array", items: { type: "string" } },
4345
},
4446
type: "object",
4547
description: stripIndent`
4648
An optional object with an optional \`checkDecorators\` property.
4749
The \`checkDecorators\` property is an array containing the names of the decorators that determine whether or not a class is checked.
50+
The \`superClass\` property is an array containing the names of classes to extend from that already implements a \`Subject\`-based \`ngOnDestroy\`.
4851
`,
4952
},
5053
],
@@ -53,14 +56,16 @@ const rule = ruleCreator({
5356
name: "prefer-composition",
5457
create: (context, unused: typeof defaultOptions) => {
5558
const { couldBeObservable, couldBeSubscription } = getTypeServices(context);
56-
const [{ checkDecorators = ["Component"] } = {}] = context.options;
59+
const [{ checkDecorators = ["Component"], superClass = [] } = {}] =
60+
context.options;
5761

5862
type Entry = {
5963
addCallExpressions: es.CallExpression[];
6064
classDeclaration: es.ClassDeclaration;
6165
propertyDefinitions: es.PropertyDefinition[];
6266
hasDecorator: boolean;
6367
ngOnDestroyDefinition?: es.MethodDefinition;
68+
extendsSuperClassDeclaration?: es.ClassDeclaration;
6469
subscribeCallExpressions: es.CallExpression[];
6570
subscriptions: Set<string>;
6671
unsubscribeCallExpressions: es.CallExpression[];
@@ -72,6 +77,7 @@ const rule = ruleCreator({
7277
classDeclaration,
7378
propertyDefinitions,
7479
ngOnDestroyDefinition,
80+
extendsSuperClassDeclaration,
7581
subscribeCallExpressions,
7682
subscriptions,
7783
unsubscribeCallExpressions,
@@ -83,6 +89,9 @@ const rule = ruleCreator({
8389
subscribeCallExpressions.forEach((callExpression) => {
8490
const { callee } = callExpression;
8591
if (isMemberExpression(callee)) {
92+
if (extendsSuperClassDeclaration) {
93+
return;
94+
}
8695
const { object, property } = callee;
8796
if (!couldBeObservable(object)) {
8897
return;
@@ -98,6 +107,9 @@ const rule = ruleCreator({
98107
});
99108

100109
if (!ngOnDestroyDefinition) {
110+
if (extendsSuperClassDeclaration) {
111+
return;
112+
}
101113
context.report({
102114
messageId: "notImplemented",
103115
node: classDeclaration.id ?? classDeclaration,
@@ -240,6 +252,20 @@ const rule = ruleCreator({
240252
return true;
241253
}
242254

255+
const extendsSuperClassDeclaration =
256+
superClass.length === 0
257+
? {}
258+
: {
259+
[`ClassDeclaration:matches(${superClass
260+
.map((className) => `[superClass.name="${className}"]`)
261+
.join()})`]: (node: es.ClassDeclaration) => {
262+
const entry = getEntry();
263+
if (entry && entry.hasDecorator) {
264+
entry.extendsSuperClassDeclaration = node;
265+
}
266+
},
267+
};
268+
243269
return {
244270
"CallExpression[callee.property.name='add']": (
245271
node: es.CallExpression
@@ -280,6 +306,7 @@ const rule = ruleCreator({
280306
entry.propertyDefinitions.push(node);
281307
}
282308
},
309+
...extendsSuperClassDeclaration,
283310
"MethodDefinition[key.name='ngOnDestroy'][kind='method']": (
284311
node: es.MethodDefinition
285312
) => {

source/rules/prefer-takeuntil.ts

Lines changed: 72 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,15 @@ const messages = {
2626
} as const;
2727
type MessageIds = keyof typeof messages;
2828

29+
const ngOnDestroyMethodSelector =
30+
"MethodDefinition[key.name='ngOnDestroy'][kind='method']";
31+
2932
const defaultOptions: readonly {
3033
alias?: string[];
3134
checkComplete?: boolean;
3235
checkDecorators?: string[];
3336
checkDestroy?: boolean;
37+
superClass?: string[];
3438
}[] = [];
3539

3640
const rule = ruleCreator({
@@ -51,6 +55,7 @@ const rule = ruleCreator({
5155
checkComplete: { type: "boolean" },
5256
checkDecorators: { type: "array", items: { type: "string" } },
5357
checkDestroy: { type: "boolean" },
58+
superClass: { type: "array", items: { type: "string" } },
5459
},
5560
type: "object",
5661
description: stripIndent`
@@ -59,6 +64,7 @@ const rule = ruleCreator({
5964
The \`checkComplete\` property is a boolean that determines whether or not \`complete\` must be called after \`next\`.
6065
The \`checkDecorators\` property is an array containing the names of the decorators that determine whether or not a class is checked.
6166
The \`checkDestroy\` property is a boolean that determines whether or not a \`Subject\`-based \`ngOnDestroy\` must be implemented.
67+
The \`superClass\` property is an array containing the names of classes to extend from that already implements a \`Subject\`-based \`ngOnDestroy\`.
6268
`,
6369
},
6470
],
@@ -78,6 +84,7 @@ const rule = ruleCreator({
7884
checkComplete = false,
7985
checkDecorators = ["Component"],
8086
checkDestroy = alias.length === 0,
87+
superClass = [],
8188
} = config;
8289

8390
type Entry = {
@@ -87,6 +94,8 @@ const rule = ruleCreator({
8794
hasDecorator: boolean;
8895
nextCallExpressions: es.CallExpression[];
8996
ngOnDestroyDefinition?: es.MethodDefinition;
97+
extendsSuperClassDeclaration?: es.ClassDeclaration;
98+
superNgOnDestroyCallExpression?: es.CallExpression;
9099
subscribeCallExpressions: es.CallExpression[];
91100
subscribeCallExpressionsToNames: Map<es.CallExpression, Set<string>>;
92101
};
@@ -117,13 +126,18 @@ const rule = ruleCreator({
117126
completeCallExpressions,
118127
nextCallExpressions,
119128
ngOnDestroyDefinition,
129+
extendsSuperClassDeclaration,
130+
superNgOnDestroyCallExpression,
120131
subscribeCallExpressionsToNames,
121132
} = entry;
122133
if (subscribeCallExpressionsToNames.size === 0) {
123134
return;
124135
}
125136

126137
if (!ngOnDestroyDefinition) {
138+
if (extendsSuperClassDeclaration) {
139+
return;
140+
}
127141
context.report({
128142
messageId: "noDestroy",
129143
node: classDeclaration.id ?? classDeclaration,
@@ -154,26 +168,39 @@ const rule = ruleCreator({
154168
};
155169
namesToChecks.set(name, check);
156170

157-
if (!checkSubjectProperty(name, entry)) {
158-
check.descriptors.push({
159-
data: { name },
160-
messageId: "notDeclared",
161-
node: classDeclaration.id ?? classDeclaration,
162-
});
163-
}
164-
if (!checkSubjectCall(name, nextCallExpressions)) {
165-
check.descriptors.push({
166-
data: { method: "next", name },
167-
messageId: "notCalled",
168-
node: ngOnDestroyDefinition.key,
169-
});
170-
}
171-
if (checkComplete && !checkSubjectCall(name, completeCallExpressions)) {
172-
check.descriptors.push({
173-
data: { method: "complete", name },
174-
messageId: "notCalled",
175-
node: ngOnDestroyDefinition.key,
176-
});
171+
if (extendsSuperClassDeclaration) {
172+
if (!superNgOnDestroyCallExpression) {
173+
check.descriptors.push({
174+
data: { method: "ngOnDestroy", name: "super" },
175+
messageId: "notCalled",
176+
node: ngOnDestroyDefinition.key,
177+
});
178+
}
179+
} else {
180+
if (!checkSubjectProperty(name, entry)) {
181+
check.descriptors.push({
182+
data: { name },
183+
messageId: "notDeclared",
184+
node: classDeclaration.id ?? classDeclaration,
185+
});
186+
}
187+
if (!checkSubjectCall(name, nextCallExpressions)) {
188+
check.descriptors.push({
189+
data: { method: "next", name },
190+
messageId: "notCalled",
191+
node: ngOnDestroyDefinition.key,
192+
});
193+
}
194+
if (
195+
checkComplete &&
196+
!checkSubjectCall(name, completeCallExpressions)
197+
) {
198+
check.descriptors.push({
199+
data: { method: "complete", name },
200+
messageId: "notCalled",
201+
node: ngOnDestroyDefinition.key,
202+
});
203+
}
177204
}
178205
});
179206

@@ -308,6 +335,20 @@ const rule = ruleCreator({
308335
);
309336
}
310337

338+
const extendsSuperClassDeclaration =
339+
superClass.length === 0
340+
? {}
341+
: {
342+
[`ClassDeclaration:matches(${superClass
343+
.map((className) => `[superClass.name="${className}"]`)
344+
.join()})`]: (node: es.ClassDeclaration) => {
345+
const entry = getEntry();
346+
if (entry && entry.hasDecorator) {
347+
entry.extendsSuperClassDeclaration = node;
348+
}
349+
},
350+
};
351+
311352
return {
312353
"CallExpression[callee.property.name='subscribe']": (
313354
node: es.CallExpression
@@ -344,22 +385,28 @@ const rule = ruleCreator({
344385
entry.propertyDefinitions.push(node);
345386
}
346387
},
347-
"MethodDefinition[key.name='ngOnDestroy'][kind='method']": (
348-
node: es.MethodDefinition
349-
) => {
388+
[ngOnDestroyMethodSelector]: (node: es.MethodDefinition) => {
350389
const entry = getEntry();
351390
if (entry && entry.hasDecorator) {
352391
entry.ngOnDestroyDefinition = node;
353392
}
354393
},
355-
"MethodDefinition[key.name='ngOnDestroy'][kind='method'] CallExpression[callee.property.name='next']":
394+
...extendsSuperClassDeclaration,
395+
[`${ngOnDestroyMethodSelector} CallExpression[callee.object.type='Super'][callee.property.name='ngOnDestroy']`]:
396+
(node: es.CallExpression) => {
397+
const entry = getEntry();
398+
if (entry && entry.hasDecorator) {
399+
entry.superNgOnDestroyCallExpression = node;
400+
}
401+
},
402+
[`${ngOnDestroyMethodSelector} CallExpression[callee.property.name='next']`]:
356403
(node: es.CallExpression) => {
357404
const entry = getEntry();
358405
if (entry && entry.hasDecorator) {
359406
entry.nextCallExpressions.push(node);
360407
}
361408
},
362-
"MethodDefinition[key.name='ngOnDestroy'][kind='method'] CallExpression[callee.property.name='complete']":
409+
[`${ngOnDestroyMethodSelector} CallExpression[callee.property.name='complete']`]:
363410
(node: es.CallExpression) => {
364411
const entry = getEntry();
365412
if (entry && entry.hasDecorator) {

tests/rules/prefer-composition.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,44 @@ ruleTester({ types: true }).run("prefer-composition", rule, {
9898
}
9999
`,
100100
},
101+
{
102+
code: stripIndent`
103+
// extends superClass
104+
// https://github.com/cartant/eslint-plugin-rxjs-angular/issues/1
105+
import { Component, Directive, OnDestroy } from "@angular/core";
106+
import { of, Subject } from "rxjs";
107+
import { switchMap, takeUntil } from "rxjs/operators";
108+
109+
const o = of("o");
110+
111+
@Directive()
112+
abstract class BaseComponent implements OnDestroy {
113+
private readonly destroySubject = new Subject<void>();
114+
protected readonly destroy = this.destroySubject.asObservable();
115+
ngOnDestroy() {
116+
this.destroySubject.next();
117+
this.destroySubject.complete();
118+
}
119+
}
120+
121+
@Component({
122+
selector: "component-with-super-class"
123+
})
124+
class CorrectComponent extends BaseComponent {
125+
someMethod() {
126+
o.pipe(
127+
switchMap(_ => o),
128+
takeUntil(this.destroy)
129+
).subscribe();
130+
}
131+
}
132+
`,
133+
options: [
134+
{
135+
superClass: ["BaseComponent"],
136+
},
137+
],
138+
},
101139
],
102140
invalid: [
103141
fromFixture(

0 commit comments

Comments
 (0)