Repository: nzakas/eslint-plugin-typescript
Pull request #4 (2016-09-12 03:46:29)

New: Add rule explicit-member-accessibility
Rule based off of TSLint's [member-access rule](http://palantir.github.io/tslint/rules/member-access/)

JamesHenry: Thanks so much @dannyfritz, it's fantastic to have another contributor already!

@nzakas I do not have the right to create a travis job for this repo, please could you do it in the same way we have for typescript-eslint-parser to save us having to manually run the tests for contributor PRs?

A few thoughts on the PR:

- Let's make sure our docs are entirely free-standing, and do not require linking to TSLint documentation. This project is not intended to be a replacement/emulation of TSLint, and whilst it is naturally a fantastic resource for us when designing rules, I don't think the docs should be linked. The implementations, capabilities and naming of the rules in the respective projects may well diverge over time, so it's good to keep them separate.
- Other than removing the link to TSLint, the documentation is currently incomplete
- You've used the term 'decorator' a couple of times when I believe you meant 'declaration'. We need to be careful with using decorator as it naturally has a fixed meaning in the context of TypeScript

Thanks again, Danny!
dannyfritz: * I'll remove the link to TSLint. **However, I'd request an issue with checkboxes for each TSLint rule implemented so far.** Just like how ESLint had a push to do all the rules JSHint does, I think we should have a push for feature parity with TSLint as that is currently the standard.
* I'll spend some time today to get the docs more filled out.
* Yeah, I thought it was a visibility `decorator` when I started, but as I was munging through `ts.SyntaxType`, I was noticed it was not.

---

I have a question for someone more knowledgeable: It doesn't seem `ts` parses the visibility keywords into the AST unless I'm mistaken. I ended up having to grab the sourcecode and do a RegExp test for the keywords on the `classProperty` and `methodDeclaration`. I don't really care for this approach. Does anyone know of a better way to find them using the AST? Or really, where can I find documentation for `ts.languageService`?
dannyfritz: http://eslint.org/docs/rules/indent#compatibility
nzakas: @dannyfritz feel free to open an issue with checkboxes for all the TSLint rules.

wrt to your AST question, if something is missing from the AST, the right place to discuss that is on the `typescript-eslint-parser` repo to see if we can do something about it.
nzakas: @JamesHenry just setup Travis. I'm not sure why you couldn't, but I'll double-check.

