diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 531b768f6d8..c55672229ba 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -97,11 +97,41 @@ namespace ts.formatting { } export function formatOnSemicolon(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { - return formatOutermostParent(position, SyntaxKind.SemicolonToken, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnSemicolon); + const semicolon = findImmediatelyPrecedingTokenOfKind(position, SyntaxKind.SemicolonToken, sourceFile); + return formatNodeLines(findOutermostNodeWithinListLevel(semicolon), sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnSemicolon); + } + + export function formatOnOpeningCurly(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { + const openingCurly = findImmediatelyPrecedingTokenOfKind(position, SyntaxKind.OpenBraceToken, sourceFile); + if (!openingCurly) { + return []; + } + const curlyBraceRange = openingCurly.parent; + const outermostNode = findOutermostNodeWithinListLevel(curlyBraceRange); + + /** + * We limit the span to end at the opening curly to handle the case where + * the brace matched to that just typed will be incorrect after further edits. + * For example, we could type the opening curly for the following method + * body without brace-matching activated: + * ``` + * class C { + * foo() + * } + * ``` + * and we wouldn't want to move the closing brace. + */ + const textRange: TextRange = { + pos: getLineStartPositionForPosition(outermostNode.getStart(sourceFile), sourceFile), + end: position + }; + + return formatSpan(textRange, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnOpeningCurlyBrace); } export function formatOnClosingCurly(position: number, sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { - return formatOutermostParent(position, SyntaxKind.CloseBraceToken, sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnClosingCurlyBrace); + const precedingToken = findImmediatelyPrecedingTokenOfKind(position, SyntaxKind.CloseBraceToken, sourceFile); + return formatNodeLines(findOutermostNodeWithinListLevel(precedingToken), sourceFile, options, rulesProvider, FormattingRequestKind.FormatOnClosingCurlyBrace); } export function formatDocument(sourceFile: SourceFile, rulesProvider: RulesProvider, options: FormatCodeSettings): TextChange[] { @@ -121,44 +151,36 @@ namespace ts.formatting { return formatSpan(span, sourceFile, options, rulesProvider, FormattingRequestKind.FormatSelection); } - function formatOutermostParent(position: number, expectedLastToken: SyntaxKind, sourceFile: SourceFile, options: FormatCodeSettings, rulesProvider: RulesProvider, requestKind: FormattingRequestKind): TextChange[] { - const parent = findOutermostParent(position, expectedLastToken, sourceFile); - if (!parent) { - return []; - } - const span = { - pos: getLineStartPositionForPosition(parent.getStart(sourceFile), sourceFile), - end: parent.end - }; - return formatSpan(span, sourceFile, options, rulesProvider, requestKind); + /** + * Validating `expectedTokenKind` ensures the token was typed in the context we expect (eg: not a comment). + * @param expectedTokenKind The kind of the last token constituting the desired parent node. + */ + function findImmediatelyPrecedingTokenOfKind(end: number, expectedTokenKind: SyntaxKind, sourceFile: SourceFile): Node | undefined { + const precedingToken = findPrecedingToken(end, sourceFile); + + return precedingToken && precedingToken.kind === expectedTokenKind && end === precedingToken.getEnd() ? + precedingToken : + undefined; } - function findOutermostParent(position: number, expectedTokenKind: SyntaxKind, sourceFile: SourceFile): Node { - const precedingToken = findPrecedingToken(position, sourceFile); - - // when it is claimed that trigger character was typed at given position - // we verify that there is a token with a matching kind whose end is equal to position (because the character was just typed). - // If this condition is not hold - then trigger character was typed in some other context, - // i.e.in comment and thus should not trigger autoformatting - if (!precedingToken || - precedingToken.kind !== expectedTokenKind || - position !== precedingToken.getEnd()) { - return undefined; - } - - // walk up and search for the parent node that ends at the same position with precedingToken. - // for cases like this - // - // let x = 1; - // while (true) { - // } - // after typing close curly in while statement we want to reformat just the while statement. - // However if we just walk upwards searching for the parent that has the same end value - - // we'll end up with the whole source file. isListElement allows to stop on the list element level - let current = precedingToken; + /** + * Finds the highest node enclosing `node` at the same list level as `node` + * and whose end does not exceed `node.end`. + * + * Consider typing the following + * ``` + * let x = 1; + * while (true) { + * } + * ``` + * Upon typing the closing curly, we want to format the entire `while`-statement, but not the preceding + * variable declaration. + */ + function findOutermostNodeWithinListLevel(node: Node) { + let current = node; while (current && current.parent && - current.parent.end === precedingToken.end && + current.parent.end === node.end && !isListElement(current.parent, current)) { current = current.parent; } @@ -315,7 +337,7 @@ namespace ts.formatting { } /* @internal */ - export function formatNode(node: Node, sourceFileLike: SourceFileLike, languageVariant: LanguageVariant, initialIndentation: number, delta: number, rulesProvider: RulesProvider): TextChange[] { + export function formatNodeGivenIndentation(node: Node, sourceFileLike: SourceFileLike, languageVariant: LanguageVariant, initialIndentation: number, delta: number, rulesProvider: RulesProvider): TextChange[] { const range = { pos: 0, end: sourceFileLike.text.length }; return formatSpanWorker( range, @@ -330,6 +352,19 @@ namespace ts.formatting { sourceFileLike); } + function formatNodeLines(node: Node, sourceFile: SourceFile, options: FormatCodeSettings, rulesProvider: RulesProvider, requestKind: FormattingRequestKind): TextChange[] { + if (!node) { + return []; + } + + const span = { + pos: getLineStartPositionForPosition(node.getStart(sourceFile), sourceFile), + end: node.end + }; + + return formatSpan(span, sourceFile, options, rulesProvider, requestKind); + } + function formatSpan(originalRange: TextRange, sourceFile: SourceFile, options: FormatCodeSettings, @@ -1096,7 +1131,7 @@ namespace ts.formatting { return; } - // edit should not be applied only if we have one line feed between elements + // edit should not be applied if we have one line feed between elements const lineDelta = currentStartLine - previousStartLine; if (lineDelta !== 1) { recordReplace(previousRange.end, currentRange.pos - previousRange.end, options.newLineCharacter); diff --git a/src/services/formatting/formattingRequestKind.ts b/src/services/formatting/formattingRequestKind.ts index d0397e4a8d9..6c671e1b888 100644 --- a/src/services/formatting/formattingRequestKind.ts +++ b/src/services/formatting/formattingRequestKind.ts @@ -7,6 +7,7 @@ namespace ts.formatting { FormatSelection, FormatOnEnter, FormatOnSemicolon, + FormatOnOpeningCurlyBrace, FormatOnClosingCurlyBrace } } \ No newline at end of file diff --git a/src/services/formatting/rules.ts b/src/services/formatting/rules.ts index 2c1becb8dd1..15bbf5041d3 100644 --- a/src/services/formatting/rules.ts +++ b/src/services/formatting/rules.ts @@ -290,15 +290,15 @@ namespace ts.formatting { // Place a space before open brace in a function declaration this.FunctionOpenBraceLeftTokenRange = Shared.TokenRange.AnyIncludingMultilineComments; - this.SpaceBeforeOpenBraceInFunction = new Rule(RuleDescriptor.create2(this.FunctionOpenBraceLeftTokenRange, SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.IsFunctionDeclContext, Rules.IsBeforeBlockContext, Rules.IsNotFormatOnEnter, Rules.IsSameLineTokenOrBeforeMultilineBlockContext), RuleAction.Space), RuleFlags.CanDeleteNewLines); + this.SpaceBeforeOpenBraceInFunction = new Rule(RuleDescriptor.create2(this.FunctionOpenBraceLeftTokenRange, SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.isOptionDisabledOrUndefinedOrTokensOnSameLine("placeOpenBraceOnNewLineForFunctions"), Rules.IsFunctionDeclContext, Rules.IsBeforeBlockContext, Rules.IsNotFormatOnEnter, Rules.IsSameLineTokenOrBeforeBlockContext), RuleAction.Space), RuleFlags.CanDeleteNewLines); // Place a space before open brace in a TypeScript declaration that has braces as children (class, module, enum, etc) this.TypeScriptOpenBraceLeftTokenRange = Shared.TokenRange.FromTokens([SyntaxKind.Identifier, SyntaxKind.MultiLineCommentTrivia, SyntaxKind.ClassKeyword, SyntaxKind.ExportKeyword, SyntaxKind.ImportKeyword]); - this.SpaceBeforeOpenBraceInTypeScriptDeclWithBlock = new Rule(RuleDescriptor.create2(this.TypeScriptOpenBraceLeftTokenRange, SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.IsTypeScriptDeclWithBlockContext, Rules.IsNotFormatOnEnter, Rules.IsSameLineTokenOrBeforeMultilineBlockContext), RuleAction.Space), RuleFlags.CanDeleteNewLines); + this.SpaceBeforeOpenBraceInTypeScriptDeclWithBlock = new Rule(RuleDescriptor.create2(this.TypeScriptOpenBraceLeftTokenRange, SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.isOptionDisabledOrUndefinedOrTokensOnSameLine("placeOpenBraceOnNewLineForFunctions"), Rules.IsTypeScriptDeclWithBlockContext, Rules.IsNotFormatOnEnter, Rules.IsSameLineTokenOrBeforeBlockContext), RuleAction.Space), RuleFlags.CanDeleteNewLines); // Place a space before open brace in a control flow construct this.ControlOpenBraceLeftTokenRange = Shared.TokenRange.FromTokens([SyntaxKind.CloseParenToken, SyntaxKind.MultiLineCommentTrivia, SyntaxKind.DoKeyword, SyntaxKind.TryKeyword, SyntaxKind.FinallyKeyword, SyntaxKind.ElseKeyword]); - this.SpaceBeforeOpenBraceInControl = new Rule(RuleDescriptor.create2(this.ControlOpenBraceLeftTokenRange, SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.IsControlDeclContext, Rules.IsNotFormatOnEnter, Rules.IsSameLineTokenOrBeforeMultilineBlockContext), RuleAction.Space), RuleFlags.CanDeleteNewLines); + this.SpaceBeforeOpenBraceInControl = new Rule(RuleDescriptor.create2(this.ControlOpenBraceLeftTokenRange, SyntaxKind.OpenBraceToken), RuleOperation.create2(new RuleOperationContext(Rules.isOptionDisabledOrUndefinedOrTokensOnSameLine("placeOpenBraceOnNewLineForControlBlocks"), Rules.IsControlDeclContext, Rules.IsNotFormatOnEnter, Rules.IsSameLineTokenOrBeforeBlockContext), RuleAction.Space), RuleFlags.CanDeleteNewLines); // Insert a space after { and before } in single-line contexts, but remove space from empty object literals {}. this.SpaceAfterOpenBrace = new Rule(RuleDescriptor.create3(SyntaxKind.OpenBraceToken, Shared.TokenRange.Any), RuleOperation.create2(new RuleOperationContext(Rules.IsOptionEnabledOrUndefined("insertSpaceAfterOpeningAndBeforeClosingNonemptyBraces"), Rules.IsBraceWrappedContext), RuleAction.Space)); @@ -585,6 +585,10 @@ namespace ts.formatting { return (context) => !context.options || !context.options.hasOwnProperty(optionName) || !context.options[optionName]; } + static isOptionDisabledOrUndefinedOrTokensOnSameLine(optionName: keyof FormatCodeSettings): (context: FormattingContext) => boolean { + return (context) => !context.options || !context.options.hasOwnProperty(optionName) || !context.options[optionName] || context.TokensAreOnSameLine(); + } + static IsOptionEnabledOrUndefined(optionName: keyof FormatCodeSettings): (context: FormattingContext) => boolean { return (context) => !context.options || !context.options.hasOwnProperty(optionName) || !!context.options[optionName]; } @@ -644,25 +648,8 @@ namespace ts.formatting { return context.contextNode.kind === SyntaxKind.ConditionalExpression; } - static IsSameLineTokenOrBeforeMultilineBlockContext(context: FormattingContext): boolean { - //// This check is mainly used inside SpaceBeforeOpenBraceInControl and SpaceBeforeOpenBraceInFunction. - //// - //// Ex: - //// if (1) { .... - //// * ) and { are on the same line so apply the rule. Here we don't care whether it's same or multi block context - //// - //// Ex: - //// if (1) - //// { ... } - //// * ) and { are on different lines. We only need to format if the block is multiline context. So in this case we don't format. - //// - //// Ex: - //// if (1) - //// { ... - //// } - //// * ) and { are on different lines. We only need to format if the block is multiline context. So in this case we format. - - return context.TokensAreOnSameLine() || Rules.IsBeforeMultilineBlockContext(context); + static IsSameLineTokenOrBeforeBlockContext(context: FormattingContext): boolean { + return context.TokensAreOnSameLine() || Rules.IsBeforeBlockContext(context); } static IsBraceWrappedContext(context: FormattingContext): boolean { diff --git a/src/services/services.ts b/src/services/services.ts index 985107a2172..402baf12eec 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1762,8 +1762,10 @@ namespace ts { function getFormattingEditsAfterKeystroke(fileName: string, position: number, key: string, options: FormatCodeOptions | FormatCodeSettings): TextChange[] { const sourceFile = syntaxTreeCache.getCurrentSourceFile(fileName); const settings = toEditorSettings(options); - - if (key === "}") { + if (key === "{") { + return formatting.formatOnOpeningCurly(position, sourceFile, getRuleProvider(settings), settings); + } + else if (key === "}") { return formatting.formatOnClosingCurly(position, sourceFile, getRuleProvider(settings), settings); } else if (key === ";") { diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 0faccb19b9c..db949ef63cf 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -511,7 +511,7 @@ namespace ts.textChanges { lineMap, getLineAndCharacterOfPosition: pos => computeLineAndCharacterOfPosition(lineMap, pos) }; - const changes = formatting.formatNode(nonFormattedText.node, file, sourceFile.languageVariant, initialIndentation, delta, rulesProvider); + const changes = formatting.formatNodeGivenIndentation(nonFormattedText.node, file, sourceFile.languageVariant, initialIndentation, delta, rulesProvider); return applyChanges(nonFormattedText.text, changes); } diff --git a/tests/cases/fourslash/autoFormattingOnPasting.ts b/tests/cases/fourslash/autoFormattingOnPasting.ts index 0c7f471d43d..5c9e2c86e37 100644 --- a/tests/cases/fourslash/autoFormattingOnPasting.ts +++ b/tests/cases/fourslash/autoFormattingOnPasting.ts @@ -4,11 +4,11 @@ /////**/ ////} goTo.marker(""); -edit.paste(" class TestClass{\r\n\ -private foo;\r\n\ -public testMethod( )\r\n\ -{}\r\n\ -}"); +edit.paste(` class TestClass{ +private foo; +public testMethod( ) +{} +}`); // We're missing scenarios of formatting option settings due to bug 693273 - [TypeScript] Need to improve fourslash support for formatting options. // Missing scenario ** Uncheck Tools->Options->Text Editor->TypeScript->Formatting->General->Format on paste ** //verify.currentFileContentIs("module TestModule {\r\n\ @@ -19,10 +19,9 @@ public testMethod( )\r\n\ //}\r\n\ //}"); // Missing scenario ** Check Tools->Options->Text Editor->TypeScript->Formatting->General->Format on paste ** -verify.currentFileContentIs("module TestModule {\r\n\ - class TestClass {\r\n\ - private foo;\r\n\ - public testMethod()\r\n\ - { }\r\n\ - }\r\n\ -}"); \ No newline at end of file +verify.currentFileContentIs(`module TestModule { + class TestClass { + private foo; + public testMethod() { } + } +}`); \ No newline at end of file diff --git a/tests/cases/fourslash/forceIndentAfterNewLineInsert.ts b/tests/cases/fourslash/forceIndentAfterNewLineInsert.ts index a076213ebb6..d4c41be01ee 100644 --- a/tests/cases/fourslash/forceIndentAfterNewLineInsert.ts +++ b/tests/cases/fourslash/forceIndentAfterNewLineInsert.ts @@ -1,18 +1,23 @@ /// -////function f() +////function f1() ////{ return 0; } +////function f2() +////{ +////return 0; +////} ////function g() ////{ function h() { ////return 0; ////}} format.document(); verify.currentFileContentIs( - "function f()\n" + - "{ return 0; }\n" + - "function g() {\n" + - " function h() {\n" + - " return 0;\n" + - " }\n" + - "}" - ); +`function f1() { return 0; } +function f2() { + return 0; +} +function g() { + function h() { + return 0; + } +}`); diff --git a/tests/cases/fourslash/formatOnEnterOpenBraceAddNewLine.ts b/tests/cases/fourslash/formatOnEnterOpenBraceAddNewLine.ts new file mode 100644 index 00000000000..c5045717df5 --- /dev/null +++ b/tests/cases/fourslash/formatOnEnterOpenBraceAddNewLine.ts @@ -0,0 +1,24 @@ +/// + +//// if(true) {/*0*/} +//// if(false)/*1*/{ +//// } + +format.setOption("PlaceOpenBraceOnNewLineForControlBlocks", true); +goTo.marker("0"); +edit.insertLine(""); +verify.currentFileContentIs( +`if (true) +{ +} +if(false){ +}`); +goTo.marker("1"); +edit.insertLine(""); +verify.currentFileContentIs( +`if (true) +{ +} +if (false) +{ +}`); \ No newline at end of file diff --git a/tests/cases/fourslash/formatOnOpenCurlyBraceRemoveNewLine.ts b/tests/cases/fourslash/formatOnOpenCurlyBraceRemoveNewLine.ts new file mode 100644 index 00000000000..ad6aa4da1db --- /dev/null +++ b/tests/cases/fourslash/formatOnOpenCurlyBraceRemoveNewLine.ts @@ -0,0 +1,10 @@ +/// + +//// if(true) +//// /**/ } + +format.setOption("PlaceOpenBraceOnNewLineForControlBlocks", false); +goTo.marker(""); +edit.insert("{"); +verify.currentFileContentIs( + `if (true) { }`); diff --git a/tests/cases/fourslash/formatRemoveNewLineAfterOpenBrace.ts b/tests/cases/fourslash/formatRemoveNewLineAfterOpenBrace.ts new file mode 100644 index 00000000000..6309ef21805 --- /dev/null +++ b/tests/cases/fourslash/formatRemoveNewLineAfterOpenBrace.ts @@ -0,0 +1,15 @@ +/// + +//// function foo() +//// { +//// } +//// if (true) +//// { +//// } + +format.document(); +verify.currentFileContentIs( +`function foo() { +} +if (true) { +}`); diff --git a/tests/cases/fourslash/formattingOfMultilineBlockConstructs.ts b/tests/cases/fourslash/formattingOfMultilineBlockConstructs.ts index 18f497496d8..fc52df62da3 100644 --- a/tests/cases/fourslash/formattingOfMultilineBlockConstructs.ts +++ b/tests/cases/fourslash/formattingOfMultilineBlockConstructs.ts @@ -47,7 +47,7 @@ verify.currentLineContentIs("enum E {"); goTo.marker('4'); verify.currentLineContentIs("class MyClass {"); goTo.marker('cons'); -verify.currentLineContentIs(" constructor()"); +verify.currentLineContentIs(" constructor() { }"); goTo.marker('5'); verify.currentLineContentIs(" public MyFunction() {"); goTo.marker('6'); diff --git a/tests/cases/fourslash/formattingOnSingleLineBlocks.ts b/tests/cases/fourslash/formattingOnSingleLineBlocks.ts index fc1a6818b95..67990056274 100644 --- a/tests/cases/fourslash/formattingOnSingleLineBlocks.ts +++ b/tests/cases/fourslash/formattingOnSingleLineBlocks.ts @@ -1,16 +1,11 @@ /// -/////*1*/class C -////{}/*2*/ -/////*3*/if (true) -////{}/*4*/ +////class C +////{} +////if (true) +////{} format.document(); -goTo.marker("1"); -verify.currentLineContentIs("class C"); -goTo.marker("2"); -verify.currentLineContentIs("{ }"); -goTo.marker("3"); -verify.currentLineContentIs("if (true)"); -goTo.marker("4"); -verify.currentLineContentIs("{ }"); \ No newline at end of file +verify.currentFileContentIs( +`class C { } +if (true) { }`); \ No newline at end of file diff --git a/tests/cases/fourslash/semicolonFormattingNestedStatements.ts b/tests/cases/fourslash/semicolonFormattingNestedStatements.ts index 20d49a35f47..9e7d0643d08 100644 --- a/tests/cases/fourslash/semicolonFormattingNestedStatements.ts +++ b/tests/cases/fourslash/semicolonFormattingNestedStatements.ts @@ -11,7 +11,7 @@ goTo.marker("innermost"); edit.insert(";"); -// Adding smicolon should format the innermost statement +// Adding semicolon should format the innermost statement verify.currentLineContentIs(' var x = 0;'); // Also should format any parent statement that is terminated by the semicolon