gn format: Make parenthesizing better

tools/gn/command_format.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <sstream>
 
 #include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/strings/string_split.h"
 #include "tools/gn/commands.h"
@@ skipping 50 matching lines @@
 const int kPenaltyExcess = 10000;
 const int kPenaltyBrokenLineOnOneLiner = 5000;
 
 enum Precedence {
   kPrecedenceLowest,
   kPrecedenceAssign,
   kPrecedenceOr,
   kPrecedenceAnd,
   kPrecedenceCompare,
   kPrecedenceAdd,
+  kPrecedenceUnary,
   kPrecedenceSuffix,
-  kPrecedenceUnary,
 };
 
 int CountLines(const std::string& str) {
   std::vector<std::string> lines;
   base::SplitStringDontTrim(str, '\n', &lines);
   return static_cast<int>(lines.size());
 }
 
 class Printer {
  public:
@@ skipping 88 matching lines @@
   std::string output_;           // Output buffer.
   std::vector<Token> comments_;  // Pending end-of-line comments.
   int margin() const { return stack_.back().margin; }
 
   int penalty_depth_;
   int GetPenaltyForLineBreak() const {
     return penalty_depth_ * kPenaltyLineBreak;
   }
 
   struct IndentState {
-    IndentState() : margin(0), continuation_requires_indent(false) {}
-    IndentState(int margin, bool continuation_requires_indent)
+    IndentState()
+        : margin(0),
+          continuation_requires_indent(false),
+          parent_is_boolean_or(false) {}
+    IndentState(int margin,
+                bool continuation_requires_indent,
+                bool parent_is_boolean_or)
         : margin(margin),
-          continuation_requires_indent(continuation_requires_indent) {}
+          continuation_requires_indent(continuation_requires_indent),
+          parent_is_boolean_or(parent_is_boolean_or) {}
 
     // The left margin (number of spaces).
     int margin;
 
     bool continuation_requires_indent;
+
+    bool parent_is_boolean_or;
   };
   // Stack used to track
   std::vector<IndentState> stack_;
 
   // Gives the precedence for operators in a BinaryOpNode.
   std::map<base::StringPiece, Precedence> precedence_;
 
   DISALLOW_COPY_AND_ASSIGN(Printer);
 };
 
