Include comments when calculating AST node size

The formatter prints method arguments one-per-line unless they all fit on the
same line, or they're all very short. This fixes a bug where the formatter
would fill the arguments even if they had long parameter comments attached to
them.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=106897312

Author: cushon

core/src/main/java/com/google/googlejavaformat/OpsBuilder.java

@@ -20,6 +20,8 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Multimap;
+import com.google.googlejavaformat.Input.Tok;
+import com.google.googlejavaformat.Input.Token;
 import com.google.googlejavaformat.OpsBuilder.BlankLineWanted;
 import com.google.googlejavaformat.Output.BreakTag;
 
@@ -32,6 +34,25 @@
  */
 public final class OpsBuilder {
 
+  /** @return the actual size of the AST node at position, including comments. */
+  public int actualSize(int position, int length) {
+    Token startToken = input.getPositionTokenMap().floorEntry(position).getValue();
+    int start = startToken.getTok().getPosition();
+    for (Tok tok : startToken.getToksBefore()) {
+      if (tok.isComment()) {
+        start = Math.min(start, tok.getPosition());
+      }
+    }
+    Token endToken = input.getPositionTokenMap().lowerEntry(position + length).getValue();
+    int end = endToken.getTok().getPosition() + endToken.getTok().getText().length();
+    for (Tok tok : endToken.getToksAfter()) {
+      if (tok.isComment()) {
+        end = Math.max(end, tok.getPosition() + tok.getText().length());
+      }
+    }
+    return end - start;
+  }
+
   /** A request to add or remove a blank line in the output. */
   public abstract static class BlankLineWanted {
 

core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java

@@ -571,14 +571,14 @@ public boolean visit(ArrayInitializer node) {
    * Returns {@code defaultThreshold} if bin-packing can be used for the given
    * expression list, and {code NEVER_FILL} otherwise.
    */
-  private static int maxLinesFilledForItems(List<Expression> expressions, int defaultThreshold) {
+  private int maxLinesFilledForItems(List<Expression> expressions, int defaultThreshold) {
     return hasOnlyShortItems(expressions) ? ALWAYS_FILL : defaultThreshold;
   }
 
-  private static boolean hasOnlyShortItems(List<Expression> expressions) {
+  private boolean hasOnlyShortItems(List<Expression> expressions) {
     for (Expression expression : expressions) {
-      // TODO(cushon): this ignores attached comments, so `/*myParameterName=*/ true` has length 4.
-      if (expression.getLength() >= MAX_ITEM_LENGTH_FOR_FILLING) {
+      if (builder.actualSize(expression.getStartPosition(), expression.getLength())
+          >= MAX_ITEM_LENGTH_FOR_FILLING) {
         return false;
       }
     }

core/src/test/resources/com/google/googlejavaformat/java/testdata/B20844369.output

@@ -17,7 +17,8 @@ public class B20844369 {
 
   int x =
       //foo
-      42 + //bar
+      42
+          + //bar
           1;
 
   int x = /*foo*/
@@ -26,6 +27,7 @@ public class B20844369 {
 
   int x =
       /*foo*/
-      42 + //bar
+      42
+          + //bar
           1;
 }

core/src/test/resources/com/google/googlejavaformat/java/testdata/B25372815.input

@@ -0,0 +1,32 @@
+class Test {
+  {
+    // For "All goals" view, show only advertiser goals tab.
+    return newPageTypeSet(
+        false /* advertisers */,
+        false /* userManagement */,
+        false /* campaigns */,
+        false /* adGroups */,
+        false /* ads */,
+        false /* keywords */,
+        false /* negativeKeywords */,
+        false /* advertiserBidStrategies */,
+        false /* bidStrategies */,
+        false /* bidStrategyRecommendations */,
+        false /* bidKeywords */,
+        false /* engineBidStrategies */,
+        false /* conversionTrackers */,
+        false /* labels */,
+        false /* labelKeywords */,
+        false /* evergreen labels */,
+        false /* searchQueries */,
+        false /* engineSearchQueries */,
+        false /* remarketingTarget */,
+        false /* sitelinks */,
+        false /* feedSitelinks */,
+        false /* locationExtensions */,
+        false /* callExtensions */,
+        false /* calloutExtensions */,
+        false /* appExtensions */,
+        false /* events */);
+  }
+}

core/src/test/resources/com/google/googlejavaformat/java/testdata/B25372815.output

@@ -0,0 +1,32 @@
+class Test {
+  {
+    // For "All goals" view, show only advertiser goals tab.
+    return newPageTypeSet(
+        false /* advertisers */,
+        false /* userManagement */,
+        false /* campaigns */,
+        false /* adGroups */,
+        false /* ads */,
+        false /* keywords */,
+        false /* negativeKeywords */,
+        false /* advertiserBidStrategies */,
+        false /* bidStrategies */,
+        false /* bidStrategyRecommendations */,
+        false /* bidKeywords */,
+        false /* engineBidStrategies */,
+        false /* conversionTrackers */,
+        false /* labels */,
+        false /* labelKeywords */,
+        false /* evergreen labels */,
+        false /* searchQueries */,
+        false /* engineSearchQueries */,
+        false /* remarketingTarget */,
+        false /* sitelinks */,
+        false /* feedSitelinks */,
+        false /* locationExtensions */,
+        false /* callExtensions */,
+        false /* calloutExtensions */,
+        false /* appExtensions */,
+        false /* events */);
+  }
+}