[6.4.0] Make module extension tag's debugPrint useful for error mes…

…sages (#19285)

When passed to `print` or `fail`, a module extension tag now results in
a string such as `'foo' tag at /ws/MODULE.bazel:3:4`, which can be used
to form error messages referencing tags without leaking non-hermetic
information to the extension implementation function.

Closes #19274.

Commit
cc5889c

PiperOrigin-RevId: 558163928
Change-Id: If20086c62356bb0635d0a4bdf91029267122f62b

Co-authored-by: Fabian Meumertzheim <[email protected]>
Co-authored-by: Xùdōng Yáng <[email protected]>

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

@@ -51,7 +51,12 @@ public class StarlarkBazelModule implements StarlarkValue {
           "Contains the tags in a module for the module extension currently being processed. This"
               + " object has a field for each tag class of the extension, and the value of the"
               + " field is a list containing an object for each tag instance. This \"tag instance\""
-              + " object in turn has a field for each attribute of the tag class.")
+              + " object in turn has a field for each attribute of the tag class.\n\n"
+              + "When passed as positional arguments to <code>print()</code> or <code>fail()"
+              + "</code>, tag instance objects turn into a meaningful string representation of the"
+              + " form \"'install' tag at /home/user/workspace/MODULE.bazel:3:4\". This can be used"
+              + " to construct error messages that point to the location of the tag in the module"
+              + " file, e.g. <code>fail(\"Conflict between\", tag1, \"and\", tag2)</code>.")
   static class Tags implements Structure {
     private final ImmutableMap<String, StarlarkList<TypeCheckedTag>> typeCheckedTags;
 

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

@@ -23,8 +23,11 @@
 import javax.annotation.Nullable;
 import net.starlark.java.annot.StarlarkBuiltin;
 import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.Printer;
+import net.starlark.java.eval.StarlarkSemantics;
 import net.starlark.java.eval.Structure;
 import net.starlark.java.spelling.SpellChecker;
+import net.starlark.java.syntax.Location;
 
 /**
  * A {@link Tag} whose attribute values have been type-checked against the attribute schema define
@@ -36,10 +39,21 @@ public class TypeCheckedTag implements Structure {
   private final Object[] attrValues;
   private final boolean devDependency;
 
-  private TypeCheckedTag(TagClass tagClass, Object[] attrValues, boolean devDependency) {
+  // The properties below are only used for error reporting.
+  private final Location location;
+  private final String tagClassName;
+
+  private TypeCheckedTag(
+      TagClass tagClass,
+      Object[] attrValues,
+      boolean devDependency,
+      Location location,
+      String tagClassName) {
     this.tagClass = tagClass;
     this.attrValues = attrValues;
     this.devDependency = devDependency;
+    this.location = location;
+    this.tagClassName = tagClassName;
   }
 
   /** Creates a {@link TypeCheckedTag}. */
@@ -97,7 +111,8 @@ public static TypeCheckedTag create(TagClass tagClass, Tag tag, LabelConverter l
         attrValues[i] = Attribute.valueToStarlark(attr.getDefaultValueUnchecked());
       }
     }
-    return new TypeCheckedTag(tagClass, attrValues, tag.isDevDependency());
+    return new TypeCheckedTag(
+        tagClass, attrValues, tag.isDevDependency(), tag.getLocation(), tag.getTagName());
   }
 
   /**
@@ -133,4 +148,9 @@ public ImmutableCollection<String> getFieldNames() {
   public String getErrorMessageForUnknownField(String field) {
     return "unknown attribute " + field;
   }
+
+  @Override
+  public void debugPrint(Printer printer, StarlarkSemantics semantics) {
+    printer.append(String.format("'%s' tag at %s", tagClassName, location));
+  }
 }

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

@@ -2348,4 +2348,40 @@ public void isDevDependency_usages() throws Exception {
     assertThat(result.get(skyKey).getModule().getGlobal("ext2_data")).isEqualTo("ext2: False");
     assertThat(result.get(skyKey).getModule().getGlobal("ext3_data")).isEqualTo("ext3: True");
   }
+
+  @Test
+  public void printAndFailOnTag() throws Exception {
+    scratch.file(
+        workspaceRoot.getRelative("MODULE.bazel").getPathString(),
+        "ext = use_extension('//:defs.bzl', 'ext')",
+        "ext.foo()",
+        "ext.foo()");
+    scratch.file(
+        workspaceRoot.getRelative("defs.bzl").getPathString(),
+        "repo = repository_rule(lambda ctx: True)",
+        "def _ext_impl(ctx):",
+        "  tag1 = ctx.modules[0].tags.foo[0]",
+        "  tag2 = ctx.modules[0].tags.foo[1]",
+        "  print('Conflict between', tag1, 'and', tag2)",
+        "  fail('Fatal conflict between', tag1, 'and', tag2)",
+        "foo = tag_class()",
+        "ext = module_extension(implementation=_ext_impl,tag_classes={'foo':foo})");
+    scratch.file(workspaceRoot.getRelative("BUILD").getPathString());
+
+    ModuleExtensionId extensionId =
+        ModuleExtensionId.create(Label.parseCanonical("//:defs.bzl"), "ext", Optional.empty());
+    reporter.removeHandler(failFastHandler);
+    var result =
+        evaluator.<SingleExtensionEvalValue>evaluate(
+            ImmutableList.of(SingleExtensionEvalValue.key(extensionId)), evaluationContext);
+
+    assertThat(result.hasError()).isTrue();
+    assertContainsEvent(
+        "Fatal conflict between 'foo' tag at /ws/MODULE.bazel:2:8 and 'foo' tag at "
+            + "/ws/MODULE.bazel:3:8",
+        ImmutableSet.of(EventKind.ERROR));
+    assertContainsEvent(
+        "Conflict between 'foo' tag at /ws/MODULE.bazel:2:8 and 'foo' tag at /ws/MODULE.bazel:3:8",
+        ImmutableSet.of(EventKind.DEBUG));
+  }
 }