Change PathFragmentListConverter to return an ImmutableList, because …

…why not. PiperOrigin-RevId: 229986883
weixiao-huang · Jan 31, 2019 · 4559782 · 4559782
1 parent dfa160e
commit 4559782
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 26 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/util/OptionsUtils.java b/src/main/java/com/google/devtools/build/lib/util/OptionsUtils.java
@@ -20,8 +20,6 @@
 import com.google.devtools.common.options.Converter;
 import com.google.devtools.common.options.OptionsParsingResult;
 import com.google.devtools.common.options.ParsedOptionDescription;
-import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 
 /**
@@ -117,24 +115,21 @@ public String getTypeDescription() {
     }
   }
 
-  /**
-   * Converts from a colon-separated list of strings into a list of PathFragment instances.
-   */
-  public static class PathFragmentListConverter
-      implements Converter<List<PathFragment>> {
+  /** Converts from a colon-separated list of strings into a list of PathFragment instances. */
+  public static class PathFragmentListConverter implements Converter<ImmutableList<PathFragment>> {
 
     @Override
-    public List<PathFragment> convert(String input) {
-      List<PathFragment> list = new ArrayList<>();
+    public ImmutableList<PathFragment> convert(String input) {
+      ImmutableList.Builder<PathFragment> result = ImmutableList.builder();
       for (String piece : input.split(":")) {
         if (!piece.isEmpty()) {
           if (piece.startsWith("~/")) {
             piece = piece.replace("~", System.getProperty("user.home"));
           }
-          list.add(PathFragment.create(piece));
+          result.add(PathFragment.create(piece));
         }
       }
-      return Collections.unmodifiableList(list);
+      return result.build();
     }
 
     @Override

diff --git a/src/test/java/com/google/devtools/build/lib/util/OptionsUtilsTest.java b/src/test/java/com/google/devtools/build/lib/util/OptionsUtilsTest.java
@@ -14,9 +14,7 @@
 package com.google.devtools.build.lib.util;
 
 import static com.google.common.truth.Truth.assertThat;
-import static org.junit.Assert.fail;
 
-import com.google.common.collect.Lists;
 import com.google.devtools.build.lib.util.OptionsUtils.PathFragmentConverter;
 import com.google.devtools.build.lib.util.OptionsUtils.PathFragmentListConverter;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -151,10 +149,6 @@ public void asStringOfExplicitOptionsMultipleOptionsAreMultipleTimes() throws Ex
         .inOrder();
   }
 
-  private static List<PathFragment> list(PathFragment... fragments) {
-    return Lists.newArrayList(fragments);
-  }
-
   private PathFragment fragment(String string) {
     return PathFragment.create(string);
   }
@@ -169,7 +163,7 @@ private PathFragment convertOne(String input) throws Exception {
 
   @Test
   public void emptyStringYieldsEmptyList() throws Exception {
-    assertThat(convert("")).isEqualTo(list());
+    assertThat(convert("")).isEmpty();
   }
 
   @Test
@@ -200,12 +194,4 @@ public void singlePath() throws Exception {
     assertThat(convertOne("foo/bar/baz")).isEqualTo(fragment("foo/bar/baz"));
     assertThat(convertOne("~/foo")).isEqualTo(fragment(System.getProperty("user.home") + "/foo"));
   }
-
-  @Test
-  public void valueisUnmodifiable() throws Exception {
-    try {
-      new PathFragmentListConverter().convert("value").add(PathFragment.create("other"));
-      fail("could modify value");
-    } catch (UnsupportedOperationException expected) {}
-  }
 }