From 247ce0c8b7250c7f40c3c66131ad00e9b98b5b34 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Thu, 17 Aug 2023 13:16:32 -0700 Subject: [PATCH] Use `debugPrint` instead of `str` for `fail` arguments This allows non-hermetic information (such as Starlark `Location`s) to be included in `fail` messages without allowing access to it from Starlark. It also aligns the content of `fail` output with that of `print`. This change has very mild implications for backwards compatibility: it only affects the contents of `fail` messages, which aren't accessible from Starlark; `debugPrint`'s default implementation is `str` and no built-in Starlark type overrides it; if a type overrides `debugPrint`, it would usually contain at least as detailed information as `str`; `debugPrint` is a "Bazelism" that isn't governed by the Starlark spec. Work towards #17375 Closes #18818. Co-authored-by: Alexandre Rostovtsev PiperOrigin-RevId: 557916312 Change-Id: I8f202cd8530bcebb2d99f57745289b3992d03cac --- .../net/starlark/java/eval/MethodLibrary.java | 32 +++++++++++-------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/main/java/net/starlark/java/eval/MethodLibrary.java b/src/main/java/net/starlark/java/eval/MethodLibrary.java index 5610061162982a..be5cf11651363b 100644 --- a/src/main/java/net/starlark/java/eval/MethodLibrary.java +++ b/src/main/java/net/starlark/java/eval/MethodLibrary.java @@ -15,13 +15,10 @@ package net.starlark.java.eval; import com.google.common.base.Ascii; -import com.google.common.base.Joiner; import com.google.common.collect.Ordering; -import java.util.ArrayList; import java.util.Arrays; import java.util.Comparator; import java.util.Iterator; -import java.util.List; import java.util.NoSuchElementException; import net.starlark.java.annot.Param; import net.starlark.java.annot.ParamType; @@ -679,24 +676,33 @@ public StarlarkList dir(Object object, StarlarkThread thread) throws EvalExce @Param( name = "args", doc = - "A list of values, formatted with str and joined with spaces, that appear in the" - + " error message."), + "A list of values, formatted with debugPrint (which is equivalent to str by" + + " default) and joined with spaces, that appear in the error message."), useStarlarkThread = true) public void fail(Object msg, Object attr, Tuple args, StarlarkThread thread) throws EvalException { - List elems = new ArrayList<>(); + Printer printer = new Printer(); + boolean needSeparator = false; + if (attr != Starlark.NONE) { + printer.append("attribute ").append((String) attr).append(":"); + needSeparator = true; + } // msg acts like a leading element of args. if (msg != Starlark.NONE) { - elems.add(Starlark.str(msg, thread.getSemantics())); + if (needSeparator) { + printer.append(" "); + } + printer.debugPrint(msg, thread.getSemantics()); + needSeparator = true; } for (Object arg : args) { - elems.add(Starlark.str(arg, thread.getSemantics())); - } - String str = Joiner.on(" ").join(elems); - if (attr != Starlark.NONE) { - str = String.format("attribute %s: %s", attr, str); + if (needSeparator) { + printer.append(" "); + } + printer.debugPrint(arg, thread.getSemantics()); + needSeparator = true; } - throw Starlark.errorf("%s", str); + throw Starlark.errorf("%s", printer.toString()); } @StarlarkMethod(