Merge pull request #16657 from aozgaa/formatOnOpenCurly

Format on open curly

Author: aozgaa

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);
 
-    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;
-        }
+        return precedingToken && precedingToken.kind === expectedTokenKind && end === precedingToken.getEnd() ?
+            precedingToken :
+            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);

src/services/formatting/formattingRequestKind.ts

@@ -7,6 +7,7 @@ namespace ts.formatting {
         FormatSelection,
         FormatOnEnter,
         FormatOnSemicolon,
+        FormatOnOpeningCurlyBrace,
         FormatOnClosingCurlyBrace
     }
 }

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 {

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 === ";") {

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);
     }
 

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\
-}");
+verify.currentFileContentIs(`module TestModule {
+    class TestClass {
+        private foo;
+        public testMethod() { }
+    }
+}`);

tests/cases/fourslash/forceIndentAfterNewLineInsert.ts

@@ -1,18 +1,23 @@
 ///<reference path="fourslash.ts"/>
 
-////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;
+    }
+}`);

tests/cases/fourslash/formatOnEnterOpenBraceAddNewLine.ts

@@ -0,0 +1,24 @@
+/// <reference path="fourslash.ts"/>
+
+//// 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)
+{
+}`);

tests/cases/fourslash/formatOnOpenCurlyBraceRemoveNewLine.ts

@@ -0,0 +1,10 @@
+/// <reference path="fourslash.ts"/>
+
+//// if(true)
+//// /**/ }
+
+format.setOption("PlaceOpenBraceOnNewLineForControlBlocks", false);
+goTo.marker("");
+edit.insert("{");
+verify.currentFileContentIs(
+    `if (true) { }`);

tests/cases/fourslash/formatRemoveNewLineAfterOpenBrace.ts

@@ -0,0 +1,15 @@
+/// <reference path="fourslash.ts"/>
+
+//// function foo()
+//// {
+//// }
+//// if (true)
+//// {
+//// }
+
+format.document();
+verify.currentFileContentIs(
+`function foo() {
+}
+if (true) {
+}`);