Clean up BaseJavaCompilationHelper a bit.

- move constant to top of class
- extract a method to get the ijar FilesToRunProvider
- move some methods to JavaCompilationHelper that are only used from there
- inline derivedArtifact, which only had a single call site
- make the other derivedArtifact method private, since it's only used here

--
MOS_MIGRATED_REVID=114519285

2 files changed, 56 insertions, 58 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/BaseJavaCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/BaseJavaCompilationHelper.java
index 85573f8e3d..c586c6a42f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/BaseJavaCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/BaseJavaCompilationHelper.java
@@ -17,7 +17,6 @@ package com.google.devtools.build.lib.rules.java;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
-import com.google.devtools.build.lib.actions.Root;
 import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
 import com.google.devtools.build.lib.analysis.AnalysisUtils;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
@@ -45,7 +44,13 @@ public class BaseJavaCompilationHelper {
    */
   private static final String SINGLEJAR_MAX_MEMORY = "-Xmx256m";
 
-  private final RuleContext ruleContext;
+  private static final ImmutableList<String> SOURCE_JAR_COMMAND_LINE_ARGS = ImmutableList.of(
+      "--compression",
+      "--normalize",
+      "--exclude_build_data",
+      "--warn_duplicate_resources");
+
+  protected final RuleContext ruleContext;
 
   public BaseJavaCompilationHelper(RuleContext ruleContext) {
     this.ruleContext = ruleContext;
@@ -61,12 +66,6 @@ public class BaseJavaCompilationHelper {
     return AnalysisUtils.getMiddlemanFor(ruleContext, ":host_jdk");
   }
 
-  private static final ImmutableList<String> SOURCE_JAR_COMMAND_LINE_ARGS = ImmutableList.of(
-      "--compression",
-      "--normalize",
-      "--exclude_build_data",
-      "--warn_duplicate_resources");
-
   private CommandLine sourceJarCommandLine(Artifact outputJar,
       Map<PathFragment, Artifact> resources, Iterable<Artifact> resourceJars) {
     CustomCommandLine.Builder args = CustomCommandLine.builder();
@@ -93,7 +92,7 @@ public class BaseJavaCompilationHelper {
         .addOutput(outputJar)
         .addInputs(resources.values())
         .addInputs(resourceJars)
-        .addTransitiveInputs(JavaCompilationHelper.getHostJavabaseInputs(ruleContext))
+        .addTransitiveInputs(getHostJavabaseInputs(ruleContext))
         .setJarExecutable(
             ruleContext.getHostConfiguration().getFragment(Jvm.class).getJavaExecutable(),
             ruleContext.getPrerequisiteArtifact("$singlejar", Mode.HOST),
@@ -119,6 +118,10 @@ public class BaseJavaCompilationHelper {
     return ruleContext.getPrerequisiteArtifact("$javabuilder", Mode.HOST);
   }
 
+  protected FilesToRunProvider getIJar() {
+    return ruleContext.getExecutablePrerequisite("$ijar", Mode.HOST);
+  }
+
   /**
    * Returns the instrumentation jar in the given semantics.
    */
@@ -161,10 +164,9 @@ public class BaseJavaCompilationHelper {
    *     name of the current rule
    * @return the Artifact to create with the Action
    */
-  protected Artifact createIjarAction(final Artifact inputJar, boolean addPrefix) {
+  protected Artifact createIjarAction(Artifact inputJar, boolean addPrefix) {
     Artifact interfaceJar = getIjarArtifact(inputJar, addPrefix);
-    final FilesToRunProvider ijarTarget =
-        ruleContext.getExecutablePrerequisite("$ijar", Mode.HOST);
+    FilesToRunProvider ijarTarget = getIJar();
     if (!ruleContext.hasErrors()) {
       ruleContext.registerAction(new SpawnAction.Builder()
           .addInput(inputJar)
@@ -179,15 +181,6 @@ public class BaseJavaCompilationHelper {
     return interfaceJar;
   }
 
-  protected final JavaCompileAction.Builder createJavaCompileActionBuilder(
-      JavaSemantics semantics) {
-    JavaCompileAction.Builder builder = new JavaCompileAction.Builder(ruleContext, semantics);
-    builder.setJavaExecutable(
-        ruleContext.getHostConfiguration().getFragment(Jvm.class).getJavaExecutable());
-    builder.setJavaBaseInputs(BaseJavaCompilationHelper.getHostJavabaseInputs(ruleContext));
-    return builder;
-  }
-
   public RuleContext getRuleContext() {
     return ruleContext;
   }
@@ -205,47 +198,14 @@ public class BaseJavaCompilationHelper {
   }
 
   /**
-   * Produces a derived directory where source files generated by annotation processors should be
-   * stored.
-   */
-  protected PathFragment sourceGenDir(Artifact outputJar) {
-    return workDir(outputJar, "_sourcegenfiles");
-  }
-
-  protected PathFragment tempDir(Artifact outputJar) {
-    return workDir(outputJar, "_temp");
-  }
-
-  protected PathFragment classDir(Artifact outputJar) {
-    return workDir(outputJar, "_classes");
-  }
-
-  /**
-   * For an output jar and a suffix, produces a derived directory under
-   * {@code bin} directory with a given suffix.
-   *
-   * <p>Note that this won't work if a rule produces two jars with the same basename.
-   */
-  private PathFragment workDir(Artifact outputJar, String suffix) {
-    String basename = FileSystemUtils.removeExtension(outputJar.getExecPath().getBaseName());
-    return getConfiguration().getBinDirectory().getExecPath()
-        .getRelative(ruleContext.getUniqueDirectory("_javac"))
-        .getRelative(basename + suffix);
-  }
-
-  /**
    * Creates a derived artifact from the given artifact by adding the given
    * prefix and removing the extension and replacing it by the given suffix.
    * The new artifact will have the same root as the given one.
    */
-  protected Artifact derivedArtifact(Artifact artifact, String prefix, String suffix) {
-    return derivedArtifact(artifact, prefix, suffix, artifact.getRoot());
-  }
-
-  protected Artifact derivedArtifact(Artifact artifact, String prefix, String suffix, Root root) {
+  private Artifact derivedArtifact(Artifact artifact, String prefix, String suffix) {
     PathFragment path = artifact.getRootRelativePath();
     String basename = FileSystemUtils.removeExtension(path.getBaseName()) + suffix;
     path = path.replaceName(prefix + basename);
-    return ruleContext.getDerivedArtifact(path, root);
+    return ruleContext.getDerivedArtifact(path, artifact.getRoot());
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java
index 04aa86392c..5500cd7226 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java
@@ -49,7 +49,7 @@ import javax.annotation.Nullable;
  * <p>
  * Also supports the creation of resource and source only Jars.
  */
-public class JavaCompilationHelper extends BaseJavaCompilationHelper {
+public final class JavaCompilationHelper extends BaseJavaCompilationHelper {
 
   /**
    * Maximum memory to use for GenClass for generating the gen jar.
@@ -250,7 +250,7 @@ public class JavaCompilationHelper extends BaseJavaCompilationHelper {
       .addInput(manifestProto)
       .addInput(classJar)
       .addOutput(genClassJar)
-      .addTransitiveInputs(JavaCompilationHelper.getHostJavabaseInputs(getRuleContext()))
+      .addTransitiveInputs(getHostJavabaseInputs(getRuleContext()))
       .setJarExecutable(
           getRuleContext().getHostConfiguration().getFragment(Jvm.class).getJavaExecutable(),
           getRuleContext().getPrerequisiteArtifact("$genclass", Mode.HOST),
@@ -333,6 +333,44 @@ public class JavaCompilationHelper extends BaseJavaCompilationHelper {
     return resourceJar;
   }
 
+  private JavaCompileAction.Builder createJavaCompileActionBuilder(
+      JavaSemantics semantics) {
+    JavaCompileAction.Builder builder = new JavaCompileAction.Builder(ruleContext, semantics);
+    builder.setJavaExecutable(
+        ruleContext.getHostConfiguration().getFragment(Jvm.class).getJavaExecutable());
+    builder.setJavaBaseInputs(BaseJavaCompilationHelper.getHostJavabaseInputs(ruleContext));
+    return builder;
+  }
+
+  /**
+   * Produces a derived directory where source files generated by annotation processors should be
+   * stored.
+   */
+  private PathFragment sourceGenDir(Artifact outputJar) {
+    return workDir(outputJar, "_sourcegenfiles");
+  }
+
+  private PathFragment tempDir(Artifact outputJar) {
+    return workDir(outputJar, "_temp");
+  }
+
+  private PathFragment classDir(Artifact outputJar) {
+    return workDir(outputJar, "_classes");
+  }
+
+  /**
+   * For an output jar and a suffix, produces a derived directory under
+   * {@code bin} directory with a given suffix.
+   *
+   * <p>Note that this won't work if a rule produces two jars with the same basename.
+   */
+  private PathFragment workDir(Artifact outputJar, String suffix) {
+    String basename = FileSystemUtils.removeExtension(outputJar.getExecPath().getBaseName());
+    return getConfiguration().getBinDirectory().getExecPath()
+        .getRelative(ruleContext.getUniqueDirectory("_javac"))
+        .getRelative(basename + suffix);
+  }
+
   /**
    * Creates an Action that packages the Java source files into a Jar.  If {@code gensrcJar} is
    * non-null, includes the contents of the {@code gensrcJar} with the output source jar.