From 9d21a5b56c97727ecbb4f6deb958b366e38ec48b Mon Sep 17 00:00:00 2001
From: Andrew Branch <andrewbranch@users.noreply.github.com>
Date: Fri, 29 Jan 2021 12:07:57 -0800
Subject: [PATCH] Fix indentation of multiline conditional expressions (#42484)

* Fix indentation of multiline conditional expressions

* Add more comment

* Fix comment typo

Co-authored-by: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>

* Fix suggestion formatting

Co-authored-by: Nathan Shively-Sanders <293473+sandersn@users.noreply.github.com>
---
 src/services/formatting/formatting.ts         |  9 ++--
 src/services/formatting/smartIndenter.ts      | 43 +++++++++++++++++++
 .../formatMultilineConditionalExpressions.ts  | 41 ++++++++++++++++++
 3 files changed, 89 insertions(+), 4 deletions(-)
 create mode 100644 tests/cases/fourslash/formatMultilineConditionalExpressions.ts

diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts
index de41f3d768f..f1caec5d564 100644
--- a/src/services/formatting/formatting.ts
+++ b/src/services/formatting/formatting.ts
@@ -498,10 +498,11 @@ namespace ts.formatting {
                     // - we need to get the indentation on last line and the delta of parent
                     return { indentation: indentationOnLastIndentedLine, delta: parentDynamicIndentation.getDelta(node) };
                 }
-                else if (SmartIndenter.childStartsOnTheSameLineWithElseInIfStatement(parent, node, startLine, sourceFile)) {
-                    return { indentation: parentDynamicIndentation.getIndentation(), delta };
-                }
-                else if (SmartIndenter.argumentStartsOnSameLineAsPreviousArgument(parent, node, startLine, sourceFile)) {
+                else if (
+                    SmartIndenter.childStartsOnTheSameLineWithElseInIfStatement(parent, node, startLine, sourceFile) ||
+                    SmartIndenter.childIsUnindentedBranchOfConditionalExpression(parent, node, startLine, sourceFile) ||
+                    SmartIndenter.argumentStartsOnSameLineAsPreviousArgument(parent, node, startLine, sourceFile)
+                ) {
                     return { indentation: parentDynamicIndentation.getIndentation(), delta };
                 }
                 else {
diff --git a/src/services/formatting/smartIndenter.ts b/src/services/formatting/smartIndenter.ts
index 96afecee30d..7bca924dd71 100644
--- a/src/services/formatting/smartIndenter.ts
+++ b/src/services/formatting/smartIndenter.ts
@@ -341,6 +341,49 @@ namespace ts.formatting {
             return false;
         }
 
+        // A multiline conditional typically increases the indentation of its whenTrue and whenFalse children:
+        //
+        // condition
+        //   ? whenTrue
+        //   : whenFalse;
+        //
+        // However, that indentation does not apply if the subexpressions themselves span multiple lines,
+        // applying their own indentation:
+        //
+        // (() => {
+        //   return complexCalculationForCondition();
+        // })() ? {
+        //   whenTrue: 'multiline object literal'
+        // } : (
+        //   whenFalse('multiline parenthesized expression')
+        // );
+        //
+        // In these cases, we must discard the indentation increase that would otherwise be applied to the
+        // whenTrue and whenFalse children to avoid double-indenting their contents. To identify this scenario,
+        // we check for the whenTrue branch beginning on the line that the condition ends, and the whenFalse
+        // branch beginning on the line that the whenTrue branch ends.
+        export function childIsUnindentedBranchOfConditionalExpression(parent: Node, child: TextRangeWithKind, childStartLine: number, sourceFile: SourceFileLike): boolean {
+            if (isConditionalExpression(parent) && (child === parent.whenTrue || child === parent.whenFalse)) {
+                const conditionEndLine = getLineAndCharacterOfPosition(sourceFile, parent.condition.end).line;
+                if (child === parent.whenTrue) {
+                    return childStartLine === conditionEndLine;
+                }
+                else {
+                    // On the whenFalse side, we have to look at the whenTrue side, because if that one was
+                    // indented, whenFalse must also be indented:
+                    //
+                    // const y = true
+                    //   ? 1 : (          L1: whenTrue indented because it's on a new line
+                    //     0              L2: indented two stops, one because whenTrue was indented
+                    //   );                   and one because of the parentheses spanning multiple lines
+                    const trueStartLine = getStartLineAndCharacterForNode(parent.whenTrue, sourceFile).line;
+                    const trueEndLine = getLineAndCharacterOfPosition(sourceFile, parent.whenTrue.end).line;
+                    return conditionEndLine === trueStartLine && trueEndLine === childStartLine;
+                }
+            }
+            return false;
+        }
+
         export function argumentStartsOnSameLineAsPreviousArgument(parent: Node, child: TextRangeWithKind, childStartLine: number, sourceFile: SourceFileLike): boolean {
             if (isCallOrNewExpression(parent)) {
                 if (!parent.arguments) return false;
diff --git a/tests/cases/fourslash/formatMultilineConditionalExpressions.ts b/tests/cases/fourslash/formatMultilineConditionalExpressions.ts
new file mode 100644
index 00000000000..de7b243bd2d
--- /dev/null
+++ b/tests/cases/fourslash/formatMultilineConditionalExpressions.ts
@@ -0,0 +1,41 @@
+/// <reference path="fourslash.ts" />
+
+// @Filename: index.tsx
+//// !!true ? (
+////     <div>a</div>
+//// ) : (
+////     <div>b</div>
+//// )
+////
+//// const y = true
+////     ? 0 : (
+////         1
+////     );
+////
+//// const yy = true
+////     ? 0
+////     : (
+////         1
+////     );
+//// 
+//// const z = true ? (() => {
+////     console.log('hello');
+//// }) : (
+////     1
+//// );
+//// 
+//// const a = true ? {
+////     p: true
+//// } : {
+////     p: false
+//// };
+//// 
+//// const b = {
+////     x: 'x'
+//// } ? (
+////     1
+//// ) : (
+////     0
+//// );
+
+verify.formatDocumentChangesNothing();