Switch from RepositoryName#toString to RepositoryName#getNameWithAt

Fixes a TODO. Also includes some code cleanups.

PiperOrigin-RevId: 447437517

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java

@@ -462,7 +462,7 @@ public Descriptor labelAttribute(
       Label label =
           ((BazelModuleContext) Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData())
               .label();
-      if (!label.getPackageIdentifier().getRepository().toString().equals("@_builtins")) {
+      if (!label.getPackageIdentifier().getRepository().getName().equals("_builtins")) {
         throw Starlark.errorf("Rule in '%s' cannot use private API", label.getPackageName());
       }
     }

src/main/java/com/google/devtools/build/lib/cmdline/Label.java

@@ -369,7 +369,7 @@ public String getCanonicalForm() {
   }
 
   public String getUnambiguousCanonicalForm() {
-    return packageIdentifier.getRepository()
+    return packageIdentifier.getRepository().getNameWithAt()
         + "//"
         + packageIdentifier.getPackageFragment()
         + ":"

src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java

@@ -169,8 +169,7 @@ public PathFragment getRunfilesPath() {
    */
   // TODO(bazel-team): Maybe rename to "getDefaultForm"?
   public String getCanonicalForm() {
-    String repository = getRepository().getCanonicalForm();
-    return repository + "//" + getPackageFragment();
+    return repository.getCanonicalForm() + "//" + getPackageFragment();
   }
 
   /**
@@ -183,9 +182,10 @@ public String getCanonicalForm() {
   // that disparity?
   @Override
   public String toString() {
-    // Avoid creating a new String object in the common case of the main repository (PathFragment
-    // stores its toString).
-    return repository.isMain() ? pkgName.toString() : (repository + "//" + pkgName);
+    if (repository.isMain()) {
+      return getPackageFragment().getPathString();
+    }
+    return getCanonicalForm();
   }
 
   @Override
@@ -218,7 +218,7 @@ public int compareTo(PackageIdentifier that) {
       return pkgName.compareTo(that.pkgName);
     }
     return ComparisonChain.start()
-        .compare(repository.toString(), that.repository.toString())
+        .compare(repository.getName(), that.repository.getName())
         .compare(pkgName, that.pkgName)
         .result();
   }

src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java

@@ -108,7 +108,7 @@ public static Pair<RepositoryName, PathFragment> fromPathFragment(
     }
 
     try {
-      RepositoryName repoName = RepositoryName.create(path.getSegment(1));
+      RepositoryName repoName = create(path.getSegment(1));
       PathFragment subPath = path.subFragment(2);
       return Pair.of(repoName, subPath);
     } catch (LabelSyntaxException e) {
@@ -188,14 +188,13 @@ public boolean isMain() {
   }
 
   /** Returns the repository name, with leading "{@literal @}". */
-  // TODO(bazel-team): Use this over toString()- easier to track its usage.
   public String getNameWithAt() {
     return name;
   }
 
   /**
-   * Returns the repository name with leading '@' except for the main repo, which is just the empty
-   * string.
+   * Returns the repository name with leading "{@literal @}" except for the main repo, which is just
+   * the empty string.
    */
   // TODO(bazel-team): Consider renaming to "getDefaultForm".
   public String getCanonicalForm() {

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

@@ -535,7 +535,7 @@ public String repositoryName(StarlarkThread thread) throws EvalException {
     BazelStarlarkContext.from(thread).checkLoadingPhase("native.repository_name");
     PackageIdentifier packageId =
         PackageFactory.getContext(thread).getBuilder().getPackageIdentifier();
-    return packageId.getRepository().toString();
+    return packageId.getRepository().getNameWithAt();
   }
 
   private static Dict<String, Object> getRuleDict(Rule rule, Mutability mu) throws EvalException {

src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java

@@ -1936,7 +1936,7 @@ public static boolean isBuiltIn(StarlarkThread thread) {
     Label label =
         ((BazelModuleContext) Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData())
             .label();
-    return label.getPackageIdentifier().getRepository().toString().equals("@_builtins");
+    return label.getPackageIdentifier().getRepository().getName().equals("_builtins");
   }
 
   protected Language parseLanguage(String string) throws EvalException {

src/main/java/com/google/devtools/build/lib/rules/java/JavaStarlarkCommon.java

@@ -338,7 +338,7 @@ static void checkPrivateAccess(StarlarkThread thread) throws EvalException {
         ((BazelModuleContext) Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData())
             .label();
     if (!PRIVATE_STARLARKIFACTION_ALLOWLIST.contains(label.getPackageName())
-        && !label.getPackageIdentifier().getRepository().toString().equals("@_builtins")) {
+        && !label.getPackageIdentifier().getRepository().getName().equals("_builtins")) {
       throw Starlark.errorf("Rule in '%s' cannot use private API", label.getPackageName());
     }
   }

src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java

@@ -374,8 +374,10 @@ && managedDirectoriesExist(directories.getWorkspace(), managedDirectories)) {
     if (!repoRoot.exists()) {
       // The repository isn't on the file system, there is nothing we can do.
       throw new RepositoryFunctionException(
-          new IOException("to fix, run\n\tbazel fetch //...\nExternal repository " + repositoryName
-              + " not found and fetching repositories is disabled."),
+          new IOException(
+              "to fix, run\n\tbazel fetch //...\nExternal repository "
+                  + repositoryName.getNameWithAt()
+                  + " not found and fetching repositories is disabled."),
           Transience.TRANSIENT);
     }
 
@@ -677,9 +679,7 @@ private static class RepositoryFetching implements FetchProgress {
     final String message;
 
     RepositoryFetching(String name, boolean finished) {
-      this.id = name;
-      this.finished = finished;
-      this.message = finished ? "finished." : "fetching";
+      this(name, finished, finished ? "finished." : "fetching");
     }
 
     RepositoryFetching(String name, boolean finished, String message) {

src/main/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupValue.java

@@ -158,7 +158,7 @@ public PathFragment getPath() {
 
     @Override
     public String toString() {
-      return "SuccessfulLocalRepositoryLookupValue(" + repositoryName + ")";
+      return "SuccessfulLocalRepositoryLookupValue(" + repositoryName.getNameWithAt() + ")";
     }
 
     @Override

src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java

@@ -146,7 +146,7 @@ public static String explainNoBuildFileValue(PackageIdentifier packageKey, Envir
       return "BUILD file not found in directory '"
           + packageKey.getPackageFragment()
           + "' of external repository "
-          + packageKey.getRepository()
+          + packageKey.getRepository().getNameWithAt()
           + ". "
           + educationalMessage;
     }

src/test/java/com/google/devtools/build/lib/buildtool/SymlinkForestTest.java

@@ -163,7 +163,7 @@ private static PackageIdentifier createExternalPkg(Root root, String repo, Strin
       repoRoot.getRelative(pkg).createDirectoryAndParents();
       FileSystemUtils.createEmptyFile(repoRoot.getRelative(pkg).getChild("file"));
     }
-    return PackageIdentifier.create(RepositoryName.create("" + repo), PathFragment.create(pkg));
+    return PackageIdentifier.create(RepositoryName.create(repo), PathFragment.create(pkg));
   }
 
   // Create package for main repo
@@ -173,7 +173,7 @@ private static PackageIdentifier createMainPkg(Root repoRoot, String pkg)
       repoRoot.getRelative(pkg).createDirectoryAndParents();
       FileSystemUtils.createEmptyFile(repoRoot.getRelative(pkg).getChild("file"));
     }
-    return PackageIdentifier.create(RepositoryName.create(""), PathFragment.create(pkg));
+    return PackageIdentifier.createInMainRepo(PathFragment.create(pkg));
   }
 
   private static void assertLinksTo(Path fromRoot, Root toRoot, String relpart) throws IOException {

src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java

@@ -187,7 +187,7 @@ public void testGetRelativeWithRemoteRepoToDefaultRepo() throws Exception {
 
     Label relative = base.getRelativeWithRemapping("@//x:y", ImmutableMap.of());
 
-    assertThat(relative.getRepository()).isEqualTo(RepositoryName.create(""));
+    assertThat(relative.getRepository()).isEqualTo(RepositoryName.MAIN);
     assertThat(relative.getPackageFragment()).isEqualTo(PathFragment.create("x"));
     assertThat(relative.getName()).isEqualTo("y");
   }

src/test/java/com/google/devtools/build/lib/cmdline/RepositoryNameTest.java

@@ -36,15 +36,15 @@ public void assertNotValid(String name, String expectedMessage) {
 
   @Test
   public void testValidateRepositoryName() throws Exception {
-    assertThat(RepositoryName.create("foo").toString()).isEqualTo("@foo");
-    assertThat(RepositoryName.create("").toString()).isEqualTo("@");
+    assertThat(RepositoryName.create("foo").getNameWithAt()).isEqualTo("@foo");
+    assertThat(RepositoryName.create("").getNameWithAt()).isEqualTo("@");
     assertThat(RepositoryName.create("")).isSameInstanceAs(RepositoryName.MAIN);
-    assertThat(RepositoryName.create("foo_bar").toString()).isEqualTo("@foo_bar");
-    assertThat(RepositoryName.create("foo-bar").toString()).isEqualTo("@foo-bar");
-    assertThat(RepositoryName.create("foo.bar").toString()).isEqualTo("@foo.bar");
-    assertThat(RepositoryName.create("..foo").toString()).isEqualTo("@..foo");
-    assertThat(RepositoryName.create("foo..").toString()).isEqualTo("@foo..");
-    assertThat(RepositoryName.create(".foo").toString()).isEqualTo("@.foo");
+    assertThat(RepositoryName.create("foo_bar").getNameWithAt()).isEqualTo("@foo_bar");
+    assertThat(RepositoryName.create("foo-bar").getNameWithAt()).isEqualTo("@foo-bar");
+    assertThat(RepositoryName.create("foo.bar").getNameWithAt()).isEqualTo("@foo.bar");
+    assertThat(RepositoryName.create("..foo").getNameWithAt()).isEqualTo("@..foo");
+    assertThat(RepositoryName.create("foo..").getNameWithAt()).isEqualTo("@foo..");
+    assertThat(RepositoryName.create(".foo").getNameWithAt()).isEqualTo("@.foo");
 
     assertNotValid(".", "repo names are not allowed to be '@.'");
     assertNotValid("..", "repo names are not allowed to be '@..'");
@@ -58,13 +58,10 @@ public void testRunfilesDir() throws Exception {
     assertThat(RepositoryName.create("foo").getRunfilesPath())
         .isEqualTo(PathFragment.create("../foo"));
     assertThat(RepositoryName.create("").getRunfilesPath()).isEqualTo(PathFragment.EMPTY_FRAGMENT);
-    assertThat(RepositoryName.create("").getRunfilesPath())
-        .isEqualTo(PathFragment.EMPTY_FRAGMENT);
   }
 
   @Test
   public void testGetDefaultCanonicalForm() throws Exception {
-    assertThat(RepositoryName.create("").getCanonicalForm()).isEqualTo("");
     assertThat(RepositoryName.create("").getCanonicalForm()).isEqualTo("");
     assertThat(RepositoryName.create("foo").getCanonicalForm()).isEqualTo("@foo");
   }