feat(subjects): Add allowProtected option.

cartant · Apr 16, 2019 · 977ecf3 · 977ecf3
1 parent fe0146d
commit 977ecf3
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 9 deletions.
diff --git a/README.md b/README.md
@@ -90,7 +90,7 @@ The package includes the following rules (none of which are enabled by default):
 | `rxjs-no-deep-operators` | Disallows deep importation from `rxjs/operators`. Deep imports won't be in available in RxJS v6. | None |
 | `rxjs-no-do` | I do without `do` operators. [Do you not?](https://youtu.be/spG-Yj0zEyc) Well, `do` isn't always a code smell, but this rule can be useful as a warning. | None |
 | `rxjs-no-explicit-generics` | Disallows the explicit specification of generic type arguments when calling operators. Rely upon TypeScript's inference instead. | None |
-| `rxjs-no-exposed-subjects` | Disallows exposed subjects. In classes, `Subject` properties and methods that return a `Subject` must be `private`. | None |
+| `rxjs-no-exposed-subjects` | Disallows exposed subjects. In classes, `Subject` properties and methods that return a `Subject` must be `private`. | [See below](#rxjs-no-exposed-subjects) |
 | `rxjs-no-finnish` | Disallows the use of [Finnish notation](https://medium.com/@benlesh/observables-and-finnish-notation-df8356ed1c9b). | None |
 | `rxjs-no-ignored-error` | Disallows the calling of `subscribe` without specifying an error handler. | None |
 | `rxjs-no-ignored-notifier` | Disallows observables not composed from the `repeatWhen` or `retryWhen` notifier. | None |
@@ -236,6 +236,23 @@ For example:
 }
 ```
 
+<a name="rxjs-no-exposed-subjects"></a>
+
+#### rxjs-no-exposed-subjects
+
+The rule has an optional `allowProtected` property that can be specified - it defaults to `false`:
+
+```json
+"rules": {
+  "rxjs-no-sharereplay": {
+    "options": [{
+      "allowProtected": true
+    }],
+    "severity": "error"
+  }
+}
+```
+
 <a name="rxjs-no-sharereplay"></a>
 
 #### rxjs-no-sharereplay

diff --git a/docs/index.md b/docs/index.md
@@ -25,7 +25,7 @@ The package includes the following rules (none of which are enabled by default):
 | `rxjs-no-deep-operators` | Disallows deep importation from `rxjs/operators`. Deep imports won't be in available in RxJS v6. | None |
 | `rxjs-no-do` | I do without `do` operators. [Do you not?](https://youtu.be/spG-Yj0zEyc) Well, `do` isn't always a code smell, but this rule can be useful as a warning. | None |
 | `rxjs-no-explicit-generics` | Disallows the explicit specification of generic type arguments when calling operators. Rely upon TypeScript's inference instead. | None |
-| `rxjs-no-exposed-subjects` | Disallows exposed subjects. In classes, `Subject` properties and methods that return a `Subject` must be `private`. | None |
+| `rxjs-no-exposed-subjects` | Disallows exposed subjects. In classes, `Subject` properties and methods that return a `Subject` must be `private`. | [See below](#rxjs-no-exposed-subjects) |
 | `rxjs-no-finnish` | Disallows the use of [Finnish notation](https://medium.com/@benlesh/observables-and-finnish-notation-df8356ed1c9b). | None |
 | `rxjs-no-ignored-error` | Disallows the calling of `subscribe` without specifying an error handler. | None |
 | `rxjs-no-ignored-notifier` | Disallows observables not composed from the `repeatWhen` or `retryWhen` notifier. | None |
@@ -172,6 +172,23 @@ For example:
 }
 ```
 
+<a name="rxjs-no-exposed-subjects"></a>
+
+#### rxjs-no-exposed-subjects
+
+The rule has an optional `allowProtected` property that can be specified - it defaults to `false`:
+
+```json
+"rules": {
+  "rxjs-no-sharereplay": {
+    "options": [{
+      "allowProtected": true
+    }],
+    "severity": "error"
+  }
+}
+```
+
 <a name="rxjs-no-sharereplay"></a>
 
 #### rxjs-no-sharereplay

diff --git a/source/rules/rxjsNoExposedSubjectsRule.ts b/source/rules/rxjsNoExposedSubjectsRule.ts
@@ -13,8 +13,14 @@ const defaultAllowedTypesRegExp = /^EventEmitter$/;
 export class Rule extends Lint.Rules.TypedRule {
   public static metadata: Lint.IRuleMetadata = {
     description: "Disallows exposed subjects.",
-    options: null,
-    optionsDescription: "Not configurable.",
+    options: {
+        properties: {
+            allowProtected: { type: "boolean" }
+        },
+        type: "object"
+    },
+    optionsDescription: Lint.Utils.dedent`
+        An optional object with optional \`allowProtected\` property - which defaults to \`false\`.`,
     requiresTypeInfo: true,
     ruleName: "rxjs-no-exposed-subjects",
     type: "functionality",
@@ -32,21 +38,26 @@ export class Rule extends Lint.Rules.TypedRule {
 }
 
 class RxjsNoExposedSubjects extends Lint.ProgramAwareRuleWalker {
+
+  private allowProtected = false;
+
   constructor(
     sourceFile: ts.SourceFile,
     rawOptions: Lint.IOptions,
     program: ts.Program
   ) {
     super(sourceFile, rawOptions, program);
+    const [options] = this.getOptions();
+    if (options) {
+      this.allowProtected = options.allowProtected;
+    }
   }
 
-  // CASE: Properties of classes declared in constructor
   protected visitConstructorDeclaration(node: ts.ConstructorDeclaration): void {
     node.parameters.forEach(param => this.validateNode(param));
     super.visitConstructorDeclaration(node);
   }
 
-  // CASE: Standard properties of Classes.
   protected visitPropertyDeclaration(node: ts.PropertyDeclaration): void {
     this.validateNode(node);
     super.visitPropertyDeclaration(node);
@@ -76,15 +87,25 @@ class RxjsNoExposedSubjects extends Lint.ProgramAwareRuleWalker {
     const { name } = node as any;
     if (name) {
       const text = name.getText();
+      const { allowProtected } = this;
+      const protectedModifier = allowProtected && node.modifiers
+        ? node.modifiers.find(
+            modifier => modifier.kind === ts.SyntaxKind.ProtectedKeyword
+          )
+        : null;
       const privateModifier = node.modifiers
         ? node.modifiers.find(
             modifier => modifier.kind === ts.SyntaxKind.PrivateKeyword
           )
         : null;
       const type = this.getTypeChecker().getTypeAtLocation(typeNode || node);
 
-      if (!privateModifier && couldBeType(type, "Subject") && !couldBeType(type, defaultAllowedTypesRegExp)) {
-        this.addFailureAtNode(name, `Subject '${text}' must be private.`);
+      if (
+        !(protectedModifier || privateModifier) &&
+        couldBeType(type, "Subject") &&
+        !couldBeType(type, defaultAllowedTypesRegExp)
+      ) {
+        this.addFailureAtNode(name, `Subject '${text}' must be private${allowProtected ? " or protected" : ""}.`);
       }
     }
   }

diff --git a/test/v6/fixtures/no-exposed-subjects/protected/fixture.ts.lint b/test/v6/fixtures/no-exposed-subjects/protected/fixture.ts.lint
@@ -20,7 +20,6 @@ class Mock {
     public i: Subject<any>,
            ~                                    [no-exposed % ("i")]
     protected j: Subject<any>,
-              ~                                 [no-exposed % ("j")]
     private k: Subject<any>,
     l: Subject<any>,
     ~                                           [no-exposed % ("l")]