Brief audit of singleton SkyValues

Minimize indirection wrt singletons, turns out we had one that was completely unused.

--
MOS_MIGRATED_REVID=105621494

6 files changed, 32 insertions, 66 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunction.java
index 6d6511af55..09debbff4a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunction.java
@@ -41,7 +41,7 @@ public class ContainingPackageLookupFunction implements SkyFunction {
 
     PathFragment parentDir = dir.getPackageFragment().getParentDirectory();
     if (parentDir == null) {
-      return ContainingPackageLookupValue.noContainingPackage();
+      return ContainingPackageLookupValue.NONE;
     }
     PackageIdentifier parentId = PackageIdentifier.create(dir.getRepository(), parentDir);
     return env.getValue(ContainingPackageLookupValue.key(parentId));
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupValue.java
index 09731a2f06..70bdf0d07d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupValue.java
@@ -25,6 +25,9 @@ import com.google.devtools.build.skyframe.SkyValue;
  * a specific package.
  */
 public abstract class ContainingPackageLookupValue implements SkyValue {
+
+  public static final NoContainingPackage NONE = new NoContainingPackage();
+
   /** Returns whether there is a containing package. */
   public abstract boolean hasContainingPackage();
 
@@ -39,16 +42,14 @@ public abstract class ContainingPackageLookupValue implements SkyValue {
     return new SkyKey(SkyFunctions.CONTAINING_PACKAGE_LOOKUP, id);
   }
 
-  static ContainingPackageLookupValue noContainingPackage() {
-    return NoContainingPackage.INSTANCE;
-  }
-
   static ContainingPackageLookupValue withContainingPackage(PackageIdentifier pkgId, Path root) {
     return new ContainingPackage(pkgId, root);
   }
 
-  private static class NoContainingPackage extends ContainingPackageLookupValue {
-    private static final NoContainingPackage INSTANCE = new NoContainingPackage();
+  /** Value indicating there is no containing package. */
+  public static class NoContainingPackage extends ContainingPackageLookupValue {
+
+    private NoContainingPackage() {}
 
     @Override
     public boolean hasContainingPackage() {
@@ -66,7 +67,8 @@ public abstract class ContainingPackageLookupValue implements SkyValue {
     }
   }
 
-  private static class ContainingPackage extends ContainingPackageLookupValue {
+  /** A successful lookup value. */
+  public static class ContainingPackage extends ContainingPackageLookupValue {
     private final PackageIdentifier containingPackage;
     private final Path containingPackageRoot;
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileStateValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileStateValue.java
index 46a5734823..1a75e3b0de 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FileStateValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileStateValue.java
@@ -53,8 +53,10 @@ import javax.annotation.Nullable;
 @VisibleForTesting
 public abstract class FileStateValue implements SkyValue {
 
-  static final FileStateValue DIRECTORY_FILE_STATE_NODE = DirectoryFileStateValue.INSTANCE;
-  static final FileStateValue NONEXISTENT_FILE_STATE_NODE = NonexistentFileStateValue.INSTANCE;
+  public static final DirectoryFileStateValue DIRECTORY_FILE_STATE_NODE =
+      new DirectoryFileStateValue();
+  public static final NonexistentFileStateValue NONEXISTENT_FILE_STATE_NODE =
+      new NonexistentFileStateValue();
 
   enum Type {
     FILE,
@@ -131,7 +133,7 @@ public abstract class FileStateValue implements SkyValue {
    * where fast digest lookups are not available.
    */
   @ThreadSafe
-  private static final class FileFileStateValue extends FileStateValue {
+  public static final class FileFileStateValue extends FileStateValue {
     private final long size;
     // Only needed for empty-file equality-checking. Otherwise is always -1.
     // TODO(bazel-team): Consider getting rid of this special case for empty files.
@@ -229,9 +231,7 @@ public abstract class FileStateValue implements SkyValue {
   }
 
   /** Implementation of {@link FileStateValue} for directories that exist. */
-  private static final class DirectoryFileStateValue extends FileStateValue {
-
-    static final DirectoryFileStateValue INSTANCE = new DirectoryFileStateValue();
+  public static final class DirectoryFileStateValue extends FileStateValue {
 
     private DirectoryFileStateValue() {
     }
@@ -259,7 +259,7 @@ public abstract class FileStateValue implements SkyValue {
   }
 
   /** Implementation of {@link FileStateValue} for symlinks. */
-  private static final class SymlinkFileStateValue extends FileStateValue {
+  public static final class SymlinkFileStateValue extends FileStateValue {
 
     private final PathFragment symlinkTarget;
 
@@ -298,9 +298,7 @@ public abstract class FileStateValue implements SkyValue {
   }
 
   /** Implementation of {@link FileStateValue} for nonexistent files. */
-  private static final class NonexistentFileStateValue extends FileStateValue {
-
-    static final NonexistentFileStateValue INSTANCE = new NonexistentFileStateValue();
+  public static final class NonexistentFileStateValue extends FileStateValue {
 
     private NonexistentFileStateValue() {
     }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index 6ba2775e5a..50d5c8a403 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -382,7 +382,6 @@ public class PackageFunction implements SkyFunction {
       switch (packageLookupValue.getErrorReason()) {
         case NO_BUILD_FILE:
         case DELETED_PACKAGE:
-        case NO_EXTERNAL_PACKAGE:
           throw new PackageFunctionException(new BuildFileNotFoundException(packageId,
               packageLookupValue.getErrorMsg()), Transience.PERSISTENT);
         case INVALID_PACKAGE_NAME:
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
index 5c612ca1b8..b49742a3ac 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
@@ -66,7 +66,7 @@ public class PackageLookupFunction implements SkyFunction {
     }
 
     if (deletedPackages.get().contains(packageKey)) {
-      return PackageLookupValue.deletedPackage();
+      return PackageLookupValue.DELETED_PACKAGE_VALUE;
     }
 
     // TODO(bazel-team): The following is O(n^2) on the number of elements on the package path due
@@ -79,7 +79,7 @@ public class PackageLookupFunction implements SkyFunction {
         return value;
       }
     }
-    return PackageLookupValue.noBuildFile();
+    return PackageLookupValue.NO_BUILD_FILE_VALUE;
   }
 
   @Nullable
@@ -129,7 +129,7 @@ public class PackageLookupFunction implements SkyFunction {
     if (fileValue.isFile()) {
       return PackageLookupValue.success(buildFileRootedPath.getRoot());
     }
-    return PackageLookupValue.noBuildFile();
+    return PackageLookupValue.NO_BUILD_FILE_VALUE;
   }
 
   /**
@@ -169,7 +169,7 @@ public class PackageLookupFunction implements SkyFunction {
         return PackageLookupValue.success(repositoryValue.getPath());
       }
     }
-    return PackageLookupValue.noBuildFile();
+    return PackageLookupValue.NO_BUILD_FILE_VALUE;
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java
index 492fdfb254..7eb743f651 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupValue.java
@@ -36,6 +36,11 @@ import java.util.Objects;
  */
 public abstract class PackageLookupValue implements SkyValue {
 
+  public static final NoBuildFilePackageLookupValue NO_BUILD_FILE_VALUE =
+      new NoBuildFilePackageLookupValue();
+  public static final DeletedPackageLookupValue DELETED_PACKAGE_VALUE =
+      new DeletedPackageLookupValue();
+
   enum ErrorReason {
     // There is no BUILD file.
     NO_BUILD_FILE,
@@ -44,11 +49,7 @@ public abstract class PackageLookupValue implements SkyValue {
     INVALID_PACKAGE_NAME,
 
     // The package is considered deleted because of --deleted_packages.
-    DELETED_PACKAGE,
-
-    // The //external package could not be loaded, either because the WORKSPACE file could not be
-    // parsed or the packages it references cannot be loaded.
-    NO_EXTERNAL_PACKAGE
+    DELETED_PACKAGE
   }
 
   protected PackageLookupValue() {
@@ -67,22 +68,10 @@ public abstract class PackageLookupValue implements SkyValue {
     return new WorkspacePackageLookupValue(root);
   }
 
-  public static PackageLookupValue noBuildFile() {
-    return NoBuildFilePackageLookupValue.INSTANCE;
-  }
-
-  public static PackageLookupValue noExternalPackage() {
-    return NoExternalPackageLookupValue.INSTANCE;
-  }
-
   public static PackageLookupValue invalidPackageName(String errorMsg) {
     return new InvalidNamePackageLookupValue(errorMsg);
   }
 
-  public static PackageLookupValue deletedPackage() {
-    return DeletedPackageLookupValue.INSTANCE;
-  }
-
   public boolean isExternalPackage() {
     return false;
   }
@@ -225,10 +214,8 @@ public abstract class PackageLookupValue implements SkyValue {
     }
   }
 
-  private static class NoBuildFilePackageLookupValue extends UnsuccessfulPackageLookupValue {
-
-    public static final NoBuildFilePackageLookupValue INSTANCE =
-        new NoBuildFilePackageLookupValue();
+  /** Marker value for no build file found. */
+  public static class NoBuildFilePackageLookupValue extends UnsuccessfulPackageLookupValue {
 
     private NoBuildFilePackageLookupValue() {
     }
@@ -244,25 +231,6 @@ public abstract class PackageLookupValue implements SkyValue {
     }
   }
 
-  private static class NoExternalPackageLookupValue extends UnsuccessfulPackageLookupValue {
-
-    public static final NoExternalPackageLookupValue INSTANCE =
-        new NoExternalPackageLookupValue();
-
-    private NoExternalPackageLookupValue() {
-    }
-
-    @Override
-    ErrorReason getErrorReason() {
-      return ErrorReason.NO_EXTERNAL_PACKAGE;
-    }
-
-    @Override
-    String getErrorMsg() {
-      return "Error loading the //external package";
-    }
-  }
-
   private static class InvalidNamePackageLookupValue extends UnsuccessfulPackageLookupValue {
 
     private final String errorMsg;
@@ -296,9 +264,8 @@ public abstract class PackageLookupValue implements SkyValue {
     }
   }
 
-  private static class DeletedPackageLookupValue extends UnsuccessfulPackageLookupValue {
-
-    public static final DeletedPackageLookupValue INSTANCE = new DeletedPackageLookupValue();
+  /** Marker value for a deleted package. */
+  public static class DeletedPackageLookupValue extends UnsuccessfulPackageLookupValue {
 
     private DeletedPackageLookupValue() {
     }