Skip to content

Commit 0bec767

Browse files
Added superClass option. Fixes cartant#1
Co-authored-by: Samuel-Therrien-Beslogic <[email protected]> Co-authored-by: Avasam <[email protected]>
1 parent 8603c9f commit 0bec767

File tree

6 files changed

+317
-29
lines changed

6 files changed

+317
-29
lines changed

docs/rules/prefer-composition.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,11 @@ export class SomeComponent implements OnInit, OnDestroy {
6060

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

63-
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"]`.
63+
This rule accepts a single option which is an object with a `checkDecorators` and `superClass` properties.
64+
65+
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"]`.
66+
67+
The `superClass` property is an array containing the names of classes to extend from that already implements a `Subject`-based `ngOnDestroy`.
6468

6569
```json
6670
{

docs/rules/prefer-takeuntil.md

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,16 @@ class SomeComponent implements OnDestroy, OnInit {
6565

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

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

7070
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.
7171

7272
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"]`.
7373

74+
The `checkDestroy` property is a boolean that determines whether or not a `Subject`-based `ngOnDestroy` must be implemented.
75+
76+
The `superClass` property is an array containing the names of classes to extend from that already implements a `Subject`-based `ngOnDestroy`.
77+
7478
The `alias` property is an array of names of operators that should be treated similarly to `takeUntil`.
7579

7680
```json
@@ -81,7 +85,8 @@ The `alias` property is an array of names of operators that should be treated si
8185
"alias": ["untilDestroyed"],
8286
"checkComplete": true,
8387
"checkDecorators": ["Component"],
84-
"checkDestroy": true
88+
"checkDestroy": true,
89+
"superClass": []
8590
}
8691
]
8792
}

src/rules/prefer-composition.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { ruleCreator } from '../utils';
1212

1313
const defaultOptions: readonly {
1414
checkDecorators?: string[];
15+
superClass?: string[];
1516
}[] = [];
1617

1718
export const preferCompositionRule = ruleCreator({
@@ -34,11 +35,13 @@ export const preferCompositionRule = ruleCreator({
3435
{
3536
properties: {
3637
checkDecorators: { type: 'array', items: { type: 'string' }, description: 'An optional array of decorator names to check.' },
38+
superClass: { type: "array", items: { type: "string" } },
3739
},
3840
type: 'object',
3941
description: stripIndent`
4042
An optional object with an optional \`checkDecorators\` property.
4143
The \`checkDecorators\` property is an array containing the names of the decorators that determine whether or not a class is checked.
44+
The \`superClass\` property is an array containing the names of classes to extend from that already implements a \`Subject\`-based \`ngOnDestroy\`.
4245
`,
4346
},
4447
],
@@ -47,14 +50,15 @@ export const preferCompositionRule = ruleCreator({
4750
name: 'prefer-composition',
4851
create: (context) => {
4952
const { couldBeObservable, couldBeSubscription } = getTypeServices(context);
50-
const [{ checkDecorators = ['Component'] } = {}] = context.options;
53+
const [{ checkDecorators = ["Component"], superClass = [] } = {}] = context.options;
5154

5255
interface Entry {
5356
addCallExpressions: es.CallExpression[];
5457
classDeclaration: es.ClassDeclaration;
5558
propertyDefinitions: es.PropertyDefinition[];
5659
hasDecorator: boolean;
5760
ngOnDestroyDefinition?: es.MethodDefinition;
61+
extendsSuperClassDeclaration?: es.ClassDeclaration;
5862
subscribeCallExpressions: es.CallExpression[];
5963
subscriptions: Set<string>;
6064
unsubscribeCallExpressions: es.CallExpression[];
@@ -66,6 +70,7 @@ export const preferCompositionRule = ruleCreator({
6670
classDeclaration,
6771
propertyDefinitions,
6872
ngOnDestroyDefinition,
73+
extendsSuperClassDeclaration,
6974
subscribeCallExpressions,
7075
subscriptions,
7176
unsubscribeCallExpressions,
@@ -77,6 +82,9 @@ export const preferCompositionRule = ruleCreator({
7782
subscribeCallExpressions.forEach((callExpression) => {
7883
const { callee } = callExpression;
7984
if (isMemberExpression(callee)) {
85+
if (extendsSuperClassDeclaration) {
86+
return;
87+
}
8088
const { object, property } = callee;
8189
if (!couldBeObservable(object)) {
8290
return;
@@ -92,6 +100,9 @@ export const preferCompositionRule = ruleCreator({
92100
});
93101

94102
if (!ngOnDestroyDefinition) {
103+
if (extendsSuperClassDeclaration) {
104+
return;
105+
}
95106
context.report({
96107
messageId: 'notImplemented',
97108
node: classDeclaration.id ?? classDeclaration,
@@ -233,6 +244,20 @@ export const preferCompositionRule = ruleCreator({
233244
return true;
234245
}
235246

247+
const extendsSuperClassDeclaration =
248+
superClass.length === 0
249+
? {}
250+
: {
251+
[`ClassDeclaration:matches(${superClass
252+
.map((className) => `[superClass.name="${className}"]`)
253+
.join()})`]: (node: es.ClassDeclaration) => {
254+
const entry = getEntry();
255+
if (entry && entry.hasDecorator) {
256+
entry.extendsSuperClassDeclaration = node;
257+
}
258+
},
259+
};
260+
236261
return {
237262
'CallExpression[callee.property.name=\'add\']': (
238263
node: es.CallExpression,
@@ -273,6 +298,7 @@ export const preferCompositionRule = ruleCreator({
273298
entry.propertyDefinitions.push(node);
274299
}
275300
},
301+
...extendsSuperClassDeclaration,
276302
'MethodDefinition[key.name=\'ngOnDestroy\'][kind=\'method\']': (
277303
node: es.MethodDefinition,
278304
) => {

src/rules/prefer-takeuntil.ts

Lines changed: 72 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,15 @@ const messages = {
2121
} as const;
2222
type MessageIds = keyof typeof messages;
2323

24+
const ngOnDestroyMethodSelector =
25+
"MethodDefinition[key.name='ngOnDestroy'][kind='method']";
26+
2427
const defaultOptions: readonly {
2528
alias?: string[];
2629
checkComplete?: boolean;
2730
checkDecorators?: string[];
2831
checkDestroy?: boolean;
32+
superClass?: string[];
2933
}[] = [];
3034

3135
export const preferTakeuntilRule = ruleCreator({
@@ -46,6 +50,7 @@ export const preferTakeuntilRule = ruleCreator({
4650
checkComplete: { type: 'boolean', description: 'Check for `complete` calls.' },
4751
checkDecorators: { type: 'array', items: { type: 'string' }, description: 'An optional array of decorator names to check.' },
4852
checkDestroy: { type: 'boolean', description: 'Check for `Subject`-based `ngOnDestroy`.' },
53+
superClass: { type: "array", items: { type: "string" } },
4954
},
5055
type: 'object',
5156
description: stripIndent`
@@ -54,6 +59,7 @@ export const preferTakeuntilRule = ruleCreator({
5459
The \`checkComplete\` property is a boolean that determines whether or not \`complete\` must be called after \`next\`.
5560
The \`checkDecorators\` property is an array containing the names of the decorators that determine whether or not a class is checked.
5661
The \`checkDestroy\` property is a boolean that determines whether or not a \`Subject\`-based \`ngOnDestroy\` must be implemented.
62+
The \`superClass\` property is an array containing the names of classes to extend from that already implements a \`Subject\`-based \`ngOnDestroy\`.
5763
`,
5864
},
5965
],
@@ -73,6 +79,7 @@ export const preferTakeuntilRule = ruleCreator({
7379
checkComplete = false,
7480
checkDecorators = ['Component'],
7581
checkDestroy = alias.length === 0,
82+
superClass = [],
7683
} = config;
7784

7885
interface Entry {
@@ -82,6 +89,8 @@ export const preferTakeuntilRule = ruleCreator({
8289
hasDecorator: boolean;
8390
nextCallExpressions: es.CallExpression[];
8491
ngOnDestroyDefinition?: es.MethodDefinition;
92+
extendsSuperClassDeclaration?: es.ClassDeclaration;
93+
superNgOnDestroyCallExpression?: es.CallExpression;
8594
subscribeCallExpressions: es.CallExpression[];
8695
subscribeCallExpressionsToNames: Map<es.CallExpression, Set<string>>;
8796
}
@@ -112,13 +121,18 @@ export const preferTakeuntilRule = ruleCreator({
112121
completeCallExpressions,
113122
nextCallExpressions,
114123
ngOnDestroyDefinition,
124+
extendsSuperClassDeclaration,
125+
superNgOnDestroyCallExpression,
115126
subscribeCallExpressionsToNames,
116127
} = entry;
117128
if (subscribeCallExpressionsToNames.size === 0) {
118129
return;
119130
}
120131

121132
if (!ngOnDestroyDefinition) {
133+
if (extendsSuperClassDeclaration) {
134+
return;
135+
}
122136
context.report({
123137
messageId: 'noDestroy',
124138
node: classDeclaration.id ?? classDeclaration,
@@ -149,26 +163,39 @@ export const preferTakeuntilRule = ruleCreator({
149163
};
150164
namesToChecks.set(name, check);
151165

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

@@ -304,6 +331,20 @@ export const preferTakeuntilRule = ruleCreator({
304331
);
305332
}
306333

334+
const extendsSuperClassDeclaration =
335+
superClass.length === 0
336+
? {}
337+
: {
338+
[`ClassDeclaration:matches(${superClass
339+
.map((className) => `[superClass.name="${className}"]`)
340+
.join()})`]: (node: es.ClassDeclaration) => {
341+
const entry = getEntry();
342+
if (entry && entry.hasDecorator) {
343+
entry.extendsSuperClassDeclaration = node;
344+
}
345+
},
346+
};
347+
307348
return {
308349
'CallExpression[callee.property.name=\'subscribe\']': (
309350
node: es.CallExpression,
@@ -340,22 +381,28 @@ export const preferTakeuntilRule = ruleCreator({
340381
entry.propertyDefinitions.push(node);
341382
}
342383
},
343-
'MethodDefinition[key.name=\'ngOnDestroy\'][kind=\'method\']': (
344-
node: es.MethodDefinition,
345-
) => {
384+
[ngOnDestroyMethodSelector]: (node: es.MethodDefinition) => {
346385
const entry = getEntry();
347386
if (entry?.hasDecorator) {
348387
entry.ngOnDestroyDefinition = node;
349388
}
350389
},
351-
'MethodDefinition[key.name=\'ngOnDestroy\'][kind=\'method\'] CallExpression[callee.property.name=\'next\']':
390+
...extendsSuperClassDeclaration,
391+
[`${ngOnDestroyMethodSelector} CallExpression[callee.object.type='Super'][callee.property.name='ngOnDestroy']`]:
392+
(node: es.CallExpression) => {
393+
const entry = getEntry();
394+
if (entry && entry.hasDecorator) {
395+
entry.superNgOnDestroyCallExpression = node;
396+
}
397+
},
398+
[`${ngOnDestroyMethodSelector} CallExpression[callee.property.name='next']`]:
352399
(node: es.CallExpression) => {
353400
const entry = getEntry();
354401
if (entry?.hasDecorator) {
355402
entry.nextCallExpressions.push(node);
356403
}
357404
},
358-
'MethodDefinition[key.name=\'ngOnDestroy\'][kind=\'method\'] CallExpression[callee.property.name=\'complete\']':
405+
[`${ngOnDestroyMethodSelector} CallExpression[callee.property.name='complete']`]:
359406
(node: es.CallExpression) => {
360407
const entry = getEntry();
361408
if (entry?.hasDecorator) {

tests/rules/prefer-composition.test.ts

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

0 commit comments

Comments
 (0)