diff --git a/docs/rules/no-redundant-notify.md b/docs/rules/no-redundant-notify.md index b38e374e..245e5fae 100644 --- a/docs/rules/no-redundant-notify.md +++ b/docs/rules/no-redundant-notify.md @@ -6,7 +6,8 @@ -This rule effects failures if an attempt is made to send a notification to an observer after a `complete` or `error` notification has already been sent. +This rule effects failures if an attempt is made to send a notification to an observer after a `complete` or `error` notification has already been sent, +or if `unsubscribe` has been called. Note that the rule _does not perform extensive analysis_. It uses a straightforward and limited approach to catch obviously redundant notifications. diff --git a/src/rules/no-redundant-notify.ts b/src/rules/no-redundant-notify.ts index f515f9e7..d0d5895c 100644 --- a/src/rules/no-redundant-notify.ts +++ b/src/rules/no-redundant-notify.ts @@ -32,7 +32,7 @@ export const noRedundantNotifyRule = ruleCreator({ const sourceCode = context.sourceCode; const { couldBeType } = getTypeServices(context); return { - 'ExpressionStatement[expression.callee.property.name=/^(complete|error)$/] + ExpressionStatement[expression.callee.property.name=/^(next|complete|error)$/]': + 'ExpressionStatement[expression.callee.property.name=/^(complete|error|unsubscribe)$/] + ExpressionStatement[expression.callee.property.name=/^(next|complete|error|unsubscribe)$/]': (node: es.ExpressionStatement) => { const parent = node.parent; if (!parent) { diff --git a/tests/rules/no-redundant-notify.test.ts b/tests/rules/no-redundant-notify.test.ts index ce124a17..8aad26ec 100644 --- a/tests/rules/no-redundant-notify.test.ts +++ b/tests/rules/no-redundant-notify.test.ts @@ -21,6 +21,14 @@ ruleTester({ types: true }).run('no-redundant-notify', noRedundantNotifyRule, { observer.error(new Error("Kaboom!")); }) `, + stripIndent` + // observable next + unsubscribe + import { Observable } from "rxjs"; + const observable = new Observable(observer => { + observer.next(42); + observer.unsubscribe(); + }) + `, stripIndent` // subject next + complete import { Subject } from "rxjs"; @@ -35,6 +43,13 @@ ruleTester({ types: true }).run('no-redundant-notify', noRedundantNotifyRule, { subject.next(42); subject.error(new Error("Kaboom!")); `, + stripIndent` + // subject next + unsubscribe + import { Subject } from "rxjs"; + const subject = new Subject(); + subject.next(42); + subject.unsubscribe(); + `, stripIndent` // different names with error import { Subject } from "rxjs"; @@ -51,6 +66,14 @@ ruleTester({ types: true }).run('no-redundant-notify', noRedundantNotifyRule, { a.complete(); b.complete(); `, + stripIndent` + // different names with unsubscribe + import { Subject } from "rxjs"; + const a = new Subject(); + const b = new Subject(); + a.unsubscribe(); + b.unsubscribe(); + `, stripIndent` // non-observer import { Subject } from "rxjs"; @@ -139,6 +162,51 @@ ruleTester({ types: true }).run('no-redundant-notify', noRedundantNotifyRule, { }); `, ), + fromFixture( + stripIndent` + // observable unsubscribe + next + import { Observable } from "rxjs"; + const observable = new Observable(observer => { + observer.unsubscribe(); + observer.next(42); + ~~~~ [forbidden] + }) + `, + ), + fromFixture( + stripIndent` + // observable unsubscribe + complete + import { Observable } from "rxjs"; + const observable = new Observable(observer => { + observer.unsubscribe(); + observer.complete(); + ~~~~~~~~ [forbidden] + }) + `, + ), + fromFixture( + stripIndent` + // observable unsubscribe + error + import { Observable } from "rxjs"; + const observable = new Observable(observer => { + observer.unsubscribe(); + observer.error(new Error("Kaboom!")); + ~~~~~ [forbidden] + }) + `, + ), + fromFixture( + stripIndent` + // observable unsubscribe + unsubscribe + import { Observable } from "rxjs"; + const observable = new Observable(observer => { + observer.unsubscribe(); + observer.unsubscribe(); + ~~~~~~~~~~~ [forbidden] + }) + `, + ), + fromFixture( stripIndent` // subject complete + next @@ -199,5 +267,45 @@ ruleTester({ types: true }).run('no-redundant-notify', noRedundantNotifyRule, { ~~~~~ [forbidden] `, ), + fromFixture( + stripIndent` + // subject unsubscribe + next + import { Subject } from "rxjs"; + const subject = new Subject(); + subject.unsubscribe(); + subject.next(42); + ~~~~ [forbidden] + `, + ), + fromFixture( + stripIndent` + // subject unsubscribe + complete + import { Subject } from "rxjs"; + const subject = new Subject(); + subject.unsubscribe(); + subject.complete(); + ~~~~~~~~ [forbidden] + `, + ), + fromFixture( + stripIndent` + // subject unsubscribe + error + import { Subject } from "rxjs"; + const subject = new Subject(); + subject.unsubscribe(); + subject.error(new Error("Kaboom!")); + ~~~~~ [forbidden] + `, + ), + fromFixture( + stripIndent` + // subject unsubscribe + unsubscribe + import { Subject } from "rxjs"; + const subject = new Subject(); + subject.unsubscribe(); + subject.unsubscribe(); + ~~~~~~~~~~~ [forbidden] + `, + ), ], });