Review a number of action subclasses and update them according to the spec.

Consists of adding @Immutable annotations, adding final modifiers, and changing
the types of fields to immutable types.

--
MOS_MIGRATED_REVID=120221067

28 files changed, 116 insertions, 68 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java b/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java
index b0b163e396..82e67978e0 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionOwner.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.actions;
 
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.util.Preconditions;
 
@@ -26,6 +27,7 @@ import javax.annotation.Nullable;
  * but to avoid storing heavyweight analysis objects in actions, and to avoid coupling between the
  * analysis and actions packages, the RuleConfiguredTarget provides an instance of this class.
  */
+@Immutable
 public final class ActionOwner {
   /** An action owner for special cases. Usage is strongly discouraged. */
   public static final ActionOwner SYSTEM_ACTION_OWNER =
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 57e680824e..0b1248c538 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -131,7 +131,7 @@ public final class RuleContext extends TargetContext
      * Targets from FilesetEntry.files, or null if the user omitted it.
      */
     @Nullable
-    public List<TransitiveInfoCollection> getFiles() {
+    public ImmutableList<TransitiveInfoCollection> getFiles() {
       return files;
     }
   }
@@ -782,7 +782,7 @@ public final class RuleContext extends TargetContext
    * @param attributeName the name of the attribute to process
    * @return a list of strings containing the expanded and tokenized values for the attribute
    */
-  public List<String> getTokenizedStringListAttr(String attributeName) {
+  public ImmutableList<String> getTokenizedStringListAttr(String attributeName) {
     return getExpandedStringListAttr(attributeName, Tokenize.YES);
   }
 
@@ -793,7 +793,7 @@ public final class RuleContext extends TargetContext
    * @param attributeName the name of the attribute to process
    * @return a list of strings containing the processed values for the attribute
    */
-  public List<String> getExpandedStringListAttr(String attributeName, Tokenize tokenize) {
+  public ImmutableList<String> getExpandedStringListAttr(String attributeName, Tokenize tokenize) {
     if (!getRule().isAttrDefined(attributeName, Type.STRING_LIST)) {
       // TODO(bazel-team): This should be an error.
       return ImmutableList.of();
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java b/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java
index 04792e8db4..f1e633967f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java
@@ -21,6 +21,7 @@ import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -49,12 +50,14 @@ import javax.annotation.Nullable;
  * <p>Note that this action carefully avoids building the manifest content in
  * memory.
  */
-public class SourceManifestAction extends AbstractFileWriteAction {
+@Immutable // if all ManifestWriter implementations are immutable
+public final class SourceManifestAction extends AbstractFileWriteAction {
 
   private static final String GUID = "07459553-a3d0-4d37-9d78-18ed942470f4";
 
   /**
-   * Interface for defining manifest formatting and reporting specifics.
+   * Interface for defining manifest formatting and reporting specifics. Implementations must be
+   * immutable.
    */
   @VisibleForTesting
   interface ManifestWriter {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/BinaryFileWriteAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/BinaryFileWriteAction.java
index 3e235bb0e1..d87772d61f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/BinaryFileWriteAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/BinaryFileWriteAction.java
@@ -20,6 +20,7 @@ import com.google.common.io.ByteStreams;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.util.Preconditions;
 
@@ -30,6 +31,7 @@ import java.io.OutputStream;
 /**
  * Action to write a binary file.
  */
+@Immutable // if source is immutable
 public final class BinaryFileWriteAction extends AbstractFileWriteAction {
 
   private static final String GUID = "eeee07fe-4b40-11e4-82d6-eba0b4f713e2";
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java
index 1907f7c223..23c1484ba8 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/FileWriteAction.java
@@ -20,6 +20,7 @@ import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.util.Fingerprint;
 
 import java.io.IOException;
@@ -31,7 +32,8 @@ import java.util.Collection;
  * <p>TODO(bazel-team): Choose a better name to distinguish this class from
  * {@link BinaryFileWriteAction}.
  */
-public class FileWriteAction extends AbstractFileWriteAction {
+@Immutable // if fileContents is immutable
+public final class FileWriteAction extends AbstractFileWriteAction {
 
   private static final String GUID = "332877c7-ca9f-4731-b387-54f620408522";
 
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/ParameterFileWriteAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/ParameterFileWriteAction.java
index 939d9fc41e..2a1c9b8e21 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/ParameterFileWriteAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/ParameterFileWriteAction.java
@@ -20,6 +20,7 @@ import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.util.ShellEscaper;
 
@@ -31,6 +32,7 @@ import java.nio.charset.Charset;
 /**
  * Action to write a parameter file for a {@link CommandLine}.
  */
+@Immutable // if commandLine and charset are immutable
 public final class ParameterFileWriteAction extends AbstractFileWriteAction {
 
   private static final String GUID = "45f678d8-e395-401e-8446-e795ccc6361f";
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java
index 10d4d7285b..fe404e1168 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java
@@ -18,10 +18,12 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
 import com.google.common.base.Objects;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.util.ResourceFileLoader;
 import com.google.devtools.build.lib.util.StringUtilities;
@@ -34,12 +36,12 @@ import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 import java.util.Collection;
 import java.util.List;
-import java.util.Map;
 
 /**
  * Action to expand a template and write the expanded content to a file.
  */
-public class TemplateExpansionAction extends AbstractFileWriteAction {
+@Immutable // if all substitutions are immutable
+public final class TemplateExpansionAction extends AbstractFileWriteAction {
 
   private static final String GUID = "786c1fe0-dca8-407a-b108-e1ecd6d1bc7f";
 
@@ -52,6 +54,7 @@ public class TemplateExpansionAction extends AbstractFileWriteAction {
    * <p>It should be assumed that the {@link #getKey} invocation is cheap, and
    * that the {@link #getValue} invocation is expensive.
    */
+  @Immutable // if the keys and values in the passed in lists and maps are all immutable
   public abstract static class Substitution {
     private Substitution() {
     }
@@ -80,7 +83,8 @@ public class TemplateExpansionAction extends AbstractFileWriteAction {
      * Returns an immutable Substitution instance for the key and list of values. The
      * values will be joined by spaces before substitution.
      */
-    public static Substitution ofSpaceSeparatedList(final String key, final List<?> value) {
+    public static Substitution ofSpaceSeparatedList(
+        final String key, final ImmutableList<?> value) {
       return new Substitution() {
         @Override
         public String getKey() {
@@ -101,7 +105,8 @@ public class TemplateExpansionAction extends AbstractFileWriteAction {
      *
      * <p>For example, the map <(a,1), (b,2), (c,3)> will become "a=1 b=2 c=3".
      */
-    public static Substitution ofSpaceSeparatedMap(final String key, final Map<?, ?> value) {
+    public static Substitution ofSpaceSeparatedMap(
+        final String key, final ImmutableMap<?, ?> value) {
       return new Substitution() {
         @Override
         public String getKey() {
@@ -164,6 +169,7 @@ public class TemplateExpansionAction extends AbstractFileWriteAction {
    * A template that contains text content, or alternatively throws an {@link
    * IOException}.
    */
+  @Immutable // all subclasses are immutable
   public abstract static class Template {
 
     private static final Charset DEFAULT_CHARSET = StandardCharsets.UTF_8;
@@ -258,7 +264,7 @@ public class TemplateExpansionAction extends AbstractFileWriteAction {
   }
 
   private final Template template;
-  private final List<Substitution> substitutions;
+  private final ImmutableList<Substitution> substitutions;
 
   /**
    * Creates a new TemplateExpansionAction instance.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 349d6d451d..fd7671d06e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -2200,7 +2200,7 @@ public final class BuildConfiguration {
    * Returns user-specified test environment variables and their values, as
    * set by the --test_env options.
    */
-  public Map<String, String> getTestEnv() {
+  public ImmutableMap<String, String> getTestEnv() {
     return testEnvironment;
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
index b140349dbd..fae681b68e 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
@@ -181,7 +181,7 @@ public class BazelJavaSemantics implements JavaSemantics {
 
     arguments.add(Substitution.of("%java_start_class%",
         ShellEscaper.escapeString(javaStartClass)));
-    arguments.add(Substitution.ofSpaceSeparatedList("%jvm_flags%", jvmFlags));
+    arguments.add(Substitution.ofSpaceSeparatedList("%jvm_flags%", ImmutableList.copyOf(jvmFlags)));
 
     ruleContext.registerAction(new TemplateExpansionAction(
         ruleContext.getActionOwner(), executable, STUB_SCRIPT, arguments, true));
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 7900cab618..69abf331cb 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
@@ -34,6 +34,9 @@ import java.util.List;
  * @see CollectionUtils#checkImmutable(Iterable)
  */
 public final class IterablesChain<T> implements Iterable<T> {
+  public static <T> Iterable<T> concat(Iterable<? extends T> a, Iterable<? extends T> b) {
+    return IterablesChain.<T>builder().add(a).add(b).build();
+  }
 
   private final Iterable<T> chain;
 
@@ -61,7 +64,7 @@ public final class IterablesChain<T> implements Iterable<T> {
  *
    */
   public static class Builder<T> {
-    private List<Iterable<T>> iterables = new ArrayList<>();
+    private List<Iterable<? extends T>> iterables = new ArrayList<>();
     private boolean deduplicate;
 
     private Builder() {
@@ -72,7 +75,7 @@ public final class IterablesChain<T> implements Iterable<T> {
      *
      * <p>If the iterable can not be confirmed to be immutable, a runtime error is thrown.
      */
-    public Builder<T> add(Iterable<T> iterable) {
+    public Builder<T> add(Iterable<? extends T> iterable) {
       CollectionUtils.checkImmutable(iterable);
       if (!Iterables.isEmpty(iterable)) {
         iterables.add(iterable);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
index 48a8a931cd..8c9fc5f2e0 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
@@ -41,6 +41,7 @@ import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.analysis.actions.SpawnAction.Builder;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.collect.IterablesChain;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
@@ -769,7 +770,7 @@ public abstract class AndroidBinary implements RuleConfiguredTargetFactory {
       ResourceApk resourceApk,
       NativeLibs nativeLibs) {
 
-    Iterable<Artifact> jars = Iterables.concat(
+    Iterable<Artifact> jars = IterablesChain.concat(
         resourceClasses.getArchiveInputs(true), androidCommon.getRuntimeJars());
 
     AndroidSdkProvider sdk = AndroidSdkProvider.fromRuleContext(ruleContext);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java
index fcd16afc46..9adf9eb8af 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidResourcesProvider.java
@@ -103,7 +103,7 @@ public final class AndroidResourcesProvider implements TransitiveInfoProvider {
     private final boolean manifestExported;
     private final Artifact javaSourceJar;
     private final Artifact rTxt;
-    private Artifact symbolsTxt;
+    private final Artifact symbolsTxt;
 
     public ResourceContainer(Label label,
         String javaPackage,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ApkManifestAction.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApkManifestAction.java
index 8218156465..d78f976ccb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ApkManifestAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ApkManifestAction.java
@@ -22,6 +22,8 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.cache.MetadataHandler;
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
 import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction;
+import com.google.devtools.build.lib.collect.CollectionUtils;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.rules.android.apkmanifest.ApkManifestOuterClass;
 import com.google.devtools.build.lib.rules.android.apkmanifest.ApkManifestOuterClass.ApkManifest;
 import com.google.devtools.build.lib.util.Fingerprint;
@@ -33,7 +35,8 @@ import java.io.OutputStream;
 import java.io.PrintStream;
 import java.util.Map;
 
-public class ApkManifestAction extends AbstractFileWriteAction {
+@Immutable
+public final class ApkManifestAction extends AbstractFileWriteAction {
 
   private static Iterable<Artifact> makeInputs(
       AndroidSdkProvider sdk,
@@ -92,8 +95,8 @@ public class ApkManifestAction extends AbstractFileWriteAction {
       Iterable<Artifact> jars,
       ResourceApk resourceApk,
       NativeLibs nativeLibs) {
-
     super(owner, makeInputs(sdk, jars, resourceApk, nativeLibs), outputFile, false);
+    CollectionUtils.checkImmutable(jars);
     this.textOutput = textOutput;
     this.sdk = sdk;
     this.jars = jars;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java b/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java
index 78c8db2cde..dbcf0bfa98 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/NativeLibs.java
@@ -26,6 +26,7 @@ import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.rules.cpp.CcLinkParams;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
 import com.google.devtools.build.lib.rules.cpp.CppFileTypes;
@@ -43,6 +44,7 @@ import java.util.Set;
 import javax.annotation.Nullable;
 
 /** Represents the collection of native libraries (.so) to be installed in the APK. */
+@Immutable
 public final class NativeLibs {
   public static final NativeLibs EMPTY =
       new NativeLibs(ImmutableMap.<String, Iterable<Artifact>>of(), null);
@@ -105,10 +107,10 @@ public final class NativeLibs {
   }
 
   // Map from architecture (CPU folder to place the library in) to libraries for that CPU
-  private final Map<String, Iterable<Artifact>> nativeLibs;
+  private final ImmutableMap<String, Iterable<Artifact>> nativeLibs;
   private final Artifact nativeLibsName;
 
-  private NativeLibs(Map<String, Iterable<Artifact>> nativeLibs, Artifact nativeLibsName) {
+  private NativeLibs(ImmutableMap<String, Iterable<Artifact>> nativeLibs, Artifact nativeLibsName) {
     this.nativeLibs = nativeLibs;
     this.nativeLibsName = nativeLibsName;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java
index 8a14507c95..0b7bbfc4fc 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceApk.java
@@ -25,7 +25,7 @@ import javax.annotation.Nullable;
  * unsigned APKs.
  */
 @Immutable
-public class ResourceApk {
+public final class ResourceApk {
   // TODO(bazel-team): The only field that is legitimately nullable is javaSrcJar. The rest is
   // marked as such due to .fromTransitiveResources(). It seems like there should be a better way
   // to do this.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java
index dfd4df3f79..f8309de42e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ResourceDependencies.java
@@ -20,6 +20,7 @@ import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.rules.android.AndroidResourcesProvider.ResourceContainer;
 
@@ -32,7 +33,8 @@ import com.google.devtools.build.lib.rules.android.AndroidResourcesProvider.Reso
  * library is included in both the transitive and direct dependencies, it will appear twice. This
  * requires consumers to manage duplicated resources gracefully.
  */
-public class ResourceDependencies {
+@Immutable
+public final class ResourceDependencies {
   /**
    * Contains all the transitive resources that are not generated by the direct ancestors of the
    * current rule.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/WriteAdbArgsAction.java b/src/main/java/com/google/devtools/build/lib/rules/android/WriteAdbArgsAction.java
index 0adf00840d..6ddf13da19 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/WriteAdbArgsAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/WriteAdbArgsAction.java
@@ -19,6 +19,7 @@ import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.common.options.EnumConverter;
 import com.google.devtools.common.options.Option;
@@ -33,7 +34,8 @@ import java.util.List;
  * An action that writes the a parameter file to {@code incremental_install.py} based on the command
  * line arguments to {@code blaze mobile-install}.
  */
-public class WriteAdbArgsAction extends AbstractFileWriteAction {
+@Immutable // note that it accesses data non-hermetically during the execution phase
+public final class WriteAdbArgsAction extends AbstractFileWriteAction {
   private static final String GUID = "16720416-3c01-4b0a-a543-ead7e563a1ca";
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMap.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMap.java
index 651eed48ef..aa365d4b1b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMap.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMap.java
@@ -20,7 +20,7 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
  * Structure for C++ module maps. Stores the name of the module and a .cppmap artifact.
  */
 @Immutable
-public class CppModuleMap {
+public final class CppModuleMap {
   // NOTE: If you add a field here, you'll likely need to update CppModuleMapAction.computeKey().
   private final Artifact artifact;
   private final String name;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java
index 71e491d3e1..3fea2c4c0a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModuleMapAction.java
@@ -21,6 +21,7 @@ import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ResourceSet;
 import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
@@ -36,7 +37,8 @@ import java.util.List;
  * Creates C++ module map artifact genfiles. These are then passed to Clang to
  * do dependency checking.
  */
-public class CppModuleMapAction extends AbstractFileWriteAction {
+@Immutable
+public final class CppModuleMapAction extends AbstractFileWriteAction {
 
   private static final String GUID = "4f407081-1951-40c1-befc-d6b4daff5de3";
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/WriteBuildInfoHeaderAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/WriteBuildInfoHeaderAction.java
index b0151eb8b4..1edef180e3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/WriteBuildInfoHeaderAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/WriteBuildInfoHeaderAction.java
@@ -23,6 +23,7 @@ import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.WorkspaceStatusAction;
 import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.util.Preconditions;
 
@@ -38,6 +39,7 @@ import java.util.Map;
  * An action that creates a C++ header containing the build information in the
  * form of #define directives.
  */
+@Immutable
 public final class WriteBuildInfoHeaderAction extends AbstractFileWriteAction {
   private static final String GUID = "b0798174-1352-4a54-854a-9785aaea491b";
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionInfoFileWriteAction.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionInfoFileWriteAction.java
index bc3f79d61c..817234c1a4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionInfoFileWriteAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraActionInfoFileWriteAction.java
@@ -21,6 +21,7 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
 import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.util.Preconditions;
 
@@ -32,11 +33,12 @@ import java.io.OutputStream;
  * .xa file for use by an extra action. This can only be done at execution time because actions may
  * store information only known at execution time into the protocol buffer.
  */
-public class ExtraActionInfoFileWriteAction extends AbstractFileWriteAction {
-  private final Action shadowedAction;
-
+@Immutable // if shadowedAction is immutable
+public final class ExtraActionInfoFileWriteAction extends AbstractFileWriteAction {
   private static final String UUID = "1759f81d-e72e-477d-b182-c4532bdbaeeb";
 
+  private final Action shadowedAction;
+
   ExtraActionInfoFileWriteAction(ActionOwner owner, Artifact extraActionInfoFile,
       Action shadowedAction) {
     super(owner, ImmutableList.<Artifact>of(), extraActionInfoFile, false);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genquery/BUILD b/src/main/java/com/google/devtools/build/lib/rules/genquery/BUILD
index b6c9ddeb2c..46edcfae09 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genquery/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/rules/genquery/BUILD
@@ -12,6 +12,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib:build-base",
         "//src/main/java/com/google/devtools/build/lib:collect",
         "//src/main/java/com/google/devtools/build/lib:common",
+        "//src/main/java/com/google/devtools/build/lib:concurrent",
         "//src/main/java/com/google/devtools/build/lib:events",
         "//src/main/java/com/google/devtools/build/lib:packages-internal",
         "//src/main/java/com/google/devtools/build/lib:util",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
index b61d6ac59b..3f2cd64703 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
@@ -22,6 +22,7 @@ import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
+import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
@@ -37,6 +38,7 @@ import com.google.devtools.build.lib.cmdline.TargetPattern;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
@@ -82,7 +84,6 @@ import java.io.OutputStream;
 import java.io.PrintStream;
 import java.nio.channels.ClosedByInterruptException;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -150,25 +151,7 @@ public class GenQuery implements RuleConfiguredTargetFactory {
     }
 
     ruleContext.registerAction(
-        new AbstractFileWriteAction(
-            ruleContext.getActionOwner(), Collections.<Artifact>emptySet(), outputArtifact, false) {
-          @Override
-          public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) {
-            return new DeterministicWriter() {
-              @Override
-              public void writeOutputFile(OutputStream out) throws IOException {
-                out.write(result);
-              }
-            };
-          }
-
-          @Override
-          protected String computeKey() {
-            Fingerprint f = new Fingerprint();
-            f.addBytes(result);
-            return f.hexDigestAndReset();
-          }
-        });
+        new QueryResultAction(ruleContext.getActionOwner(), outputArtifact, result));
 
     NestedSet<Artifact> filesToBuild = NestedSetBuilder.create(Order.STABLE_ORDER, outputArtifact);
     return new RuleConfiguredTargetBuilder(ruleContext)
@@ -329,6 +312,33 @@ public class GenQuery implements RuleConfiguredTargetFactory {
     return outputStream.toByteArray();
   }
 
+  @Immutable // assuming no other reference to result
+  private static final class QueryResultAction extends AbstractFileWriteAction {
+    private final byte[] result;
+
+    private QueryResultAction(ActionOwner owner, Artifact output, byte[] result) {
+      super(owner, ImmutableList.<Artifact>of(), output, /*makeExecutable=*/false);
+      this.result = result;
+    }
+
+    @Override
+    public DeterministicWriter newDeterministicWriter(ActionExecutionContext ctx) {
+      return new DeterministicWriter() {
+        @Override
+        public void writeOutputFile(OutputStream out) throws IOException {
+          out.write(result);
+        }
+      };
+    }
+
+    @Override
+    protected String computeKey() {
+      Fingerprint f = new Fingerprint();
+      f.addBytes(result);
+      return f.hexDigestAndReset();
+    }
+  }
+
   /**
    * Provide target pattern evaluation to the query operations using Skyframe dep lookup. For thread
    * safety, we must synchronize access to the SkyFunction.Environment.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java
index fd94eab868..00e54eae1b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/TestSupport.java
@@ -79,7 +79,7 @@ public class TestSupport {
     String runMemleaks =
         ruleContext.getFragment(ObjcConfiguration.class).runMemleaks() ? "true" : "false";
 
-    Map<String, String> testEnv = ruleContext.getConfiguration().getTestEnv();
+    ImmutableMap<String, String> testEnv = ruleContext.getConfiguration().getTestEnv();
 
     // The substitutions below are common for simulator and lab device.
     ImmutableList.Builder<Substitution> substitutions =
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/BaselineCoverageAction.java b/src/main/java/com/google/devtools/build/lib/rules/test/BaselineCoverageAction.java
index 20ae43eb27..99626d2118 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/BaselineCoverageAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/BaselineCoverageAction.java
@@ -29,6 +29,7 @@ import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
@@ -42,12 +43,13 @@ import java.util.List;
  * Generates baseline (empty) coverage for the given non-test target.
  */
 @VisibleForTesting
+@Immutable
 public final class BaselineCoverageAction extends AbstractFileWriteAction
     implements NotifyOnActionCacheHit {
-  private final Iterable<Artifact> instrumentedFiles;
+  private final NestedSet<Artifact> instrumentedFiles;
 
   private BaselineCoverageAction(
-      ActionOwner owner, Iterable<Artifact> instrumentedFiles, Artifact output) {
+      ActionOwner owner, NestedSet<Artifact> instrumentedFiles, Artifact output) {
     super(owner, ImmutableList.<Artifact>of(), output, false);
     this.instrumentedFiles = instrumentedFiles;
   }
@@ -110,7 +112,8 @@ public final class BaselineCoverageAction extends AbstractFileWriteAction
    * Returns collection of baseline coverage artifacts associated with the given target.
    * Will always return 0 or 1 elements.
    */
-  static NestedSet<Artifact> create(RuleContext ruleContext, Iterable<Artifact> instrumentedFiles) {
+  static NestedSet<Artifact> create(
+      RuleContext ruleContext, NestedSet<Artifact> instrumentedFiles) {
     // Baseline coverage artifacts will still go into "testlogs" directory.
     Artifact coverageData = ruleContext.getPackageRelativeArtifact(
         new PathFragment(ruleContext.getTarget().getName()).getChild("baseline_coverage.dat"),
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFileManifestAction.java b/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFileManifestAction.java
index 96224f0fb7..4f1884b25f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFileManifestAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/InstrumentedFileManifestAction.java
@@ -24,6 +24,7 @@ import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.util.Fingerprint;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.util.RegexFilter;
@@ -33,21 +34,20 @@ import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.io.Writer;
 import java.util.Arrays;
-import java.util.Collection;
 
 /**
  * Creates instrumented file manifest to list instrumented source files.
  */
-class InstrumentedFileManifestAction extends AbstractFileWriteAction {
-
+@Immutable
+final class InstrumentedFileManifestAction extends AbstractFileWriteAction {
   private static final String GUID = "d9ddb800-f9a1-01Da-238d-988311a8475b";
 
-  private final Collection<Artifact> collectedSourceFiles;
-  private final Collection<Artifact> metadataFiles;
+  private final ImmutableList<Artifact> collectedSourceFiles;
+  private final ImmutableList<Artifact> metadataFiles;
   private final RegexFilter instrumentationFilter;
 
-  private InstrumentedFileManifestAction(ActionOwner owner, Collection<Artifact> inputs,
-      Collection<Artifact> additionalSourceFiles, Collection<Artifact> gcnoFiles,
+  private InstrumentedFileManifestAction(ActionOwner owner, ImmutableList<Artifact> inputs,
+      ImmutableList<Artifact> additionalSourceFiles, ImmutableList<Artifact> gcnoFiles,
       Artifact output, RegexFilter instrumentationFilter) {
     super(owner, inputs, output, false);
     this.collectedSourceFiles = additionalSourceFiles;
@@ -102,17 +102,14 @@ class InstrumentedFileManifestAction extends AbstractFileWriteAction {
    * @param metadataFiles *.gcno/*.em files collected by the {@link InstrumentedFilesCollector}
    * @return instrumented file manifest artifact
    */
-  public static Artifact getInstrumentedFileManifest(final RuleContext ruleContext,
-      final Collection<Artifact> additionalSourceFiles, final Collection<Artifact> metadataFiles) {
+  public static Artifact getInstrumentedFileManifest(RuleContext ruleContext,
+      ImmutableList<Artifact> additionalSourceFiles, ImmutableList<Artifact> metadataFiles) {
     // Instrumented manifest makes sense only for rules with binary output.
     Preconditions.checkState(ruleContext.getRule().hasBinaryOutput());
     Artifact instrumentedFileManifest = ruleContext.getPackageRelativeArtifact(
         ruleContext.getTarget().getName()  + ".instrumented_files",
         ruleContext.getConfiguration().getBinDirectory());
 
-    // Instrumented manifest artifact might already exist in case when multiple test
-    // actions that use slightly different subsets of runfiles set are generated for the same rule.
-    // So check whether we need to create a new action instance.
     ImmutableList<Artifact> inputs = ImmutableList.<Artifact>builder()
         .addAll(additionalSourceFiles)
         .addAll(metadataFiles)
diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java
index 856d41d49d..73235cdac2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/test/TestActionBuilder.java
@@ -39,7 +39,6 @@ import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.common.options.EnumConverter;
 
-import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 
@@ -204,7 +203,7 @@ public final class TestActionBuilder {
     if (collectCodeCoverage) {
       // Add instrumented file manifest artifact to the list of inputs. This file will contain
       // exec paths of all source files that should be included into the code coverage output.
-      Collection<Artifact> metadataFiles =
+      ImmutableList<Artifact> metadataFiles =
           ImmutableList.copyOf(instrumentedFiles.getInstrumentationMetadataFiles());
       inputsBuilder.addTransitive(NestedSetBuilder.wrap(Order.STABLE_ORDER, metadataFiles));
       for (TransitiveInfoCollection dep :
diff --git a/src/main/java/com/google/devtools/build/lib/util/RegexFilter.java b/src/main/java/com/google/devtools/build/lib/util/RegexFilter.java
index 8484f440ed..950e276de2 100644
--- a/src/main/java/com/google/devtools/build/lib/util/RegexFilter.java
+++ b/src/main/java/com/google/devtools/build/lib/util/RegexFilter.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.util;
 
 import com.google.common.base.Joiner;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.common.options.Converter;
 import com.google.devtools.common.options.OptionsParsingException;
 
@@ -33,7 +34,8 @@ import java.util.regex.PatternSyntaxException;
  * any of the excluded regex expressions and if it matches at least one
  * included regex expression.
  */
-public class RegexFilter implements Serializable {
+@Immutable
+public final class RegexFilter implements Serializable {
   private final Pattern inclusionPattern;
   private final Pattern exclusionPattern;
   private final int hashCode;