Trailing comma rule now encompasses function decls, exprs and types (p…

…alantir#1486)

* Trailing comma rule now encompasses function decls, exprs and types

* Trailing comma support for ctor, iface, class, type params, methods, set accessor, function calls

-- Untested:
   1. ctor calls (should be handled by callSignature)
   2. set accessor calls (should be handled by callSignature)
   3. tuple/function/ctor/parameterized return types (should be
      handled syntax walker recursion and should hit one of
      vist...[TupleType, FunctionType, ConstructorType, TypeReference, TypeLiteral])
   4. Enums (I think this will fail)

-- Known not working:
   1. comma separated object literal types (object literals can
      be semicolon delineated, so need a good way to only cry
      about trailing commas if they aren't semicolon delineated)

-- Known won't fix:
   1. iface/class extension list (tsc does not support trailing
      commas)

* Handle objects and enums

src/language/walker/syntaxWalker.ts

@@ -86,6 +86,10 @@ export class SyntaxWalker {
         this.walkChildren(node);
     }
 
+    protected visitConstructSignature(node: ts.ConstructSignatureDeclaration) {
+        this.walkChildren(node);
+    }
+
     protected visitConstructorDeclaration(node: ts.ConstructorDeclaration) {
         this.walkChildren(node);
     }
@@ -294,6 +298,10 @@ export class SyntaxWalker {
         this.walkChildren(node);
     }
 
+    protected visitTupleType(node: ts.TupleTypeNode) {
+        this.walkChildren(node);
+    }
+
     protected visitTypeAliasDeclaration(node: ts.TypeAliasDeclaration) {
         this.walkChildren(node);
     }
@@ -392,6 +400,10 @@ export class SyntaxWalker {
                 this.visitConditionalExpression(<ts.ConditionalExpression> node);
                 break;
 
+            case ts.SyntaxKind.ConstructSignature:
+                this.visitConstructSignature(<ts.ConstructSignatureDeclaration> node);
+                break;
+
             case ts.SyntaxKind.Constructor:
                 this.visitConstructorDeclaration(<ts.ConstructorDeclaration> node);
                 break;
@@ -604,6 +616,10 @@ export class SyntaxWalker {
                 this.visitTryStatement(<ts.TryStatement> node);
                 break;
 
+            case ts.SyntaxKind.TupleType:
+                this.visitTupleType(<ts.TupleTypeNode> node);
+                break;
+
             case ts.SyntaxKind.TypeAliasDeclaration:
                 this.visitTypeAliasDeclaration(<ts.TypeAliasDeclaration> node);
                 break;

src/rules/trailingCommaRule.ts

@@ -23,7 +23,9 @@ export class Rule extends Lint.Rules.AbstractRule {
     /* tslint:disable:object-literal-sort-keys */
     public static metadata: Lint.IRuleMetadata = {
         ruleName: "trailing-comma",
-        description: "Requires or disallows trailing commas in array and object literals, destructuring assignments and named imports.",
+        description: Lint.Utils.dedent`
+            Requires or disallows trailing commas in array and object literals, destructuring assignments, function and tuple typings,
+            named imports and function parameters.`,
         optionsDescription: Lint.Utils.dedent`
             One argument which is an object with the keys \`multiline\` and \`singleline\`.
             Both should be set to either \`"always"\` or \`"never"\`.
@@ -33,7 +35,8 @@ export class Rule extends Lint.Rules.AbstractRule {
 
             A array is considered "multiline" if its closing bracket is on a line
             after the last array element. The same general logic is followed for
-            object literals and named import statements.`,
+            object literals, function and tuple typings, named import statements
+            and function parameters.`,
         options: {
             type: "object",
             properties: {
@@ -62,40 +65,181 @@ export class Rule extends Lint.Rules.AbstractRule {
 }
 
 class TrailingCommaWalker extends Lint.RuleWalker {
+    private static SYNTAX_LIST_WRAPPER_TOKENS: [ts.SyntaxKind, ts.SyntaxKind][] = [
+        [ts.SyntaxKind.OpenBraceToken, ts.SyntaxKind.CloseBraceToken],
+        [ts.SyntaxKind.OpenBracketToken, ts.SyntaxKind.CloseBracketToken],
+        [ts.SyntaxKind.OpenParenToken, ts.SyntaxKind.CloseParenToken],
+        [ts.SyntaxKind.LessThanToken, ts.SyntaxKind.GreaterThanToken],
+    ];
+
     public visitArrayLiteralExpression(node: ts.ArrayLiteralExpression) {
-        this.lintNode(node);
+        this.lintChildNodeWithIndex(node, 1);
         super.visitArrayLiteralExpression(node);
     }
 
+    public visitArrowFunction(node: ts.FunctionLikeDeclaration) {
+        this.lintChildNodeWithIndex(node, 1);
+        super.visitArrowFunction(node);
+    }
+
     public visitBindingPattern(node: ts.BindingPattern) {
         if (node.kind === ts.SyntaxKind.ArrayBindingPattern || node.kind === ts.SyntaxKind.ObjectBindingPattern) {
-            this.lintNode(node);
+            this.lintChildNodeWithIndex(node, 1);
         }
         super.visitBindingPattern(node);
     }
 
-    public visitNamedImports(node: ts.NamedImports) {
+    public visitCallExpression(node: ts.CallExpression) {
+        this.lintNode(node);
+        super.visitCallExpression(node);
+    }
+
+    public visitClassDeclaration(node: ts.ClassDeclaration) {
+        this.lintNode(node);
+        super.visitClassDeclaration(node);
+    }
+
+    public visitConstructSignature(node: ts.ConstructSignatureDeclaration) {
+        this.lintNode(node);
+        super.visitConstructSignature(node);
+    }
+
+    public visitConstructorDeclaration(node: ts.ConstructorDeclaration) {
+        this.lintNode(node);
+        super.visitConstructorDeclaration(node);
+    }
+
+    public visitConstructorType(node: ts.FunctionOrConstructorTypeNode) {
+        this.lintNode(node);
+        super.visitConstructorType(node);
+    }
+
+    public visitEnumDeclaration(node: ts.EnumDeclaration) {
+        this.lintNode(node, true);
+        super.visitEnumDeclaration(node);
+    }
+
+    public visitFunctionType(node: ts.FunctionOrConstructorTypeNode) {
+        this.lintChildNodeWithIndex(node, 1);
+        super.visitFunctionType(node);
+    }
+
+    public visitFunctionDeclaration(node: ts.FunctionDeclaration) {
+        this.lintNode(node);
+        super.visitFunctionDeclaration(node);
+    }
+
+    public visitFunctionExpression(node: ts.FunctionExpression) {
+        this.lintNode(node);
+        super.visitFunctionExpression(node);
+    }
+
+    public visitInterfaceDeclaration(node: ts.InterfaceDeclaration) {
+        this.lintNode(node);
+        super.visitInterfaceDeclaration(node);
+    }
+
+    public visitMethodDeclaration(node: ts.MethodDeclaration) {
+        this.lintNode(node);
+        super.visitMethodDeclaration(node);
+    }
+
+    public visitMethodSignature(node: ts.SignatureDeclaration) {
         this.lintNode(node);
+        super.visitMethodSignature(node);
+    }
+
+    public visitNamedImports(node: ts.NamedImports) {
+        this.lintChildNodeWithIndex(node, 1);
         super.visitNamedImports(node);
     }
 
-    public visitObjectLiteralExpression(node: ts.ObjectLiteralExpression) {
+    public visitNewExpression(node: ts.NewExpression) {
         this.lintNode(node);
+        super.visitNewExpression(node);
+    }
+
+    public visitObjectLiteralExpression(node: ts.ObjectLiteralExpression) {
+        this.lintChildNodeWithIndex(node, 1);
         super.visitObjectLiteralExpression(node);
     }
 
-    private lintNode(node: ts.Node) {
-        const child = node.getChildAt(1);
-        if (child != null && child.kind === ts.SyntaxKind.SyntaxList) {
+    public visitSetAccessor(node: ts.AccessorDeclaration) {
+        this.lintNode(node);
+        super.visitSetAccessor(node);
+    }
+
+    public visitTupleType(node: ts.TupleTypeNode) {
+        this.lintChildNodeWithIndex(node, 1);
+        super.visitTupleType(node);
+    }
+
+    public visitTypeLiteral(node: ts.TypeLiteralNode) {
+        this.lintNode(node);
+        // object type literals need to be inspected separately because they
+        // have a different syntax list wrapper token, and they can be semicolon delimited
+        const children = node.getChildren();
+        for (let i = 0; i < children.length - 2; i++) {
+            if (children[i].kind === ts.SyntaxKind.OpenBraceToken &&
+                children[i + 1].kind === ts.SyntaxKind.SyntaxList &&
+                children[i + 2].kind === ts.SyntaxKind.CloseBraceToken) {
+                const grandChildren = children[i + 1].getChildren();
+                // the AST is different from the grammar spec. The semicolons are included as tokens as part of *Signature,
+                // as opposed to optionals alongside it. So instead of children[i + 1] having
+                // [ PropertySignature, Semicolon, PropertySignature, Semicolon ], the AST is
+                // [ PropertySignature, PropertySignature], where the Semicolons are under PropertySignature
+                const hasSemicolon = grandChildren.some(grandChild => {
+                    return grandChild.getChildren().some(ggc => ggc.kind === ts.SyntaxKind.SemicolonToken);
+                });
+
+                if (!hasSemicolon) {
+                    const endLineOfClosingElement = this.getSourceFile().getLineAndCharacterOfPosition(children[i + 2].getEnd()).line;
+                    this.lintChildNodeWithIndex(children[i + 1], grandChildren.length - 1, endLineOfClosingElement);
+                }
+            }
+        }
+        super.visitTypeLiteral(node);
+    }
+
+    public visitTypeReference(node: ts.TypeReferenceNode) {
+        this.lintNode(node);
+        super.visitTypeReference(node);
+    }
+
+    private lintNode(node: ts.Node, includeBraces?: boolean) {
+        const children = node.getChildren();
+        const syntaxListWrapperTokens = (includeBraces === true) ?
+            TrailingCommaWalker.SYNTAX_LIST_WRAPPER_TOKENS : TrailingCommaWalker.SYNTAX_LIST_WRAPPER_TOKENS.slice(1);
+
+        for (let i = 0; i < children.length - 2; i++) {
+            syntaxListWrapperTokens.forEach(([openToken, closeToken]) => {
+                if (children[i].kind === openToken &&
+                    children[i + 1].kind === ts.SyntaxKind.SyntaxList &&
+                    children[i + 2].kind === closeToken) {
+                    this.lintChildNodeWithIndex(node, i + 1);
+                }
+            });
+        }
+    }
+
+    private lintChildNodeWithIndex(node: ts.Node, childNodeIndex: number, endLineOfClosingElement?: number) {
+        const child = node.getChildAt(childNodeIndex);
+        if (child != null) {
             const grandChildren = child.getChildren();
 
             if (grandChildren.length > 0) {
                 const lastGrandChild = grandChildren[grandChildren.length - 1];
                 const hasTrailingComma = lastGrandChild.kind === ts.SyntaxKind.CommaToken;
 
-                const endLineOfNode = this.getSourceFile().getLineAndCharacterOfPosition(node.getEnd()).line;
                 const endLineOfLastElement = this.getSourceFile().getLineAndCharacterOfPosition(lastGrandChild.getEnd()).line;
-                const isMultiline = endLineOfNode !== endLineOfLastElement;
+                if (endLineOfClosingElement === undefined) {
+                    let closingElementNode = node.getChildAt(childNodeIndex + 1);
+                    if (closingElementNode == null) {
+                        closingElementNode = node;
+                    }
+                    endLineOfClosingElement = this.getSourceFile().getLineAndCharacterOfPosition(closingElementNode.getEnd()).line;
+                }
+                const isMultiline = endLineOfClosingElement !== endLineOfLastElement;
                 const option = this.getOption(isMultiline ? "multiline" : "singleline");
 
                 if (hasTrailingComma && option === "never") {