clang-format: Introduce stricter AlignOperands flag

francoisferrand · Francois Ferrand · commit 3d61b1120e82 · 2020-04-22T17:36:38.000+02:00
Summary: Even when BreakBeforeBinaryOperators is set, AlignOperands kept aligning the beginning of the line, even when it could align the actual operands (e.g. after an assignment). With this patch, there is an option to actually align the operands, so that the operator gets right-aligned with the equal sign or return operator: int aaaaa = bbbbbb + cccccc; return aaaaaaa && bbbbbbb; This not happen in parentheses, to avoid 'breaking' the indentation: if (aaaaa && bbbbb) return; Reviewers: krasimir, djasper Subscribers: cfe-commits, klimek Differential Revision: https://reviews.llvm.org/D32478
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
@@ -270,17 +270,49 @@ the configuration (without a prefix: ``Auto``).
 
 
 
-**AlignOperands** (``bool``)
+**AlignOperands** (``OperandAlignmentStyle``)
   If ``true``, horizontally align operands of binary and ternary
   expressions.
 
-  Specifically, this aligns operands of a single expression that needs to be
-  split over multiple lines, e.g.:
+  Possible values:
+
+  * ``OAS_DontAlign`` (in configuration: ``DontAlign``)
+    Do not align operands of binary and ternary expressions.
+    The wrapped lines are indented ``ContinuationIndentWidth`` spaces from
+    the start of the line.
+
+  * ``OAS_Align`` (in configuration: ``Align``)
+    Horizontally align operands of binary and ternary expressions.
+
+    Specifically, this aligns operands of a single expression that needs
+    to be split over multiple lines, e.g.:
+
+    .. code-block:: c++
+
+      int aaa = bbbbbbbbbbbbbbb +
+                ccccccccccccccc;
+
+    When ``BreakBeforeBinaryOperators`` is set, the wrapped operator is
+    aligned with the operand on the first line.
+
+    .. code-block:: c++
+
+      int aaa = bbbbbbbbbbbbbbb
+                + ccccccccccccccc;
+
+  * ``OAS_AlignAfterOperator`` (in configuration: ``AlignAfterOperator``)
+    Horizontally align operands of binary and ternary expressions.
+
+    This is similar to ``AO_Align``, except when
+    ``BreakBeforeBinaryOperators`` is set, the operator is un-indented so
+    that the wrapped operand is aligned with the operand on the first line.
+
+    .. code-block:: c++
+
+      int aaa = bbbbbbbbbbbbbbb
+              + ccccccccccccccc;
 
-  .. code-block:: c++
 
-    int aaa = bbbbbbbbbbbbbbb +
-              ccccccccccccccc;
 
 **AlignTrailingComments** (``bool``)
   If ``true``, aligns trailing comments.
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
@@ -153,16 +153,43 @@ struct FormatStyle {
   /// Options for aligning backslashes in escaped newlines.
   EscapedNewlineAlignmentStyle AlignEscapedNewlines;
 
+  /// Different styles for aligning operands.
+  enum OperandAlignmentStyle {
+    /// Do not align operands of binary and ternary expressions.
+    /// The wrapped lines are indented ``ContinuationIndentWidth`` spaces from
+    /// the start of the line.
+    OAS_DontAlign,
+    /// Horizontally align operands of binary and ternary expressions.
+    ///
+    /// Specifically, this aligns operands of a single expression that needs
+    /// to be split over multiple lines, e.g.:
+    /// \code
+    ///   int aaa = bbbbbbbbbbbbbbb +
+    ///             ccccccccccccccc;
+    /// \endcode
+    ///
+    /// When ``BreakBeforeBinaryOperators`` is set, the wrapped operator is
+    /// aligned with the operand on the first line.
+    /// \code
+    ///   int aaa = bbbbbbbbbbbbbbb
+    ///             + ccccccccccccccc;
+    /// \endcode
+    OAS_Align,
+    /// Horizontally align operands of binary and ternary expressions.
+    ///
+    /// This is similar to ``AO_Align``, except when
+    /// ``BreakBeforeBinaryOperators`` is set, the operator is un-indented so
+    /// that the wrapped operand is aligned with the operand on the first line.
+    /// \code
+    ///   int aaa = bbbbbbbbbbbbbbb
+    ///           + ccccccccccccccc;
+    /// \endcode
+    OAS_AlignAfterOperator,
+  };
+
   /// If ``true``, horizontally align operands of binary and ternary
   /// expressions.
-  ///
-  /// Specifically, this aligns operands of a single expression that needs to be
-  /// split over multiple lines, e.g.:
-  /// \code
-  ///   int aaa = bbbbbbbbbbbbbbb +
-  ///             ccccccccccccccc;
-  /// \endcode
-  bool AlignOperands;
+  OperandAlignmentStyle AlignOperands;
 
   /// If ``true``, aligns trailing comments.
   /// \code
diff --git a/clang/lib/Format/ContinuationIndenter.cpp b/clang/lib/Format/ContinuationIndenter.cpp
@@ -693,7 +693,9 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState &State, bool DryRun,
     // does not help.
     bool HasTwoOperands =
         P->OperatorIndex == 0 && !P->NextOperator && !P->is(TT_ConditionalExpr);
-    if ((!BreakBeforeOperator && !(HasTwoOperands && Style.AlignOperands)) ||
+    if ((!BreakBeforeOperator &&
+         !(HasTwoOperands &&
+           Style.AlignOperands != FormatStyle::OAS_DontAlign)) ||
         (!State.Stack.back().LastOperatorWrapped && BreakBeforeOperator))
       State.Stack.back().NoLineBreakInOperand = true;
   }
@@ -1039,6 +1041,8 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
       //    * not remove the 'lead' ContinuationIndentWidth
       //    * always un-indent by the operator when BreakBeforeTernaryOperators=true
       unsigned Indent = State.Stack.back().Indent - Style.ContinuationIndentWidth;
+      if (Style.BreakBeforeTernaryOperators && State.Stack.back().UnindentOperator)
+        Indent -= 2;
       return Indent;
     }
     return State.Stack.back().QuestionColumn;
