Split PackagePiece Identifier out into its own file/target

... and split the identifier for a package piece for build file into a sibling rather
than parent class of the identifier of a package piece for macro.

This is in preparation for making package piece identifiers into skykeys - with the
two flavors of identifier being evaluated by different skyfunctions.

Working towards bazelbuild#23852

PiperOrigin-RevId: 747509571
Change-Id: Ib62469f8e88906c6f4beceeeb0da47cb444d4b11

Author: tetromino

src/main/java/com/google/devtools/build/lib/packages/BUILD

@@ -54,6 +54,7 @@ java_library(
             "Globber.java",
             "GlobberUtils.java",
             "LabelPrinter.java",
+            "PackagePieceIdentifier.java",
             "PackageSpecification.java",
             "RuleClassUtils.java",
         ],
@@ -64,6 +65,7 @@ java_library(
         ":globber",
         ":globber_utils",
         ":label_printer",
+        ":package_piece_identifier",
         ":package_specification",
         "//src/main/java/com/google/devtools/build/docgen/annot",
         "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
@@ -209,6 +211,16 @@ java_library(
     ],
 )
 
+java_library(
+    name = "package_piece_identifier",
+    srcs = ["PackagePieceIdentifier.java"],
+    deps = [
+        "//src/main/java/com/google/devtools/build/lib/cmdline",
+        "//src/main/java/com/google/devtools/build/lib/util:hash_codes",
+        "//third_party:guava",
+    ],
+)
+
 java_library(
     name = "rule_class_id",
     srcs = ["RuleClassId.java"],

src/main/java/com/google/devtools/build/lib/packages/PackagePiece.java

@@ -31,10 +31,8 @@
 import com.google.devtools.build.lib.packages.Package.Declarations;
 import com.google.devtools.build.lib.packages.Package.Metadata;
 import com.google.devtools.build.lib.packages.TargetRecorder.MacroNamespaceViolationException;
-import com.google.devtools.build.lib.util.HashCodes;
 import com.google.devtools.build.lib.vfs.RootedPath;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.concurrent.Semaphore;
 import javax.annotation.Nullable;
@@ -55,11 +53,7 @@
 // another class of package piece obtained by evaluating a set of macros.
 public abstract sealed class PackagePiece extends Packageoid
     permits PackagePiece.ForBuildFile, PackagePiece.ForMacro {
-  protected final Identifier identifier;
-
-  public Identifier getIdentifier() {
-    return identifier;
-  }
+  public abstract PackagePieceIdentifier getIdentifier();
 
   /**
    * Returns the {@link PackagePiece} corresponding to the evaluation of the BUILD file for this
@@ -150,100 +144,20 @@ public String getShortDescription() {
     return "package piece " + getIdentifier();
   }
 
-  private PackagePiece(Identifier identifier) {
-    this.identifier = identifier;
-  }
-
-  /** A unique identifier for a {@link PackagePiece}. */
-  public static sealed class Identifier permits PackagePiece.ForMacro.Identifier {
-    protected final PackageIdentifier packageIdentifier;
-    // BUILD file label for a {@link PackagePiece.ForBuildFile}, or the label of the macro class
-    // definition for a {@link PackagePiece.ForMacro}.
-    protected final Label definingLabel;
-
-    /**
-     * The canonical form of the package name if this is an identifier for a {@link
-     * PackagePiece.ForBuildFile}, or the canonical form of the macro instance name if this is an
-     * identifier for a {@link PackagePiece.ForMacro}.
-     *
-     * <p>This string is not unique, since multiple macro instances can have the same name. Intended
-     * to be used in combination with {@link #getCanonicalFormDefinedBy}, or with {@link
-     * #getDefiningLabel} + {@link #getDefiningSymbol} pair.
-     */
-    public String getCanonicalFormName() {
-      String pkgIdString = packageIdentifier.getCanonicalForm();
-      return getInstanceName() != null
-          ? String.format("%s:%s", pkgIdString, getInstanceName())
-          : pkgIdString;
-    }
-
-    public String getCanonicalFormDefinedBy() {
-      String definingLabelString = definingLabel.getCanonicalForm();
-      return getDefiningSymbol() != null
-          ? String.format("%s%%%s", definingLabelString, getDefiningSymbol())
-          : definingLabelString;
-    }
-
-    @Nullable
-    protected String getInstanceName() {
-      return null;
-    }
-
-    /**
-     * BUILD file label for a {@link PackagePiece.ForBuildFile}, or the label of the macro class
-     * definition for a {@link PackagePiece.ForMacro}.
-     */
-    public Label getDefiningLabel() {
-      return definingLabel;
-    }
-
-    @Nullable
-    public String getDefiningSymbol() {
-      return null;
-    }
-
-    @Override
-    public String toString() {
-      return String.format("%s defined by %s", getCanonicalFormName(), getCanonicalFormDefinedBy());
-    }
-
-    @Override
-    public boolean equals(Object other) {
-      if (this == other) {
-        return true;
-      }
-      if (!(other instanceof PackagePiece.Identifier that)) {
-        return false;
-      }
-      return this.packageIdentifier.equals(that.packageIdentifier)
-          && this.definingLabel.equals(that.definingLabel)
-          // Valid because PackagePiece.ForMacro.Identifier requires defining symbol and instance
-          // name to be non-null.
-          && Objects.equals(this.getDefiningSymbol(), that.getDefiningSymbol())
-          && Objects.equals(this.getInstanceName(), that.getInstanceName());
-    }
-
-    @Override
-    public int hashCode() {
-      return HashCodes.hashObjects(
-          packageIdentifier, definingLabel, getDefiningSymbol(), getInstanceName());
-    }
-
-    @VisibleForTesting
-    Identifier(PackageIdentifier packageIdentifier, Label definingLabel) {
-      this.packageIdentifier = packageIdentifier;
-      this.definingLabel = definingLabel;
-    }
-  }
-
   /**
    * A {@link PackagePiece} obtained by evaluating a BUILD file, without expanding any symbolic
    * macros.
    */
   public static final class ForBuildFile extends PackagePiece {
+    private final PackagePieceIdentifier.ForBuildFile identifier;
     private final Metadata metadata;
     private final Declarations declarations;
 
+    @Override
+    public PackagePieceIdentifier.ForBuildFile getIdentifier() {
+      return identifier;
+    }
+
     @Override
     public PackagePiece.ForBuildFile getPackagePieceForBuildFile() {
       return this;
@@ -266,7 +180,9 @@ public void checkMacroNamespaceCompliance(Target target) {
     }
 
     private ForBuildFile(Metadata metadata) {
-      super(new Identifier(metadata.packageIdentifier(), metadata.buildFileLabel()));
+      this.identifier =
+          new PackagePieceIdentifier.ForBuildFile(
+              metadata.packageIdentifier(), metadata.buildFileLabel());
       this.metadata = metadata;
       this.declarations = new Declarations();
     }
@@ -394,11 +310,17 @@ private Builder(
 
   /** A {@link PackagePiece} obtained by evaluating a symbolic macro instance. */
   public static final class ForMacro extends PackagePiece {
+    private final PackagePieceIdentifier.ForMacro identifier;
     private final MacroInstance evaluatedMacro;
     private final PackagePiece.ForBuildFile pieceForBuildFile;
     // Null until the package piece is fully initialized by its builder's {@code finishBuild()}.
     @Nullable private ImmutableSet<String> macroNamespaceViolations = null;
 
+    @Override
+    public PackagePieceIdentifier.ForMacro getIdentifier() {
+      return identifier;
+    }
+
     @Override
     public PackagePiece.ForBuildFile getPackagePieceForBuildFile() {
       return pieceForBuildFile;
@@ -444,12 +366,12 @@ public void checkMacroNamespaceCompliance(Target target)
     }
 
     private ForMacro(MacroInstance evaluatedMacro, PackagePiece.ForBuildFile pieceForBuildFile) {
-      super(
-          new PackagePiece.ForMacro.Identifier(
+      this.identifier =
+          new PackagePieceIdentifier.ForMacro(
               pieceForBuildFile.getPackageIdentifier(),
               evaluatedMacro.getMacroClass().getDefiningBzlLabel(),
               /* definingSymbol= */ evaluatedMacro.getMacroClass().getName(),
-              /* instanceName= */ evaluatedMacro.getName()));
+              /* instanceName= */ evaluatedMacro.getName());
       this.evaluatedMacro = evaluatedMacro;
       this.pieceForBuildFile = pieceForBuildFile;
     }
@@ -480,38 +402,6 @@ public static Builder newBuilder(
           trackFullMacroInformation);
     }
 
-    /**
-     * A unique identifier for a {@link PackagePiece.ForMacro}.
-     *
-     * <p>Exists purely as a memory optimization to avoid allocating the defining symbol and
-     * instance name for {@link PackagePiece.ForBuildFile} objects.
-     */
-    public static final class Identifier extends PackagePiece.Identifier {
-      private final String definingSymbol;
-      private final String instanceName;
-
-      @Override
-      public String getDefiningSymbol() {
-        return definingSymbol;
-      }
-
-      @Override
-      public String getInstanceName() {
-        return instanceName;
-      }
-
-      @VisibleForTesting
-      Identifier(
-          PackageIdentifier packageIdentifier,
-          Label definingLabel,
-          String definingSymbol,
-          String instanceName) {
-        super(packageIdentifier, definingLabel);
-        this.definingSymbol = checkNotNull(definingSymbol);
-        this.instanceName = checkNotNull(instanceName);
-      }
-    }
-
     /** A builder for {@link PackagePieceForMacro} objects. */
     public static class Builder extends TargetDefinitionContext {