And yeah, let's stick with the ESLint conventions for commit messages because that will let us use the same release tool.
dannyfritz: Renamed `Visibility Declaration` to `Visibility Modifier` as per the [TypeScript docs](https://www.typescriptlang.org/docs/handbook/classes.html#public-private-and-protected-modifiers).

---

Followed ESLint's [contributing guidelines](http://eslint.org/docs/developer-guide/contributing/pull-requests).

nzakas: Code looks good overall (I'm fixing Travis right now). I wonder if we can come up with a better rule name, though. `member-access` doesn't seem to indicate what the rule actually does. If it enforces the use of `public` or `private`, maybe a better name would be `class-member-visibility`?
dannyfritz: Renamed to `use-accessibility` and bumped version of `typescript-eslint-parser` to get new accessibility features.
JamesHenry: @dannyfritz @nzakas how do you feel about `explicit-class-member-accessibility` or `explicit-member-accessibility` as the name of the rule?

My reasoning:

- Including `class-member-` / `member-` in the name, because "accessibility" is too generic a term to not include some kind of context
- "explicit-" prefix over "use-", because you are not opting into a behaviour, or making use of a feature of accessibility as such (members are public by default, so adding a "public" modifier simply makes this behaviour explicit
AdamWillden: Just to chime in as an outsider excitedly watching the progress going on here (fantastic work all of you!)... 

I agree with @JamesHenry on the naming. Not precious about which but the shortness of  `explicit-member-accessibility` probably wins it out of the two, for me.
nzakas: Yeah, I like `explicit-member-accessibility` as I think it clearly describes what the rule is doing (at first, I wasn't sure if it was also checking for `public` or `private` elsewhere).

Patch:
diff --git a/README.md b/README.md
index 9f5177d..2132ea2 100644
--- a/README.md
+++ b/README.md
@@ -50,8 +50,4 @@ Then configure the rules you want to use under the rules section.
 ## Supported Rules
 
 * `typescript/type-annotation-spacing` - enforces one space after the colon and zero spaces before the colon of a type annotation.
-
-
-
-
-
+* `typescript/explicit-member-accessibility` - enforces accessibility modifiers on class properties and methods.
diff --git a/docs/rules/explicit-member-accessibility.md b/docs/rules/explicit-member-accessibility.md
new file mode 100644
index 0000000..0144469
--- /dev/null
+++ b/docs/rules/explicit-member-accessibility.md
@@ -0,0 +1,42 @@
+# Enforces accessibility modifiers on class properties and methods (explicit-member-accessibility)
+
+Leaving off accessibility modifier and making everything public can make
+your interface hard to use by others.
+If you make all internal pieces private or protected, your interface will
+be easier to use.
+
+## Rule Details
+
+This rule aims to make code more readable and explicit about who can use
+which properties.
+
+The following patterns are considered warnings:
+
+```ts
+class Animal {
+  name: string // No accessibility modifier
+  getName (): string {} // No accessibility modifier
+}
+```
+
+The following patterns are not warnings:
+
+```ts
+class Animal {
+  private name: string // explicit accessibility modifier
+  public getName (): string {} // explicit accessibility modifier
+}
+```
+
+## When Not To Use It
+
+If you think defaulting to public is a good default, then you will not need
+this rule.
+
+## Further Reading
+
+* TypeScript [Accessibility Modifiers](https://www.typescriptlang.org/docs/handbook/classes.html#public-private-and-protected-modifiers)
+
+## Compatibility
+
+* TSLint: [member-access](http://palantir.github.io/tslint/rules/member-access/)
diff --git a/lib/rules/explicit-member-accessibility.js b/lib/rules/explicit-member-accessibility.js
new file mode 100644
index 0000000..6890f6d
--- /dev/null
+++ b/lib/rules/explicit-member-accessibility.js
@@ -0,0 +1,64 @@
+/**
+ * @fileoverview Enforces explicit accessibility modifier for class members
+ * @author Danny Fritz
+ */
+"use strict";
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = {
+    meta: {
+        docs: {
+            description: "Enforces explicity accessibility modifiers for class members",
+            category: "TypeScript"
+        },
+        schema: []
+    },
+
+    create: function(context) {
+
+        //----------------------------------------------------------------------
+        // Helpers
+        //----------------------------------------------------------------------
+
+        /**
+         * Checks if a method declaration has an accessibility modifier.
+         * @param {ASTNode} methodDefinition The node representing a MethodDefinition.
+         * @returns {void}
+         * @private
+         */
+        function checkMethodAccessibilityModifier(methodDefinition) {
+            if (!methodDefinition.accessibility) {
+                context.report({
+                    node: methodDefinition,
+                    message: "Missing accessibility modifier on method definition " + methodDefinition.key.name + "."
+                });
+            }
+        }
+
+        /**
+         * Checks if property has an accessibility modifier.
+         * @param {ASTNode} classProperty The node representing a ClassProperty.
+         * @returns {void}
+         * @private
+         */
+        function checkPropertyAccessibilityModifier(classProperty) {
+            if (!classProperty.accessibility) {
+                context.report({
+                    node: classProperty,
+                    message: "Missing accessibility modifier on class property " + classProperty.key.name + "."
+                });
+            }
+        }
+
+        //----------------------------------------------------------------------
+        // Public
+        //----------------------------------------------------------------------
+        return {
+            ClassProperty: checkPropertyAccessibilityModifier,
+            MethodDefinition: checkMethodAccessibilityModifier
+        };
+    }
+};
diff --git a/package.json b/package.json
index c3e8e51..b68fac5 100644
--- a/package.json
+++ b/package.json
@@ -21,7 +21,7 @@
     "eslint": "~3.0.0",
     "eslint-config-eslint": "^3.0.0",
     "mocha": "^2.4.5",
-    "typescript-eslint-parser": "~0.1.1"
+    "typescript-eslint-parser": "^0.4.0"
   },
   "engines": {
     "node": ">=4"


Test patch:
diff --git a/tests/lib/rules/explicit-member-accessibility.js b/tests/lib/rules/explicit-member-accessibility.js
new file mode 100644
index 0000000..639e8f7
--- /dev/null
+++ b/tests/lib/rules/explicit-member-accessibility.js
@@ -0,0 +1,70 @@
+/**
+ * @fileoverview Enforces explicit accessibility modifiers for class members
+ * @author Danny Fritz
+ */
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+var rule = require("../../../lib/rules/explicit-member-accessibility"),
+    RuleTester = require("eslint").RuleTester;
+
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+var ruleTester = new RuleTester();
+ruleTester.run("explicit-member-accessibility", rule, {
+
+    valid: [
+        {
+            code: `
+class Test {
+  protected name: string
+  private x: number
+  public getX () {
+    return this.x
+  }
+}
+            `,
+            parser: "typescript-eslint-parser"
+        }
+    ],
+    invalid: [
+        {
+            code: `
+class Test {
+  x: number
+  public getX () {
+    return this.x
+  }
+}
+          `,
+            parser: "typescript-eslint-parser",
+            errors: [{
+                message: "Missing accessibility modifier on class property x.",
+                line: 3,
+                column: 3
+            }]
+        },
+        {
+            code: `
+class Test {
+  private x: number
+  getX () {
+    return this.x
+  }
+}
+          `,
+            parser: "typescript-eslint-parser",
+            errors: [{
+                message: "Missing accessibility modifier on method definition getX.",
+                line: 4,
+                column: 3
+            }]
+        }
+    ]
+});