@@ -1118,6 +1122,13 @@ unsigned ContinuationIndenter::getNewLineColumn(const LineState &State) {
     return ContinuationIndent;
   if (Current.is(TT_ProtoExtensionLSquare))
     return State.Stack.back().Indent;
+  if (Current.isBinaryOperator() && State.Stack.back().UnindentOperator)
+    return State.Stack.back().Indent - Current.Tok.getLength() -
+           Current.SpacesRequiredBefore;
+  if (Current.isOneOf(tok::comment, TT_BlockComment, TT_LineComment) &&
+      NextNonComment->isBinaryOperator() && State.Stack.back().UnindentOperator)
+    return State.Stack.back().Indent - NextNonComment->Tok.getLength() -
+           NextNonComment->SpacesRequiredBefore;
   if (State.Stack.back().Indent == State.FirstIndent && PreviousNonComment &&
       !PreviousNonComment->isOneOf(tok::r_brace, TT_CtorInitializerComma))
     // Ensure that we fall back to the continuation indent width instead of
@@ -1297,7 +1308,7 @@ void ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
       (Previous && (Previous->opensScope() ||
                     Previous->isOneOf(tok::semi, tok::kw_return) ||
                     (Previous->getPrecedence() == prec::Assignment &&
-                     Style.AlignOperands) ||
+                     Style.AlignOperands != FormatStyle::OAS_DontAlign) ||
                     Previous->is(TT_ObjCMethodExpr)));
   for (SmallVectorImpl<prec::Level>::const_reverse_iterator
            I = Current.FakeLParens.rbegin(),
@@ -1309,6 +1320,7 @@ void ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
     NewParenState.LastOperatorWrapped = true;
     NewParenState.IsChainedConditional = false;
     NewParenState.IsWrappedConditional = false;
+    NewParenState.UnindentOperator = false;
     NewParenState.NoLineBreak =
         NewParenState.NoLineBreak || State.Stack.back().NoLineBreakInOperand;
 
@@ -1320,14 +1332,26 @@ void ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
     // a builder type call after 'return' or, if the alignment after opening
     // brackets is disabled.
     if (!Current.isTrailingComment() &&
-        (Style.AlignOperands || *I < prec::Assignment) &&
+        (Style.AlignOperands != FormatStyle::OAS_DontAlign ||
+         *I < prec::Assignment) &&
         (!Previous || Previous->isNot(tok::kw_return) ||
          (Style.Language != FormatStyle::LK_Java && *I > 0)) &&
         (Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign ||
-         *I != prec::Comma || Current.NestingLevel == 0))
+         *I != prec::Comma || Current.NestingLevel == 0)) {
       NewParenState.Indent =
           std::max(std::max(State.Column, NewParenState.Indent),
                    State.Stack.back().LastSpace);
+    }
+
+    // If BreakBeforeBinaryOperators is set, un-indent a bit to account for
+    // the operator and keep the operands aligned
+    if (Style.AlignOperands == FormatStyle::OAS_AlignAfterOperator && Previous &&
+        (Previous->getPrecedence() == prec::Assignment ||
+         Previous->is(tok::kw_return) ||
+         (*I == prec::Conditional && Previous->is(tok::question) &&
+          Previous->is(TT_ConditionalExpr))) &&
+        !Newline)
+      NewParenState.UnindentOperator = true;
 
     // Do not indent relative to the fake parentheses inserted for "." or "->".
     // This is a special case to make the following to statements consistent:
