Treat expressions ending in .stream() as a syntactic unit

------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=206354173
google · Jul 30, 2018 · 1827f79 · 1827f79
1 parent b17ced2
commit 1827f79
Show file tree

Hide file tree

Showing 6 changed files with 102 additions and 36 deletions.
diff --git a/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java b/core/src/main/java/com/google/googlejavaformat/java/JavaInputAstVisitor.java
@@ -43,15 +43,18 @@
 import static org.openjdk.source.tree.Tree.Kind.VARIABLE;
 
 import com.google.common.base.MoreObjects;
+import com.google.common.base.Predicate;
 import com.google.common.base.Throwables;
 import com.google.common.base.Verify;
 import com.google.common.collect.HashMultiset;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSortedSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Iterators;
 import com.google.common.collect.Multiset;
 import com.google.common.collect.PeekingIterator;
+import com.google.common.collect.Streams;
 import com.google.googlejavaformat.CloseOp;
 import com.google.googlejavaformat.Doc;
 import com.google.googlejavaformat.Doc.FillMode;
@@ -67,12 +70,16 @@
 import com.google.googlejavaformat.java.DimensionHelpers.TypeWithDims;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Deque;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 import java.util.regex.Pattern;
+import java.util.stream.Stream;
 import javax.annotation.Nullable;
 import org.openjdk.javax.lang.model.element.Name;
 import org.openjdk.source.tree.AnnotatedTypeTree;
@@ -1522,7 +1529,7 @@ private boolean handleLogStatement(MethodInvocationTree node) {
       return false;
     }
     parts.addFirst(curr);
-    visitDotWithPrefix(ImmutableList.copyOf(parts), false, parts.size() - 1);
+    visitDotWithPrefix(ImmutableList.copyOf(parts), false, ImmutableList.of(parts.size() - 1));
     return true;
   }
 
@@ -1544,6 +1551,20 @@ private boolean handleLogStatement(MethodInvocationTree node) {
           "withCause",
           "withStackTrace");
 
+  private static Optional<Long> handleStream(List<ExpressionTree> parts) {
+    return indexIn(
+        parts.stream(),
+        p ->
+            (p instanceof MethodInvocationTree)
+                && getMethodName((MethodInvocationTree) p).contentEquals("stream"));
+  }
+
+  private static <T> Optional<Long> indexIn(Stream<T> stream, Predicate<T> predicate) {
+    return Streams.mapWithIndex(stream, (x, i) -> predicate.apply(x) ? i : -1)
+        .filter(x -> x != -1)
+        .findFirst();
+  }
+
   @Override
   public Void visitMemberSelect(MemberSelectTree node, Void unused) {
     sync(node);
@@ -2609,9 +2630,11 @@ void visitDot(ExpressionTree node0) {
       }
     }
 
+    Set<Integer> prefixes = new LinkedHashSet<>();
+
     // Check if the dot chain has a prefix that looks like a type name, so we can
     // treat the type name-shaped part as a single syntactic unit.
-    int prefixIndex = TypeNameClassifier.typePrefixLength(simpleNames(stack));
+    TypeNameClassifier.typePrefixLength(simpleNames(stack)).ifPresent(prefixes::add);
 
     int invocationCount = 0;
     int firstInvocationIndex = -1;
