Remove obsolete Label.getDefaultCanonicalForm method.

Also move some processing to PackageIdentifier and clean up related methods. PiperOrigin-RevId: 359276948
bazelbuild · Feb 24, 2021 · 0b1e612 · 0b1e612
1 parent 31d160a
commit 0b1e612
Show file tree

Hide file tree

Showing 12 changed files with 20 additions and 25 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
@@ -189,8 +189,7 @@ private static void createStubFile(
                 Substitution.of(
                     "%enable_host_version_warning%",
                     boolToLiteral(common.shouldWarnAboutHostVersionUponFailure())),
-                Substitution.of(
-                    "%target%", ruleContext.getRule().getLabel().getDefaultCanonicalForm()),
+                Substitution.of("%target%", ruleContext.getRule().getLabel().getCanonicalForm()),
                 Substitution.of(
                     "%python_version_from_config%", versionToLiteral(common.getVersion())),
                 Substitution.of("%python_version_from_attr%", versionToLiteral(attrVersion)),

diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
@@ -442,7 +442,7 @@ public String toString() {
    * <p>invariant: {@code parseAbsolute(x.getCanonicalForm(), false).equals(x)}
    */
   public String getCanonicalForm() {
-    return getDefaultCanonicalForm();
+    return packageIdentifier.getCanonicalForm() + ":" + name;
   }
 
   public String getUnambiguousCanonicalForm() {
@@ -465,15 +465,6 @@ public String getWorkspaceName() {
     return packageIdentifier.getRepository().strippedName();
   }
 
-  /**
-   * Renders this label in canonical form, except with labels in the main and default repositories
-   * conflated.
-   */
-  public String getDefaultCanonicalForm() {
-    String repository = packageIdentifier.getRepository().getDefaultCanonicalForm();
-    return repository + "//" + packageIdentifier.getPackageFragment() + ":" + name;
-  }
-
   /**
    * Renders this label in shorthand form.
    *

diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java
@@ -210,12 +210,18 @@ public PackageIdentifier makeAbsolute() {
     return create(RepositoryName.MAIN, pkgName);
   }
 
+  /** Returns the package in label syntax format. */
+  public String getCanonicalForm() {
+    String repository = getRepository().getCanonicalForm();
+    return repository + "//" + getPackageFragment();
+  }
+
   /**
    * Returns the name of this package.
    *
    * <p>There are certain places that expect the path fragment as the package name ('foo/bar') as a
    * package identifier. This isn't specific enough for packages in other repositories, so their
-   * stringified version is '@baz//foo/bar'.</p>
+   * stringified version is '@baz//foo/bar'.
    */
   @Override
   public String toString() {

diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java
@@ -240,7 +240,7 @@ public String getName() {
    * Returns the repository name, except that the main repo is conflated with the default repo
    * ({@code "@"} becomes the empty string).
    */
-  public String getDefaultCanonicalForm() {
+  public String getCanonicalForm() {
     return isMain() ? "" : getName();
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/output/LabelOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/query/output/LabelOutputFormatter.java
@@ -53,7 +53,7 @@ public void processOutput(Iterable<Target> partialResult) throws IOException {
             writer.append(' ');
           }
           Label label = target.getLabel();
-          writer.append(label.getDefaultCanonicalForm()).append(lineTerm);
+          writer.append(label.getCanonicalForm()).append(lineTerm);
         }
       }
     };

diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/output/LocationOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/query/output/LocationOutputFormatter.java
@@ -87,7 +87,7 @@ public void processOutput(Iterable<Target> partialResult) throws IOException {
               .append(": ")
               .append(target.getTargetKind())
               .append(" ")
-              .append(target.getLabel().getDefaultCanonicalForm())
+              .append(target.getLabel().getCanonicalForm())
               .append(lineTerm);
         }
       }

diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/output/MinrankOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/query/output/MinrankOutputFormatter.java
@@ -59,7 +59,7 @@ private static void outputToStreamOrSave(
     if (toSave != null) {
       toSave.add(new RankAndLabel(rank, label));
     } else {
-      out.print(rank + " " + label.getDefaultCanonicalForm() + lineTerminator);
+      out.print(rank + " " + label.getCanonicalForm() + lineTerminator);
     }
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/query2/query/output/RankAndLabel.java b/src/main/java/com/google/devtools/build/lib/query2/query/output/RankAndLabel.java
@@ -39,6 +39,6 @@ public int compareTo(RankAndLabel o) {
 
   @Override
   public String toString() {
-    return rank + " " + label.getDefaultCanonicalForm();
+    return rank + " " + label.getCanonicalForm();
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java b/src/main/java/com/google/devtools/build/lib/rules/config/ConfigSetting.java
@@ -302,7 +302,7 @@ private static boolean matchesConfig(
             errorMessage +=
                 String.format(
                     " (it is allowlisted to %s//tools/... only)",
-                    getToolsRepository(ruleContext).getDefaultCanonicalForm());
+                    getToolsRepository(ruleContext).getCanonicalForm());
           }
           if (selectRestriction.getErrorMessage() != null) {
             errorMessage += ". " + selectRestriction.getErrorMessage();

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java b/src/main/java/com/google/devtools/build/lib/runtime/StarlarkOptionsParser.java
@@ -246,8 +246,7 @@ private void parseArg(
       // Use the canonical form to ensure we don't have
       // duplicate options getting into the starlark options map.
       unparsedOptions.put(
-          buildSettingTarget.getLabel().getDefaultCanonicalForm(),
-          new Pair<>(value, buildSettingTarget));
+          buildSettingTarget.getLabel().getCanonicalForm(), new Pair<>(value, buildSettingTarget));
     } else {
       boolean booleanValue = true;
       // check --noflag form

diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/RepositoryNameTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/RepositoryNameTest.java
@@ -65,8 +65,8 @@ public void testRunfilesDir() throws Exception {
 
   @Test
   public void testGetDefaultCanonicalForm() throws Exception {
-    assertThat(RepositoryName.create("").getDefaultCanonicalForm()).isEqualTo("");
-    assertThat(RepositoryName.create("@").getDefaultCanonicalForm()).isEqualTo("");
-    assertThat(RepositoryName.create("@foo").getDefaultCanonicalForm()).isEqualTo("@foo");
+    assertThat(RepositoryName.create("").getCanonicalForm()).isEqualTo("");
+    assertThat(RepositoryName.create("@").getCanonicalForm()).isEqualTo("");
+    assertThat(RepositoryName.create("@foo").getCanonicalForm()).isEqualTo("@foo");
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/ConfigSettingTest.java
@@ -292,7 +292,7 @@ public void nonselectableAllowlistedOption_OutOfToolsPackage() throws Exception
         String.format(
             "option 'nonselectable_allowlisted_option' cannot be used in a config_setting (it is "
                 + "allowlisted to %s//tools/... only)",
-            RepositoryName.create(TestConstants.TOOLS_REPOSITORY).getDefaultCanonicalForm()),
+            RepositoryName.create(TestConstants.TOOLS_REPOSITORY).getCanonicalForm()),
         "config_setting(",
         "    name = 'badoption',",
         "    values = {",