@@ -1350,6 +1374,7 @@ void ContinuationIndenter::moveStatePastFakeLParens(LineState &State,
         Previous->is(TT_ConditionalExpr) && I == Current.FakeLParens.rbegin() &&
         !State.Stack.back().IsWrappedConditional) {
       NewParenState.IsChainedConditional = true;
+      NewParenState.UnindentOperator = State.Stack.back().UnindentOperator;
     } else if (*I == prec::Conditional ||
                (!SkipFirstExtraIndent && *I > prec::Assignment &&
                 !Current.isTrailingComment())) {
diff --git a/clang/lib/Format/ContinuationIndenter.h b/clang/lib/Format/ContinuationIndenter.h
@@ -210,7 +210,7 @@ struct ParenState {
         ObjCSelectorNameFound(false), HasMultipleNestedBlocks(false),
         NestedBlockInlined(false), IsInsideObjCArrayLiteral(false),
         IsCSharpGenericTypeConstraint(false), IsChainedConditional(false),
-        IsWrappedConditional(false) {}
+        IsWrappedConditional(false), UnindentOperator(false) {}
 
   /// \brief The token opening this parenthesis level, or nullptr if this level
   /// is opened by fake parenthesis.
@@ -344,6 +344,10 @@ struct ParenState {
   /// question mark)
   bool IsWrappedConditional : 1;
 
+  /// \brief Indicates the indent should be reduced by the length of the
+  /// operator.
+  bool UnindentOperator : 1;
+
   bool operator<(const ParenState &Other) const {
     if (Indent != Other.Indent)
       return Indent < Other.Indent;
@@ -389,6 +393,8 @@ struct ParenState {
       return IsChainedConditional;
     if (IsWrappedConditional != Other.IsWrappedConditional)
       return IsWrappedConditional;
+    if (UnindentOperator != Other.UnindentOperator)
+      return UnindentOperator;
     return false;
   }
 };
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
@@ -308,6 +308,18 @@ struct ScalarEnumerationTraits<FormatStyle::EscapedNewlineAlignmentStyle> {
   }
 };
 
+template <> struct ScalarEnumerationTraits<FormatStyle::OperandAlignmentStyle> {
+  static void enumeration(IO &IO, FormatStyle::OperandAlignmentStyle &Value) {
+    IO.enumCase(Value, "DontAlign", FormatStyle::OAS_DontAlign);
+    IO.enumCase(Value, "Align", FormatStyle::OAS_Align);
+    IO.enumCase(Value, "AlignAfterOperator", FormatStyle::OAS_AlignAfterOperator);
+
+    // For backward compatibility.
+    IO.enumCase(Value, "true", FormatStyle::OAS_Align);
+    IO.enumCase(Value, "false", FormatStyle::OAS_DontAlign);
+  }
+};
+
 template <> struct ScalarEnumerationTraits<FormatStyle::PointerAlignmentStyle> {
   static void enumeration(IO &IO, FormatStyle::PointerAlignmentStyle &Value) {
     IO.enumCase(Value, "Middle", FormatStyle::PAS_Middle);
@@ -744,7 +756,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   LLVMStyle.AccessModifierOffset = -2;
   LLVMStyle.AlignEscapedNewlines = FormatStyle::ENAS_Right;
   LLVMStyle.AlignAfterOpenBracket = FormatStyle::BAS_Align;
-  LLVMStyle.AlignOperands = true;
+  LLVMStyle.AlignOperands = FormatStyle::OAS_Align;
   LLVMStyle.AlignTrailingComments = true;
   LLVMStyle.AlignConsecutiveAssignments = false;
   LLVMStyle.AlignConsecutiveDeclarations = false;
@@ -943,7 +955,7 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
 
   if (Language == FormatStyle::LK_Java) {
     GoogleStyle.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
-    GoogleStyle.AlignOperands = false;
+    GoogleStyle.AlignOperands = FormatStyle::OAS_DontAlign;
     GoogleStyle.AlignTrailingComments = false;
     GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
     GoogleStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
@@ -954,7 +966,7 @@ FormatStyle getGoogleStyle(FormatStyle::LanguageKind Language) {
     GoogleStyle.SpacesBeforeTrailingComments = 1;
   } else if (Language == FormatStyle::LK_JavaScript) {
     GoogleStyle.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
-    GoogleStyle.AlignOperands = false;
+    GoogleStyle.AlignOperands = FormatStyle::OAS_DontAlign;
     GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
     // TODO: still under discussion whether to switch to SLS_All.
     GoogleStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Empty;
@@ -1085,7 +1097,7 @@ FormatStyle getWebKitStyle() {
   FormatStyle Style = getLLVMStyle();
   Style.AccessModifierOffset = -4;
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
-  Style.AlignOperands = false;
+  Style.AlignOperands = FormatStyle::OAS_DontAlign;
   Style.AlignTrailingComments = false;
   Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;
   Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
diff --git a/clang/unittests/Format/FormatTestJS.cpp b/clang/unittests/Format/FormatTestJS.cpp
