Move more fields into Metadata

Each of the three affected fields is initialized when the builder is constructed, so they may as well go alongside the other fields that are determined prior to BUILD evaluation. The affected fields are also conceivably useful to symbolic macros as well, so they probably belong on Metadata for that reason too.

Work toward #19922.

PiperOrigin-RevId: 592602808
Change-Id: Ia171bb67ff6d386cc9687ad46e05e8c6aeade1f8

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

@@ -124,25 +124,6 @@ private NameConflictException(String message) {
     }
   }
 
-  /** Governs the error message behavior of {@link Package#getTarget}. */
-  // TODO(bazel-team): Arguably, this could be replaced by a boolean param to getTarget(), or some
-  // separate action taken by the caller. But there's a lot of call sites that would need updating.
-  private final boolean succinctTargetNotFoundErrors;
-
-  /**
-   * The name of the Bzlmod module associated with the repo this package is in. If this package is
-   * not from a Bzlmod repo, this is empty. For repos generated by module extensions, this is the
-   * name of the module hosting the extension.
-   */
-  private final Optional<String> associatedModuleName;
-
-  /**
-   * The version of the Bzlmod module associated with the repo this package is in. If this package
-   * is not from a Bzlmod repo, this is empty. For repos generated by module extensions, this is the
-   * version of the module hosting the extension.
-   */
-  private final Optional<String> associatedModuleVersion;
-
   /** The collection of all targets defined in this package, indexed by name. */
   private ImmutableSortedMap<String, Target> targets;
 
@@ -229,16 +210,8 @@ public long getComputationSteps() {
    */
   // TODO(#19922): Better separate fields that must be known a priori from those determined through
   // BUILD evaluation.
-  private Package(
-      Metadata metadata,
-      Optional<String> associatedModuleName,
-      Optional<String> associatedModuleVersion,
-      boolean succinctTargetNotFoundErrors) {
+  private Package(Metadata metadata) {
     this.metadata = metadata;
-    // TODO(#19922): move these three fields into Metadata
-    this.associatedModuleName = associatedModuleName;
-    this.associatedModuleVersion = associatedModuleVersion;
-    this.succinctTargetNotFoundErrors = succinctTargetNotFoundErrors;
   }
 
   /** Returns this package's identifier. */
@@ -630,7 +603,7 @@ public Target getTarget(String targetName) throws NoSuchTargetException {
       throw new IllegalArgumentException(targetName, e);
     }
 
-    if (succinctTargetNotFoundErrors) {
+    if (metadata.succinctTargetNotFoundErrors) {
       throw new NoSuchTargetException(
           label, String.format("target '%s' not declared in package '%s'", targetName, getName()));
     } else {
@@ -1011,12 +984,11 @@ public T intern(T sample) {
       metadata.packageIdentifier = Preconditions.checkNotNull(id);
       metadata.workspaceName = Preconditions.checkNotNull(workspaceName);
       metadata.repositoryMapping = Preconditions.checkNotNull(repositoryMapping);
-      this.pkg =
-          new Package(
-              metadata,
-              associatedModuleName,
-              associatedModuleVersion,
-              packageSettings.succinctTargetNotFoundErrors());
+      metadata.associatedModuleName = Preconditions.checkNotNull(associatedModuleName);
+      metadata.associatedModuleVersion = Preconditions.checkNotNull(associatedModuleVersion);
+      metadata.succinctTargetNotFoundErrors = packageSettings.succinctTargetNotFoundErrors();
+
+      this.pkg = new Package(metadata);
       this.precomputeTransitiveLoads = packageSettings.precomputeTransitiveLoads();
       this.noImplicitFileExport = noImplicitFileExport;
       this.labelConverter = new LabelConverter(id, repositoryMapping);
@@ -1049,7 +1021,7 @@ String getPackageWorkspaceName() {
      * this is the name of the module hosting the extension.
      */
     Optional<String> getAssociatedModuleName() {
-      return pkg.associatedModuleName;
+      return pkg.metadata.associatedModuleName;
     }
 
     /**
@@ -1058,7 +1030,7 @@ Optional<String> getAssociatedModuleName() {
      * this is the version of the module hosting the extension.
      */
     Optional<String> getAssociatedModuleVersion() {
-      return pkg.associatedModuleVersion;
+      return pkg.metadata.associatedModuleVersion;
     }
 
     /**
@@ -2029,6 +2001,20 @@ public RepositoryMapping getRepositoryMapping() {
       return repositoryMapping;
     }
 
+    /**
+     * The name of the Bzlmod module associated with the repo this package is in. If this package is
+     * not from a Bzlmod repo, this is empty. For repos generated by module extensions, this is the
+     * name of the module hosting the extension.
+     */
+    private Optional<String> associatedModuleName;
+
+    /**
+     * The version of the Bzlmod module associated with the repo this package is in. If this package
+     * is not from a Bzlmod repo, this is empty. For repos generated by module extensions, this is
+     * the version of the module hosting the extension.
+     */
+    private Optional<String> associatedModuleVersion;
+
     private ConfigSettingVisibilityPolicy configSettingVisibilityPolicy;
 
     /** Returns the visibility policy. */
@@ -2041,6 +2027,12 @@ public ConfigSettingVisibilityPolicy getConfigSettingVisibilityPolicy() {
     @Nullable private ImmutableList<Module> directLoads;
     @Nullable private ImmutableList<Label> transitiveLoads;
 
+    /** Governs the error message behavior of {@link Package#getTarget}. */
+    // TODO(bazel-team): Arguably, this could be replaced by a boolean param to getTarget(), or some
+    // separate action taken by the caller. But there's a lot of call sites that would need
+    // updating.
+    private boolean succinctTargetNotFoundErrors;
+
     // Fields that are updated during BUILD file execution.
 
     private PackageArgs packageArgs = PackageArgs.DEFAULT;