Use RepositoryName instead of String wherever appropriate

Some more type safety for the upcoming repo-name-related changes.

* Module[Key]#getCanonicalRepoName now returns RepositoryName
* RepositoryName#ownerRepo is now a RepositoryName (which itself can't have ownerRepo set)
* BzlmodRepoRuleValue.Key now contains a RepositoryName
* Canonical repository name lookups in BazelModuleResolutionValue & ModuleExtensionResolutionValue now have RepositoryName as the key type

PiperOrigin-RevId: 454148957
Change-Id: I574a04d1f777ee924062dc50334b046d7a0a57c1

Author: Wyverald

src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java

@@ -246,10 +246,10 @@ public void workspaceInit(
             //   - The canonical name "local_config_platform" is hardcoded in Bazel code.
             //     See {@link PlatformOptions}
             "local_config_platform",
-            (String repoName) ->
+            (RepositoryName repoName) ->
                 RepoSpec.builder()
                     .setRuleClassName("local_config_platform")
-                    .setAttributes(ImmutableMap.of("name", repoName))
+                    .setAttributes(ImmutableMap.of("name", repoName.getName()))
                     .build());
     builder
         .addSkyFunction(SkyFunctions.REPOSITORY_DIRECTORY, repositoryDelegatorFunction)

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/AbridgedModule.java

@@ -16,6 +16,7 @@
 package com.google.devtools.build.lib.bazel.bzlmod;
 
 import com.google.auto.value.AutoValue;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
 
 /**
  * An abridged version of a {@link Module}, with a reduced set of information available, used for
@@ -29,7 +30,7 @@ public abstract class AbridgedModule {
 
   public abstract ModuleKey getKey();
 
-  public final String getCanonicalRepoName() {
+  public final RepositoryName getCanonicalRepoName() {
     return getKey().getCanonicalRepoName();
   }
 

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ArchiveOverride.java

@@ -17,6 +17,7 @@
 
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
 
 /** Specifies that a module should be retrieved from an archive. */
 @AutoValue
