From c145b6bec52d15f7531bed1111a180d398980400 Mon Sep 17 00:00:00 2001 From: Nicholas Jamieson Date: Sun, 24 Nov 2019 19:15:28 +1000 Subject: [PATCH] feat: Add prefer-composition rule. --- package.json | 2 +- .../rules/rxjsPreferAngularCompositionRule.ts | 227 ++++++++++++++++++ .../default/fixture.ts.lint | 19 +- .../default/fixture.ts.lint | 85 +++++++ .../default/tsconfig.json | 13 + .../default/tslint.json | 8 + 6 files changed, 348 insertions(+), 6 deletions(-) create mode 100644 source/rules/rxjsPreferAngularCompositionRule.ts create mode 100644 test/v6/fixtures/prefer-angular-composition/default/fixture.ts.lint create mode 100644 test/v6/fixtures/prefer-angular-composition/default/tsconfig.json create mode 100644 test/v6/fixtures/prefer-angular-composition/default/tslint.json diff --git a/package.json b/package.json index 1f4d89f7..8068969e 100644 --- a/package.json +++ b/package.json @@ -78,7 +78,7 @@ "test": "yarn run lint && yarn run test:build && yarn run test:mocha && yarn run test:tslint-v5 && yarn run test:tslint-v6 && yarn run test:tslint-v6-compat", "test:build": "yarn run test:clean && tsc -p tsconfig.json", "test:clean": "rimraf build", - "test:debug": "tslint --test \"./test/v6/fixtures/no-redundant-notify/default/tslint.json\"", + "test:debug": "tslint --test \"./test/v6/fixtures/prefer-angular-composition/default/tslint.json\"", "test:issues": "yarn run test:clean && tsc -p tsconfig.json && tslint --test \"./test/v6/fixtures/issues/**/tslint.json\"", "test:mocha": "mocha \"./build/**/*-spec.js\"", "test:tslint-v5": "yarn --cwd ./test/v5 install && yarn --cwd ./test/v5 upgrade && tslint --test \"./test/v5/fixtures/**/tslint.json\"", diff --git a/source/rules/rxjsPreferAngularCompositionRule.ts b/source/rules/rxjsPreferAngularCompositionRule.ts new file mode 100644 index 00000000..96018dc9 --- /dev/null +++ b/source/rules/rxjsPreferAngularCompositionRule.ts @@ -0,0 +1,227 @@ +/** + * @license Use of this source code is governed by an MIT-style license that + * can be found in the LICENSE file at https://github.com/cartant/rxjs-tslint-rules + */ + +import { tsquery } from "@phenomnomnominal/tsquery"; +import * as Lint from "tslint"; +import * as ts from "typescript"; +import { couldBeType, isThis } from "../support/util"; + +export class Rule extends Lint.Rules.TypedRule { + public static metadata: Lint.IRuleMetadata = { + description: "? within an Angular component.", + options: null, + optionsDescription: "Not configurable.", + requiresTypeInfo: true, + ruleName: "rxjs-prefer-angular-composition", + type: "style", + typescriptOnly: true + }; + + public applyWithProgram( + sourceFile: ts.SourceFile, + program: ts.Program + ): Lint.RuleFailure[] { + const failures: Lint.RuleFailure[] = []; + const typeChecker = program.getTypeChecker(); + + const classDeclarations = tsquery( + sourceFile, + `ClassDeclaration:has(Decorator[expression.expression.name="Component"])` + ) as ts.ClassDeclaration[]; + classDeclarations.forEach(classDeclaration => { + const subscriptions = new Set(); + const callExpressions = tsquery( + classDeclaration, + `CallExpression[expression.name.text="subscribe"]` + ) as ts.CallExpression[]; + callExpressions.forEach(callExpression => { + const { expression } = callExpression; + if (ts.isPropertyAccessExpression(expression)) { + const { expression: object, name: property } = expression; + const type = typeChecker.getTypeAtLocation(object); + if (!couldBeType(type, "Observable")) { + return; + } + if (isComposed(callExpression, typeChecker, subscriptions)) { + return; + } + failures.push( + new Lint.RuleFailure( + sourceFile, + property.getStart(), + property.getStart() + property.getWidth(), + "Subscription not composed", + this.ruleName + ) + ); + } + }); + if (callExpressions.length === 0) { + return; + } + + const methodDeclarations = tsquery( + classDeclaration, + `MethodDeclaration[name.text="ngOnDestroy"]` + ) as ts.MethodDeclaration[]; + if (methodDeclarations.length === 0) { + const { name } = classDeclaration; + failures.push( + new Lint.RuleFailure( + sourceFile, + name.getStart(), + name.getStart() + name.getWidth(), + "ngOnDestroy not implemented", + this.ruleName + ) + ); + return; + } + const [methodDeclaration] = methodDeclarations; + + subscriptions.forEach(subscription => { + const propertyDeclarations = tsquery( + classDeclaration, + `PropertyDeclaration[name.text="${subscription}"]` + ) as ts.PropertyDeclaration[]; + if (propertyDeclarations.length === 0) { + const { name } = classDeclaration; + failures.push( + new Lint.RuleFailure( + sourceFile, + name.getStart(), + name.getStart() + name.getWidth(), + `Composed subscription '${subscription}' not declared`, + this.ruleName + ) + ); + return; + } + const [propertyDeclaration] = propertyDeclarations; + + const callExpressions = tsquery( + methodDeclaration, + `CallExpression[expression.expression.name.text="${subscription}"][expression.name.text="unsubscribe"], CallExpression[expression.expression.text="${subscription}"][expression.name.text="unsubscribe"]` + ); + if (callExpressions.length === 0) { + const { name } = propertyDeclaration; + failures.push( + new Lint.RuleFailure( + sourceFile, + name.getStart(), + name.getStart() + name.getWidth(), + `Composed subscription not unsubscribed`, + this.ruleName + ) + ); + } + }); + }); + return failures; + } +} + +function getBlock(node: ts.Node): ts.Block | undefined { + let { parent } = node; + while (parent && !ts.isBlock(parent)) { + parent = parent.parent; + } + return parent as ts.Block | undefined; +} + +function isComposed( + callExpression: ts.CallExpression, + typeChecker: ts.TypeChecker, + subscriptions: Set +): boolean { + const { parent } = callExpression; + if ( + ts.isCallExpression(parent) && + ts.isPropertyAccessExpression(parent.expression) + ) { + const { expression: object, name: property } = parent.expression; + const { text } = property; + if (text !== "add") { + return false; + } + if (!couldBeType(typeChecker.getTypeAtLocation(object), "Subscription")) { + return false; + } + // TODO: Need a general mechanism to get the subscription name from either + // an identifier or a this-accessed property: + if (ts.isPropertyAccessExpression(object)) { + subscriptions.add((object as any).name.text); + } else { + subscriptions.add((object as any).text); + } + return true; + } + if (ts.isVariableDeclaration(parent) && ts.isIdentifier(parent.name)) { + return isVariableComposed(parent.name, typeChecker, subscriptions); + } + if ( + ts.isBinaryExpression(parent) && + ts.isIdentifier(parent.left) && + parent.operatorToken.kind === ts.SyntaxKind.EqualsToken + ) { + return isVariableComposed(parent.left, typeChecker, subscriptions); + } + return false; +} + +function isVariableComposed( + identifier: ts.Identifier, + typeChecker: ts.TypeChecker, + subscriptions: Set +): boolean { + const { text } = identifier; + const block = getBlock(identifier); + if (block) { + const callExpressions = tsquery( + block, + `CallExpression[expression.name.text="add"] > Identifier[text="${text}"]` + ) + .map(identifier => identifier.parent) + .filter((callExpression: ts.CallExpression) => { + const { expression } = callExpression; + if (!ts.isPropertyAccessExpression(expression)) { + return false; + } + if (callExpression.end < identifier.pos) { + return false; + } + if ( + !couldBeType( + typeChecker.getTypeAtLocation(expression.expression), + "Subscription" + ) + ) { + return false; + } + return true; + }) as ts.CallExpression[]; + if (callExpressions.length === 0) { + return false; + } + const [callExpression] = callExpressions; + const { expression } = callExpression; + if (!ts.isPropertyAccessExpression(expression)) { + return false; + } + if (ts.isIdentifier(expression.expression)) { + subscriptions.add(expression.expression.text); + return true; + } + if ( + ts.isPropertyAccessExpression(expression.expression) && + isThis(expression.expression.expression) + ) { + subscriptions.add(expression.expression.name.text); + return true; + } + return false; + } + return false; +} diff --git a/test/v6/fixtures/prefer-angular-async-pipe/default/fixture.ts.lint b/test/v6/fixtures/prefer-angular-async-pipe/default/fixture.ts.lint index 02b33b01..613d89a1 100644 --- a/test/v6/fixtures/prefer-angular-async-pipe/default/fixture.ts.lint +++ b/test/v6/fixtures/prefer-angular-async-pipe/default/fixture.ts.lint @@ -1,13 +1,22 @@ -import { Component OnInit } from "@angular/core"; +import { Component, OnInit } from "@angular/core"; import { of } from "rxjs"; @Component({ - selector: "some-component", - template: "something" + selector: "piped-component", + template: "{{ value | async }}" }) -export class SomeComponent implements OnInit { +export class PipedComponent { + value = of("foo"); +} + +@Component({ + selector: "not-piped-component", + template: "{{ value }}" +}) +export class NotPipedComponent implements OnInit { + value: string; ngOnInit() { - of("foo").subscribe(); + of("foo").subscribe(value => this.value = value); ~~~~~~~~~ [prefer-async-pipe] } } diff --git a/test/v6/fixtures/prefer-angular-composition/default/fixture.ts.lint b/test/v6/fixtures/prefer-angular-composition/default/fixture.ts.lint new file mode 100644 index 00000000..04685cdf --- /dev/null +++ b/test/v6/fixtures/prefer-angular-composition/default/fixture.ts.lint @@ -0,0 +1,85 @@ +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; + private subscription = new Subscription(); + ngOnInit() { + this.subscription.add(of("foo").subscribe(value => this.value = value)); + let subscription = of("bar").subscribe(value => this.value = value); + this.subscription.add(subscription); + subscription = of("baz").subscribe(value => this.value = value); + this.subscription.add(subscription); + } + ngOnDestroy() { + this.subscription.unsubscribe(); + } +} + +@Component({ + selector: "destructured-composed-component", + template: "{{ value }}" +}) +export class DestructuredComposedComponent implements OnInit, OnDestroy { + value: string; + private subscription = new Subscription(); + ngOnInit() { + const { subscription } = this; + subscription.add(of("foo").subscribe(value => this.value = value)); + } + ngOnDestroy() { + const { subscription } = this; + subscription.unsubscribe(); + } +} + +@Component({ + selector: "not-composed-component", + template: "{{ value }}" +}) +export class NotComposedComponent implements OnInit, OnDestroy { + value: string; + ngOnInit() { + of("foo").subscribe(value => this.value = value); + ~~~~~~~~~ [prefer-composition % ("Subscription not composed")] + const subscription = of("bar").subscribe(value => this.value = value); + ~~~~~~~~~ [prefer-composition % ("Subscription not composed")] + + } + ngOnDestroy() { + } +} + +@Component({ + selector: "not-unsubscribed-component", + template: "{{ value }}" +}) +export class NotUnsubscribedComponent implements OnInit, OnDestroy { + value: string; + private subscription = new Subscription(); + ~~~~~~~~~~~~ [prefer-composition % ("Composed subscription not unsubscribed")] + ngOnInit() { + this.subscription.add(of("foo").subscribe(value => this.value = value)); + } + ngOnDestroy() { + } +} + +@Component({ + selector: "not-destroyed-component", + template: "{{ value }}" +}) +export class NotDestroyedComponent implements OnInit { + ~~~~~~~~~~~~~~~~~~~~~ [prefer-composition % ("ngOnDestroy not implemented")] + value: string; + private subscription = new Subscription(); + ngOnInit() { + this.subscription.add(of("foo").subscribe(value => this.value = value)); + } +} + +[prefer-composition]: %s diff --git a/test/v6/fixtures/prefer-angular-composition/default/tsconfig.json b/test/v6/fixtures/prefer-angular-composition/default/tsconfig.json new file mode 100644 index 00000000..690be78e --- /dev/null +++ b/test/v6/fixtures/prefer-angular-composition/default/tsconfig.json @@ -0,0 +1,13 @@ +{ + "compilerOptions": { + "baseUrl": ".", + "lib": ["es2015"], + "noEmit": true, + "paths": { + "rxjs": ["../../node_modules/rxjs"] + }, + "skipLibCheck": true, + "target": "es5" + }, + "include": ["fixture.ts"] +} diff --git a/test/v6/fixtures/prefer-angular-composition/default/tslint.json b/test/v6/fixtures/prefer-angular-composition/default/tslint.json new file mode 100644 index 00000000..98e93772 --- /dev/null +++ b/test/v6/fixtures/prefer-angular-composition/default/tslint.json @@ -0,0 +1,8 @@ +{ + "defaultSeverity": "error", + "jsRules": {}, + "rules": { + "rxjs-prefer-angular-composition": { "severity": "error" } + }, + "rulesDirectory": "../../../../../build/rules" +}