Clean up IterablesChain.

* Change builder return type to Iterable<T> instead of IterableChain<T>. It is over-specified and unnecessary to state the return type so precisely.

* Optimize builder for cases where we add 0 or 1 iterables to the chain. In this case, we can simply return the underlying iterables without adding wrappers.

* Extract DedupingIterable, it doesn't have anything to do with IterablesChain and is only used in one place

RELNOTES: None
PiperOrigin-RevId: 193363048

4 files changed, 102 insertions, 80 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/collect/CollectionUtils.java b/src/main/java/com/google/devtools/build/lib/collect/CollectionUtils.java
index 9198ba2ba7..a400be4cb2 100644
--- a/src/main/java/com/google/devtools/build/lib/collect/CollectionUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/collect/CollectionUtils.java
@@ -122,6 +122,7 @@ public final class CollectionUtils {
     return iterable instanceof ImmutableList<?>
         || iterable instanceof ImmutableSet<?>
         || iterable instanceof IterablesChain<?>
+        || iterable instanceof DedupingIterable<?>
         || iterable instanceof NestedSet<?>
         || iterable instanceof ImmutableIterable<?>;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/collect/DedupingIterable.java b/src/main/java/com/google/devtools/build/lib/collect/DedupingIterable.java
new file mode 100644
index 0000000000..25c6d9fc91
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/collect/DedupingIterable.java
@@ -0,0 +1,60 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.collect;
+
+import com.google.common.collect.AbstractIterator;
+import java.util.HashSet;
+import java.util.Iterator;
+
+/**
+ * An iterable implementation that removes duplicate elements (as determined by equals), using a
+ * hash set.
+ */
+public final class DedupingIterable<T> implements Iterable<T> {
+  private final Iterable<T> iterable;
+
+  DedupingIterable(Iterable<T> iterable) {
+    CollectionUtils.checkImmutable(iterable);
+    this.iterable = iterable;
+  }
+
+  public static <T> Iterable<T> of(Iterable<T> iterable) {
+    return new DedupingIterable<>(iterable);
+  }
+
+  @Override
+  public Iterator<T> iterator() {
+    return new DedupingIterator<>(iterable.iterator());
+  }
+
+  private static final class DedupingIterator<T> extends AbstractIterator<T> {
+    private final HashSet<T> set = new HashSet<>();
+    private final Iterator<T> it;
+
+    DedupingIterator(Iterator<T> it) {
+      this.it = it;
+    }
+
+    @Override
+    protected T computeNext() {
+      while (it.hasNext()) {
+        T next = it.next();
+        if (set.add(next)) {
+          return next;
+        }
+      }
+      return endOfData();
+    }
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/collect/IterableCodecs.java b/src/main/java/com/google/devtools/build/lib/collect/IterableCodecs.java
index e61055ecfc..43c6033a7e 100644
--- a/src/main/java/com/google/devtools/build/lib/collect/IterableCodecs.java
+++ b/src/main/java/com/google/devtools/build/lib/collect/IterableCodecs.java
@@ -67,6 +67,26 @@ class IterableCodecs {
     }
   }
 
+  static class DedupingIterableCodec implements ObjectCodec<DedupingIterable> {
+    @Override
+    public Class<DedupingIterable> getEncodedClass() {
+      return DedupingIterable.class;
+    }
+
+    @Override
+    public void serialize(
+        SerializationContext context, DedupingIterable obj, CodedOutputStream codedOut)
+        throws SerializationException, IOException {
+      IterableCodecs.serialize(context, obj, codedOut);
+    }
+
+    @Override
+    public DedupingIterable deserialize(DeserializationContext context, CodedInputStream codedIn)
+        throws SerializationException, IOException {
+      return new DedupingIterable<>(IterableCodecs.deserialize(context, codedIn));
+    }
+  }
+
   private static void serialize(
       SerializationContext context, Iterable obj, CodedOutputStream codedOut)
       throws SerializationException, IOException {
diff --git a/src/main/java/com/google/devtools/build/lib/collect/IterablesChain.java b/src/main/java/com/google/devtools/build/lib/collect/IterablesChain.java
index fe0f4bbe0c..496f9712f6 100644
--- a/src/main/java/com/google/devtools/build/lib/collect/IterablesChain.java
+++ b/src/main/java/com/google/devtools/build/lib/collect/IterablesChain.java
@@ -15,21 +15,19 @@
 package com.google.devtools.build.lib.collect;
 
 import com.google.common.base.Joiner;
-import com.google.common.collect.AbstractIterator;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import java.util.ArrayList;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 
 /**
  * An immutable chain of immutable Iterables.
  *
- * <p>This class is defined for the sole purpose of being able to check for immutability
- * (using instanceof). Otherwise, we could use a plain Iterable (as returned by
- * {@code Iterables.concat()}).
+ * <p>This class is defined for the sole purpose of being able to check for immutability (using
+ * instanceof). Otherwise, we could use a plain Iterable (as returned by {@code
+ * Iterables.concat()}).
  *
  * @see CollectionUtils#checkImmutable(Iterable)
  */
@@ -58,17 +56,11 @@ public final class IterablesChain<T> implements Iterable<T> {
     return "[" + Joiner.on(", ").join(this) + "]";
   }
 
-  /**
-   * Builder for IterablesChain.
-   *
- *
-   */
+  /** Builder for IterablesChain. */
   public static class Builder<T> {
     private List<Iterable<? extends T>> iterables = new ArrayList<>();
-    private boolean deduplicate;
 
-    private Builder() {
-    }
+    private Builder() {}
 
     /**
      * Adds an immutable iterable to the end of the chain.
@@ -78,89 +70,38 @@ public final class IterablesChain<T> implements Iterable<T> {
     public Builder<T> add(Iterable<? extends T> iterable) {
       CollectionUtils.checkImmutable(iterable);
       // Avoid unnecessarily expanding a NestedSet.
-      boolean isEmpty = iterable instanceof NestedSet
-          ? ((NestedSet<?>) iterable).isEmpty()
-          : Iterables.isEmpty(iterable);
+      boolean isEmpty =
+          iterable instanceof NestedSet
+              ? ((NestedSet<?>) iterable).isEmpty()
+              : Iterables.isEmpty(iterable);
       if (!isEmpty) {
         iterables.add(iterable);
       }
       return this;
     }
 
-    /**
-     * Adds a single element to the chain.
-     */
+    /** Adds a single element to the chain. */
     public Builder<T> addElement(T element) {
       iterables.add(ImmutableList.of(element));
       return this;
     }
 
-    /**
-     * Returns true if the chain is empty.
-     */
+    /** Returns true if the chain is empty. */
     public boolean isEmpty() {
       return iterables.isEmpty();
     }
 
-    /**
-     * If this is called, the resulting {@link IterablesChain} object uses a hash set to remove
-     * duplicate elements.
-     */
-    public Builder<T> deduplicate() {
-      this.deduplicate = true;
-      return this;
-    }
-
-    /**
-     * Builds an iterable that iterates through all elements in this chain.
-     */
-    public IterablesChain<T> build() {
-      if (isEmpty()) {
-        return new IterablesChain<>(ImmutableList.of());
-      }
-      Iterable<T> concat = Iterables.concat(ImmutableList.copyOf(iterables));
-      return new IterablesChain<>(deduplicate ? new Deduper<>(concat) : concat);
-    }
-  }
-
-  /**
-   * An iterable implementation that removes duplicate elements (as determined by equals), using a
-   * hash set.
-   */
-  private static final class Deduper<T> implements Iterable<T> {
-    private final Iterable<T> iterable;
-
-    public Deduper(Iterable<T> iterable) {
-      this.iterable = iterable;
-    }
-
-    @Override
-    public Iterator<T> iterator() {
-      return new DedupingIterator<>(iterable.iterator());
-    }
-  }
-
-  /**
-   * An iterator implementation that removes duplicate elements (as determined by equals), using a
-   * hash set.
-   */
-  private static final class DedupingIterator<T> extends AbstractIterator<T> {
-    private final HashSet<T> set = new HashSet<>();
-    private final Iterator<T> it;
-
-    public DedupingIterator(Iterator<T> it) {
-      this.it = it;
-    }
-
-    @Override
-    protected T computeNext() {
-      while (it.hasNext()) {
-        T next = it.next();
-        if (set.add(next)) {
-          return next;
-        }
+    /** Builds an iterable that iterates through all elements in this chain. */
+    @SuppressWarnings("unchecked")
+    public Iterable<T> build() {
+      int size = iterables.size();
+      if (size == 0) {
+        return ImmutableList.of();
+      } else if (size == 1) {
+        // cast is type safe since Iterable<T> doesn't allow mutation
+        return (Iterable<T>) iterables.get(0);
       }
-      return endOfData();
+      return new IterablesChain<>(Iterables.concat(ImmutableList.copyOf(iterables)));
     }
   }
 }