From 39f735acaa68fb0daa6d57f430897f38232bbb68 Mon Sep 17 00:00:00 2001 From: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com> Date: Thu, 9 Jan 2020 20:15:32 +0100 Subject: [PATCH] feat: add ngrx-no-effects-in-providers (#26) --- README.md | 1 + src/recommended.ts | 3 ++ src/rules/ngrxNoEffectsInProvidersRule.ts | 49 +++++++++++++++++++ src/utils/queries.ts | 7 +++ .../fixture.ts.lint | 21 ++++++++ .../tsconfig.json | 12 +++++ .../ngrx-no-effects-in-providers/tslint.json | 7 +++ 7 files changed, 100 insertions(+) create mode 100644 src/rules/ngrxNoEffectsInProvidersRule.ts create mode 100644 src/utils/queries.ts create mode 100644 test/rules/ngrx-no-effects-in-providers/fixture.ts.lint create mode 100644 test/rules/ngrx-no-effects-in-providers/tsconfig.json create mode 100644 test/rules/ngrx-no-effects-in-providers/tslint.json diff --git a/README.md b/README.md index 88ebb68..4277b5f 100755 --- a/README.md +++ b/README.md @@ -54,6 +54,7 @@ To enable all rules, use the `recommended` configuration file. | ngrx-no-dispatch-in-effects | An Effect should not call `store.dispatch`, but should return an action | [Example](https://github.com/timdeschryver/ngrx-tslint-rules/tree/master/test/rules/ngrx-no-dispatch-in-effects/fixture.ts.lint) | | ngrx-no-duplicate-action-types | An action type must be unique | [Example](https://github.com/timdeschryver/ngrx-tslint-rules/tree/master/test/rules/ngrx-no-duplicate-action-types/fixture.ts.lint) | | ngrx-no-effect-decorator | The createEffect creator function is preferred | [Example](https://github.com/timdeschryver/ngrx-tslint-rules/tree/master/test/rules/ngrx-no-effect-decorator/fixture.ts.lint) | +| ngrx-no-effects-in-providers | The Effect should not be listed as a provider if it is added to the EffectsModule | [Example](https://github.com/timdeschryver/ngrx-tslint-rules/tree/master/test/rules/ngrx-no-effects-in-providers/fixture.ts.lint) | | ngrx-no-multiple-actions-in-effects | An Effect should not return multiple actions | [Example](https://github.com/timdeschryver/ngrx-tslint-rules/tree/master/test/rules/ngrx-no-multiple-actions-in-effects/fixture.ts.lint) | | ngrx-no-multiple-stores | Store should at most be one time injected | [Example](https://github.com/timdeschryver/ngrx-tslint-rules/tree/master/test/rules/ngrx-no-multiple-stores/fixture.ts.lint) | | ngrx-no-reducer-in-key-names | Avoid the word "reducer" in the key names | [Example](https://github.com/timdeschryver/ngrx-tslint-rules/tree/master/test/rules/ngrx-no-reducer-in-key-names/fixture.ts.lint) | diff --git a/src/recommended.ts b/src/recommended.ts index 4b11546..48932eb 100644 --- a/src/recommended.ts +++ b/src/recommended.ts @@ -19,6 +19,9 @@ module.exports = { 'ngrx-no-effect-decorator': { severity: 'warning', }, + 'ngrx-no-effects-in-providers': { + severity: 'error', + }, 'ngrx-no-multiple-actions-in-effects': { severity: 'warning', }, diff --git a/src/rules/ngrxNoEffectsInProvidersRule.ts b/src/rules/ngrxNoEffectsInProvidersRule.ts new file mode 100644 index 0000000..18b0323 --- /dev/null +++ b/src/rules/ngrxNoEffectsInProvidersRule.ts @@ -0,0 +1,49 @@ +import { tsquery } from '@phenomnomnominal/tsquery' +import * as Lint from 'tslint' +import * as ts from 'typescript' +import { NG_MODULE_IMPORTS, NG_MODULE_PROVIDERS } from '../utils/queries' + +export class Rule extends Lint.Rules.TypedRule { + public static metadata: Lint.IRuleMetadata = { + description: 'The Effect should not be listed as a provider', + descriptionDetails: + 'If an Effect is registered to the EffectsModule and is added as a provider, it will be registered twice', + options: null, + optionsDescription: 'Not configurable', + requiresTypeInfo: false, + ruleName: 'ngrx-no-effects-in-providers', + type: 'functionality', + typescriptOnly: true, + } + + public static FAILURE_STRING = + 'The Effect should not be listed as a provider if it is added to the EffectsModule' + + private static QUERY = `${NG_MODULE_IMPORTS} > CallExpression[expression.expression.text="EffectsModule"][expression.name.text=/forFeature|forRoot/] > ArrayLiteralExpression > Identifier` + + public applyWithProgram(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + const effectIdentifierNodes = tsquery( + sourceFile, + Rule.QUERY, + ) as ts.Identifier[] + + const effectNames = effectIdentifierNodes.map(node => node.text).join('|') + const hits = tsquery( + sourceFile, + `${NG_MODULE_PROVIDERS} > Identifier[name=/${effectNames}/]`, + ) + + const failures = hits.map( + (node): Lint.RuleFailure => + new Lint.RuleFailure( + sourceFile, + node.getStart(), + node.getStart() + node.getWidth(), + Rule.FAILURE_STRING, + this.ruleName, + ), + ) + + return failures + } +} diff --git a/src/utils/queries.ts b/src/utils/queries.ts new file mode 100644 index 0000000..c4a3c5e --- /dev/null +++ b/src/utils/queries.ts @@ -0,0 +1,7 @@ +const queryNgModuleProperty = (property: string) => + `${NG_MODULE_QUERY} > ObjectLiteralExpression PropertyAssignment[name.text="${property}"] > ArrayLiteralExpression` + +export const NG_MODULE_QUERY = + 'ClassDeclaration > Decorator > CallExpression[expression.text="NgModule"]' +export const NG_MODULE_IMPORTS = queryNgModuleProperty('imports') +export const NG_MODULE_PROVIDERS = queryNgModuleProperty('providers') diff --git a/test/rules/ngrx-no-effects-in-providers/fixture.ts.lint b/test/rules/ngrx-no-effects-in-providers/fixture.ts.lint new file mode 100644 index 0000000..e4ead2a --- /dev/null +++ b/test/rules/ngrx-no-effects-in-providers/fixture.ts.lint @@ -0,0 +1,21 @@ +import { Injectable } from '@angular/core'; +import { StoreModule } from '@ngrx/store' +import { EffectsModule } from '@ngrx/effects' + +@NgModule({ + imports: [ + StoreModule.forFeature('persons', {"foo": "bar"}), + EffectsModule.forRoot([RootEffectOne, RootEffectTwo]), + EffectsModule.forFeature([FeatEffectOne, FeatEffectTwo]), + EffectsModule.forFeature([FeatEffectThree]), + ], + providers: [FeatEffectTwo, UnRegisteredEffect, FeatEffectThree, RootEffectTwo], + ~~~~~~~~~~~~~ [error] + ~~~~~~~~~~~~~~~ [error] + ~~~~~~~~~~~~~ [error] + + bootstrap: [AppComponent], +}) +export class AppModule {} + +[error]: The Effect should not be listed as a provider if it is added to the EffectsModule diff --git a/test/rules/ngrx-no-effects-in-providers/tsconfig.json b/test/rules/ngrx-no-effects-in-providers/tsconfig.json new file mode 100644 index 0000000..e816ee8 --- /dev/null +++ b/test/rules/ngrx-no-effects-in-providers/tsconfig.json @@ -0,0 +1,12 @@ +{ + "compilerOptions": { + "baseUrl": ".", + "lib": ["es2015"], + "noEmit": true, + "paths": { + "@ngrx/effects": ["../../../node_modules/@ngrx/effects"] + }, + "skipLibCheck": true, + "target": "es5" + } +} diff --git a/test/rules/ngrx-no-effects-in-providers/tslint.json b/test/rules/ngrx-no-effects-in-providers/tslint.json new file mode 100644 index 0000000..162d117 --- /dev/null +++ b/test/rules/ngrx-no-effects-in-providers/tslint.json @@ -0,0 +1,7 @@ +{ + "defaultSeverity": "error", + "rules": { + "ngrx-no-effects-in-providers": true + }, + "rulesDirectory": ["../../../dist/rules"] +}