From 854462d383c679b6e20137b6a2713112b5492bbc Mon Sep 17 00:00:00 2001 From: Andy Date: Tue, 17 Jul 2018 15:21:35 -0700 Subject: [PATCH] Fix formatting at trailing comma (#25706) --- src/harness/fourslash.ts | 12 +++++++++ src/services/formatting/formatting.ts | 27 +++++++++++--------- tests/cases/fourslash/formatTrailingComma.ts | 8 ++++++ tests/cases/fourslash/fourslash.ts | 1 + 4 files changed, 36 insertions(+), 12 deletions(-) create mode 100644 tests/cases/fourslash/formatTrailingComma.ts diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index b7da07f603d..dcfefdbab26 100644 --- 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) { this.state.verifyGoToDefinitionIs(endMarkers); } diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 82f9f6796bb..c1de1574e88 100644 --- 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 new file mode 100644 index 00000000000..dcdc6412467 --- /dev/null +++ b/tests/cases/fourslash/formatTrailingComma.ts @@ -0,0 +1,8 @@ +/// + +////foo +//// .bar( +//// x, +//// ); + +verify.formatDocumentChangesNothing(); diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 51989a05030..5585b49ee71 100644 --- 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): void; goToDefinitionName(name: string, containerName: string): void;