@@ -2647,23 +2670,25 @@ void visitDot(ExpressionTree node0) {
     // myField
     //     .foo();
     //
-    if (invocationCount == 1) {
+    if (invocationCount == 1 && firstInvocationIndex > 0) {
-      prefixIndex = firstInvocationIndex;
+      prefixes.add(firstInvocationIndex);
     }
 
-    if (prefixIndex == -1 && items.get(0) instanceof IdentifierTree) {
+    if (prefixes.isEmpty() && items.get(0) instanceof IdentifierTree) {
       switch (((IdentifierTree) items.get(0)).getName().toString()) {
         case "this":
         case "super":
-          prefixIndex = 1;
+          prefixes.add(1);
           break;
         default:
           break;
       }
     }
 
-    if (prefixIndex > 0) {
+    handleStream(items).ifPresent(x -> prefixes.add(x.intValue()));
-      visitDotWithPrefix(items, needDot, prefixIndex);
+
+    if (!prefixes.isEmpty()) {
+      visitDotWithPrefix(items, needDot, prefixes);
     } else {
       visitRegularDot(items, needDot);
     }
@@ -2755,21 +2780,26 @@ private boolean fillFirstArgument(ExpressionTree e, List<ExpressionTree> items,
    *
    * @param items in the chain
    * @param needDot whether a leading dot is needed
-   * @param prefixIndex the index of the last item in the prefix
+   * @param prefixes the terminal indices of 'prefixes' of the expression that should be treated as
+   *     a syntactic unit
    */
-  private void visitDotWithPrefix(List<ExpressionTree> items, boolean needDot, int prefixIndex) {
+  private void visitDotWithPrefix(
+      List<ExpressionTree> items, boolean needDot, Collection<Integer> prefixes) {
     // Are there method invocations or field accesses after the prefix?
-    boolean trailingDereferences = prefixIndex >= 0 && prefixIndex < items.size() - 1;
+    boolean trailingDereferences = !prefixes.isEmpty() && getLast(prefixes) < items.size() - 1;
 
     builder.open(plusFour);
-    builder.open(trailingDereferences ? ZERO : ZERO);
+    for (int times = 0; times < prefixes.size(); times++) {
+      builder.open(ZERO);
+    }
 
+    Deque<Integer> unconsumedPrefixes = new ArrayDeque<>(ImmutableSortedSet.copyOf(prefixes));
     BreakTag nameTag = genSym();
     for (int i = 0; i < items.size(); i++) {
       ExpressionTree e = items.get(i);
       if (needDot) {
         FillMode fillMode;
-        if (prefixIndex >= 0 && i <= prefixIndex) {
+        if (!unconsumedPrefixes.isEmpty() && i <= unconsumedPrefixes.peekFirst()) {
           fillMode = FillMode.INDEPENDENT;
         } else {
           fillMode = FillMode.UNIFIED;
@@ -2780,8 +2810,9 @@ private void visitDotWithPrefix(List<ExpressionTree> items, boolean needDot, int
       }
       BreakTag tyargTag = genSym();
       dotExpressionUpToArgs(e, Optional.of(tyargTag));
-      if (prefixIndex >= 0 && i == prefixIndex) {
+      if (!unconsumedPrefixes.isEmpty() && i == unconsumedPrefixes.peekFirst()) {
         builder.close();
+        unconsumedPrefixes.removeFirst();
       }
 
       Indent tyargIndent = Indent.If.make(tyargTag, plusFour, ZERO);

diff --git a/core/src/main/java/com/google/googlejavaformat/java/TypeNameClassifier.java b/core/src/main/java/com/google/googlejavaformat/java/TypeNameClassifier.java
@@ -16,6 +16,7 @@
 
 import com.google.common.base.Verify;
 import java.util.List;
+import java.util.Optional;
 
 /** Heuristics for classifying qualified names as types. */
 public final class TypeNameClassifier {
@@ -121,16 +122,16 @@ public boolean isSingleUnit() {
    *   <li>com.google.ClassName.InnerClass.staticMemberName
    * </ul>
    */
-  static int typePrefixLength(List<String> nameParts) {
+  static Optional<Integer> typePrefixLength(List<String> nameParts) {
     TyParseState state = TyParseState.START;
-    int typeLength = -1;
+    Optional<Integer> typeLength = Optional.empty();
     for (int i = 0; i < nameParts.size(); i++) {
       state = state.next(JavaCaseFormat.from(nameParts.get(i)));
       if (state == TyParseState.REJECT) {
         break;
       }
       if (state.isSingleUnit()) {
-        typeLength = i;
+        typeLength = Optional.of(i);
       }
     }
     return typeLength;

diff --git a/core/src/test/java/com/google/googlejavaformat/java/TypeNameClassifierTest.java b/core/src/test/java/com/google/googlejavaformat/java/TypeNameClassifierTest.java
@@ -15,9 +15,11 @@
 package com.google.googlejavaformat.java;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
 
 import com.google.common.base.Splitter;
 import com.google.googlejavaformat.java.TypeNameClassifier.JavaCaseFormat;
+import java.util.Optional;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -43,30 +45,30 @@ public void caseFormat() throws Exception {
     assertThat(JavaCaseFormat.from("_A")).isEqualTo(JavaCaseFormat.UPPERCASE);
   }
 
-  private static int getPrefix(String qualifiedName) {
+  private static Optional<Integer> getPrefix(String qualifiedName) {
     return TypeNameClassifier.typePrefixLength(Splitter.on('.').splitToList(qualifiedName));
   }
 
   @Test
   public void typePrefixLength() {
-    assertThat(getPrefix("fieldName")).isEqualTo(-1);
+    assertThat(getPrefix("fieldName")).isEmpty();
-    assertThat(getPrefix("CONST")).isEqualTo(-1);
+    assertThat(getPrefix("CONST")).isEmpty();
-    assertThat(getPrefix("ClassName")).isEqualTo(0);
+    assertThat(getPrefix("ClassName")).hasValue(0);
-    assertThat(getPrefix("com.ClassName")).isEqualTo(1);
+    assertThat(getPrefix("com.ClassName")).hasValue(1);
-    assertThat(getPrefix("ClassName.foo")).isEqualTo(1);
+    assertThat(getPrefix("ClassName.foo")).hasValue(1);
-    assertThat(getPrefix("com.ClassName.foo")).isEqualTo(2);
+    assertThat(getPrefix("com.ClassName.foo")).hasValue(2);
-    assertThat(getPrefix("ClassName.foo.bar")).isEqualTo(1);
+    assertThat(getPrefix("ClassName.foo.bar")).hasValue(1);
-    assertThat(getPrefix("com.ClassName.foo.bar")).isEqualTo(2);
+    assertThat(getPrefix("com.ClassName.foo.bar")).hasValue(2);
-    assertThat(getPrefix("ClassName.CONST")).isEqualTo(1);
+    assertThat(getPrefix("ClassName.CONST")).hasValue(1);
-    assertThat(getPrefix("ClassName.varName")).isEqualTo(1);
+    assertThat(getPrefix("ClassName.varName")).hasValue(1);
-    assertThat(getPrefix("ClassName.Inner.varName")).isEqualTo(2);
+    assertThat(getPrefix("ClassName.Inner.varName")).hasValue(2);
   }
 
   @Test
   public void ambiguousClass() {
-    assertThat(getPrefix("com.google.security.acl.proto2api.ACL.Entry.newBuilder")).isEqualTo(7);
+    assertThat(getPrefix("com.google.security.acl.proto2api.ACL.Entry.newBuilder")).hasValue(7);
     // A human would probably identify this as "class-shaped", but just looking
     // at the case we have to assume it could be something like `field1.field2.CONST`.
-    assertThat(getPrefix("com.google.security.acl.proto2api.ACL.newBuilder")).isEqualTo(-1);
+    assertThat(getPrefix("com.google.security.acl.proto2api.ACL.newBuilder")).isEmpty();
   }
 }
diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/B20128760.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/B20128760.output
@@ -92,14 +92,11 @@ class B20128760 {
         stream(members)
             .flatMap(
                 m ->
-                    m.getFieldValues()
+                    m.getFieldValues().entrySet().stream()
-                        .entrySet()
-                        .stream()
                         .filter(fv -> itemLinkFieldIds.contains(fv.getKey()))
                         .flatMap(
                             fv ->
-                                FieldDTO.deserializeStringToListOfStrings(fv.getValue())
+                                FieldDTO.deserializeStringToListOfStrings(fv.getValue()).stream()
-                                    .stream()
                                     .map(
                                         id ->
                                             new ItemKey(

diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/B35644813.input b/core/src/test/resources/com/google/googlejavaformat/java/testdata/B35644813.input
@@ -0,0 +1,19 @@
+class B35644813 {
+  {
+    return foo____________
+        .bar__________()
+        .baz____________()
+        .stream()
+        .map(Baz::getId)
+        .collect(toList());
+  }
+
+  private static final ImmutableSet<String> SCANDINAVIA =
+      ImmutableSet.of(
+              DENMARK____________________________________________________,
+              NORWAY_____________________________________________________,
+              SWEDEN_____________________________________________________)
+          .stream()
+          .map(x -> String.format("country: %s", x.toLowerCase()))
+          .collect(toImmutableSet());
+}
diff --git a/core/src/test/resources/com/google/googlejavaformat/java/testdata/B35644813.output b/core/src/test/resources/com/google/googlejavaformat/java/testdata/B35644813.output
@@ -0,0 +1,16 @@
+class B35644813 {
+  {
+    return foo____________.bar__________().baz____________().stream()
+        .map(Baz::getId)
+        .collect(toList());
+  }
+
+  private static final ImmutableSet<String> SCANDINAVIA =
+      ImmutableSet.of(
+              DENMARK____________________________________________________,
+              NORWAY_____________________________________________________,
+              SWEDEN_____________________________________________________)
+          .stream()
+          .map(x -> String.format("country: %s", x.toLowerCase()))
+          .collect(toImmutableSet());
+}