clang-format: support aligned nested conditionals formatting

When multiple ternary operators are chained, e.g. like an if/else-if/
else-if/.../else sequence, clang-format will keep aligning the colon
with the question mark, which increases the indent for each
conditionals:

  int a = condition1 ? result1
                     : condition2 ? result2
                                  : condition3 ? result3
                                               : result4;

This patch detects the situation (e.g. conditionals used in false branch
of another conditional), to avoid indenting in that case:

  int a = condition1   ? result1
          : condition2 ? result2
          : condition3 ? result3
                       : result4;

When BreakBeforeTernaryOperators is false, this will format like this:

  int a = condition1 ? result1 :
          condition2 ? result2 :
          conditino3 ? result3 :
                       result4;

Author: francoisferrand

clang/lib/Format/ContinuationIndenter.cpp

@@ -367,6 +367,12 @@ bool ContinuationIndenter::mustBreak(const LineState &State) {
       State.Stack.back().BreakBeforeParameter && !Current.isTrailingComment() &&
       !Current.isOneOf(tok::r_paren, tok::r_brace))
     return true;
+  if (State.Stack.back().IsChainedConditional &&
+      ((Style.BreakBeforeTernaryOperators && Current.is(TT_ConditionalExpr) &&
+            Current.is(tok::colon)) ||
+       (!Style.BreakBeforeTernaryOperators && Previous.is(TT_ConditionalExpr) &&
+            Previous.is(tok::colon))))
+    return true;
   if (((Previous.is(TT_DictLiteral) && Previous.is(tok::l_brace)) ||
        (Previous.is(TT_ArrayInitializerLSquare) &&
         Previous.ParameterCount > 1) ||
@@ -1022,8 +1028,21 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
   if (State.Stack.back().QuestionColumn != 0 &&
       ((NextNonComment->is(tok::colon) &&
         NextNonComment->is(TT_ConditionalExpr)) ||
-       Previous.is(TT_ConditionalExpr)))
+       Previous.is(TT_ConditionalExpr))) {
+    if (((NextNonComment->is(tok::colon) && NextNonComment->Next &&
+          !NextNonComment->Next->FakeLParens.empty() &&
+          NextNonComment->Next->FakeLParens.back() == prec::Conditional) ||
+         (Previous.is(tok::colon) && !Current.FakeLParens.empty() &&
+          Current.FakeLParens.back() == prec::Conditional)) &&
+        !State.Stack.back().IsWrappedConditional) {
+      //NOTE: we may tweak this slightly:
+      //    * not remove the 'lead' ContinuationIndentWidth
+      //    * always un-indent by the operator when BreakBeforeTernaryOperators=true
+      unsigned Indent = State.Stack.back().Indent - Style.ContinuationIndentWidth;
+      return Indent;
+    }
     return State.Stack.back().QuestionColumn;
+  }
   if (Previous.is(tok::comma) && State.Stack.back().VariablePos != 0)
     return State.Stack.back().VariablePos;
   if ((PreviousNonComment &&
@@ -1144,6 +1163,10 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
   if (Current.is(TT_ArraySubscriptLSquare) &&
       State.Stack.back().StartOfArraySubscripts == 0)
     State.Stack.back().StartOfArraySubscripts = State.Column;
+  if (Current.is(TT_ConditionalExpr) && Current.is(tok::question) &&
+      ((Current.MustBreakBefore) ||
+       (Current.getNextNonComment() && Current.getNextNonComment()->MustBreakBefore)))
+    State.Stack.back().IsWrappedConditional = true;
   if (Style.BreakBeforeTernaryOperators && Current.is(tok::question))
     State.Stack.back().QuestionColumn = State.Column;
   if (!Style.BreakBeforeTernaryOperators && Current.isNot(tok::colon)) {
@@ -1284,6 +1307,8 @@ void ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
     NewParenState.Tok = nullptr;
     NewParenState.ContainsLineBreak = false;
     NewParenState.LastOperatorWrapped = true;
+    NewParenState.IsChainedConditional = false;
+    NewParenState.IsWrappedConditional = false;
     NewParenState.NoLineBreak =
         NewParenState.NoLineBreak || State.Stack.back().NoLineBreakInOperand;
 
@@ -1316,14 +1341,20 @@ void ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
         Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign)
       NewParenState.StartOfFunctionCall = State.Column;
 
-    // Always indent conditional expressions. Never indent expression where
-    // the 'operator' is ',', ';' or an assignment (i.e. *I <=
-    // prec::Assignment) as those have different indentation rules. Indent
-    // other expression, unless the indentation needs to be skipped.
-    if (*I == prec::Conditional ||
-        (!SkipFirstExtraIndent && *I > prec::Assignment &&
-         !Current.isTrailingComment()))
+    // Indent conditional expressions, unless they are chained "else-if"
+    // conditionals. Never indent expression where the 'operator' is ',', ';' or
+    // an assignment (i.e. *I <= prec::Assignment) as those have different
+    // indentation rules. Indent other expression, unless the indentation needs
+    // to be skipped.
+    if (*I == prec::Conditional && Previous && Previous->is(tok::colon) &&
+        Previous->is(TT_ConditionalExpr) && I == Current.FakeLParens.rbegin() &&
+        !State.Stack.back().IsWrappedConditional) {
+      NewParenState.IsChainedConditional = true;
+    } else if (*I == prec::Conditional ||
+               (!SkipFirstExtraIndent && *I > prec::Assignment &&
+                !Current.isTrailingComment())) {
       NewParenState.Indent += Style.ContinuationIndentWidth;
+    }
     if ((Previous && !Previous->opensScope()) || *I != prec::Comma)
       NewParenState.BreakBeforeParameter = false;
     State.Stack.push_back(NewParenState);

clang/lib/Format/ContinuationIndenter.h

@@ -202,14 +202,15 @@ struct ParenState {
   ParenState(const FormatToken *Tok, unsigned Indent, unsigned LastSpace,
              bool AvoidBinPacking, bool NoLineBreak)
       : Tok(Tok), Indent(Indent), LastSpace(LastSpace),
-        NestedBlockIndent(Indent), IsAligned(false),
-        BreakBeforeClosingBrace(false), AvoidBinPacking(AvoidBinPacking),
-        BreakBeforeParameter(false), NoLineBreak(NoLineBreak),
-        NoLineBreakInOperand(false), LastOperatorWrapped(true),
-        ContainsLineBreak(false), ContainsUnwrappedBuilder(false),
-        AlignColons(true), ObjCSelectorNameFound(false),
-        HasMultipleNestedBlocks(false), NestedBlockInlined(false),
-        IsInsideObjCArrayLiteral(false), IsCSharpGenericTypeConstraint(false) {}
+        NestedBlockIndent(Indent), BreakBeforeClosingBrace(false),
+        AvoidBinPacking(AvoidBinPacking), BreakBeforeParameter(false),
+        NoLineBreak(NoLineBreak), NoLineBreakInOperand(false),
+        LastOperatorWrapped(true), ContainsLineBreak(false),
+        ContainsUnwrappedBuilder(false), AlignColons(true),
+        ObjCSelectorNameFound(false), HasMultipleNestedBlocks(false),
+        NestedBlockInlined(false), IsInsideObjCArrayLiteral(false),
+        IsCSharpGenericTypeConstraint(false), IsChainedConditional(false),
+        IsWrappedConditional(false) {}
 
   /// \brief The token opening this parenthesis level, or nullptr if this level
   /// is opened by fake parenthesis.
@@ -335,6 +336,14 @@ struct ParenState {
 
   bool IsCSharpGenericTypeConstraint : 1;
 
+  /// \brief true if the current \c ParenState represents the false branch of
+  /// a chained conditional expression (e.g. else-if)
+  bool IsChainedConditional : 1;
+
+  /// \brief true if there conditionnal was wrapped on the first operator (the
+  /// question mark)
+  bool IsWrappedConditional : 1;
+
   bool operator<(const ParenState &Other) const {
     if (Indent != Other.Indent)
       return Indent < Other.Indent;
@@ -376,6 +385,10 @@ struct ParenState {
       return NestedBlockInlined;
     if (IsCSharpGenericTypeConstraint != Other.IsCSharpGenericTypeConstraint)
       return IsCSharpGenericTypeConstraint;
+    if (IsChainedConditional != Other.IsChainedConditional)
+      return IsChainedConditional;
+    if (IsWrappedConditional != Other.IsWrappedConditional)
+      return IsWrappedConditional;
     return false;
   }
 };

clang/lib/Format/WhitespaceManager.cpp

@@ -95,6 +95,7 @@ const tooling::Replacements &WhitespaceManager::generateReplacements() {
   alignConsecutiveMacros();
   alignConsecutiveDeclarations();
   alignConsecutiveAssignments();
+  alignChainedConditionals();
   alignTrailingComments();
   alignEscapedNewlines();
   generateChanges();
@@ -227,6 +228,32 @@ void WhitespaceManager::calculateLineBreakInformation() {
       LastBlockComment = nullptr;
     }
   }
+
+  // Compute conditional nesting level
+  // Level is increased for each conditional, unless this conditional continues
+  // a chain of conditional, i.e. starts immediately after the colon of another
+  // conditional.
+  SmallVector<bool, 16> ScopeStack;
+  int ConditionalsLevel = 0;
+  for (auto &Change : Changes) {
+    for (unsigned i = 0, e = Change.Tok->FakeLParens.size(); i != e; ++i) {
+      bool isNestedConditional =
+          Change.Tok->FakeLParens[e - 1 - i] == prec::Conditional &&
+          !(i == 0 && Change.Tok->Previous &&
+            Change.Tok->Previous->is(TT_ConditionalExpr) &&
+            Change.Tok->Previous->is(tok::colon));
+      if (isNestedConditional)
+        ++ConditionalsLevel;
+      ScopeStack.push_back(isNestedConditional);
+    }
+
+    Change.ConditionalsLevel = ConditionalsLevel;
+
+    for (unsigned i = Change.Tok->FakeRParens; i > 0 && ScopeStack.size(); --i) {
+      if (ScopeStack.pop_back_val())
+        --ConditionalsLevel;
+    }
+  }
 }
 
 // Align a single sequence of tokens, see AlignTokens below.
@@ -248,6 +275,7 @@ AlignTokenSequence(unsigned Start, unsigned End, unsigned Column, F &&Matches,
   //          double z);
   // In the above example, we need to take special care to ensure that
   // 'double z' is indented along with it's owning function 'b'.
+  // Special handling is required for 'nested' ternary operators.
   SmallVector<unsigned, 16> ScopeStack;
 
   for (unsigned i = Start; i != End; ++i) {
@@ -288,7 +316,10 @@ AlignTokenSequence(unsigned Start, unsigned End, unsigned Column, F &&Matches,
       unsigned ScopeStart = ScopeStack.back();
       if (Changes[ScopeStart - 1].Tok->is(TT_FunctionDeclarationName) ||
           (ScopeStart > Start + 1 &&
-           Changes[ScopeStart - 2].Tok->is(TT_FunctionDeclarationName)))
+           Changes[ScopeStart - 2].Tok->is(TT_FunctionDeclarationName)) ||
+          Changes[i].Tok->is(TT_ConditionalExpr) ||
+          (Changes[i].Tok->Previous &&
+           Changes[i].Tok->Previous->is(TT_ConditionalExpr)))
         Changes[i].Spaces += Shift;
     }
 
@@ -341,7 +372,7 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
   // abort when we hit any token in a higher scope than the starting one.
   auto IndentAndNestingLevel = StartAt < Changes.size()
                                    ? Changes[StartAt].indentAndNestingLevel()
-                                   : std::pair<unsigned, unsigned>(0, 0);
+                                   : std::tuple<unsigned, unsigned, unsigned>();
 
   // Keep track of the number of commas before the matching tokens, we will only
   // align a sequence of matching tokens if they are preceded by the same number
@@ -409,8 +440,8 @@ static unsigned AlignTokens(const FormatStyle &Style, F &&Matches,
       StartOfSequence = i;
 
     unsigned ChangeMinColumn = Changes[i].StartOfTokenColumn;
-    int LineLengthAfter = -Changes[i].Spaces;
-    for (unsigned j = i; j != e && Changes[j].NewlinesBefore == 0; ++j)
+    int LineLengthAfter = Changes[i].TokenLength;
+    for (unsigned j = i + 1; j != e && Changes[j].NewlinesBefore == 0; ++j)
       LineLengthAfter += Changes[j].Spaces + Changes[j].TokenLength;
     unsigned ChangeMaxColumn = Style.ColumnLimit - LineLengthAfter;
 
@@ -608,6 +639,52 @@ void WhitespaceManager::alignConsecutiveDeclarations() {
       Changes, /*StartAt=*/0);
 }
 
+void WhitespaceManager::alignChainedConditionals()
+{
+  if (Style.BreakBeforeTernaryOperators) {
+    AlignTokens(Style,
+                [](Change const &C) {
+                  // Align question operators and last colon
+                  return C.Tok->is(TT_ConditionalExpr) &&
+                         ((C.Tok->is(tok::question) && !C.NewlinesBefore) ||
+                          (C.Tok->is(tok::colon) && C.Tok->Next &&
+                           (C.Tok->Next->FakeLParens.size() == 0 ||
+                            C.Tok->Next->FakeLParens.back() !=
+                                    prec::Conditional)));
+                },
+                Changes, /*StartAt=*/0);
+  } else {
+    static auto AlignWrappedOperand = [](Change const &C) {
+      auto Previous = C.Tok->getPreviousNonComment();//Previous;
+      return C.NewlinesBefore && Previous && Previous->is(TT_ConditionalExpr) &&
+             (Previous->is(tok::question) ||
+              (Previous->is(tok::colon) &&
+               (C.Tok->FakeLParens.size() == 0 ||
+                C.Tok->FakeLParens.back() != prec::Conditional)));
+    };
+    // Ensure we keep alignment of wrapped operands with non-wrapped operands
+    // Since we actually align the operators, the wrapped operands need the
+    // extra offset to be properly aligned.
+    for (Change & C: Changes) {
+      if (AlignWrappedOperand(C))
+        C.StartOfTokenColumn -= 2;
+    }
+    AlignTokens(Style,
+                [this](Change const &C) {
+                  // Align question operators if next operand is not wrapped, as
+                  // well as wrapped operands after question operator or last
+                  // colon in conditional sequence
+                  return (C.Tok->is(TT_ConditionalExpr) &&
+                          C.Tok->is(tok::question) &&
+                          &C != &Changes.back() &&
+                          (&C + 1)->NewlinesBefore == 0 &&
+                          !(&C + 1)->IsTrailingComment) ||
+                         AlignWrappedOperand(C);
+                },
+                Changes, /*StartAt=*/0);
+  }
+}
+
 void WhitespaceManager::alignTrailingComments() {
   unsigned MinColumn = 0;
   unsigned MaxColumn = UINT_MAX;

clang/lib/Format/WhitespaceManager.h

@@ -19,6 +19,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Format/Format.h"
 #include <string>
+#include <tuple>
 
 namespace clang {
 namespace format {
@@ -158,11 +159,16 @@ class WhitespaceManager {
     const Change *StartOfBlockComment;
     int IndentationOffset;
 
-    // A combination of indent level and nesting level, which are used in
-    // tandem to compute lexical scope, for the purposes of deciding
+    // Depth of conditionals. Computed from tracking fake parenthesis, except
+    // it does not increase the indent for "chained" conditionals.
+    int ConditionalsLevel;
+
+    // A combination of indent, nesting and conditionals levels, which are used
+    // in tandem to compute lexical scope, for the purposes of deciding
     // when to stop consecutive alignment runs.
-    std::pair<unsigned, unsigned> indentAndNestingLevel() const {
-      return std::make_pair(Tok->IndentLevel, Tok->NestingLevel);
+    std::tuple<unsigned, unsigned, unsigned> indentAndNestingLevel() const {
+      return std::make_tuple(Tok->IndentLevel, Tok->NestingLevel,
+                             ConditionalsLevel);
     }
   };
 
@@ -181,6 +187,9 @@ class WhitespaceManager {
   /// Align consecutive declarations over all \c Changes.
   void alignConsecutiveDeclarations();
 
+  /// Align consecutive declarations over all \c Changes.
+  void alignChainedConditionals();
+
   /// Align trailing comments over all \c Changes.
   void alignTrailingComments();