diff --git a/.husky/pre-commit b/.husky/pre-commit old mode 100644 new mode 100755 diff --git a/package.json b/package.json index 925adeb..49771e7 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,7 @@ "dependencies": { "@typescript-eslint/experimental-utils": "^5.0.0", "common-tags": "^1.8.0", - "eslint-etc": "^5.0.0", + "eslint-etc": "^5.2.0", "requireindex": "~1.2.0", "tslib": "^2.0.0" }, diff --git a/source/rules/prefer-composition.ts b/source/rules/prefer-composition.ts index 90a2962..cc8b97e 100644 --- a/source/rules/prefer-composition.ts +++ b/source/rules/prefer-composition.ts @@ -11,6 +11,7 @@ import { isAssignmentExpression, isCallExpression, isIdentifier, + isPrivateIdentifier, isMemberExpression, isVariableDeclarator, } from "eslint-etc"; @@ -145,7 +146,11 @@ const rule = ruleCreator({ const { callee } = callExpression; if (isMemberExpression(callee)) { const { object } = callee; - if (isMemberExpression(object) && isIdentifier(object.property)) { + if ( + isMemberExpression(object) && + (isIdentifier(object.property) || + isPrivateIdentifier(object.property)) + ) { return object.property.name; } if (isIdentifier(object)) { @@ -186,6 +191,7 @@ const rule = ruleCreator({ // subscription or if it's assigned to a variable that is added to a // subscription. const { addCallExpressions, subscriptions } = entry; + const parent = getParent(callExpression); if (!parent) { return false; @@ -208,6 +214,7 @@ const rule = ruleCreator({ subscriptions.add(name); return true; } + if (isVariableDeclarator(parent) && isIdentifier(parent.id)) { return isVariableComposed(parent.id, entry); } diff --git a/source/rules/prefer-takeuntil.ts b/source/rules/prefer-takeuntil.ts index f707665..21d148c 100644 --- a/source/rules/prefer-takeuntil.ts +++ b/source/rules/prefer-takeuntil.ts @@ -15,7 +15,7 @@ import { isMemberExpression, isThisExpression, } from "eslint-etc"; -import { ruleCreator } from "../utils"; +import { ruleCreator, isPrivateIdentifier } from "../utils"; const messages = { noDestroy: "`ngOnDestroy` is not implemented.", @@ -207,7 +207,7 @@ const rule = ruleCreator({ if ( isMemberExpression(arg) && isThisExpression(arg.object) && - isIdentifier(arg.property) + (isIdentifier(arg.property) || isPrivateIdentifier(arg.property)) ) { return { found: true, name: arg.property.name }; } else if (arg && isIdentifier(arg)) { @@ -233,7 +233,8 @@ const rule = ruleCreator({ (isMemberExpression(callee) && isMemberExpression(callee.object) && isThisExpression(callee.object.object) && - isIdentifier(callee.object.property) && + (isIdentifier(callee.object.property) || + isPrivateIdentifier(callee.object.property)) && callee.object.property.name === name) ); return Boolean(callExpression); diff --git a/tests/rules/prefer-composition.ts b/tests/rules/prefer-composition.ts index 7084a8e..70ae37c 100644 --- a/tests/rules/prefer-composition.ts +++ b/tests/rules/prefer-composition.ts @@ -32,6 +32,28 @@ ruleTester({ types: true }).run("prefer-composition", rule, { } `, }, + { + code: stripIndent` + // composed component with private JavaScript property + import { Component, OnDestroy, OnInit } from "@angular/core"; + import { of, Subscription } from "rxjs"; + + @Component({ + selector: "composed-component", + template: "{{ value }}" + }) + export class ComposedComponent implements OnInit, OnDestroy { + value: string; + #subscription = new Subscription(); + ngOnInit() { + this.#subscription.add(of("foo").subscribe(value => this.value = value)); + } + ngOnDestroy() { + this.#subscription.unsubscribe(); + } + } + `, + }, { code: stripIndent` // variable composed component @@ -47,7 +69,7 @@ ruleTester({ types: true }).run("prefer-composition", rule, { private subscription = new Subscription(); ngOnInit() { let subscription = of("foo").subscribe(value => this.value = value); - this.subscription.add(subscription);1 + this.subscription.add(subscription); subscription = of("bar").subscribe(value => this.value = value); this.subscription.add(subscription); } @@ -57,6 +79,31 @@ ruleTester({ types: true }).run("prefer-composition", rule, { } `, }, + { + code: stripIndent` + // variable composed component with private JavaScript property + import { Component, OnDestroy, OnInit } from "@angular/core"; + import { of, Subscription } from "rxjs"; + + @Component({ + selector: "variable-composed-component", + template: "{{ value }}" + }) + export class VariableComposedComponent implements OnInit, OnDestroy { + value: string; + #subscription = new Subscription(); + ngOnInit() { + let subscription = of("foo").subscribe(value => this.value = value); + this.#subscription.add(subscription); + subscription = of("bar").subscribe(value => this.value = value); + this.#subscription.add(subscription); + } + ngOnDestroy() { + this.#subscription.unsubscribe(); + } + } + `, + }, { code: stripIndent` // destructured composed component @@ -145,6 +192,28 @@ ruleTester({ types: true }).run("prefer-composition", rule, { } ` ), + fromFixture( + stripIndent` + // not unsubscribed component with private JavaScript property + import { Component, OnDestroy, OnInit } from "@angular/core"; + import { of, Subscription } from "rxjs"; + + @Component({ + selector: "not-unsubscribed-component", + template: "{{ value }}" + }) + export class NotUnsubscribedComponent implements OnInit, OnDestroy { + value: string; + #subscription = new Subscription(); + ~~~~~~~~~~~~~ [notUnsubscribed] + ngOnInit() { + this.#subscription.add(of("foo").subscribe(value => this.value = value)); + } + ngOnDestroy() { + } + } + ` + ), fromFixture( stripIndent` // not destroyed component @@ -165,6 +234,26 @@ ruleTester({ types: true }).run("prefer-composition", rule, { } ` ), + fromFixture( + stripIndent` + // not destroyed component with Private JavaScript property + import { Component, OnDestroy, OnInit } from "@angular/core"; + import { of, Subscription } from "rxjs"; + + @Component({ + selector: "not-destroyed-component", + template: "{{ value }}" + }) + export class NotDestroyedComponent implements OnInit { + ~~~~~~~~~~~~~~~~~~~~~ [notImplemented] + value: string; + #subscription = new Subscription(); + ngOnInit() { + this.#subscription.add(of("foo").subscribe(value => this.value = value)); + } + } + ` + ), fromFixture( stripIndent` // not declared diff --git a/tests/rules/prefer-takeuntil.ts b/tests/rules/prefer-takeuntil.ts index 1a6b2d2..c8011d2 100644 --- a/tests/rules/prefer-takeuntil.ts +++ b/tests/rules/prefer-takeuntil.ts @@ -37,6 +37,33 @@ ruleTester({ types: true }).run("prefer-takeuntil", rule, { } `, }, + { + code: stripIndent` + // correct component with Private JavaScript property + import { Component, OnDestroy } from "@angular/core"; + import { of, Subject } from "rxjs"; + import { switchMap, takeUntil } from "rxjs/operators"; + + const o = of("o"); + + @Component({ + selector: "correct-component" + }) + class CorrectComponent implements OnDestroy { + #destroy = new Subject(); + someMethod() { + o.pipe( + switchMap(_ => o), + takeUntil(this.#destroy) + ).subscribe(); + } + ngOnDestroy() { + this.#destroy.next(); + this.#destroy.complete(); + } + } + `, + }, { code: stripIndent` // correct component, not last @@ -65,6 +92,34 @@ ruleTester({ types: true }).run("prefer-takeuntil", rule, { } `, }, + { + code: stripIndent` + // correct component, not last with Private JavaScript property + import { Component, OnDestroy } from "@angular/core"; + import { of, Subject } from "rxjs"; + import { map, switchMap, takeUntil } from "rxjs/operators"; + + const o = of("o"); + + @Component({ + selector: "correct-component" + }) + class CorrectComponent implements OnDestroy { + #destroy = new Subject(); + someMethod() { + o.pipe( + switchMap(_ => o), + takeUntil(this.#destroy), + map(value => value) + ).subscribe(); + } + ngOnDestroy() { + this.#destroy.next(); + this.#destroy.complete(); + } + } + `, + }, { code: stripIndent` // destructured component @@ -122,6 +177,34 @@ ruleTester({ types: true }).run("prefer-takeuntil", rule, { } `, }, + { + code: stripIndent` + // secondary takeuntil component with Private JavaScript property + import { Component, OnDestroy } from "@angular/core"; + import { of, Subject } from "rxjs"; + import { switchMap, takeUntil } from "rxjs/operators"; + + const o = of("o"); + + @Component({ + selector: "secondary-takeuntil-component" + }) + class SecondaryTakeUntilComponent implements OnDestroy { + #destroy = new Subject(); + someMethod() { + o.pipe( + takeUntil(o), + switchMap(_ => o), + takeUntil(this.#destroy) + ).subscribe(); + } + ngOnDestroy() { + this.#destroy.next(); + this.#destroy.complete(); + } + } + `, + }, { code: stripIndent` // not components @@ -226,6 +309,35 @@ ruleTester({ types: true }).run("prefer-takeuntil", rule, { `, options: [{ alias: ["someAlias"] }], }, + { + code: stripIndent` + // with alias with Private JavaScript property + import { Component, OnDestroy } from "@angular/core"; + import { of, Subject } from "rxjs"; + import { switchMap, takeUntil } from "rxjs/operators"; + + const o = of("o"); + const someAlias = takeUntil; + + @Component({ + selector: "component-with-alias" + }) + class CorrectComponent implements OnDestroy { + #destroy = new Subject(); + someMethod() { + o.pipe( + switchMap(_ => o), + someAlias(this.#destroy) + ).subscribe(); + } + ngOnDestroy() { + this.#destroy.next(); + this.#destroy.complete(); + } + } + `, + options: [{ alias: ["someAlias"] }], + }, { code: stripIndent` // decorators with takeuntil @@ -307,6 +419,87 @@ ruleTester({ types: true }).run("prefer-takeuntil", rule, { }, ], }, + { + code: stripIndent` + // decorators with takeuntil + import { Component, OnDestroy } from "@angular/core"; + import { of, Subject } from "rxjs"; + import { switchMap, takeUntil } from "rxjs/operators"; + + const o = of("o"); + + @Component({ + selector: "correct-component" + }) + class CorrectComponent implements OnDestroy { + #destroy = new Subject(); + someMethod() { + o.pipe( + switchMap(_ => o), + takeUntil(this.#destroy) + ).subscribe(); + } + ngOnDestroy() { + this.#destroy.next(); + this.#destroy.complete(); + } + } + + @Injectable() + class CorrectService implements OnDestroy { + #destroy = new Subject(); + someMethod() { + o.pipe( + switchMap(_ => o), + takeUntil(this.#destroy) + ).subscribe(); + } + ngOnDestroy() { + this.#destroy.next(); + this.#destroy.complete(); + } + } + + @Pipe({ + name: 'controlByName', + }) + class CorrectPipe implements OnDestroy { + #destroy = new Subject(); + someMethod() { + o.pipe( + switchMap(_ => o), + takeUntil(this.#destroy) + ).subscribe(); + } + ngOnDestroy() { + this.#destroy.next(); + this.#destroy.complete(); + } + } + + @Directive({ + selector: 'my-directive' + }) + class CorrectDirective implements OnDestroy { + #destroy = new Subject(); + someMethod() { + o.pipe( + switchMap(_ => o), + takeUntil(this.#destroy) + ).subscribe(); + } + ngOnDestroy() { + this.#destroy.next(); + this.#destroy.complete(); + } + } + `, + options: [ + { + checkDecorators: ["Component", "Pipe", "Injectable", "Directive"], + }, + ], + }, { code: stripIndent` // https://github.com/cartant/rxjs-tslint-rules/issues/115 @@ -341,6 +534,40 @@ ruleTester({ types: true }).run("prefer-takeuntil", rule, { }, ], }, + { + code: stripIndent` + // https://github.com/cartant/rxjs-tslint-rules/issues/115 with Private JavaScript property + import { Component } from "@angular/core"; + import { of, Subject } from "rxjs"; + import { switchMap, takeUntil } from "rxjs/operators"; + + const o = of("o"); + const someAlias = (cmp) => takeUntil(cmp.destroy); + + @Component({ + selector: "component-with-alias" + }) + class CorrectComponent implements OnDestroy { + #destroy = new Subject(); + someMethod() { + o.pipe( + switchMap(_ => o), + someAlias(this) + ).subscribe(); + } + ngOnDestroy() { + this.#destroy.next(); + this.#destroy.complete(); + } + } + `, + options: [ + { + alias: ["someAlias"], + checkDestroy: false, + }, + ], + }, { code: stripIndent` // https://github.com/cartant/eslint-plugin-rxjs-angular/issues/5 @@ -398,6 +625,32 @@ ruleTester({ types: true }).run("prefer-takeuntil", rule, { `, { options: [{ checkComplete: true }] } ), + fromFixture( + stripIndent` + // no pipe component with Private JavaScript property + import { Component, OnDestroy } from "@angular/core"; + import { of, Subject } from "rxjs"; + import { switchMap, takeUntil } from "rxjs/operators"; + + const o = of("o"); + + @Component({ + selector: "no-pipe-component" + }) + class NoPipeComponent { + #destroy = new Subject(); + someMethod() { + o.subscribe(); + ~~~~~~~~~ [noTakeUntil] + } + ngOnDestroy() { + this.#destroy.next(); + this.#destroy.complete(); + } + } + `, + { options: [{ checkComplete: true }] } + ), fromFixture( stripIndent` // no takeuntil component @@ -427,6 +680,34 @@ ruleTester({ types: true }).run("prefer-takeuntil", rule, { `, { options: [{ checkComplete: true }] } ), + fromFixture( + stripIndent` + // no takeuntil component with Private JavaScript property + import { Component, OnDestroy } from "@angular/core"; + import { of, Subject } from "rxjs"; + import { switchMap, takeUntil } from "rxjs/operators"; + + const o = of("o"); + + @Component({ + selector: "no-takeuntil-component" + }) + class NoTakeUntilComponent { + #destroy = new Subject(); + someMethod() { + o.pipe( + switchMap(_ => o) + ).subscribe(); + ~~~~~~~~~ [noTakeUntil] + } + ngOnDestroy() { + this.#destroy.next(); + this.#destroy.complete(); + } + } + `, + { options: [{ checkComplete: true }] } + ), fromFixture( stripIndent` // no subject component @@ -481,6 +762,31 @@ ruleTester({ types: true }).run("prefer-takeuntil", rule, { `, { options: [{ checkComplete: true }] } ), + fromFixture( + stripIndent` + // no destroy component + import { Component, OnDestroy } from "@angular/core"; + import { of, Subject } from "rxjs"; + import { switchMap, takeUntil } from "rxjs/operators"; + + const o = of("o"); + + @Component({ + selector: "no-destroy-component" + }) + class NoDestroyComponent { + ~~~~~~~~~~~~~~~~~~ [noDestroy] + #destroy = new Subject(); + someMethod() { + o.pipe( + switchMap(_ => o), + takeUntil(this.#destroy) + ).subscribe(); + } + } + `, + { options: [{ checkComplete: true }] } + ), fromFixture( stripIndent` // no next component @@ -509,6 +815,34 @@ ruleTester({ types: true }).run("prefer-takeuntil", rule, { `, { options: [{ checkComplete: true }] } ), + fromFixture( + stripIndent` + // no next component with Private JavaScript property + import { Component, OnDestroy } from "@angular/core"; + import { of, Subject } from "rxjs"; + import { switchMap, takeUntil } from "rxjs/operators"; + + const o = of("o"); + + @Component({ + selector: "no-next-component" + }) + class NoNextComponent implements OnDestroy { + #destroy = new Subject(); + someMethod() { + o.pipe( + switchMap(_ => o), + takeUntil(this.#destroy) + ).subscribe(); + } + ngOnDestroy() { + ~~~~~~~~~~~ [notCalled { "method": "next", "name": "destroy" }] + this.#destroy.complete(); + } + } + `, + { options: [{ checkComplete: true }] } + ), fromFixture( stripIndent` // no complete component @@ -537,6 +871,34 @@ ruleTester({ types: true }).run("prefer-takeuntil", rule, { `, { options: [{ checkComplete: true }] } ), + fromFixture( + stripIndent` + // no complete component with Private JavaScript property + import { Component, OnDestroy } from "@angular/core"; + import { of, Subject } from "rxjs"; + import { switchMap, takeUntil } from "rxjs/operators"; + + const o = of("o"); + + @Component({ + selector: "no-complete-component" + }) + class NoCompleteComponent implements OnDestroy { + #destroy = new Subject(); + someMethod() { + o.pipe( + switchMap(_ => o), + takeUntil(this.#destroy) + ).subscribe(); + } + ngOnDestroy() { + ~~~~~~~~~~~ [notCalled { "method": "complete", "name": "destroy" }] + this.#destroy.next(); + } + } + `, + { options: [{ checkComplete: true }] } + ), fromFixture( stripIndent` // no destroy and no takeuntil component @@ -590,6 +952,35 @@ ruleTester({ types: true }).run("prefer-takeuntil", rule, { `, { options: [{ alias: ["someAlias"] }] } ), + fromFixture( + stripIndent` + // without alias + import { Component, OnDestroy } from "@angular/core"; + import { of, Subject } from "rxjs"; + import { switchMap, takeUntil } from "rxjs/operators"; + + const o = of("o"); + const someAlias = takeUntil; + + @Component({ + selector: "component-without-alias" + }) + class NoTakeUntilComponent { + #destroy = new Subject(); + someMethod() { + o.pipe( + switchMap(_ => o) + ).subscribe(); + ~~~~~~~~~ [noTakeUntil] + } + ngOnDestroy() { + this.#destroy.next(); + this.#destroy.complete(); + } + } + `, + { options: [{ alias: ["someAlias"] }] } + ), fromFixture( stripIndent` // decorators without takeuntil diff --git a/yarn.lock b/yarn.lock index 27a7f3f..e58843e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -699,7 +699,7 @@ escape-string-regexp@4.0.0, escape-string-regexp@^4.0.0: resolved "https://registry.yarnpkg.com/escape-string-regexp/-/escape-string-regexp-4.0.0.tgz#14ba83a5d373e3d311e5afca29cf5bfad965bf34" integrity sha512-TtpcNJ3XAzx3Gq8sWRzJaVajRs0uVxA2YAkdb1jm2YkPz4G6egUFAyA3n5vtEIZefPk5Wa4UXbKuS5fKkJWdgA== -eslint-etc@^5.0.0: +eslint-etc@^5.2.0: version "5.2.0" resolved "https://registry.yarnpkg.com/eslint-etc/-/eslint-etc-5.2.0.tgz#c51c19a2ffb6447af76a1c5ffd13b9a212db859b" integrity sha512-Gcm/NMa349FOXb1PEEfNMMyIANuorIc2/mI5Vfu1zENNsz+FBVhF62uY6gPUCigm/xDOc8JOnl+71WGnlzlDag==