Narrow error location of member-access (#1964)

Error is reported from the start of the member until the end of its name instead of marking whole function bodies.
palantir · Jan 2, 2017 · 436e4a7 · 436e4a7
1 parent b6e40ea
commit 436e4a7
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 20 deletions.
diff --git a/src/rules/memberAccessRule.ts b/src/rules/memberAccessRule.ts
@@ -92,7 +92,7 @@ export class MemberAccessWalker extends Lint.RuleWalker {
         super.visitSetAccessor(node);
     }
 
-    private validateVisibilityModifiers(node: ts.Node) {
+    private validateVisibilityModifiers(node: ts.ClassElement) {
         if (node.parent!.kind === ts.SyntaxKind.ObjectLiteralExpression) {
             return;
         }
@@ -108,26 +108,34 @@ export class MemberAccessWalker extends Lint.RuleWalker {
             let memberType: string;
             let publicOnly = false;
 
+            let end: number;
             if (node.kind === ts.SyntaxKind.MethodDeclaration) {
                 memberType = "class method";
+                end = (node as ts.MethodDeclaration).name.getEnd();
             } else if (node.kind === ts.SyntaxKind.PropertyDeclaration) {
                 memberType = "class property";
+                end = (node as ts.PropertyDeclaration).name.getEnd();
             } else if (node.kind === ts.SyntaxKind.Constructor) {
                 memberType = "class constructor";
                 publicOnly = true;
+                end = Lint.childOfKind(node, ts.SyntaxKind.ConstructorKeyword)!.getEnd();
             } else if (node.kind === ts.SyntaxKind.GetAccessor) {
                 memberType = "get property accessor";
+                end = (node as ts.AccessorDeclaration).name.getEnd();
             } else if (node.kind === ts.SyntaxKind.SetAccessor) {
                 memberType = "set property accessor";
+                end = (node as ts.AccessorDeclaration).name.getEnd();
             } else {
                 throw new Error("unhandled node type");
             }
 
+            let memberName: string|undefined = undefined;
             // look for the identifier and get its text
-            const member = Lint.childOfKind(node, ts.SyntaxKind.Identifier);
-            const memberName = member && member.getText();
+            if (node.name !== undefined && node.name.kind === ts.SyntaxKind.Identifier) {
+                memberName = (node.name as ts.Identifier).text;
+            }
             const failureString = Rule.FAILURE_STRING_FACTORY(memberType, memberName, publicOnly);
-            this.addFailureAtNode(node, failureString);
+            this.addFailureFromStartToEnd(node.getStart(), end, failureString);
         }
     }
 }
diff --git a/test/rules/member-access/accessor/test.ts.lint b/test/rules/member-access/accessor/test.ts.lint
@@ -1,12 +1,10 @@
 class Members {
     get g() {
-    ~~~~~~~~~
+    ~~~~~ [The get property accessor 'g' must be marked either 'private', 'public', or 'protected']
         return 1;
-~~~~~~~~~~~~~~~~~
     }
-~~~~~ [The get property accessor 'g' must be marked either 'private', 'public', or 'protected']
     set s(o: any) {}
-    ~~~~~~~~~~~~~~~~ [The set property accessor 's' must be marked either 'private', 'public', or 'protected']
+    ~~~~~ [The set property accessor 's' must be marked either 'private', 'public', or 'protected']
 
     public get publicG() {
         return 1;

diff --git a/test/rules/member-access/constructor/test.ts.lint b/test/rules/member-access/constructor/test.ts.lint
@@ -1,8 +1,8 @@
 class ContructorsNoAccess {
     constructor(i: number);
-    ~~~~~~~~~~~~~~~~~~~~~~~ [The class constructor must be marked as 'public']
+    ~~~~~~~~~~~ [The class constructor must be marked as 'public']
     constructor(o: any) {}
-    ~~~~~~~~~~~~~~~~~~~~~~ [The class constructor must be marked as 'public']
+    ~~~~~~~~~~~ [The class constructor must be marked as 'public']
 }
 
 class ContructorsAccess {

diff --git a/test/rules/member-access/default/test.ts.lint b/test/rules/member-access/default/test.ts.lint
@@ -1,9 +1,9 @@
 declare class AmbientNoAccess {
     a(): number;
-    ~~~~~~~~~~~~ [The class method 'a' must be marked either 'private', 'public', or 'protected']
+    ~ [The class method 'a' must be marked either 'private', 'public', or 'protected']
 
     static b(): number;
-    ~~~~~~~~~~~~~~~~~~~ [The class method 'b' must be marked either 'private', 'public', or 'protected']
+    ~~~~~~~~ [The class method 'b' must be marked either 'private', 'public', or 'protected']
 }
 
 declare class AmbientAccess {
@@ -13,9 +13,9 @@ declare class AmbientAccess {
 
 class Members {
     i: number;
-    ~~~~~~~~~~ [The class property 'i' must be marked either 'private', 'public', or 'protected']
+    ~ [The class property 'i' must be marked either 'private', 'public', or 'protected']
     static j: number;
-    ~~~~~~~~~~~~~~~~~ [The class property 'j' must be marked either 'private', 'public', or 'protected']
+    ~~~~~~~~ [The class property 'j' must be marked either 'private', 'public', or 'protected']
 
     public nPublic: number;
     protected nProtected: number;
@@ -26,14 +26,14 @@ class Members {
     private static nsPrivate: number;
 
     noAccess(x: number): number;
-    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [The class method 'noAccess' must be marked either 'private', 'public', or 'protected']
+    ~~~~~~~~ [The class method 'noAccess' must be marked either 'private', 'public', or 'protected']
     noAccess(o: any): any {}
-    ~~~~~~~~~~~~~~~~~~~~~~~~ [The class method 'noAccess' must be marked either 'private', 'public', or 'protected']
+    ~~~~~~~~ [The class method 'noAccess' must be marked either 'private', 'public', or 'protected']
 
     static noAccess(x: number): number;
-    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [The class method 'noAccess' must be marked either 'private', 'public', or 'protected']
+    ~~~~~~~~~~~~~~~ [The class method 'noAccess' must be marked either 'private', 'public', or 'protected']
     static noAccess(o: any): any {}
-    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [The class method 'noAccess' must be marked either 'private', 'public', or 'protected']
+    ~~~~~~~~~~~~~~~ [The class method 'noAccess' must be marked either 'private', 'public', or 'protected']
 
     public access(x: number): number;
     public access(o: any): any {}
@@ -49,9 +49,9 @@ const obj = {
 function main() {
     class A {
         i: number;
-        ~~~~~~~~~~ [The class property 'i' must be marked either 'private', 'public', or 'protected']
+        ~ [The class property 'i' must be marked either 'private', 'public', or 'protected']
         static j: number;
-        ~~~~~~~~~~~~~~~~~ [The class property 'j' must be marked either 'private', 'public', or 'protected']
+        ~~~~~~~~ [The class property 'j' must be marked either 'private', 'public', or 'protected']
         public n: number;
     }
 }