Merge pull request microsoft#22849 from aozgaa/dev/aozgaa/cSharpObjLi…

…teralFormatting CSharp Style Object Literal Formatting
dtarc · Mar 30, 2018 · 9dd3ef4 · 9dd3ef4
2 parents e149641 + 8c88ce7
commit 9dd3ef4
Show file tree

Hide file tree

Showing 11 changed files with 206 additions and 12 deletions.
diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts
@@ -328,7 +328,7 @@ namespace ts.formatting {
                 break;
             }
 
-            if (SmartIndenter.shouldIndentChildNode(n, child)) {
+            if (SmartIndenter.shouldIndentChildNode(options, n, child, sourceFile)) {
                 return options.indentSize;
             }
 
@@ -470,7 +470,7 @@ namespace ts.formatting {
             parentDynamicIndentation: DynamicIndentation,
             effectiveParentStartLine: number
         ): { indentation: number, delta: number } {
-            const delta = SmartIndenter.shouldIndentChildNode(node) ? options.indentSize : 0;
+            const delta = SmartIndenter.shouldIndentChildNode(options, node) ? options.indentSize : 0;
 
             if (effectiveParentStartLine === startLine) {
                 // if node is located on the same line with the parent
@@ -514,7 +514,7 @@ namespace ts.formatting {
                     if ((<MethodDeclaration>node).asteriskToken) {
                         return SyntaxKind.AsteriskToken;
                     }
-                    // falls through
+                // falls through
                 case SyntaxKind.PropertyDeclaration:
                 case SyntaxKind.Parameter:
                     return getNameOfDeclaration(<Declaration>node).kind;
@@ -541,9 +541,9 @@ namespace ts.formatting {
                 getIndentation: () => indentation,
                 getDelta,
                 recomputeIndentation: lineAdded => {
-                    if (node.parent && SmartIndenter.shouldIndentChildNode(node.parent, node)) {
+                    if (node.parent && SmartIndenter.shouldIndentChildNode(options, node.parent, node, sourceFile)) {
                         indentation += lineAdded ? options.indentSize : -options.indentSize;
-                        delta = SmartIndenter.shouldIndentChildNode(node) ? options.indentSize : 0;
+                        delta = SmartIndenter.shouldIndentChildNode(options, node) ? options.indentSize : 0;
                     }
                 }
             };
@@ -583,7 +583,7 @@ namespace ts.formatting {
 
             function getDelta(child: TextRangeWithKind) {
                 // Delta value should be zero when the node explicitly prevents indentation of the child node
-                return SmartIndenter.nodeWillIndentChild(node, child, /*indentByDefault*/ true) ? delta : 0;
+                return SmartIndenter.nodeWillIndentChild(options, node, child, sourceFile, /*indentByDefault*/ true) ? delta : 0;
             }
         }
 

diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts
@@ -112,7 +112,7 @@ namespace ts.formatting {
             let previous: Node | undefined;
             let current = precedingToken;
             while (current) {
-                if (positionBelongsToNode(current, position, sourceFile) && shouldIndentChildNode(current, previous, /*isNextChild*/ true)) {
+                if (positionBelongsToNode(current, position, sourceFile) && shouldIndentChildNode(options, current, previous, sourceFile, /*isNextChild*/ true)) {
                     const currentStart = getStartLineAndCharacterForNode(current, sourceFile);
                     const nextTokenKind = nextTokenIsCurlyBraceOnSameLineAsCursor(precedingToken, current, lineAtPosition, sourceFile);
                     const indentationDelta = nextTokenKind !== NextTokenKind.Unknown
@@ -193,7 +193,7 @@ namespace ts.formatting {
                 }
 
                 // increase indentation if parent node wants its content to be indented and parent and child nodes don't start on the same line
-                if (shouldIndentChildNode(parent, current, isNextChild) && !parentAndChildShareLine) {
+                if (shouldIndentChildNode(options, parent, current, sourceFile, isNextChild) && !parentAndChildShareLine) {
                     indentationDelta += options.indentSize;
                 }
 
@@ -531,9 +531,17 @@ namespace ts.formatting {
             return false;
         }
 
-        export function nodeWillIndentChild(parent: TextRangeWithKind, child: TextRangeWithKind | undefined, indentByDefault: boolean): boolean {
+        export function nodeWillIndentChild(settings: FormatCodeSettings | undefined, parent: TextRangeWithKind, child: TextRangeWithKind | undefined, sourceFile: SourceFileLike | undefined, indentByDefault: boolean): boolean {
             const childKind = child ? child.kind : SyntaxKind.Unknown;
+
             switch (parent.kind) {
+                case SyntaxKind.VariableDeclaration:
+                case SyntaxKind.PropertyAssignment:
+                case SyntaxKind.ObjectLiteralExpression:
+                    if (!settings.indentMultiLineObjectLiteralBeginningOnBlankLine && sourceFile && childKind === SyntaxKind.ObjectLiteralExpression) {
+                        return rangeIsOnOneLine(sourceFile, child);
+                    }
+                    break;
                 case SyntaxKind.DoStatement:
                 case SyntaxKind.WhileStatement:
                 case SyntaxKind.ForInStatement:
@@ -585,9 +593,16 @@ namespace ts.formatting {
          * True when the parent node should indent the given child by an explicit rule.
          * @param isNextChild If true, we are judging indent of a hypothetical child *after* this one, not the current child.
          */
-        export function shouldIndentChildNode(parent: TextRangeWithKind, child?: TextRangeWithKind, isNextChild = false): boolean {
-            return (nodeContentIsAlwaysIndented(parent.kind) || nodeWillIndentChild(parent, child, /*indentByDefault*/ false))
+        export function shouldIndentChildNode(settings: FormatCodeSettings | undefined, parent: TextRangeWithKind, child?: Node, sourceFile?: SourceFileLike, isNextChild = false): boolean {
+            return (nodeContentIsAlwaysIndented(parent.kind) || nodeWillIndentChild(settings, parent, child, sourceFile, /*indentByDefault*/ false))
                 && !(isNextChild && child && isControlFlowEndingStatement(child.kind, parent));
         }
+
+        function rangeIsOnOneLine(sourceFile: SourceFileLike, range: TextRangeWithKind) {
+            const rangeStart = skipTrivia(sourceFile.text, range.pos);
+            const startLine = sourceFile.getLineAndCharacterOfPosition(rangeStart).line;
+            const endLine = sourceFile.getLineAndCharacterOfPosition(range.end).line;
+            return startLine === endLine;
+        }
     }
 }
diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts
@@ -654,8 +654,9 @@ namespace ts.textChanges {
                     ? indentation
                     : formatting.SmartIndenter.getIndentation(pos, sourceFile, formatOptions, prefix === newLineCharacter || getLineStartPositionForPosition(pos, sourceFile) === pos);
             if (delta === undefined) {
-                delta = formatting.SmartIndenter.shouldIndentChildNode(nodeIn) ? (formatOptions.indentSize || 0) : 0;
+                delta = formatting.SmartIndenter.shouldIndentChildNode(formatContext.options, nodeIn) ? (formatOptions.indentSize || 0) : 0;
             }
+
             const file: SourceFileLike = { text, getLineAndCharacterOfPosition(pos) { return getLineAndCharacterOfPosition(this, pos); } };
             const changes = formatting.formatNodeGivenIndentation(node, file, sourceFile.languageVariant, initialIndentation, delta, formatContext);
             return applyChanges(text, changes);

diff --git a/src/services/types.ts b/src/services/types.ts
@@ -630,6 +630,7 @@ namespace ts {
         placeOpenBraceOnNewLineForFunctions?: boolean;
         placeOpenBraceOnNewLineForControlBlocks?: boolean;
         insertSpaceBeforeTypeAnnotation?: boolean;
+        indentMultiLineObjectLiteralBeginningOnBlankLine?: boolean;
     }
 
     export interface DefinitionInfo {

diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts
@@ -4407,6 +4407,7 @@ declare namespace ts {
         placeOpenBraceOnNewLineForFunctions?: boolean;
         placeOpenBraceOnNewLineForControlBlocks?: boolean;
         insertSpaceBeforeTypeAnnotation?: boolean;
+        indentMultiLineObjectLiteralBeginningOnBlankLine?: boolean;
     }
     interface DefinitionInfo {
         fileName: string;

diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts
@@ -4660,6 +4660,7 @@ declare namespace ts {
         placeOpenBraceOnNewLineForFunctions?: boolean;
         placeOpenBraceOnNewLineForControlBlocks?: boolean;
         insertSpaceBeforeTypeAnnotation?: boolean;
+        indentMultiLineObjectLiteralBeginningOnBlankLine?: boolean;
     }
     interface DefinitionInfo {
         fileName: string;

diff --git a/tests/cases/fourslash/codeFixClassImplementInterfaceObjectLiteral.ts b/tests/cases/fourslash/codeFixClassImplementInterfaceObjectLiteral.ts
@@ -0,0 +1,23 @@
+/// <reference path='fourslash.ts' />
+
+//// interface IPerson {
+////     coordinate: {
+////         x: number;
+////         y: number;
+////     }
+//// }
+//// class Person implements IPerson { }
+
+verify.codeFix({
+    description: "Implement interface 'IPerson'",
+    newFileContent:
+`interface IPerson {
+    coordinate: {
+        x: number;
+        y: number;
+    }
+}
+class Person implements IPerson {
+    coordinate: { x: number; y: number; };
+}`,
+});
diff --git a/tests/cases/fourslash/extract-method-formatting-objectliteral.ts b/tests/cases/fourslash/extract-method-formatting-objectliteral.ts
@@ -0,0 +1,50 @@
+/// <reference path='fourslash.ts' />
+
+//// 
+//// namespace M {
+////     class C {
+////         foo() {
+////             /*a*/let x = {a:1};
+////             let y = {
+////             b: 2
+////             };
+////             let z =
+////             {
+////             c: 3
+////             };/*b*/
+////             return x.a + y.b + z.c;
+////         }
+////     }
+//// }
+//// 
+
+goTo.select('a', 'b');
+edit.applyRefactor({
+    refactorName: "Extract Symbol",
+    actionName: "function_scope_1",
+    actionDescription: "Extract to method in class 'C'",
+    newContent:
+`
+namespace M {
+    class C {
+        foo() {
+            let { x, y, z } = this./*RENAME*/newMethod();
+            return x.a + y.b + z.c;
+        }
+
+        private newMethod() {
+            let x = { a: 1 };
+            let y = {
+                b: 2
+            };
+            let z = {
+                c: 3
+            };
+            return { x, y, z };
+        }
+    }
+}
+`
+});
+
+
diff --git a/tests/cases/fourslash/formattingObjectLiteralOpenCurlyNewline.ts b/tests/cases/fourslash/formattingObjectLiteralOpenCurlyNewline.ts
@@ -0,0 +1,44 @@
+/// <reference path='fourslash.ts' />
+
+//// 
+//// var clear =
+//// {
+////     outerKey:
+////     {
+////         innerKey: 1,
+////         innerKey2:
+////             2
+////     }
+//// };
+//// 
+
+format.document();
+verify.currentFileContentIs(
+`
+var clear =
+{
+    outerKey:
+    {
+        innerKey: 1,
+        innerKey2:
+            2
+    }
+};
+`
+);
+
+format.setOption("indentMultiLineObjectLiteralBeginningOnBlankLine", true);
+format.document();
+verify.currentFileContentIs(
+`
+var clear =
+    {
+        outerKey:
+            {
+                innerKey: 1,
+                innerKey2:
+                    2
+            }
+    };
+`
+);
diff --git a/tests/cases/fourslash/formattingObjectLiteralOpenCurlyNewlineTyping.ts b/tests/cases/fourslash/formattingObjectLiteralOpenCurlyNewlineTyping.ts
@@ -0,0 +1,36 @@
+/// <reference path='fourslash.ts' />
+
+//// 
+//// var varName =/**/
+//// 
+
+goTo.marker();
+edit.insert("\n{");
+verify.currentFileContentIs(
+`
+var varName =
+    {
+`
+);
+
+edit.insert("\na: 1");
+format.document();
+verify.currentFileContentIs(
+`
+var varName =
+{
+    a: 1
+`
+);
+
+edit.insert("\n};");
+
+format.document();
+verify.currentFileContentIs(
+`
+var varName =
+{
+    a: 1
+};
+`
+);
diff --git a/tests/cases/fourslash/formattingObjectLiteralOpenCurlySingleLine.ts b/tests/cases/fourslash/formattingObjectLiteralOpenCurlySingleLine.ts
@@ -0,0 +1,22 @@
+/// <reference path='fourslash.ts' />
+
+//// 
+//// let obj1 =
+//// { x: 10 };
+//// 
+//// let obj2 =
+////     // leading trivia
+//// { y: 10 };
+//// 
+
+format.document();
+verify.currentFileContentIs(
+`
+let obj1 =
+    { x: 10 };
+
+let obj2 =
+    // leading trivia
+    { y: 10 };
+`
+);