Add option to restrict format strings to %s

This is useful for contexts like ctx.actions.args()'s methods, where %d and %r aren't appropriate placeholders.

RELNOTES: None
PiperOrigin-RevId: 191629195

2 files changed, 69 insertions, 15 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Printer.java b/src/main/java/com/google/devtools/build/lib/syntax/Printer.java
index b2248f8940..52a466b759 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Printer.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Printer.java
@@ -24,9 +24,11 @@ import java.io.IOException;
 import java.util.Arrays;
 import java.util.Formattable;
 import java.util.Formatter;
+import java.util.IllegalFormatException;
 import java.util.List;
 import java.util.Map;
 import java.util.MissingFormatWidthException;
+import java.util.UnknownFormatConversionException;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import javax.annotation.Nullable;
@@ -271,17 +273,35 @@ public class Printer {
     protected final Appendable buffer;
 
     /**
-     * Creates a printer instance.
+     * If true, the only percent sequences allowed in format strings are %s substitutions and %%
+     * escapes.
+     */
+    protected final boolean simplifiedFormatStrings;
+
+    /**
+     * Creates a printer.
      *
-     * @param buffer the {@link Appendable} to which to print the representation
+     * @param buffer the {@link Appendable} that will be written to
+     * @param simplifiedFormatStrings if true, format strings will allow only %s and %%
      */
-    protected BasePrinter(Appendable buffer) {
+    protected BasePrinter(Appendable buffer, boolean simplifiedFormatStrings) {
       this.buffer = buffer;
+      this.simplifiedFormatStrings = simplifiedFormatStrings;
+    }
+
+    /**
+     * Creates a printer that writes to the given buffer and that does not use simplified format
+     * strings.
+     */
+    protected BasePrinter(Appendable buffer) {
+      this(buffer, /*simplifiedFormatStrings=*/ false);
     }
 
-    /** Creates a printer instance with a new StringBuilder. */
+    /**
+     * Creates a printer that uses a fresh buffer and that does not use simplified format strings.
+     */
     protected BasePrinter() {
-      this.buffer = new StringBuilder();
+      this(new StringBuilder());
     }
 
     @Override
@@ -490,12 +510,25 @@ public class Printer {
     }
 
     /**
-     * Perform Python-style string formatting, as per pattern % tuple Limitations: only %d %s %r %%
-     * are supported.
+     * Perform Python-style string formatting, similar to the {@code pattern % tuple} syntax.
      *
-     * @param pattern a format string.
-     * @param arguments an array containing positional arguments.
-     * @return the formatted string.
+     * <p>The only supported placeholder patterns are
+     * <ul>
+     *   <li>{@code %s} (convert as if by {@code str()})
+     *   <li>{@code %r} (convert as if by {@code repr()})
+     *   <li>{@code %d} (convert an integer to its decimal representation)
+     * </ul>
+     * To encode a literal percent character, escape it as {@code %%}. It is an error to have a
+     * non-escaped {@code %} at the end of the string or followed by any character not listed above.
+     *
+     * <p>If this printer has {@code simplifiedFormatStrings} set, only {@code %s} and {@code %%}
+     * are permitted.
+     *
+     * @param pattern a format string that may contain placeholders
+     * @param arguments an array containing arguments to substitute into the placeholders in order
+     * @return the formatted string
+     * @throws IllegalFormatException if {@code pattern} is not a valid format string, or if
+     *     {@code arguments} mismatches the number or type of placeholders in {@code pattern}
      */
     @Override
     public BasePrinter format(String pattern, Object... arguments) {
@@ -503,12 +536,9 @@ public class Printer {
     }
 
     /**
-     * Perform Python-style string formatting, as per pattern % tuple Limitations: only %d %s %r %%
-     * are supported.
+     * Perform Python-style string formatting, similar to the {@code pattern % tuple} syntax.
      *
-     * @param pattern a format string.
-     * @param arguments a tuple containing positional arguments.
-     * @return the formatted string.
+     * <p>Same as {@link #format(String, Object...)}, but with a list instead of variadic args.
      */
     @Override
     public BasePrinter formatWithList(String pattern, List<?> arguments) {
@@ -543,6 +573,11 @@ public class Printer {
           case 'd':
           case 'r':
           case 's':
+            if (simplifiedFormatStrings && (directive != 's')) {
+              throw new UnknownFormatConversionException(
+                  "cannot use %" + directive + " substitution placeholder when "
+                      + "simplifiedFormatStrings is set");
+            }
             if (a >= argLength) {
               throw new MissingFormatWidthException(
                   "not enough arguments for format pattern "
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java b/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java
index 5adbd10b4e..c19d145936 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/PrinterTest.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.syntax;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
 import static org.junit.Assert.fail;
 
 import com.google.common.base.Joiner;
@@ -137,6 +138,24 @@ public class PrinterTest {
         "%.s");
   }
 
+  private SkylarkPrinter makeSimplifiedFormatPrinter() {
+    return new Printer.BasePrinter(new StringBuilder(), /*simplifiedFormatStrings=*/ true);
+  }
+
+  @Test
+  public void testSimplifiedDisallowsPlaceholdersBesidesPercentS() {
+    assertThat(makeSimplifiedFormatPrinter().format("Allowed: %%").toString())
+        .isEqualTo("Allowed: %");
+    assertThat(makeSimplifiedFormatPrinter().format("Allowed: %s", "abc").toString())
+        .isEqualTo("Allowed: abc");
+    assertThrows(
+        IllegalFormatException.class,
+        () -> makeSimplifiedFormatPrinter().format("Disallowed: %r", "abc"));
+    assertThrows(
+        IllegalFormatException.class,
+        () -> makeSimplifiedFormatPrinter().format("Disallowed: %d", 5));
+  }
+
   @Test
   public void testListLimitStringLength() throws Exception {
     int lengthDivisibleByTwo = Printer.SUGGESTED_CRITICAL_LIST_ELEMENTS_STRING_LENGTH;