@@ -48,9 +49,9 @@ public static ArchiveOverride create(
 
   /** Returns the {@link RepoSpec} that defines this repository. */
   @Override
-  public RepoSpec getRepoSpec(String repoName) {
+  public RepoSpec getRepoSpec(RepositoryName repoName) {
     return new ArchiveRepoSpecBuilder()
-        .setRepoName(repoName)
+        .setRepoName(repoName.getName())
         .setUrls(getUrls())
         .setIntegrity(getIntegrity())
         .setStripPrefix(getStripPrefix())

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD

@@ -21,6 +21,7 @@ java_library(
         "Version.java",
     ],
     deps = [
+        "//src/main/java/com/google/devtools/build/lib/cmdline:cmdline-primitives",
         "//src/main/java/net/starlark/java/eval",
         "//third_party:auto_value",
         "//third_party:guava",
@@ -73,6 +74,7 @@ java_library(
     deps = [
         ":common",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
+        "//src/main/java/com/google/devtools/build/lib/cmdline:cmdline-primitives",
         "//src/main/java/com/google/devtools/build/lib/events",
         "//third_party:gson",
         "//third_party:guava",
@@ -177,6 +179,7 @@ java_library(
         "BzlmodRepoRuleValue.java",
     ],
     deps = [
+        "//src/main/java/com/google/devtools/build/lib/cmdline:cmdline-primitives",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/packages",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
@@ -195,6 +198,7 @@ java_library(
         ":common",
         ":registry",
         ":resolution",
+        "//src/main/java/com/google/devtools/build/lib/cmdline:cmdline-primitives",
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/skyframe",
         "//third_party:guava",

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java

@@ -27,6 +27,7 @@
 import com.google.devtools.build.lib.bazel.bzlmod.Selection.SelectionResult;
 import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.CheckDirectDepsMode;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.packages.LabelConverter;
 import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps.Code;
@@ -114,7 +115,7 @@ static BazelModuleResolutionValue createValue(
       ImmutableMap<String, ModuleOverride> overrides)
       throws BazelModuleResolutionFunctionException {
     // Build some reverse lookups for later use.
-    ImmutableMap<String, ModuleKey> canonicalRepoNameLookup =
+    ImmutableMap<RepositoryName, ModuleKey> canonicalRepoNameLookup =
         depGraph.keySet().stream()
             .collect(toImmutableMap(ModuleKey::getCanonicalRepoName, key -> key));
     ImmutableMap<String, ModuleKey> moduleNameLookup =

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionValue.java

@@ -39,7 +39,7 @@ public abstract class BazelModuleResolutionValue implements SkyValue {
   public static BazelModuleResolutionValue create(
       ImmutableMap<ModuleKey, Module> depGraph,
       ImmutableMap<ModuleKey, Module> unprunedDepGraph,
-      ImmutableMap<String, ModuleKey> canonicalRepoNameLookup,
+      ImmutableMap<RepositoryName, ModuleKey> canonicalRepoNameLookup,
       ImmutableMap<String, ModuleKey> moduleNameLookup,
       ImmutableList<AbridgedModule> abridgedModules,
       ImmutableTable<ModuleExtensionId, ModuleKey, ModuleExtensionUsage> extensionUsagesTable,
@@ -69,7 +69,7 @@ public static BazelModuleResolutionValue create(
   public abstract ImmutableMap<ModuleKey, Module> getUnprunedDepGraph();
 
   /** A mapping from a canonical repo name to the key of the module backing it. */
-  public abstract ImmutableMap<String, ModuleKey> getCanonicalRepoNameLookup();
+  public abstract ImmutableMap<RepositoryName, ModuleKey> getCanonicalRepoNameLookup();
 
   /**
    * A mapping from a plain module name to the key of the module. Does not include the root module,

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleHelper.java

@@ -14,12 +14,13 @@
 
 package com.google.devtools.build.lib.bazel.bzlmod;
 
+import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.skyframe.SkyFunction.Environment;
 import java.io.IOException;
 import java.util.Optional;
 
 /** A helper to get {@link RepoSpec} for Bzlmod generated repositories. */
 public interface BzlmodRepoRuleHelper {
-  Optional<RepoSpec> getRepoSpec(Environment env, String repositoryName)
+  Optional<RepoSpec> getRepoSpec(Environment env, RepositoryName repositoryName)
       throws InterruptedException, IOException;
 }

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleHelperImpl.java

@@ -19,6 +19,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.skyframe.SkyFunction.Environment;
 import java.io.IOException;
@@ -28,7 +29,7 @@
 public final class BzlmodRepoRuleHelperImpl implements BzlmodRepoRuleHelper {
 
   @Override
-  public Optional<RepoSpec> getRepoSpec(Environment env, String repositoryName)
+  public Optional<RepoSpec> getRepoSpec(Environment env, RepositoryName repositoryName)
       throws InterruptedException, IOException {
 
     RootModuleFileValue root =
@@ -58,7 +59,7 @@ public Optional<RepoSpec> getRepoSpec(Environment env, String repositoryName)
   }
 
   private static Optional<RepoSpec> checkRepoFromNonRegistryOverrides(
-      RootModuleFileValue root, String repositoryName) {
+      RootModuleFileValue root, RepositoryName repositoryName) {
     String moduleName = root.getNonRegistryOverrideCanonicalRepoNameLookup().get(repositoryName);
     if (moduleName == null) {
       return Optional.empty();
@@ -71,7 +72,7 @@ private static Optional<RepoSpec> checkRepoFromBazelModules(
       BazelModuleResolutionValue bazelModuleResolutionValue,
       ImmutableMap<String, ModuleOverride> overrides,
       ExtendedEventHandler eventListener,
-      String repositoryName)
+      RepositoryName repositoryName)
       throws InterruptedException, IOException {
     ModuleKey moduleKey =
         bazelModuleResolutionValue.getCanonicalRepoNameLookup().get(repositoryName);

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleValue.java

@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.bazel.bzlmod;
 
 import com.google.common.collect.Interner;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.concurrent.BlazeInterners;
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.Rule;
@@ -41,7 +42,7 @@ public Rule getRule() {
     return pkg.getRule(ruleName);
   }
 
-  public static Key key(String repositoryName) {
+  public static Key key(RepositoryName repositoryName) {
     return Key.create(repositoryName);
   }
 
@@ -61,16 +62,16 @@ public Rule getRule() {
 
   /** Argument for the SkyKey to request a BzlmodRepoRuleValue. */
   @AutoCodec
-  public static class Key extends AbstractSkyKey<String> {
+  public static class Key extends AbstractSkyKey<RepositoryName> {
     private static final Interner<Key> interner = BlazeInterners.newWeakInterner();
 
-    private Key(String arg) {
+    private Key(RepositoryName arg) {
       super(arg);
     }
 
     @AutoCodec.VisibleForSerialization
     @AutoCodec.Instantiator
-    static Key create(String arg) {
+    static Key create(RepositoryName arg) {
       return interner.intern(new Key(arg));
     }
 

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GitOverride.java

@@ -18,6 +18,7 @@
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
 
 /** Specifies that a module should be retrieved from a Git repository. */
 @AutoValue
@@ -41,10 +42,10 @@ public static GitOverride create(
 
   /** Returns the {@link RepoSpec} that defines this repository. */
   @Override
-  public RepoSpec getRepoSpec(String repoName) {
+  public RepoSpec getRepoSpec(RepositoryName repoName) {
     ImmutableMap.Builder<String, Object> attrBuilder = ImmutableMap.builder();
     attrBuilder
-        .put("name", repoName)
+        .put("name", repoName.getName())
         .put("remote", getRemote())
         .put("commit", getCommit())
         .put("patches", getPatches())

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java

@@ -21,6 +21,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.gson.FieldNamingPolicy;
 import com.google.gson.Gson;
@@ -127,7 +128,8 @@ private <T> Optional<T> grabJson(String url, Class<T> klass, ExtendedEventHandle
   }
 
   @Override
-  public RepoSpec getRepoSpec(ModuleKey key, String repoName, ExtendedEventHandler eventHandler)
+  public RepoSpec getRepoSpec(
+      ModuleKey key, RepositoryName repoName, ExtendedEventHandler eventHandler)
       throws IOException, InterruptedException {
     Optional<BazelRegistryJson> bazelRegistryJson =
         grabJson(
@@ -184,7 +186,7 @@ public RepoSpec getRepoSpec(ModuleKey key, String repoName, ExtendedEventHandler
     }
 
     return new ArchiveRepoSpecBuilder()
-        .setRepoName(repoName)
+        .setRepoName(repoName.getName())
         .setUrls(urls.build())
         .setIntegrity(sourceJson.get().integrity)
         .setStripPrefix(Strings.nullToEmpty(sourceJson.get().stripPrefix))

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LocalPathOverride.java

@@ -17,6 +17,7 @@
 
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
 
 /** Specifies that a module should be retrieved from a local directory. */
 @AutoValue
@@ -30,10 +31,10 @@ public static LocalPathOverride create(String path) {
 
   /** Returns the {@link RepoSpec} that defines this repository. */
   @Override
-  public RepoSpec getRepoSpec(String repoName) {
+  public RepoSpec getRepoSpec(RepositoryName repoName) {
     return RepoSpec.builder()
         .setRuleClassName("local_repository")
-        .setAttributes(ImmutableMap.of("name", repoName, "path", getPath()))
+        .setAttributes(ImmutableMap.of("name", repoName.getName(), "path", getPath()))
         .build();
   }
 }

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Module.java

@@ -63,7 +63,7 @@ public abstract class Module {
    */
   public abstract ModuleKey getKey();
 
-  public final String getCanonicalRepoName() {
+  public final RepositoryName getCanonicalRepoName() {
     return getKey().getCanonicalRepoName();
   }
 
@@ -114,13 +114,12 @@ public final RepositoryMapping getRepoMappingWithBazelDepsOnly() {
     // Every module should be able to reference itself as @<module name>.
     // If this is the root module, this perfectly falls into @<module name> => @
     if (!getName().isEmpty()) {
-      mapping.put(getName(), RepositoryName.createUnvalidated(getCanonicalRepoName()));
+      mapping.put(getName(), getCanonicalRepoName());
     }
     for (Map.Entry<String, ModuleKey> dep : getDeps().entrySet()) {
       // Special note: if `dep` is actually the root module, its ModuleKey would be ROOT whose
       // canonicalRepoName is the empty string. This perfectly maps to the main repo ("@").
-      mapping.put(
-          dep.getKey(), RepositoryName.createUnvalidated(dep.getValue().getCanonicalRepoName()));
+      mapping.put(dep.getKey(), dep.getValue().getCanonicalRepoName());
     }
     return RepositoryMapping.create(mapping.buildOrThrow(), getCanonicalRepoName());
   }

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionFunction.java

@@ -21,6 +21,7 @@
 import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyFunctionException;
@@ -58,8 +59,9 @@ public SkyValue compute(SkyKey skyKey, Environment env)
     // through RepositoryDelegatorFunction again. This isn't a normally a problem, but since we
     // always refetch "local" (aka "always fetch") repos, this could be an unnecessary performance
     // hit.
-    ImmutableMap.Builder<String, Package> canonicalRepoNameToPackage = ImmutableMap.builder();
-    ImmutableMap.Builder<String, ModuleExtensionId> canonicalRepoNameToExtensionId =
+    ImmutableMap.Builder<RepositoryName, Package> canonicalRepoNameToPackage =
+        ImmutableMap.builder();
+    ImmutableMap.Builder<RepositoryName, ModuleExtensionId> canonicalRepoNameToExtensionId =
         ImmutableMap.builder();
     ImmutableListMultimap.Builder<ModuleExtensionId, String> extensionIdToRepoInternalNames =
         ImmutableListMultimap.builder();
@@ -75,11 +77,12 @@ public SkyValue compute(SkyKey skyKey, Environment env)
       String repoPrefix =
           bazelModuleResolutionValue.getExtensionUniqueNames().get(extensionId) + '.';
       for (Map.Entry<String, Package> entry : generatedRepos.entrySet()) {
-            String canonicalRepoName = repoPrefix + entry.getKey();
-            canonicalRepoNameToPackage.put(canonicalRepoName, entry.getValue());
-            canonicalRepoNameToExtensionId.put(canonicalRepoName, extensionId);
-          }
-          extensionIdToRepoInternalNames.putAll(extensionId, generatedRepos.keySet());
+        RepositoryName canonicalRepoName =
+            RepositoryName.createUnvalidated(repoPrefix + entry.getKey());
+        canonicalRepoNameToPackage.put(canonicalRepoName, entry.getValue());
+        canonicalRepoNameToExtensionId.put(canonicalRepoName, extensionId);
+      }
+      extensionIdToRepoInternalNames.putAll(extensionId, generatedRepos.keySet());
     }
     return ModuleExtensionResolutionValue.create(
         canonicalRepoNameToPackage.buildOrThrow(),

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionValue.java

@@ -17,6 +17,7 @@
 import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.skyframe.SkyFunctions;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
@@ -33,13 +34,14 @@ public abstract class ModuleExtensionResolutionValue implements SkyValue {
    * A mapping from a canonical repo name of a repo generated by a module extension, to the package
    * containing (only) that repo.
    */
-  public abstract ImmutableMap<String, Package> getCanonicalRepoNameToPackage();
+  public abstract ImmutableMap<RepositoryName, Package> getCanonicalRepoNameToPackage();
 
   /**
    * A mapping from a canonical repo name of a repo generated by a module extension, to the ID of
    * that extension.
    */
-  public abstract ImmutableMap<String, ModuleExtensionId> getCanonicalRepoNameToExtensionId();
+  public abstract ImmutableMap<RepositoryName, ModuleExtensionId>
+      getCanonicalRepoNameToExtensionId();
 
   /**
    * A mapping from a module extension ID, to the list of "internal" names of the repos generated by
@@ -50,8 +52,8 @@ public abstract class ModuleExtensionResolutionValue implements SkyValue {
       getExtensionIdToRepoInternalNames();
 
   public static ModuleExtensionResolutionValue create(
-      ImmutableMap<String, Package> canonicalRepoNameToPackage,
-      ImmutableMap<String, ModuleExtensionId> canonicalRepoNameToExtensionId,
+      ImmutableMap<RepositoryName, Package> canonicalRepoNameToPackage,
+      ImmutableMap<RepositoryName, ModuleExtensionId> canonicalRepoNameToExtensionId,
       ImmutableListMultimap<ModuleExtensionId, String> extensionIdToRepoInternalNames) {
     return new AutoValue_ModuleExtensionResolutionValue(
         canonicalRepoNameToPackage, canonicalRepoNameToExtensionId, extensionIdToRepoInternalNames);

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java

@@ -162,7 +162,7 @@ private SkyValue computeForRootModule(StarlarkSemantics starlarkSemantics, Envir
     if (rootOverride != null) {
       throw errorf(Code.BAD_MODULE, "invalid override for the root module found: %s", rootOverride);
     }
-    ImmutableMap<String, String> nonRegistryOverrideCanonicalRepoNameLookup =
+    ImmutableMap<RepositoryName, String> nonRegistryOverrideCanonicalRepoNameLookup =
         Maps.filterValues(overrides, override -> override instanceof NonRegistryOverride)
             .keySet()
             .stream()
@@ -222,12 +222,9 @@ private GetModuleFileResult getModuleFile(
     // If there is a non-registry override for this module, we need to fetch the corresponding repo
     // first and read the module file from there.
     if (override instanceof NonRegistryOverride) {
-      String canonicalRepoName = key.getCanonicalRepoName();
+      RepositoryName canonicalRepoName = key.getCanonicalRepoName();
       RepositoryDirectoryValue repoDir =
-          (RepositoryDirectoryValue)
-              env.getValue(
-                  RepositoryDirectoryValue.key(
-                      RepositoryName.createUnvalidated(canonicalRepoName)));
+          (RepositoryDirectoryValue) env.getValue(RepositoryDirectoryValue.key(canonicalRepoName));
       if (repoDir == null) {
         return null;
       }