Skip to content

Commit

Permalink
Make module extension tag's debugPrint useful for error messages
Browse files Browse the repository at this point in the history
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 bazelbuild#19274.

PiperOrigin-RevId: 558163928
Change-Id: If20086c62356bb0635d0a4bdf91029267122f62b
  • Loading branch information
fmeum authored and iancha1992 committed Aug 18, 2023
1 parent 91bfb53 commit a0a751e
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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}. */
Expand Down Expand Up @@ -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());
}

/**
Expand Down Expand Up @@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

0 comments on commit a0a751e

Please sign in to comment.