Streamline ModuleExtensionId#toString()

Currently `ModuleExtensionId#toString()` uses the default Java record repr, which isn't very user-friendly. There's really no reason not to use the `asTargetString()` method instead. This PR just renames `asTargetString()` to `toString()` and updates all call sites.

Closes bazelbuild#24450.

PiperOrigin-RevId: 700050362
Change-Id: I82238d2134e1642694f0b20235fcfe9307ceaa7d

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

@@ -172,7 +172,7 @@ public ImmutableMap<String, RepoSpec> createRepos(StarlarkSemantics starlarkSema
         var attributesValue = AttributeValues.create(attributes);
         AttributeValues.validateAttrs(
             attributesValue,
-            String.format("in the extension '%s'", extensionId.asTargetString()),
+            String.format("in the extension '%s'", extensionId),
             String.format("%s '%s'", rule.getRuleClass(), name));
         repoSpecs.put(
             name,

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

@@ -24,19 +24,7 @@ public class ModuleExtensionEvaluationProgress implements FetchProgress {
 
   /** Returns the unique identifying string for a module extension evaluation event. */
   public static String moduleExtensionEvaluationContextString(ModuleExtensionId extensionId) {
-    String suffix =
-        extensionId
-            .isolationKey()
-            .map(
-                isolationKey ->
-                    String.format(
-                        " for %s in %s", isolationKey.usageExportedName(), isolationKey.module()))
-            .orElse("");
-    return String.format(
-        "module extension %s in %s%s",
-        extensionId.extensionName(),
-        extensionId.bzlFileLabel().getUnambiguousCanonicalForm(),
-        suffix);
+    return "module extension " + extensionId;
   }
 
   public static ModuleExtensionEvaluationProgress ongoing(

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

@@ -63,7 +63,7 @@ public static IsolationKey create(ModuleKey module, String usageExportedName) {
     }
 
     @Override
-    public final String toString() {
+    public String toString() {
       return module() + "+" + usageExportedName();
     }
 
@@ -79,11 +79,12 @@ public static ModuleExtensionId create(
     return new ModuleExtensionId(bzlFileLabel, extensionName, isolationKey);
   }
 
-  public final boolean isInnate() {
+  public boolean isInnate() {
     return extensionName().contains("%");
   }
 
-  public String asTargetString() {
+  @Override
+  public String toString() {
     String isolationKeyPart = isolationKey().map(key -> "%" + key).orElse("");
     return String.format(
         "%s%%%s%s",

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

@@ -225,7 +225,7 @@ public RunModuleExtensionResult run(
       try {
         return state.startOrContinueWork(
             env,
-            "module-extension-" + extensionId.asTargetString(),
+            "module-extension-" + extensionId,
             (workerEnv) ->
                 runInternal(
                     workerEnv, usagesValue, starlarkSemantics, extensionId, mainRepositoryMapping));
@@ -281,19 +281,15 @@ private RunModuleExtensionResult runInternal(
       thread.setThreadLocal(Label.RepoMappingRecorder.class, repoMappingRecorder);
       try (SilentCloseable c =
           Profiler.instance()
-              .profile(
-                  ProfilerTask.BZLMOD,
-                  () -> "evaluate module extension: " + extensionId.asTargetString())) {
+              .profile(ProfilerTask.BZLMOD, () -> "evaluate module extension: " + extensionId)) {
         Object returnValue =
             Starlark.fastcall(
                 thread, extension.implementation(), new Object[] {moduleContext}, new Object[0]);
         if (returnValue != Starlark.NONE && !(returnValue instanceof ModuleExtensionMetadata)) {
           throw ExternalDepsException.withMessage(
               ExternalDeps.Code.BAD_MODULE,
-              "expected module extension %s in %s to return None or extension_metadata, got"
-                  + " %s",
-              extensionId.extensionName(),
-              extensionId.bzlFileLabel(),
+              "expected module extension %s to return None or extension_metadata, got %s",
+              extensionId,
               Starlark.type(returnValue));
         }
         if (returnValue instanceof ModuleExtensionMetadata retMetadata) {
@@ -317,10 +313,7 @@ private RunModuleExtensionResult runInternal(
     } catch (EvalException e) {
       env.getListener().handle(Event.error(e.getInnermostLocation(), e.getMessageWithStack()));
       throw ExternalDepsException.withMessage(
-          ExternalDeps.Code.BAD_MODULE,
-          "error evaluating module extension %s in %s",
-          extensionId.extensionName(),
-          extensionId.bzlFileLabel());
+          ExternalDeps.Code.BAD_MODULE, "error evaluating module extension %s", extensionId);
     } catch (IOException e) {
       throw ExternalDepsException.withCauseAndMessage(
           ExternalDeps.Code.EXTERNAL_DEPS_UNKNOWN,

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

@@ -199,7 +199,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
                     String.format(
                         "The module extension %s produced an invalid lockfile entry because it"
                             + " referenced %s. Please report this issue to its maintainers.",
-                        extensionId.asTargetString(), nonVisibleRepoNames)));
+                        extensionId, nonVisibleRepoNames)));
       }
     }
     if (lockfileMode.equals(LockfileMode.ERROR)) {

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

@@ -57,10 +57,9 @@ public SkyValue compute(SkyKey skyKey, Environment env)
             throw new SingleExtensionFunctionException(
                 ExternalDepsException.withMessage(
                     Code.INVALID_EXTENSION_IMPORT,
-                    "module extension \"%s\" from \"%s\" does not generate repository \"%s\", yet"
+                    "module extension %s does not generate repository \"%s\", yet"
                         + " it is imported as \"%s\" in the usage at %s%s",
-                    extensionId.extensionName(),
-                    extensionId.bzlFileLabel(),
+                    extensionId,
                     repoImport.getValue(),
                     repoImport.getKey(),
                     proxy.getLocation(),
@@ -80,23 +79,21 @@ public SkyValue compute(SkyKey skyKey, Environment env)
           throw new SingleExtensionFunctionException(
               ExternalDepsException.withMessage(
                   Code.INVALID_EXTENSION_IMPORT,
-                  "module extension \"%s\" from \"%s\" generates repository \"%s\", yet"
+                  "module extension %s generates repository \"%s\", yet"
                       + " it is injected via inject_repo() at %s. Use override_repo() instead to"
                       + " override an existing repository.",
-                  extensionId.extensionName(),
-                  extensionId.bzlFileLabel(),
+                  extensionId,
                   override.getKey(),
                   override.getValue().location()),
               Transience.PERSISTENT);
         } else if (!repoExists && override.getValue().mustExist()) {
           throw new SingleExtensionFunctionException(
               ExternalDepsException.withMessage(
                   Code.INVALID_EXTENSION_IMPORT,
-                  "module extension \"%s\" from \"%s\" does not generate repository \"%s\", yet"
+                  "module extension %s does not generate repository \"%s\", yet"
                       + " it is overridden via override_repo() at %s. Use inject_repo() instead to"
                       + " inject a new repository.",
-                  extensionId.extensionName(),
-                  extensionId.bzlFileLabel(),
+                  extensionId,
                   override.getKey(),
                   override.getValue().location()),
               Transience.PERSISTENT);

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

@@ -120,7 +120,7 @@ private String toId(ModuleKey key) {
   }
 
   private String toId(ModuleExtensionId id) {
-    return id.asTargetString();
+    return id.toString();
   }
 
   private String toId(ModuleExtensionId id, String repo) {

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

@@ -60,7 +60,7 @@ public String printKey(ModuleKey key) {
   private JsonObject printExtension(
       ModuleKey key, ModuleExtensionId extensionId, boolean unexpanded) {
     JsonObject json = new JsonObject();
-    json.addProperty("key", extensionId.asTargetString());
+    json.addProperty("key", extensionId.toString());
     json.addProperty("unexpanded", unexpanded);
     if (options.extensionInfo == ExtensionShow.USAGES) {
       return json;

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

@@ -467,13 +467,12 @@ private boolean filterBuiltin(ModuleKey key) {
   /** Helper to display show_extension info. */
   private void displayExtension(ModuleExtensionId extension, ImmutableSet<ModuleKey> fromUsages)
       throws InvalidArgumentException {
-    printer.printf("## %s:\n", extension.asTargetString());
+    printer.printf("## %s:\n", extension.toString());
     printer.println();
     printer.println("Fetched repositories:");
     if (!extensionRepoImports.containsKey(extension)) {
       throw new InvalidArgumentException(
-          String.format(
-              "No extension %s exists in the dependency graph", extension.asTargetString()));
+          String.format("No extension %s exists in the dependency graph", extension));
     }
     ImmutableSortedSet<String> usedRepos =
         ImmutableSortedSet.copyOf(extensionRepoImports.get(extension).keySet());

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

@@ -96,7 +96,7 @@ private void printExtension(
       ModuleKey key, ModuleExtensionId extensionId, boolean unexpanded, int depth) {
     printTreeDrawing(IsIndirect.FALSE, depth);
     str.append('$');
-    str.append(extensionId.asTargetString());
+    str.append(extensionId);
     str.append(' ');
     if (unexpanded && options.extensionInfo == ExtensionShow.ALL) {
       str.append("... ");

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

@@ -107,9 +107,7 @@ public boolean maybeReportCycle(
             }
             Preconditions.checkArgument(input.argument() instanceof ModuleExtensionId);
             ModuleExtensionId id = (ModuleExtensionId) input.argument();
-            return String.format(
-                "extension '%s' defined in %s",
-                id.extensionName(), id.bzlFileLabel().getCanonicalForm());
+            return "module extension " + id;
           };
 
       StringBuilder cycleMessage =

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

@@ -156,9 +156,7 @@ public boolean maybeReportCycle(
             if (input instanceof RepositoryDirectoryValue.Key) {
               return ((RepositoryDirectoryValue.Key) input).argument().toString();
             } else if (input.argument() instanceof ModuleExtensionId id) {
-              return String.format(
-                  "extension '%s' defined in %s",
-                  id.extensionName(), id.bzlFileLabel().getCanonicalForm());
+              return "module extension " + id;
             } else if (input.argument() instanceof RepositoryMappingValue.Key key) {
               if (key == RepositoryMappingValue.KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS) {
                 return "repository mapping of @@ without WORKSPACE repos";

src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java

@@ -1339,7 +1339,7 @@ public void importNonExistentRepo() throws Exception {
     assertThat(result.getError().getException())
         .hasMessageThat()
         .contains(
-            "module extension \"ext\" from \"//:defs.bzl\" does not generate repository"
+            "module extension @@//:defs.bzl%ext does not generate repository"
                 + " \"missing_repo\", yet it is imported as \"my_repo\" in the usage at"
                 + " /ws/MODULE.bazel:1:20");
   }
@@ -1373,7 +1373,7 @@ public void invalidAttributeValue() throws Exception {
             + " of 'data_repo', but got 42 (int)");
     assertThat(result.getError().getException())
         .hasMessageThat()
-        .isEqualTo("error evaluating module extension ext in //:defs.bzl");
+        .isEqualTo("error evaluating module extension @@//:defs.bzl%ext");
   }
 
   @Test
@@ -1753,13 +1753,15 @@ public void testReportRepoAndBzlCycles_circularExtReposCtxRead() throws Exceptio
     cyclesReporter.reportCycles(
         result.getError().getCycleInfo(), skyKey, evaluationContext.getEventHandler());
     assertContainsEvent(
-        "ERROR <no location>: Circular definition of repositories generated by module extensions"
-            + " and/or .bzl files:\n"
-            + ".-> @@+my_ext+candy1\n"
-            + "|   extension 'my_ext' defined in //:defs.bzl\n"
-            + "|   @@+my_ext2+candy2\n"
-            + "|   extension 'my_ext2' defined in //:defs.bzl\n"
-            + "`-- @@+my_ext+candy1");
+        """
+        ERROR <no location>: Circular definition of repositories generated by module extensions\
+         and/or .bzl files:
+        .-> @@+my_ext+candy1
+        |   module extension @@//:defs.bzl%my_ext
+        |   @@+my_ext2+candy2
+        |   module extension @@//:defs.bzl%my_ext2
+        `-- @@+my_ext+candy1\
+        """);
   }
 
   @Test
@@ -1798,15 +1800,17 @@ public void testReportRepoAndBzlCycles_circularExtReposLoadInDefFile() throws Ex
     cyclesReporter.reportCycles(
         result.getError().getCycleInfo(), skyKey, evaluationContext.getEventHandler());
     assertContainsEvent(
-        "ERROR <no location>: Circular definition of repositories generated by module extensions"
-            + " and/or .bzl files:\n"
-            + ".-> @@+my_ext+candy1\n"
-            + "|   extension 'my_ext' defined in //:defs.bzl\n"
-            + "|   @@+my_ext2+candy2\n"
-            + "|   extension 'my_ext2' defined in //:defs2.bzl\n"
-            + "|   //:defs2.bzl\n"
-            + "|   @@+my_ext+candy1//:data.bzl\n"
-            + "`-- @@+my_ext+candy1");
+        """
+        ERROR <no location>: Circular definition of repositories generated by module extensions\
+         and/or .bzl files:
+        .-> @@+my_ext+candy1
+        |   module extension @@//:defs.bzl%my_ext
+        |   @@+my_ext2+candy2
+        |   module extension @@//:defs2.bzl%my_ext2
+        |   //:defs2.bzl
+        |   @@+my_ext+candy1//:data.bzl
+        `-- @@+my_ext+candy1\
+        """);
   }
 
   @Test
@@ -1836,13 +1840,15 @@ public void testReportRepoAndBzlCycles_extRepoLoadSelfCycle() throws Exception {
     cyclesReporter.reportCycles(
         result.getError().getCycleInfo(), skyKey, evaluationContext.getEventHandler());
     assertContainsEvent(
-        "ERROR <no location>: Circular definition of repositories generated by module extensions"
-            + " and/or .bzl files:\n"
-            + ".-> @@+my_ext+candy1\n"
-            + "|   extension 'my_ext' defined in //:defs.bzl\n"
-            + "|   //:defs.bzl\n"
-            + "|   @@+my_ext+candy1//:data.bzl\n"
-            + "`-- @@+my_ext+candy1");
+        """
+        ERROR <no location>: Circular definition of repositories generated by module extensions\
+         and/or .bzl files:
+        .-> @@+my_ext+candy1
+        |   module extension @@//:defs.bzl%my_ext
+        |   //:defs.bzl
+        |   @@+my_ext+candy1//:data.bzl
+        `-- @@+my_ext+candy1\
+        """);
   }
 
   @Test
@@ -2334,7 +2340,7 @@ public void extensionMetadata_all() throws Exception {
     assertThat(result.getError().getException())
         .hasMessageThat()
         .isEqualTo(
-            "module extension \"ext\" from \"@@ext+//:defs.bzl\" does not generate repository "
+            "module extension @@ext+//:defs.bzl%ext does not generate repository "
                 + "\"invalid_dep\", yet it is imported as \"invalid_dep\" in the usage at "
                 + "/ws/MODULE.bazel:3:20");
 
@@ -2430,7 +2436,7 @@ public void extensionMetadata_allDev() throws Exception {
     assertThat(result.getError().getException())
         .hasMessageThat()
         .isEqualTo(
-            "module extension \"ext\" from \"@@ext+//:defs.bzl\" does not generate repository "
+            "module extension @@ext+//:defs.bzl%ext does not generate repository "
                 + "\"invalid_dep\", yet it is imported as \"invalid_dep\" in the usage at "
                 + "/ws/MODULE.bazel:3:20");
 
@@ -3287,7 +3293,7 @@ def _ext_impl(ctx):
     assertThat(result.getError().getException())
         .hasMessageThat()
         .isEqualTo(
-            "module extension \"ext\" from \"//:defs.bzl\" does not generate repository \"foo\","
+            "module extension @@//:defs.bzl%ext does not generate repository \"foo\","
                 + " yet it is overridden via override_repo() at /ws/MODULE.bazel:6:14. Use"
                 + " inject_repo() instead to inject a new repository.");
   }