From 6b2e0db0a5ec6c3e56551195611abe1d1510b06d Mon Sep 17 00:00:00 2001
From: Fabian Meumertzheim <fabian@meumertzhe.im>
Date: Mon, 15 Apr 2024 11:18:11 -0700
Subject: [PATCH] Add `sep` keyword argument to `fail`

The argument was already available on 'print' and is necessary for full control over the formatting of error messages that rely on StarlarkValue's `debugPrint`.

Work towards #20486

Closes #21961.

PiperOrigin-RevId: 625022818
Change-Id: I895b5844d8543f936fc31d367b12bc6291a10bf8
---
 .../net/starlark/java/eval/MethodLibrary.java   | 17 ++++++++++++-----
 .../starlark/java/eval/MethodLibraryTest.java   |  1 +
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/src/main/java/net/starlark/java/eval/MethodLibrary.java b/src/main/java/net/starlark/java/eval/MethodLibrary.java
index f5c50fb35999fa..9e3f978bd8494c 100644
--- a/src/main/java/net/starlark/java/eval/MethodLibrary.java
+++ b/src/main/java/net/starlark/java/eval/MethodLibrary.java
@@ -812,16 +812,23 @@ public StarlarkList<?> dir(Object object, StarlarkThread thread) throws EvalExce
                 "Deprecated. Causes an optional prefix containing this string to be added to the"
                     + " error message.",
             positional = false,
-            named = true)
+            named = true),
+        @Param(
+            name = "sep",
+            defaultValue = "\" \"",
+            named = true,
+            positional = false,
+            doc = "The separator string between the objects, default is space (\" \").")
       },
       extraPositionals =
           @Param(
               name = "args",
               doc =
                   "A list of values, formatted with debugPrint (which is equivalent to str by"
-                      + " default) and joined with spaces, that appear in the error message."),
+                      + " default) and joined with sep (defaults to \" \"), that appear in the"
+                      + " error message."),
       useStarlarkThread = true)
-  public void fail(Object msg, Object attr, Tuple args, StarlarkThread thread)
+  public void fail(Object msg, Object attr, String sep, Tuple args, StarlarkThread thread)
       throws EvalException {
     Printer printer = new Printer();
     boolean needSeparator = false;
@@ -832,14 +839,14 @@ public void fail(Object msg, Object attr, Tuple args, StarlarkThread thread)
     // msg acts like a leading element of args.
     if (msg != Starlark.NONE) {
       if (needSeparator) {
-        printer.append(" ");
+        printer.append(sep);
       }
       printer.debugPrint(msg, thread.getSemantics());
       needSeparator = true;
     }
     for (Object arg : args) {
       if (needSeparator) {
-        printer.append(" ");
+        printer.append(sep);
       }
       printer.debugPrint(arg, thread.getSemantics());
       needSeparator = true;
diff --git a/src/test/java/net/starlark/java/eval/MethodLibraryTest.java b/src/test/java/net/starlark/java/eval/MethodLibraryTest.java
index f2204aee53676e..933c951b1d7a3d 100644
--- a/src/test/java/net/starlark/java/eval/MethodLibraryTest.java
+++ b/src/test/java/net/starlark/java/eval/MethodLibraryTest.java
@@ -755,6 +755,7 @@ ev.new Scenario()
         .testIfErrorContains("abc", "fail('abc')")
         .testIfErrorContains("18", "fail(18)")
         .testIfErrorContains("1 2 3", "fail(1, 2, 3)")
+        .testIfErrorContains("1, 2, 3", "fail(1, 2, 3, sep=', ')")
         .testIfErrorContains("attribute foo: 1 2 3", "fail(1, 2, 3, attr='foo')") // deprecated
         .testIfErrorContains("0 1 2 3", "fail(1, 2, 3, msg=0)"); // deprecated
   }