From 31f507f16c2b615bcf9b3a9fb6312d4d4d014f2e Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 19 Nov 2021 12:53:03 -0800 Subject: [PATCH 1/3] Fix formatting edits at end of range --- src/services/formatting/formatting.ts | 91 ++++++++++--------- src/services/formatting/formattingScanner.ts | 5 +- src/services/utilities.ts | 26 +++++- .../reference/api/tsserverlibrary.d.ts | 3 +- tests/baselines/reference/api/typescript.d.ts | 3 +- .../fourslash/completionsOverridingMethod5.ts | 2 +- .../formatSelectionEditAtEndOfRange.ts | 16 ++++ .../cases/fourslash/refactorExtractType12.ts | 2 +- .../cases/fourslash/refactorExtractType13.ts | 2 +- 9 files changed, 97 insertions(+), 53 deletions(-) create mode 100644 tests/cases/fourslash/formatSelectionEditAtEndOfRange.ts diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 1a8542741ea46..3b68b44063e62 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -344,7 +344,7 @@ namespace ts.formatting { } export function formatNodeGivenIndentation(node: Node, sourceFileLike: SourceFileLike, languageVariant: LanguageVariant, initialIndentation: number, delta: number, formatContext: FormatContext): TextChange[] { - const range = { pos: 0, end: sourceFileLike.text.length }; + const range = { pos: node.pos, end: node.end }; return getFormattingScanner(sourceFileLike.text, languageVariant, range.pos, range.end, scanner => formatSpanWorker( range, node, @@ -438,6 +438,25 @@ namespace ts.formatting { } } + if (previousRange! && formattingScanner.getStartPos() >= originalRange.end) { + const token = + formattingScanner.isOnEOF() ? formattingScanner.readEOFTokenRange() : + formattingScanner.isOnToken() ? formattingScanner.readTokenInfo(enclosingNode).token : + undefined; + + if (token) { + processPair( + token, + sourceFile.getLineAndCharacterOfPosition(token.pos).line, + enclosingNode, + previousRange, + previousRangeStartLine!, + previousParent!, + enclosingNode, + /*dynamicIndentation*/ undefined); + } + } + return edits; // local functions @@ -653,29 +672,14 @@ namespace ts.formatting { }); // proceed any tokens in the node that are located after child nodes - while (formattingScanner.isOnToken()) { + while (formattingScanner.isOnToken() && formattingScanner.getStartPos() < originalRange.end) { const tokenInfo = formattingScanner.readTokenInfo(node); - if (tokenInfo.token.end > node.end) { + if (tokenInfo.token.end > Math.min(node.end, originalRange.end)) { break; } consumeTokenAndAdvanceScanner(tokenInfo, node, nodeDynamicIndentation, node); } - if (!node.parent && formattingScanner.isOnEOF()) { - const token = formattingScanner.readEOFTokenRange(); - if (token.end <= node.end && previousRange) { - processPair( - token, - sourceFile.getLineAndCharacterOfPosition(token.pos).line, - node, - previousRange, - previousRangeStartLine, - previousParent, - contextNode, - nodeDynamicIndentation); - } - } - function processChildNode( child: Node, inheritedIndentation: number, @@ -717,9 +721,12 @@ namespace ts.formatting { return inheritedIndentation; } - while (formattingScanner.isOnToken()) { + while (formattingScanner.isOnToken() && formattingScanner.getStartPos() < originalRange.end) { // proceed any parent tokens that are located prior to child.getStart() const tokenInfo = formattingScanner.readTokenInfo(node); + if (tokenInfo.token.end > originalRange.end) { + return inheritedIndentation; + } if (tokenInfo.token.end > childStartPos) { if (tokenInfo.token.pos > childStartPos) { formattingScanner.skipToStartOf(child); @@ -731,7 +738,7 @@ namespace ts.formatting { consumeTokenAndAdvanceScanner(tokenInfo, node, parentDynamicIndentation, node); } - if (!formattingScanner.isOnToken()) { + if (!formattingScanner.isOnToken() || formattingScanner.getStartPos() >= originalRange.end) { return inheritedIndentation; } @@ -773,7 +780,7 @@ namespace ts.formatting { if (listStartToken !== SyntaxKind.Unknown) { // introduce a new indentation scope for lists (including list start and end tokens) - while (formattingScanner.isOnToken()) { + while (formattingScanner.isOnToken() && formattingScanner.getStartPos() < originalRange.end) { const tokenInfo = formattingScanner.readTokenInfo(parent); if (tokenInfo.token.end > nodes.pos) { // stop when formatting scanner moves past the beginning of node list @@ -814,7 +821,7 @@ namespace ts.formatting { } const listEndToken = getCloseTokenForOpenToken(listStartToken); - if (listEndToken !== SyntaxKind.Unknown && formattingScanner.isOnToken()) { + if (listEndToken !== SyntaxKind.Unknown && formattingScanner.isOnToken() && formattingScanner.getStartPos() < originalRange.end) { let tokenInfo: TokenInfo | undefined = formattingScanner.readTokenInfo(parent); if (tokenInfo.token.kind === SyntaxKind.CommaToken && isCallLikeExpression(parent)) { const commaTokenLine = sourceFile.getLineAndCharacterOfPosition(tokenInfo.token.pos).line; @@ -969,7 +976,7 @@ namespace ts.formatting { previousStartLine: number, previousParent: Node, contextNode: Node, - dynamicIndentation: DynamicIndentation): LineAction { + dynamicIndentation: DynamicIndentation | undefined): LineAction { formattingContext.updateContext(previousItem, previousParent, currentItem, currentParent, contextNode); @@ -982,24 +989,26 @@ namespace ts.formatting { // win in a conflict with lower priority rules. forEachRight(rules, rule => { lineAction = applyRuleEdits(rule, previousItem, previousStartLine, currentItem, currentStartLine); - switch (lineAction) { - case LineAction.LineRemoved: - // Handle the case where the next line is moved to be the end of this line. - // In this case we don't indent the next line in the next pass. - if (currentParent.getStart(sourceFile) === currentItem.pos) { - dynamicIndentation.recomputeIndentation(/*lineAddedByFormatting*/ false, contextNode); - } - break; - case LineAction.LineAdded: - // Handle the case where token2 is moved to the new line. - // In this case we indent token2 in the next pass but we set - // sameLineIndent flag to notify the indenter that the indentation is within the line. - if (currentParent.getStart(sourceFile) === currentItem.pos) { - dynamicIndentation.recomputeIndentation(/*lineAddedByFormatting*/ true, contextNode); - } - break; - default: - Debug.assert(lineAction === LineAction.None); + if (dynamicIndentation) { + switch (lineAction) { + case LineAction.LineRemoved: + // Handle the case where the next line is moved to be the end of this line. + // In this case we don't indent the next line in the next pass. + if (currentParent.getStart(sourceFile) === currentItem.pos) { + dynamicIndentation.recomputeIndentation(/*lineAddedByFormatting*/ false, contextNode); + } + break; + case LineAction.LineAdded: + // Handle the case where token2 is moved to the new line. + // In this case we indent token2 in the next pass but we set + // sameLineIndent flag to notify the indenter that the indentation is within the line. + if (currentParent.getStart(sourceFile) === currentItem.pos) { + dynamicIndentation.recomputeIndentation(/*lineAddedByFormatting*/ true, contextNode); + } + break; + default: + Debug.assert(lineAction === LineAction.None); + } } // We need to trim trailing whitespace between the tokens if they were on different lines, and no rule was applied to put them on the same line diff --git a/src/services/formatting/formattingScanner.ts b/src/services/formatting/formattingScanner.ts index b619f98eb2b85..05bbc34041600 100644 --- a/src/services/formatting/formattingScanner.ts +++ b/src/services/formatting/formattingScanner.ts @@ -5,6 +5,7 @@ namespace ts.formatting { export interface FormattingScanner { advance(): void; + getStartPos(): number; isOnToken(): boolean; isOnEOF(): boolean; readTokenInfo(n: Node): TokenInfo; @@ -49,6 +50,7 @@ namespace ts.formatting { lastTrailingTriviaWasNewLine: () => wasNewLine, skipToEndOf, skipToStartOf, + getStartPos: () => lastTokenInfo?.token.pos ?? scanner.getTokenPos(), }); lastTokenInfo = undefined; @@ -265,8 +267,7 @@ namespace ts.formatting { function isOnToken(): boolean { const current = lastTokenInfo ? lastTokenInfo.token.kind : scanner.getToken(); - const startPos = lastTokenInfo ? lastTokenInfo.token.pos : scanner.getStartPos(); - return startPos < endPos && current !== SyntaxKind.EndOfFileToken && !isTrivia(current); + return current !== SyntaxKind.EndOfFileToken && !isTrivia(current); } function isOnEOF(): boolean { diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 30552aa9b9d3b..89979d4cef46a 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -2726,7 +2726,7 @@ namespace ts { return typeIsAccessible ? res : undefined; } - export function syntaxRequiresTrailingCommaOrSemicolonOrASI(kind: SyntaxKind) { + function syntaxRequiresTrailingCommaOrSemicolonOrASI(kind: SyntaxKind) { return kind === SyntaxKind.CallSignature || kind === SyntaxKind.ConstructSignature || kind === SyntaxKind.IndexSignature @@ -2734,7 +2734,7 @@ namespace ts { || kind === SyntaxKind.MethodSignature; } - export function syntaxRequiresTrailingFunctionBlockOrSemicolonOrASI(kind: SyntaxKind) { + function syntaxRequiresTrailingFunctionBlockOrSemicolonOrASI(kind: SyntaxKind) { return kind === SyntaxKind.FunctionDeclaration || kind === SyntaxKind.Constructor || kind === SyntaxKind.MethodDeclaration @@ -2742,7 +2742,7 @@ namespace ts { || kind === SyntaxKind.SetAccessor; } - export function syntaxRequiresTrailingModuleBlockOrSemicolonOrASI(kind: SyntaxKind) { + function syntaxRequiresTrailingModuleBlockOrSemicolonOrASI(kind: SyntaxKind) { return kind === SyntaxKind.ModuleDeclaration; } @@ -2831,13 +2831,29 @@ namespace ts { forEachChild(sourceFile, function visit(node): boolean | undefined { if (syntaxRequiresTrailingSemicolonOrASI(node.kind)) { const lastToken = node.getLastToken(sourceFile); - if (lastToken && lastToken.kind === SyntaxKind.SemicolonToken) { + if (lastToken?.kind === SyntaxKind.SemicolonToken) { withSemicolon++; } else { withoutSemicolon++; } } + else if (syntaxRequiresTrailingCommaOrSemicolonOrASI(node.kind)) { + const lastToken = node.getLastToken(sourceFile); + if (lastToken?.kind === SyntaxKind.SemicolonToken) { + withSemicolon++; + } + else if (lastToken && lastToken.kind !== SyntaxKind.CommaToken) { + const lastTokenLine = getLineAndCharacterOfPosition(sourceFile, lastToken.getStart(sourceFile)).line; + const nextTokenLine = getLineAndCharacterOfPosition(sourceFile, getSpanOfTokenAtPosition(sourceFile, lastToken.end).start).line; + // Avoid counting missing semicolon in single-line objects: + // `function f(p: { x: string /*no semicolon here is insignificant*/ }) {` + if (lastTokenLine !== nextTokenLine) { + withoutSemicolon++; + } + } + } + if (withSemicolon + withoutSemicolon >= nStatementsToObserve) { return true; } @@ -2845,7 +2861,7 @@ namespace ts { return forEachChild(node, visit); }); - // One statement missing a semicolon isn’t sufficient evidence to say the user + // One statement missing a semicolon isn't sufficient evidence to say the user // doesn’t want semicolons, because they may not even be done writing that statement. if (withSemicolon === 0 && withoutSemicolon <= 1) { return true; diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 2ea26a94f1bc6..1fd8966f53047 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -5754,8 +5754,9 @@ declare namespace ts { * @param position A zero-based index of the character where you want the entries * @param options An object describing how the request was triggered and what kinds * of code actions can be returned with the completions. + * @param formattingSettings settings needed for calling formatting functions. */ - getCompletionsAtPosition(fileName: string, position: number, options: GetCompletionsAtPositionOptions | undefined): WithMetadata | undefined; + getCompletionsAtPosition(fileName: string, position: number, options: GetCompletionsAtPositionOptions | undefined, formattingSettings?: FormatCodeSettings): WithMetadata | undefined; /** * Gets the extended details for a completion entry retrieved from `getCompletionsAtPosition`. * diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 022504652a940..21123e9d2829b 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -5754,8 +5754,9 @@ declare namespace ts { * @param position A zero-based index of the character where you want the entries * @param options An object describing how the request was triggered and what kinds * of code actions can be returned with the completions. + * @param formattingSettings settings needed for calling formatting functions. */ - getCompletionsAtPosition(fileName: string, position: number, options: GetCompletionsAtPositionOptions | undefined): WithMetadata | undefined; + getCompletionsAtPosition(fileName: string, position: number, options: GetCompletionsAtPositionOptions | undefined, formattingSettings?: FormatCodeSettings): WithMetadata | undefined; /** * Gets the extended details for a completion entry retrieved from `getCompletionsAtPosition`. * diff --git a/tests/cases/fourslash/completionsOverridingMethod5.ts b/tests/cases/fourslash/completionsOverridingMethod5.ts index 58c03962e47ba..6fa1564180fe7 100644 --- a/tests/cases/fourslash/completionsOverridingMethod5.ts +++ b/tests/cases/fourslash/completionsOverridingMethod5.ts @@ -6,7 +6,7 @@ ////abstract class Ab { //// abstract met(n: string): void; //// met2(n: number): void { -//// +//// return; //// } ////} //// diff --git a/tests/cases/fourslash/formatSelectionEditAtEndOfRange.ts b/tests/cases/fourslash/formatSelectionEditAtEndOfRange.ts new file mode 100644 index 0000000000000..82bd202ec1560 --- /dev/null +++ b/tests/cases/fourslash/formatSelectionEditAtEndOfRange.ts @@ -0,0 +1,16 @@ +/// + + +//// /*1*/interface Base { +//// c(a: string): string +//// c(a: number): number;/*2*/ +//// } + +format.setFormatOptions({ semicolons: ts.SemicolonPreference.Remove }); +format.selection("1", "2"); +verify.currentFileContentIs( +`interface Base { + c(a: string): string + c(a: number): number +}` +); diff --git a/tests/cases/fourslash/refactorExtractType12.ts b/tests/cases/fourslash/refactorExtractType12.ts index 8c4a9e316e4aa..2ae1d914f5be3 100644 --- a/tests/cases/fourslash/refactorExtractType12.ts +++ b/tests/cases/fourslash/refactorExtractType12.ts @@ -11,7 +11,7 @@ edit.applyRefactor({ refactorName: "Extract type", actionName: "Extract to type alias", actionDescription: "Extract to type alias", - newContent: `type /*RENAME*/NewType = string; + newContent: `type /*RENAME*/NewType = string interface A { a: boolean diff --git a/tests/cases/fourslash/refactorExtractType13.ts b/tests/cases/fourslash/refactorExtractType13.ts index e72eadb2dd136..cc6ba779a9149 100644 --- a/tests/cases/fourslash/refactorExtractType13.ts +++ b/tests/cases/fourslash/refactorExtractType13.ts @@ -11,7 +11,7 @@ edit.applyRefactor({ refactorName: "Extract type", actionName: "Extract to type alias", actionDescription: "Extract to type alias", - newContent: `type /*RENAME*/NewType = boolean; + newContent: `type /*RENAME*/NewType = boolean interface A { a: NewType From cdde69e42e6e1f5590362b3a9a7bde3a33af1761 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 19 Nov 2021 13:15:39 -0800 Subject: [PATCH 2/3] Adjust test --- .../fourslash/formatSelectionEditAtEndOfRange.ts | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/tests/cases/fourslash/formatSelectionEditAtEndOfRange.ts b/tests/cases/fourslash/formatSelectionEditAtEndOfRange.ts index 82bd202ec1560..9e60b686ab0b5 100644 --- a/tests/cases/fourslash/formatSelectionEditAtEndOfRange.ts +++ b/tests/cases/fourslash/formatSelectionEditAtEndOfRange.ts @@ -1,16 +1,12 @@ /// -//// /*1*/interface Base { -//// c(a: string): string -//// c(a: number): number;/*2*/ -//// } +//// /*1*/var x = 1;/*2*/ +//// void 0; -format.setFormatOptions({ semicolons: ts.SemicolonPreference.Remove }); +format.setOption("semicolons", "remove"); format.selection("1", "2"); verify.currentFileContentIs( -`interface Base { - c(a: string): string - c(a: number): number -}` +`var x = 1 +void 0;` ); From 8bd3da4671587bc64787859a4ba11b4597aef2d2 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Fri, 19 Nov 2021 14:49:30 -0800 Subject: [PATCH 3/3] Revert API baseline changes --- tests/baselines/reference/api/tsserverlibrary.d.ts | 3 +-- tests/baselines/reference/api/typescript.d.ts | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 1fd8966f53047..2ea26a94f1bc6 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -5754,9 +5754,8 @@ declare namespace ts { * @param position A zero-based index of the character where you want the entries * @param options An object describing how the request was triggered and what kinds * of code actions can be returned with the completions. - * @param formattingSettings settings needed for calling formatting functions. */ - getCompletionsAtPosition(fileName: string, position: number, options: GetCompletionsAtPositionOptions | undefined, formattingSettings?: FormatCodeSettings): WithMetadata | undefined; + getCompletionsAtPosition(fileName: string, position: number, options: GetCompletionsAtPositionOptions | undefined): WithMetadata | undefined; /** * Gets the extended details for a completion entry retrieved from `getCompletionsAtPosition`. * diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index 21123e9d2829b..022504652a940 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -5754,9 +5754,8 @@ declare namespace ts { * @param position A zero-based index of the character where you want the entries * @param options An object describing how the request was triggered and what kinds * of code actions can be returned with the completions. - * @param formattingSettings settings needed for calling formatting functions. */ - getCompletionsAtPosition(fileName: string, position: number, options: GetCompletionsAtPositionOptions | undefined, formattingSettings?: FormatCodeSettings): WithMetadata | undefined; + getCompletionsAtPosition(fileName: string, position: number, options: GetCompletionsAtPositionOptions | undefined): WithMetadata | undefined; /** * Gets the extended details for a completion entry retrieved from `getCompletionsAtPosition`. *