CustomCommandLine.Builder: clean up its interface

In this commit:
- remove unused methods and classes
- turn CustomCommandLine.ArgvFragment into an
  interface
- remove the
  CustomCommandLine.TreeFileArtifactArgvFragment
  abstract class; it only had one remaining
  subclass
- add @Nullable annotations where nulls are fine
- add Precondition checks for non-nullable args
- simplify the interface by removing add* methods
  that can be composed of other add* methods; this
  makes it easier to see what the callers do with
  the Builder
- remove add* methods that add a single argument
  followed by a list of other elements (or a
  joined string of them); these had a bug in that
  they didn't check if the collection was empty
  (only that it was not null), and if it was empty
  then the single argument was still added though
  it was not followed by any value
- fix call sites of add* methods where we
  previously could have added a flag with an empty
  collection
- audit every affected call site

RELNOTES: none
PiperOrigin-RevId: 161957521

1 files changed, 103 insertions, 209 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java
index 601229bd14..b05b0394c6 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java
@@ -20,6 +20,7 @@ import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableList.Builder;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
 import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
@@ -43,40 +44,20 @@ import javax.annotation.Nullable;
 @Immutable
 public final class CustomCommandLine extends CommandLine {
 
-  private abstract static class ArgvFragment {
-    abstract void eval(ImmutableList.Builder<String> builder);
+  private interface ArgvFragment {
+    void eval(ImmutableList.Builder<String> builder);
   }
 
   /**
-   * A command line argument for {@link TreeFileArtifact}.
+   * A command line argument that can expand enclosed TreeArtifacts into a list of child {@link
+   * TreeFileArtifact}s at execution time before argument evaluation.
    *
-   * <p>Since {@link TreeFileArtifact} is not known or available at analysis time, subclasses should
-   * enclose its parent TreeFileArtifact instead at analysis time. This interface provides method
-   * {@link #substituteTreeArtifact} to generate another argument object that replaces the enclosed
-   * TreeArtifact with one of its {@link TreeFileArtifact} at execution time.
-   */
-  private abstract static class TreeFileArtifactArgvFragment {
-    /**
-     * Substitutes this ArgvFragment with another arg object, with the original TreeArtifacts
-     * contained in this ArgvFragment replaced by their associated TreeFileArtifacts.
-     *
-     * @param substitutionMap A map between TreeArtifacts and their associated TreeFileArtifacts
-     *     used to replace them.
-     */
-    abstract Object substituteTreeArtifact(Map<Artifact, TreeFileArtifact> substitutionMap);
-  }
-
-  /**
-   * A command line argument that can expand enclosed TreeArtifacts into a list of child
-   * {@link TreeFileArtifact}s at execution time before argument evaluation.
-   *
-   * <p>The main difference between this class and {@link TreeFileArtifactArgvFragment} is that
-   * {@link TreeFileArtifactArgvFragment} is used in {@link SpawnActionTemplate} to substitutes a
+   * <p>The main difference between this class and {@link TreeFileArtifactExecPathArg} is that
+   * {@link TreeFileArtifactExecPathArg} is used in {@link SpawnActionTemplate} to substitutes a
    * TreeArtifact with *one* of its child TreeFileArtifacts, while this class expands a TreeArtifact
    * into *all* of its child TreeFileArtifacts.
-   *
    */
-  private abstract static class TreeArtifactExpansionArgvFragment extends ArgvFragment {
+  private abstract static class TreeArtifactExpansionArgvFragment implements ArgvFragment {
     /**
      * Evaluates this argument fragment into an argument string and adds it into {@code builder}.
      * The enclosed TreeArtifact will be expanded using {@code artifactExpander}.
@@ -98,13 +79,13 @@ public final class CustomCommandLine extends CommandLine {
      * <p>Internally this method just calls {@link #describe}.
      */
     @Override
-    void eval(ImmutableList.Builder<String> builder) {
+    public void eval(ImmutableList.Builder<String> builder) {
       builder.add(describe());
     }
   }
 
   // It's better to avoid anonymous classes if we want to serialize command lines
-  private static final class JoinExecPathsArg extends ArgvFragment {
+  private static final class JoinExecPathsArg implements ArgvFragment {
 
     private final String delimiter;
     private final Iterable<Artifact> artifacts;
@@ -115,7 +96,7 @@ public final class CustomCommandLine extends CommandLine {
     }
 
     @Override
-    void eval(ImmutableList.Builder<String> builder) {
+    public void eval(ImmutableList.Builder<String> builder) {
       builder.add(Artifact.joinExecPaths(delimiter, artifacts));
     }
   }
@@ -180,7 +161,7 @@ public final class CustomCommandLine extends CommandLine {
     }
   }
 
-  private static final class PathWithTemplateArg extends ArgvFragment {
+  private static final class PathWithTemplateArg implements ArgvFragment {
 
     private final String template;
     private final PathFragment[] paths;
@@ -191,13 +172,13 @@ public final class CustomCommandLine extends CommandLine {
     }
 
     @Override
-    void eval(ImmutableList.Builder<String> builder) {
+    public void eval(ImmutableList.Builder<String> builder) {
       // PathFragment.toString() uses getPathString()
       builder.add(String.format(template, (Object[]) paths));
     }
   }
 
-  private static final class ParamFileArgument extends ArgvFragment {
+  private static final class ParamFileArgument implements ArgvFragment {
     private final String paramFilePrefix;
     private final PathFragment path;
 
@@ -207,84 +188,37 @@ public final class CustomCommandLine extends CommandLine {
     }
 
     @Override
-    void eval(ImmutableList.Builder<String> builder) {
+    public void eval(ImmutableList.Builder<String> builder) {
       builder.add(paramFilePrefix + path);
     }
   }
 
-  /**
-   * An argument object that evaluates to a formatted string for {@link TreeFileArtifact} exec
-   * paths, enclosing the associated string format template and {@link TreeFileArtifact}s.
-   */
-  private static final class TreeFileArtifactExecPathWithTemplateArg
-      extends TreeFileArtifactArgvFragment {
-
-    private final String template;
-    private final Artifact placeHolderTreeArtifact;
-
-    private TreeFileArtifactExecPathWithTemplateArg(String template, Artifact artifact) {
-      Preconditions.checkArgument(artifact.isTreeArtifact(), "%s must be a TreeArtifact",
-          artifact);
-      this.template = template;
-      this.placeHolderTreeArtifact = artifact;
-    }
-
-    @Override
-    ArgvFragment substituteTreeArtifact(Map<Artifact, TreeFileArtifact> substitutionMap) {
-      Artifact treeFileArtifact = substitutionMap.get(placeHolderTreeArtifact);
-      Preconditions.checkNotNull(treeFileArtifact, "Artifact to substitute: %s",
-          placeHolderTreeArtifact);
-
-      return new PathWithTemplateArg(template, treeFileArtifact.getExecPath());
-    }
-  }
-
   // TODO(bazel-team): CustomArgv and CustomMultiArgv is  going to be difficult to expose
   // in Skylark. Maybe we can get rid of them by refactoring JavaCompileAction. It also
   // raises immutability / serialization issues.
-  /**
-   * Custom Java code producing a String argument. Usage of this class is discouraged.
-   */
-  public abstract static class CustomArgv extends ArgvFragment {
+  /** Custom Java code producing a String argument. Usage of this class is discouraged. */
+  public abstract static class CustomArgv implements ArgvFragment {
 
     @Override
-    void eval(ImmutableList.Builder<String> builder) {
+    public void eval(ImmutableList.Builder<String> builder) {
       builder.add(argv());
     }
 
     public abstract String argv();
   }
 
-  /**
-   * Custom Java code producing a List of String arguments. Usage of this class is discouraged.
-   */
-  public abstract static class CustomMultiArgv extends ArgvFragment {
+  /** Custom Java code producing a List of String arguments. Usage of this class is discouraged. */
+  public abstract static class CustomMultiArgv implements ArgvFragment {
 
     @Override
-    void eval(ImmutableList.Builder<String> builder) {
+    public void eval(ImmutableList.Builder<String> builder) {
       builder.addAll(argv());
     }
 
     public abstract Iterable<String> argv();
   }
 
-  private static final class JoinPathsArg extends ArgvFragment {
-
-    private final String delimiter;
-    private final Iterable<PathFragment> paths;
-
-    private JoinPathsArg(String delimiter, Iterable<PathFragment> paths) {
-      this.delimiter = delimiter;
-      this.paths = CollectionUtils.makeImmutable(paths);
-    }
-
-    @Override
-    void eval(ImmutableList.Builder<String> builder) {
-      builder.add(Joiner.on(delimiter).join(paths));
-    }
-  }
-
-  private static final class JoinStringsArg extends ArgvFragment {
+  private static final class JoinStringsArg implements ArgvFragment {
 
     private final String delimiter;
     private final Iterable<String> strings;
@@ -295,12 +229,12 @@ public final class CustomCommandLine extends CommandLine {
     }
 
     @Override
-    void eval(ImmutableList.Builder<String> builder) {
+    public void eval(ImmutableList.Builder<String> builder) {
       builder.add(Joiner.on(delimiter).join(strings));
     }
   }
 
-  private static final class JoinValuesTransformed<T> extends ArgvFragment {
+  private static final class JoinValuesTransformed<T> implements ArgvFragment {
 
     private final String delimiter;
     private final Iterable<T> values;
@@ -314,7 +248,7 @@ public final class CustomCommandLine extends CommandLine {
     }
 
     @Override
-    void eval(ImmutableList.Builder<String> builder) {
+    public void eval(ImmutableList.Builder<String> builder) {
       StringBuilder arg = new StringBuilder();
       Iterator<T> parts = values.iterator();
       if (parts.hasNext()) {
@@ -331,18 +265,19 @@ public final class CustomCommandLine extends CommandLine {
   /**
    * Arguments that intersperse strings between the items in a sequence. There are two forms of
    * interspersing, and either may be used by this implementation:
+   *
    * <ul>
-   *   <li>before each - a string is added before each item in a sequence. e.g.
-   *       {@code -f foo -f bar -f baz}
-   *   <li>format each - a format string is used to format each item in a sequence. e.g.
-   *       {@code -I/foo -I/bar -I/baz} for the format {@code "-I%s"}
+   *   <li>before each - a string is added before each item in a sequence. e.g. {@code -f foo -f bar
+   *       -f baz}
+   *   <li>format each - a format string is used to format each item in a sequence. e.g. {@code
+   *       -I/foo -I/bar -I/baz} for the format {@code "-I%s"}
    * </ul>
    *
-   * <p>This class could be used both with both the "before" and "format" features at the same
-   * time, but this is probably more confusion than it is worth. If you need this functionality,
-   * consider using "before" only but storing the strings pre-formated in a {@link NestedSet}.
+   * <p>This class could be used both with both the "before" and "format" features at the same time,
+   * but this is probably more confusion than it is worth. If you need this functionality, consider
+   * using "before" only but storing the strings pre-formatted in a {@link NestedSet}.
    */
-  private static final class InterspersingArgs extends ArgvFragment {
+  private static final class InterspersingArgs implements ArgvFragment {
     private final Iterable<?> sequence;
     private final String beforeEach;
     private final String formatEach;
@@ -370,7 +305,7 @@ public final class CustomCommandLine extends CommandLine {
     }
 
     @Override
-    void eval(ImmutableList.Builder<String> builder) {
+    public void eval(ImmutableList.Builder<String> builder) {
       for (Object item : sequence) {
         if (item == null) {
           continue;
@@ -388,12 +323,11 @@ public final class CustomCommandLine extends CommandLine {
     }
   }
 
-
   /**
-   * An argument object that evaluates to the exec path of a {@link TreeFileArtifact}, enclosing
-   * the associated {@link TreeFileArtifact}.
+   * An argument object that evaluates to the exec path of a {@link TreeFileArtifact}, enclosing the
+   * associated {@link TreeFileArtifact}.
    */
-  private static final class TreeFileArtifactExecPathArg extends TreeFileArtifactArgvFragment {
+  private static final class TreeFileArtifactExecPathArg {
     private final Artifact placeHolderTreeArtifact;
 
     private TreeFileArtifactExecPathArg(Artifact artifact) {
@@ -401,7 +335,13 @@ public final class CustomCommandLine extends CommandLine {
       placeHolderTreeArtifact = artifact;
     }
 
-    @Override
+    /**
+     * Substitutes this ArgvFragment with another arg object, with the original TreeArtifacts
+     * contained in this ArgvFragment replaced by their associated TreeFileArtifacts.
+     *
+     * @param substitutionMap A map between TreeArtifacts and their associated TreeFileArtifacts
+     *     used to replace them.
+     */
     Object substituteTreeArtifact(Map<Artifact, TreeFileArtifact> substitutionMap) {
       Artifact artifact = substitutionMap.get(placeHolderTreeArtifact);
       Preconditions.checkNotNull(artifact, "Artifact to substitute: %s", placeHolderTreeArtifact);
@@ -431,56 +371,39 @@ public final class CustomCommandLine extends CommandLine {
     // toString() results.
     private final List<Object> arguments = new ArrayList<>();
 
-    public Builder add(CharSequence arg) {
+    public Builder add(@Nullable CharSequence arg) {
       if (arg != null) {
         arguments.add(arg);
       }
       return this;
     }
 
-    public Builder add(Label arg) {
+    public Builder add(@Nullable Label arg) {
       if (arg != null) {
         arguments.add(arg);
       }
       return this;
     }
 
-    public Builder add(String arg, Iterable<String> args) {
-      if (arg != null && args != null) {
-        arguments.add(arg);
-        arguments.add(
-            InterspersingArgs.fromStrings(args, /*beforeEach=*/ null, /*formatEach=*/ null));
-      }
-      return this;
-    }
-
-    public Builder add(Iterable<String> args) {
-      if (args != null) {
+    public Builder add(@Nullable Iterable<String> args) {
+      if (args != null && !Iterables.isEmpty(args)) {
         arguments.add(
             InterspersingArgs.fromStrings(args, /*beforeEach=*/ null, /*formatEach=*/ null));
       }
       return this;
     }
 
-    public Builder addExecPath(String arg, Artifact artifact) {
-      if (arg != null && artifact != null) {
+    public Builder addExecPath(String arg, @Nullable Artifact artifact) {
+      Preconditions.checkNotNull(arg);
+      if (artifact != null) {
         arguments.add(arg);
         arguments.add(artifact.getExecPath());
       }
       return this;
     }
 
-    public Builder addExecPaths(String arg, Iterable<Artifact> artifacts) {
-      if (arg != null && artifacts != null) {
-        arguments.add(arg);
-        arguments.add(
-            InterspersingArgs.fromExecPaths(artifacts, /*beforeEach=*/ null, /*formatEach=*/ null));
-      }
-      return this;
-    }
-
-    public Builder addExecPaths(Iterable<Artifact> artifacts) {
-      if (artifacts != null) {
+    public Builder addExecPaths(@Nullable Iterable<Artifact> artifacts) {
+      if (artifacts != null && !Iterables.isEmpty(artifacts)) {
         arguments.add(
             InterspersingArgs.fromExecPaths(artifacts, /*beforeEach=*/ null, /*formatEach=*/ null));
       }
@@ -488,40 +411,23 @@ public final class CustomCommandLine extends CommandLine {
     }
 
     /**
-     * Adds a flag with the exec path of a placeholder TreeArtifact. When the command line is used
-     * in an action template, the placeholder will be replaced by the exec path of a
-     * {@link TreeFileArtifact} inside the TreeArtifact at execution time for each expanded action.
-     *
-     * @param arg the name of the argument
-     * @param treeArtifact the TreeArtifact that will be evaluated to one of its child
-     *     {@link TreeFileArtifact} at execution time
-     */
-    public Builder addPlaceholderTreeArtifactExecPath(String arg, Artifact treeArtifact) {
-      if (arg != null && treeArtifact != null) {
-        arguments.add(arg);
-        arguments.add(new TreeFileArtifactExecPathArg(treeArtifact));
-      }
-      return this;
-    }
-
-    /**
      * Adds a placeholder TreeArtifact exec path. When the command line is used in an action
      * template, the placeholder will be replaced by the exec path of a {@link TreeFileArtifact}
      * inside the TreeArtifact at execution time for each expanded action.
      *
-     * @param treeArtifact the TreeArtifact that will be evaluated to one of its child
-     *     {@link TreeFileArtifact} at execution time
+     * @param treeArtifact the TreeArtifact that will be evaluated to one of its child {@link
+     *     TreeFileArtifact} at execution time
      */
-    public Builder addPlaceholderTreeArtifactExecPath(Artifact treeArtifact) {
+    public Builder addPlaceholderTreeArtifactExecPath(@Nullable Artifact treeArtifact) {
       if (treeArtifact != null) {
         arguments.add(new TreeFileArtifactExecPathArg(treeArtifact));
       }
       return this;
     }
 
-    public Builder addJoinStrings(String arg, String delimiter, Iterable<String> strings) {
-      if (arg != null && strings != null) {
-        arguments.add(arg);
+    public Builder addJoinStrings(String delimiter, @Nullable Iterable<String> strings) {
+      Preconditions.checkNotNull(delimiter);
+      if (strings != null && !Iterables.isEmpty(strings)) {
         arguments.add(new JoinStringsArg(delimiter, strings));
       }
       return this;
@@ -534,37 +440,38 @@ public final class CustomCommandLine extends CommandLine {
      * this class, expansion of the nested set is deferred until action execution instead of
      * retained on the heap.
      *
-     * @param arg The argument
      * @param delimiter A delimiter string placed in between each transformed value
      * @param values The values to expand into a list
      * @param toString A function that transforms a value into a string
      */
     public <T> Builder addJoinValues(
-        String arg, String delimiter, Iterable<T> values, Function<T, String> toString) {
-      if (arg != null && arguments != null) {
-        arguments.add(arg);
+        String delimiter, @Nullable Iterable<T> values, Function<T, String> toString) {
+      Preconditions.checkNotNull(delimiter);
+      Preconditions.checkNotNull(toString);
+      if (values != null && !Iterables.isEmpty(values)) {
         arguments.add(new JoinValuesTransformed<T>(delimiter, values, toString));
       }
       return this;
     }
- 
-    public Builder addJoinExecPaths(String arg, String delimiter, Iterable<Artifact> artifacts) {
-      if (arg != null && artifacts != null) {
-        arguments.add(arg);
+
+    public Builder addJoinExecPaths(String delimiter, @Nullable Iterable<Artifact> artifacts) {
+      Preconditions.checkNotNull(delimiter);
+      if (artifacts != null && !Iterables.isEmpty(artifacts)) {
         arguments.add(new JoinExecPathsArg(delimiter, artifacts));
       }
       return this;
     }
 
-    public Builder addPath(PathFragment path) {
+    public Builder addPath(@Nullable PathFragment path) {
       if (path != null) {
         arguments.add(path);
       }
       return this;
     }
 
-    public Builder addPaths(String template, PathFragment... path) {
-      if (template != null && path != null) {
+    public Builder addPaths(String template, @Nullable PathFragment... path) {
+      Preconditions.checkNotNull(template);
+      if (path != null && path.length > 0) {
         arguments.add(new PathWithTemplateArg(template, path));
       }
       return this;
@@ -573,41 +480,22 @@ public final class CustomCommandLine extends CommandLine {
     /**
      * Adds a param file as an argument.
      *
+     * <p>Memory consumption consideration: though `addPaths` could also do the job of this method,
+     * this one is more memory-efficient because it delays string constructions as much as possible.
+     * Using `addPaths` would look like: <code>.addPaths(paramFilePrefix + "%s", paramFile)</code>,
+     * meaning we'd eagerly create an extra String for every param file.
+     *
      * @param paramFilePrefix The character that denotes a param file, commonly '@'
      * @param paramFile The param file artifact
      */
     public Builder addParamFile(String paramFilePrefix, Artifact paramFile) {
+      Preconditions.checkNotNull(paramFilePrefix);
+      Preconditions.checkNotNull(paramFile);
       arguments.add(new ParamFileArgument(paramFilePrefix, paramFile.getExecPath()));
       return this;
     }
 
     /**
-     * Adds a formatted string containing the exec path of a placeholder TreeArtifact. When the
-     * command line is used in an action template, the placeholder will be replaced by the exec path
-     * of a {@link TreeFileArtifact} inside the TreeArtifact at execution time for each expanded
-     * action.
-     *
-     * @param template the string format template containing a single string format specifier (%s)
-     *     to be replaced by the artifact exec path string.
-     * @param treeArtifact the TreeArtifact that will be evaluated to one of their child
-     *     {@link TreeFileArtifact} at execution time
-     */
-    public Builder addPlaceholderTreeArtifactFormattedExecPath(
-        String template, Artifact treeArtifact) {
-      if (template != null && treeArtifact != null) {
-        arguments.add(new TreeFileArtifactExecPathWithTemplateArg(template, treeArtifact));
-      }
-      return this;
-    }
-
-    public Builder addJoinPaths(String delimiter, Iterable<PathFragment> paths) {
-      if (delimiter != null && paths != null) {
-        arguments.add(new JoinPathsArg(delimiter, paths));
-      }
-      return this;
-    }
-
-    /**
      * Adds a string joined together by the exec paths of all {@link TreeFileArtifact}s under
      * {@code treeArtifact}.
      *
@@ -615,6 +503,8 @@ public final class CustomCommandLine extends CommandLine {
      * @param treeArtifact the TreeArtifact containing the {@link TreeFileArtifact}s to join.
      */
     public Builder addJoinExpandedTreeArtifactExecPath(String delimiter, Artifact treeArtifact) {
+      Preconditions.checkNotNull(delimiter);
+      Preconditions.checkNotNull(treeArtifact);
       arguments.add(new JoinExpandedTreeArtifactExecPathsArg(delimiter, treeArtifact));
       return this;
     }
@@ -626,46 +516,51 @@ public final class CustomCommandLine extends CommandLine {
      * @param treeArtifact the TreeArtifact containing the {@link TreeFileArtifact}s to add.
      */
     public Builder addExpandedTreeArtifactExecPaths(Artifact treeArtifact) {
+      Preconditions.checkNotNull(treeArtifact);
       arguments.add(new ExpandedTreeArtifactExecPathsArg(treeArtifact));
       return this;
     }
 
-    public Builder addBeforeEachPath(String repeated, Iterable<PathFragment> paths) {
-      if (repeated != null && paths != null) {
+    public Builder addBeforeEachPath(String repeated, @Nullable Iterable<PathFragment> paths) {
+      Preconditions.checkNotNull(repeated);
+      if (paths != null && !Iterables.isEmpty(paths)) {
         arguments.add(InterspersingArgs.fromStrings(paths, repeated, /*formatEach=*/ null));
       }
       return this;
     }
 
-    public Builder addBeforeEach(String repeated, Iterable<String> strings) {
-      if (repeated != null && strings != null) {
+    public Builder addBeforeEach(String repeated, @Nullable Iterable<String> strings) {
+      Preconditions.checkNotNull(repeated);
+      if (strings != null && !Iterables.isEmpty(strings)) {
         arguments.add(InterspersingArgs.fromStrings(strings, repeated, /*formatEach=*/ null));
       }
       return this;
     }
 
-    public Builder addBeforeEachExecPath(String repeated, Iterable<Artifact> artifacts) {
-      if (repeated != null && artifacts != null) {
+    public Builder addBeforeEachExecPath(String repeated, @Nullable Iterable<Artifact> artifacts) {
+      Preconditions.checkNotNull(repeated);
+      if (artifacts != null && !Iterables.isEmpty(artifacts)) {
         arguments.add(InterspersingArgs.fromExecPaths(artifacts, repeated, /*formatEach=*/ null));
       }
       return this;
     }
 
-    public Builder addFormatEach(String format, Iterable<String> strings) {
-      if (format != null && strings != null) {
+    public Builder addFormatEach(String format, @Nullable Iterable<String> strings) {
+      Preconditions.checkNotNull(format);
+      if (strings != null && !Iterables.isEmpty(strings)) {
         arguments.add(InterspersingArgs.fromStrings(strings, /*beforeEach=*/null, format));
       }
       return this;
     }
 
-    public Builder add(CustomArgv arg) {
+    public Builder add(@Nullable CustomArgv arg) {
       if (arg != null) {
         arguments.add(arg);
       }
       return this;
     }
 
-    public Builder add(CustomMultiArgv arg) {
+    public Builder add(@Nullable CustomMultiArgv arg) {
       if (arg != null) {
         arguments.add(arg);
       }
@@ -683,13 +578,12 @@ public final class CustomCommandLine extends CommandLine {
 
   private final ImmutableList<Object> arguments;
 
-
   /**
    * A map between enclosed TreeArtifacts and their associated {@link TreeFileArtifacts} for
    * substitution.
    *
-   * <p> This map is used to support TreeArtifact substitutions in
-   * {@link TreeFileArtifactArgvFragment}s.
+   * <p>This map is used to support TreeArtifact substitutions in {@link
+   * TreeFileArtifactExecPathArg}s.
    */
   private final Map<Artifact, TreeFileArtifact> substitutionMap;
 
@@ -706,8 +600,8 @@ public final class CustomCommandLine extends CommandLine {
 
   /**
    * Given the list of {@link TreeFileArtifact}s, returns another CustomCommandLine that replaces
-   * their parent TreeArtifacts with the TreeFileArtifacts in all
-   * {@link TreeFileArtifactArgvFragment} argument objects.
+   * their parent TreeArtifacts with the TreeFileArtifacts in all {@link
+   * TreeFileArtifactExecPathArg} argument objects.
    */
   @VisibleForTesting
   public CustomCommandLine evaluateTreeFileArtifacts(Iterable<TreeFileArtifact> treeFileArtifacts) {
@@ -750,14 +644,14 @@ public final class CustomCommandLine extends CommandLine {
   }
 
   /**
-   * If the given arg is a {@link TreeFileArtifactArgvFragment} and we have its associated
+   * If the given arg is a {@link TreeFileArtifactExecPathArg} and we have its associated
    * TreeArtifact substitution map, returns another argument object that has its enclosing
    * TreeArtifact substituted by one of its {@link TreeFileArtifact}. Otherwise, returns the given
    * arg unmodified.
    */
   private Object substituteTreeFileArtifactArgvFragment(Object arg) {
-    if (arg instanceof TreeFileArtifactArgvFragment) {
-      TreeFileArtifactArgvFragment argvFragment = (TreeFileArtifactArgvFragment) arg;
+    if (arg instanceof TreeFileArtifactExecPathArg) {
+      TreeFileArtifactExecPathArg argvFragment = (TreeFileArtifactExecPathArg) arg;
       return argvFragment.substituteTreeArtifact(
           Preconditions.checkNotNull(substitutionMap, argvFragment));
     } else {