Fix #612

README.md

@@ -152,6 +152,8 @@ A sample configuration file with all options is available [here](https://github.
 * `label-undefined` checks that labels are defined before usage.
 * `max-line-length` sets the maximum length of a line.
 * `member-access` enforces using explicit visibility on class members
+    * `"check-accessor"` enforces explicit visibility on get/set accessors (can only be public)
+    * `"check-constructor"` enforces explicit visibility on constructors (can only be public)
 * `member-ordering` enforces member ordering. Rule options:
     * `public-before-private` All public members must be declared before private members
     * `static-before-instance` All static members must be declared before instance members

src/rules/memberAccessRule.ts

@@ -30,15 +30,44 @@ export class MemberAccessWalker extends Lint.RuleWalker {
         super(sourceFile, options);
     }
 
+    public visitConstructorDeclaration(node: ts.ConstructorDeclaration) {
+        if (this.hasOption("check-constructor")) {
+            // constructor is only allowed to have public or nothing, but the compiler will catch this
+            this.validateVisibilityModifiers(node);
+        }
+
+        super.visitConstructorDeclaration(node);
+    }
+
     public visitMethodDeclaration(node: ts.MethodDeclaration) {
         this.validateVisibilityModifiers(node);
+        super.visitMethodDeclaration(node);
     }
 
     public visitPropertyDeclaration(node: ts.PropertyDeclaration) {
         this.validateVisibilityModifiers(node);
+        super.visitPropertyDeclaration(node);
+    }
+
+    public visitGetAccessor(node: ts.AccessorDeclaration) {
+        if (this.hasOption("check-accessor")) {
+            this.validateVisibilityModifiers(node);
+        }
+        super.visitGetAccessor(node);
+    }
+
+    public visitSetAccessor(node: ts.AccessorDeclaration) {
+        if (this.hasOption("check-accessor")) {
+            this.validateVisibilityModifiers(node);
+        }
+        super.visitSetAccessor(node);
     }
 
     private validateVisibilityModifiers(node: ts.Node) {
+        if (node.parent.kind === ts.SyntaxKind.ObjectLiteralExpression) {
+            return;
+        }
+
         const hasAnyVisibilityModifiers = Lint.hasModifier(
             node.modifiers,
             ts.SyntaxKind.PublicKeyword,

test/files/rules/memberaccess-accessor.test.ts

@@ -0,0 +1,17 @@
+class Members {
+    get g() {
+        return 1;
+    }
+    set s(o: any) {}
+
+    public get publicG() {
+        return 1;
+    }
+    public set publicS(o: any) {}
+}
+
+const obj = {
+    get g() {
+        return 1;
+    }
+};

test/files/rules/memberaccess-constructor.test.ts

@@ -0,0 +1,9 @@
+class ContructorsNoAccess {
+    constructor(i: number);
+    constructor(o: any) {}
+}
+
+class ContructorsAccess {
+    public constructor(i: number);
+    public constructor(o: any) {}
+}

test/files/rules/memberaccess.test.ts

@@ -1,18 +1,32 @@
-class Foo {
-    constructor() {
-    }
+declare class AmbientNoAccess {
+    a(): number;
+}
 
-    public w: number;
-    private x: number;
-    protected y: number;
-    z: number;
+declare class AmbientAccess {
+    public a(): number;
+}
 
-    public barW(): any {
-    }
-    private barX(): any {
-    }
-    protected barY(): any {
-    }
-    barZ(): any {
+class Members {
+    i: number;
+
+    public nPublic: number;
+    protected nProtected: number;
+    private nPrivate: number;
+
+    noAccess(x: number): number;
+    noAccess(o: any): any {}
+
+    public access(x: number): number;
+    public access(o: any): any {}
+}
+
+const obj = {
+    func() {}
+};
+
+function main() {
+    class A {
+        i: number;
+        public n: number;
     }
 }

test/rules/memberAccessTests.ts

@@ -16,14 +16,47 @@
 import * as Lint from "../lint";
 
 describe("<member-access>", () => {
-    it("enforces using explicit visibility on class members", () => {
-        let fileName = "rules/memberaccess.test.ts";
-        let MemberAccessRule = Lint.Test.getRule("member-access");
-        let actualFailures = Lint.Test.applyRuleOnFile(fileName, MemberAccessRule);
-
-        Lint.Test.assertFailuresEqual(actualFailures, [
-            Lint.Test.createFailure(fileName, [8, 5], [8, 15], MemberAccessRule.FAILURE_STRING),
-            Lint.Test.createFailure(fileName, [16, 5], [17, 6], MemberAccessRule.FAILURE_STRING)
-        ]);
+    it("ensures that class properties have access modifiers", () => {
+        const fileName = "rules/memberaccess.test.ts";
+        const expectedFailures = [
+            [[2, 5], [2, 17]],
+            [[10, 5], [10, 15]],
+            [[16, 5], [16, 33]],
+            [[17, 5], [17, 29]],
+            [[29, 9], [29, 19]]
+        ];
+
+        assertFailuresInFile(fileName, expectedFailures);
     });
+
+    it("ensures that constructors have access modifiers", () => {
+        const fileName = "rules/memberaccess-constructor.test.ts";
+        const expectedFailures = [
+            [[2, 5], [2, 28]],
+            [[3, 5], [3, 27]]
+        ];
+        const options = [true, "check-constructor"];
+
+        assertFailuresInFile(fileName, expectedFailures, options);
+    });
+
+    it("ensures that accessors have access modifiers", () => {
+        const fileName = "rules/memberaccess-accessor.test.ts";
+        const expectedFailures = [
+            [[2, 5], [4, 6]],
+            [[5, 5], [5, 21]]
+        ];
+        const options = [true, "check-accessor"];
+
+        assertFailuresInFile(fileName, expectedFailures, options);
+    });
+
+    function assertFailuresInFile(fileName: string, expectedFailures: number[][][], options: any[] = [true]) {
+        const MemberAccessRule = Lint.Test.getRule("member-access");
+        const createFailure = Lint.Test.createFailuresOnFile(fileName, MemberAccessRule.FAILURE_STRING);
+        const expectedFileFailures = expectedFailures.map(failure => createFailure(failure[0], failure[1]));
+
+        const actualFailures = Lint.Test.applyRuleOnFile(fileName, MemberAccessRule, options);
+        Lint.Test.assertFailuresEqual(actualFailures, expectedFileFailures);
+    }
 });