Fix formatting at trailing comma (microsoft#25706)

Andy · web-flow · commit 854462d383c6 · 2018-07-17T15:21:35.000-07:00
diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts
@@ -2307,6 +2307,14 @@ Actual: ${stringify(fullActual)}`);
             }
         }
 
+        public verifyFormatDocumentChangesNothing(): void {
+            const { fileName } = this.activeFile;
+            const before = this.getFileContent(fileName);
+            this.formatDocument();
+            const after = this.getFileContent(fileName);
+            this.assertObjectsEqual(after, before);
+        }
+
         public verifyTextAtCaretIs(text: string) {
             const actual = this.getFileContent(this.activeFile.fileName).substring(this.currentCaretPosition, this.currentCaretPosition + text.length);
             if (actual !== text) {
@@ -4206,6 +4214,10 @@ namespace FourSlashInterface {
             this.state.verifyCurrentFileContent(text);
         }
 
+        public formatDocumentChangesNothing(): void {
+            this.state.verifyFormatDocumentChangesNothing();
+        }
+
         public goToDefinitionIs(endMarkers: ArrayOrSingle<string>) {
             this.state.verifyGoToDefinitionIs(endMarkers);
         }
diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts
@@ -717,7 +717,6 @@ namespace ts.formatting {
                 Debug.assert(isNodeArray(nodes));
 
                 const listStartToken = getOpenTokenForList(parent, nodes);
-                const listEndToken = getCloseTokenForOpenToken(listStartToken);
 
                 let listDynamicIndentation = parentDynamicIndentation;
                 let startLine = parentStartLine;
@@ -752,17 +751,21 @@ namespace ts.formatting {
                     inheritedIndentation = processChildNode(child, inheritedIndentation, node, listDynamicIndentation, startLine, startLine, /*isListItem*/ true, /*isFirstListItem*/ i === 0);
                 }
 
-                if (listEndToken !== SyntaxKind.Unknown) {
-                    if (formattingScanner.isOnToken()) {
-                        const tokenInfo = formattingScanner.readTokenInfo(parent);
-                        // consume the list end token only if it is still belong to the parent
-                        // there might be the case when current token matches end token but does not considered as one
-                        // function (x: function) <--
-                        // without this check close paren will be interpreted as list end token for function expression which is wrong
-                        if (tokenInfo.token.kind === listEndToken && rangeContainsRange(parent, tokenInfo.token)) {
-                            // consume list end token
-                            consumeTokenAndAdvanceScanner(tokenInfo, parent, listDynamicIndentation, parent);
-                        }
+                const listEndToken = getCloseTokenForOpenToken(listStartToken);
+                if (listEndToken !== SyntaxKind.Unknown && formattingScanner.isOnToken()) {
+                    let tokenInfo = formattingScanner.readTokenInfo(parent);
+                    if (tokenInfo.token.kind === SyntaxKind.CommaToken && isCallLikeExpression(parent)) {
+                        formattingScanner.advance();
+                        tokenInfo = formattingScanner.readTokenInfo(parent);
+                    }
+
+                    // consume the list end token only if it is still belong to the parent
+                    // there might be the case when current token matches end token but does not considered as one
+                    // function (x: function) <--
+                    // without this check close paren will be interpreted as list end token for function expression which is wrong
+                    if (tokenInfo.token.kind === listEndToken && rangeContainsRange(parent, tokenInfo.token)) {
+                        // consume list end token
+                        consumeTokenAndAdvanceScanner(tokenInfo, parent, listDynamicIndentation, parent);
                     }
                 }
             }
diff --git a/tests/cases/fourslash/formatTrailingComma.ts b/tests/cases/fourslash/formatTrailingComma.ts
@@ -0,0 +1,8 @@
+/// <reference path='fourslash.ts' />
+
+////foo
+////    .bar(
+////        x,
+////    );
+
+verify.formatDocumentChangesNothing();
diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts
@@ -228,6 +228,7 @@ declare namespace FourSlashInterface {
         eval(expr: string, value: any): void;
         currentLineContentIs(text: string): void;
         currentFileContentIs(text: string): void;
+        formatDocumentChangesNothing(): void;
         /** Verifies that goToDefinition at the current position would take you to `endMarker`. */
         goToDefinitionIs(endMarkers: ArrayOrSingle<string>): void;
         goToDefinitionName(name: string, containerName: string): void;

Original file line number	Diff line number	Diff line change
@@ -2307,6 +2307,14 @@ Actual: ${stringify(fullActual)}`);
`2307`	`2307`	`}`
`2308`	`2308`	`}`
`2309`	`2309`
	`2310`	`+ public verifyFormatDocumentChangesNothing(): void {`
	`2311`	`+ const { fileName } = this.activeFile;`
	`2312`	`+ const before = this.getFileContent(fileName);`
	`2313`	`+ this.formatDocument();`
	`2314`	`+ const after = this.getFileContent(fileName);`
	`2315`	`+ this.assertObjectsEqual(after, before);`
	`2316`	`+ }`
	`2317`	`+`
`2310`	`2318`	`public verifyTextAtCaretIs(text: string) {`
`2311`	`2319`	`const actual = this.getFileContent(this.activeFile.fileName).substring(this.currentCaretPosition, this.currentCaretPosition + text.length);`
`2312`	`2320`	`if (actual !== text) {`
`@@ -4206,6 +4214,10 @@ namespace FourSlashInterface {`
`4206`	`4214`	`this.state.verifyCurrentFileContent(text);`
`4207`	`4215`	`}`
`4208`	`4216`
	`4217`	`+ public formatDocumentChangesNothing(): void {`
	`4218`	`+ this.state.verifyFormatDocumentChangesNothing();`
	`4219`	`+ }`
	`4220`	`+`
`4209`	`4221`	`public goToDefinitionIs(endMarkers: ArrayOrSingle<string>) {`
`4210`	`4222`	`this.state.verifyGoToDefinitionIs(endMarkers);`
`4211`	`4223`	`}`