Skip to content

Commit

Permalink
feat: Add prefer-composition rule.
Browse files Browse the repository at this point in the history
  • Loading branch information
cartant committed Nov 27, 2019
1 parent 65922db commit c145b6b
Show file tree
Hide file tree
Showing 6 changed files with 348 additions and 6 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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\"",
Expand Down
227 changes: 227 additions & 0 deletions source/rules/rxjsPreferAngularCompositionRule.ts
Original file line number Diff line number Diff line change
@@ -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<string>();
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<string>
): 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<string>
): 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;
}
19 changes: 14 additions & 5 deletions test/v6/fixtures/prefer-angular-async-pipe/default/fixture.ts.lint
Original file line number Diff line number Diff line change
@@ -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: "<span>something</span>"
selector: "piped-component",
template: "<span>{{ value | async }}</span>"
})
export class SomeComponent implements OnInit {
export class PipedComponent {
value = of("foo");
}

@Component({
selector: "not-piped-component",
template: "<span>{{ value }}</span>"
})
export class NotPipedComponent implements OnInit {
value: string;
ngOnInit() {
of("foo").subscribe();
of("foo").subscribe(value => this.value = value);
~~~~~~~~~ [prefer-async-pipe]
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { Component, OnDestroy, OnInit } from "@angular/core";
import { of, Subscription } from "rxjs";

@Component({
selector: "composed-component",
template: "<span>{{ value }}</span>"
})
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: "<span>{{ value }}</span>"
})
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: "<span>{{ value }}</span>"
})
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: "<span>{{ value }}</span>"
})
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: "<span>{{ value }}</span>"
})
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
13 changes: 13 additions & 0 deletions test/v6/fixtures/prefer-angular-composition/default/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"compilerOptions": {
"baseUrl": ".",
"lib": ["es2015"],
"noEmit": true,
"paths": {
"rxjs": ["../../node_modules/rxjs"]
},
"skipLibCheck": true,
"target": "es5"
},
"include": ["fixture.ts"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"defaultSeverity": "error",
"jsRules": {},
"rules": {
"rxjs-prefer-angular-composition": { "severity": "error" }
},
"rulesDirectory": "../../../../../build/rules"
}

0 comments on commit c145b6b

Please sign in to comment.