@@ skipping 32 matching lines @@
   TrimWhitespaceASCII(token.value().as_string(), base::TRIM_ALL, &trimmed);
   Print(trimmed);
 }
 
 void Printer::Newline() {
   if (!comments_.empty()) {
     Print("  ");
     // Save the margin, and temporarily set it to where the first comment
     // starts so that multiple suffix comments are vertically aligned. This
     // will need to be fancier once we enforce 80 col.
-    stack_.push_back(IndentState(CurrentColumn(), false));
+    stack_.push_back(IndentState(CurrentColumn(), false, false));
     int i = 0;
     for (const auto& c : comments_) {
       if (i > 0) {
         Trim();
         Print("\n");
         PrintMargin();
       }
       TrimAndPrintToken(c);
       ++i;
     }
@@ skipping 153 matching lines @@
       Print(".");
       Expr(accessor->member(), kPrecedenceLowest, std::string());
     } else {
       CHECK(accessor->index());
       Print("[");
       Expr(accessor->index(), kPrecedenceLowest, "]");
     }
   } else if (const BinaryOpNode* binop = root->AsBinaryOp()) {
     CHECK(precedence_.find(binop->op().value()) != precedence_.end());
     AnnotatePreferedMultilineAssignment(binop);
+
     Precedence prec = precedence_[binop->op().value()];
-    AddParen(prec, outer_prec, &parenthesized);
+
+    // Since binary operators format left-to-right, it is ok for the left side
+    // use the same operator without parentheses, so the left uses prec. For the
+    // same reason, the right side cannot reuse the same operator, or else "x +
+    // (y + z)" would format as "x + y + z" which means "(x + y) + z". So, treat
+    // the right expression as appearing one precedence level higher.
+    // However, because the source parens are not in the parse tree, as a
+    // special case for && and || we insert strictly-redundant-but-helpful-for-
+    // human-readers parentheses.
+    int prec_left = prec;
+    int prec_right = prec + 1;
+    if (binop->op().value() == "&&" && stack_.back().parent_is_boolean_or) {
+      Print("(");
+      parenthesized = true;
+    } else {
+      AddParen(prec_left, outer_prec, &parenthesized);
+    }
+
     int start_line = CurrentLine();
     int start_column = CurrentColumn();
     bool is_assignment = binop->op().value() == "=" ||
                          binop->op().value() == "+=" ||
                          binop->op().value() == "-=";
     // A sort of funny special case for the long lists that are common in .gn
     // files, don't indent them + 4, even though they're just continuations when
     // they're simple lists like "x = [ a, b, c, ... ]"
     const ListNode* right_as_list = binop->right()->AsList();
     int indent_column =
         (is_assignment &&
          (!right_as_list || (!right_as_list->prefer_multiline() &&
                              !ListWillBeMultiline(right_as_list->contents(),
                                                   right_as_list->End()))))
             ? margin() + kIndentSize * 2
             : start_column;
     if (stack_.back().continuation_requires_indent)
       indent_column += kIndentSize * 2;
 
-    stack_.push_back(IndentState(indent_column, false));
+    stack_.push_back(
+        IndentState(indent_column, false, binop->op().value() == "||"));
     Printer sub_left;
     InitializeSub(&sub_left);
     sub_left.Expr(binop->left(),
-                  prec,
+                  prec_left,
                   std::string(" ") + binop->op().value().as_string());
     bool left_is_multiline = CountLines(sub_left.String()) > 1;
     // Avoid walking the whole left redundantly times (see timing of Format.046)
     // so pull the output and comments from subprinter.
     Print(sub_left.String().substr(start_column));
     std::copy(sub_left.comments_.begin(),
               sub_left.comments_.end(),
               std::back_inserter(comments_));
 
     // Single line.
     Printer sub1;
     InitializeSub(&sub1);
     sub1.Print(" ");
     int penalty_current_line =
-        sub1.Expr(binop->right(), prec + 1, std::string());
+        sub1.Expr(binop->right(), prec_right, std::string());
     sub1.Print(suffix);
     penalty_current_line += AssessPenalty(sub1.String());
     if (!is_assignment && left_is_multiline) {
       // In e.g. xxx + yyy, if xxx is already multiline, then we want a penalty
       // for trying to continue as if this were one line.
       penalty_current_line +=
           (CountLines(sub1.String()) - 1) * kPenaltyBrokenLineOnOneLiner;
     }
 
     // Break after operator.
     Printer sub2;
     InitializeSub(&sub2);
     sub2.Newline();
-    int penalty_next_line = sub2.Expr(binop->right(), prec + 1, std::string());
+    int penalty_next_line =
+        sub2.Expr(binop->right(), prec_right, std::string());
     sub2.Print(suffix);
     penalty_next_line += AssessPenalty(sub2.String());
 
     if (penalty_current_line < penalty_next_line) {
       Print(" ");
-      Expr(binop->right(), prec + 1, std::string());
+      Expr(binop->right(), prec_right, std::string());
     } else {
       // Otherwise, put first argument and op, and indent next.
       Newline();
       penalty += std::abs(CurrentColumn() - start_column) *
                  kPenaltyHorizontalSeparation;
-      Expr(binop->right(), prec + 1, std::string());
+      Expr(binop->right(), prec_right, std::string());
     }
     stack_.pop_back();
     penalty += (CurrentLine() - start_line) * GetPenaltyForLineBreak();
   } else if (const BlockNode* block = root->AsBlock()) {
     Sequence(
         kSequenceStyleBracedBlock, block->statements(), block->End(), false);
   } else if (const ConditionNode* condition = root->AsConditionNode()) {
     Print("if (");
     // TODO(scottmg): The { needs to be included in the suffix here.
     Expr(condition->condition(), kPrecedenceLowest, ") ");
@@ skipping 70 matching lines @@
   force_multiline |= ListWillBeMultiline(list, end);
 
   if (list.size() == 0 && !force_multiline) {
     // No elements, and not forcing newlines, print nothing.
   } else if (list.size() == 1 && !force_multiline) {
     Print(" ");
     Expr(list[0], kPrecedenceLowest, std::string());
     CHECK(!list[0]->comments() || list[0]->comments()->after().empty());
     Print(" ");
   } else {
-    stack_.push_back(
-        IndentState(margin() + kIndentSize, style == kSequenceStyleList));
+    stack_.push_back(IndentState(margin() + kIndentSize,
+                                 style == kSequenceStyleList,
+                                 false));
     size_t i = 0;
     for (const auto& x : list) {
       Newline();
       // If:
       // - we're going to output some comments, and;
       // - we haven't just started this multiline list, and;
       // - there isn't already a blank line here;
       // Then: insert one.
       if (i != 0 && x->comments() && !x->comments()->before().empty() &&
           !HaveBlankLine()) {
@@ skipping 68 matching lines @@
   // 2. starting on same line, broken at each comma but paren aligned;
   // 3. broken to next line + 4, broken at each comma.
   std::string terminator = ")";
   if (have_block)
     terminator += " {";
   terminator += suffix;
 
   // 1: Same line.
   Printer sub1;
   InitializeSub(&sub1);
-  sub1.stack_.push_back(IndentState(CurrentColumn(), true));
+  sub1.stack_.push_back(IndentState(CurrentColumn(), true, false));
   int penalty_one_line = 0;
   for (size_t i = 0; i < list.size(); ++i) {
     penalty_one_line += sub1.Expr(list[i], kPrecedenceLowest,
                                   i < list.size() - 1 ? ", " : std::string());
   }
   sub1.Print(terminator);
   penalty_one_line += AssessPenalty(sub1.String());
   // This extra penalty prevents a short second argument from being squeezed in
   // after a first argument that went multiline (and instead preferring a
   // variant below).
   penalty_one_line +=
       (CountLines(sub1.String()) - 1) * kPenaltyBrokenLineOnOneLiner;
 
   // 2: Starting on same line, broken at commas.
   Printer sub2;
   InitializeSub(&sub2);
-  sub2.stack_.push_back(IndentState(CurrentColumn(), true));
+  sub2.stack_.push_back(IndentState(CurrentColumn(), true, false));
   int penalty_multiline_start_same_line = 0;
   for (size_t i = 0; i < list.size(); ++i) {
     penalty_multiline_start_same_line += sub2.Expr(
         list[i], kPrecedenceLowest, i < list.size() - 1 ? "," : std::string());
     if (i < list.size() - 1) {
       sub2.Newline();
     }
   }
   sub2.Print(terminator);
   penalty_multiline_start_same_line += AssessPenalty(sub2.String());
 
   // 3: Starting on next line, broken at commas.
   Printer sub3;
   InitializeSub(&sub3);
-  sub3.stack_.push_back(IndentState(margin() + kIndentSize * 2, true));
+  sub3.stack_.push_back(IndentState(margin() + kIndentSize * 2, true, false));
   sub3.Newline();
   int penalty_multiline_start_next_line = 0;
   for (size_t i = 0; i < list.size(); ++i) {
     if (i == 0) {
       penalty_multiline_start_next_line +=
           std::abs(sub3.CurrentColumn() - start_column) *
           kPenaltyHorizontalSeparation;
     }
     penalty_multiline_start_next_line += sub3.Expr(
         list[i], kPrecedenceLowest, i < list.size() - 1 ? "," : std::string());
@@ skipping 15 matching lines @@
       force_multiline = true;
     }
   } else {
     force_multiline = true;
   }
 
   if (list.size() == 0 && !force_multiline) {
     // No elements, and not forcing newlines, print nothing.
   } else {
     if (penalty_multiline_start_next_line < penalty_multiline_start_same_line) {
-      stack_.push_back(IndentState(margin() + kIndentSize * 2, true));
+      stack_.push_back(IndentState(margin() + kIndentSize * 2, true, false));
       Newline();
     } else {
-      stack_.push_back(IndentState(CurrentColumn(), true));
+      stack_.push_back(IndentState(CurrentColumn(), true, false));
     }
 
     for (size_t i = 0; i < list.size(); ++i) {
       const auto& x = list[i];
       if (i > 0) {
         if (fits_on_current_line && !force_multiline)
           Print(" ");
         else
           Newline();
       }
@@ skipping 190 matching lines @@
       printf("Wrote formatted to '%s'.\n", to_write.AsUTF8Unsafe().c_str());
     } else {
       printf("%s", output_string.c_str());
     }
   }
 
   return 0;
 }
 
 }  // namespace commands