Clean up string representations for labels
If --incompatible_descriptive_string_representations is passed, labels are converted
to strings using `repr` differently: `Label("//package:name")` instead of
`"//package:name"`
This CL doesn't affect representations of other object types but provides the
necessary infrastructure for it.
PiperOrigin-RevId: 160955284
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
index 9c22901..247ea4d 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java
@@ -23,8 +23,8 @@
import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
-import com.google.devtools.build.lib.skylarkinterface.SkylarkPrintableValue;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.util.StringCanonicalizer;
import com.google.devtools.build.lib.util.StringUtilities;
@@ -51,7 +51,7 @@
)
@Immutable
@ThreadSafe
-public final class Label implements Comparable<Label>, Serializable, SkylarkPrintableValue, SkyKey {
+public final class Label implements Comparable<Label>, Serializable, SkylarkValue, SkyKey {
public static final PathFragment EXTERNAL_PACKAGE_NAME = PathFragment.create("external");
public static final PathFragment EXTERNAL_PACKAGE_FILE_NAME = PathFragment.create("WORKSPACE");
public static final String DEFAULT_REPOSITORY_DIRECTORY = "__main__";
@@ -562,11 +562,23 @@
}
@Override
- public void repr(SkylarkPrinter printer) {
+ public void reprLegacy(SkylarkPrinter printer) {
printer.repr(getCanonicalForm());
}
@Override
+ public void repr(SkylarkPrinter printer) {
+ printer.append("Label(");
+ printer.repr(getCanonicalForm());
+ printer.append(")");
+ }
+
+ @Override
+ public void strLegacy(SkylarkPrinter printer) {
+ printer.append(getCanonicalForm());
+ }
+
+ @Override
public void str(SkylarkPrinter printer) {
printer.append(getCanonicalForm());
}
diff --git a/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java b/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java
index 29cfb47..acc5a54 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java
@@ -466,7 +466,8 @@
if (attributeMap.isConfigurable(attr.getName())) {
// We don't know the actual value for configurable attributes, so we reconstruct
// the select without trying to resolve it.
- printStream.printf(outputAttributePattern,
+ printStream.printf(
+ outputAttributePattern,
attr.getPublicName(),
outputConfigurableAttrValue(rule, attributeMap, attr));
continue;
@@ -479,20 +480,17 @@
// Computed defaults that depend on configurable attributes can have multiple values.
continue;
}
- printStream.printf(outputAttributePattern,
+ printStream.printf(
+ outputAttributePattern,
attr.getPublicName(),
outputAttrValue(Iterables.getOnlyElement(values)));
}
printStream.printf(")\n%s", lineTerm);
}
- /**
- * Returns the given attribute value with BUILD output syntax. Does not support selects.
- */
+ /** Returns the given attribute value with BUILD output syntax. Does not support selects. */
private String outputAttrValue(Object value) {
- if (value instanceof Label) {
- value = ((Label) value).getDefaultCanonicalForm();
- } else if (value instanceof License) {
+ if (value instanceof License) {
List<String> licenseTypes = new ArrayList<>();
for (License.LicenseType licenseType : ((License) value).getLicenseTypes()) {
licenseTypes.add(licenseType.toString().toLowerCase());
@@ -504,7 +502,7 @@
} else if (value instanceof TriState) {
value = ((TriState) value).toInt();
}
- return Printer.repr(value);
+ return new LabelPrinter().repr(value).toString();
}
/**
@@ -513,14 +511,16 @@
* <p>Since query doesn't know which select path should be chosen, this doesn't try to
* resolve the final value. Instead it just reconstructs the select.
*/
- private String outputConfigurableAttrValue(Rule rule, RawAttributeMapper attributeMap,
- Attribute attr) {
+ private String outputConfigurableAttrValue(
+ Rule rule, RawAttributeMapper attributeMap, Attribute attr) {
List<String> selectors = new ArrayList<>();
- for (BuildType.Selector<?> selector : ((BuildType.SelectorList<?>)
- attributeMap.getRawAttributeValue(rule, attr)).getSelectors()) {
+ for (BuildType.Selector<?> selector :
+ ((BuildType.SelectorList<?>) attributeMap.getRawAttributeValue(rule, attr))
+ .getSelectors()) {
if (selector.isUnconditional()) {
- selectors.add(outputAttrValue(
- Iterables.getOnlyElement(selector.getEntries().entrySet()).getValue()));
+ selectors.add(
+ outputAttrValue(
+ Iterables.getOnlyElement(selector.getEntries().entrySet()).getValue()));
} else {
selectors.add(String.format("select(%s)", outputAttrValue(selector.getEntries())));
}
@@ -834,4 +834,16 @@
target.getPackage().getNameFragment())
: location.print();
}
+
+ private static class LabelPrinter extends Printer.BasePrinter {
+ @Override
+ public LabelPrinter repr(Object o) {
+ if (o instanceof Label) {
+ writeString(((Label) o).getCanonicalForm());
+ } else {
+ super.repr(o);
+ }
+ return this;
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkPrintableValue.java b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkPrintableValue.java
deleted file mode 100644
index 69426ea..0000000
--- a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkPrintableValue.java
+++ /dev/null
@@ -1,26 +0,0 @@
-// Copyright 2015 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-package com.google.devtools.build.lib.skylarkinterface;
-
-/**
- * A Skylark value that is represented by {@code str()} differently than by {@code repr()}.
- */
-public interface SkylarkPrintableValue extends SkylarkValue {
- /**
- * Print an informal, human-readable representation of the value.
- *
- * @param printer a printer to be used for formatting nested values.
- */
- void str(SkylarkPrinter printer);
-}
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkValue.java b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkValue.java
index a7c8c27..bf6a5c5 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkValue.java
@@ -37,4 +37,38 @@
* @param printer a printer to be used for formatting nested values.
*/
void repr(SkylarkPrinter printer);
+
+ /**
+ * Print a legacy representation of object x.
+ *
+ * <p>By default dispatches to the {@code repr} method. Should be called instead of {@code repr}
+ * if --incompatible_descriptive_string_representations=false is used.
+ *
+ * @param printer an instance of a printer to be used for formatting nested values
+ */
+ default void reprLegacy(SkylarkPrinter printer) {
+ repr(printer);
+ }
+
+ /**
+ * Print an informal, human-readable representation of the value.
+ *
+ * <p>By default dispatches to the {@code repr} method.
+ *
+ * @param printer a printer to be used for formatting nested values.
+ */
+ default void str(SkylarkPrinter printer) {
+ repr(printer);
+ }
+
+ /**
+ * Print a legacy informal, human-readable representation of the value.
+ *
+ * <p>By default dispatches to the {@code reprLegacy} method.
+ *
+ * @param printer a printer to be used for formatting nested values.
+ */
+ default void strLegacy(SkylarkPrinter printer) {
+ reprLegacy(printer);
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
index 06e46db..6022768 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java
@@ -84,7 +84,7 @@
}
/** Implements the "in" operator. */
- private static boolean in(Object lval, Object rval, Location location, Environment env)
+ private static boolean in(Object lval, Object rval, Environment env, Location location)
throws EvalException {
if (env.getSemantics().incompatibleDepsetIsNotIterable && rval instanceof SkylarkNestedSet) {
throw new EvalException(
@@ -163,7 +163,7 @@
return divide(lval, rval, location);
case PERCENT:
- return percent(lval, rval, location);
+ return percent(lval, rval, env, location);
case EQUALS_EQUALS:
return lval.equals(rval);
@@ -184,10 +184,10 @@
return compare(lval, rval, location) >= 0;
case IN:
- return in(lval, rval, location, env);
+ return in(lval, rval, env, location);
case NOT_IN:
- return !in(lval, rval, location, env);
+ return !in(lval, rval, env, location);
default:
throw new AssertionError("Unsupported binary operator: " + operator);
@@ -352,7 +352,7 @@
}
/** Implements Operator.PERCENT. */
- private static Object percent(Object lval, Object rval, Location location)
+ private static Object percent(Object lval, Object rval, Environment env, Location location)
throws EvalException {
// int % int
if (lval instanceof Integer && rval instanceof Integer) {
@@ -376,9 +376,9 @@
String pattern = (String) lval;
try {
if (rval instanceof Tuple) {
- return Printer.formatWithList(pattern, (Tuple) rval);
+ return Printer.getPrinter(env).formatWithList(pattern, (Tuple) rval).toString();
}
- return Printer.format(pattern, rval);
+ return Printer.getPrinter(env).format(pattern, rval).toString();
} catch (IllegalFormatException e) {
throw new EvalException(location, e.getMessage());
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FormatParser.java b/src/main/java/com/google/devtools/build/lib/syntax/FormatParser.java
index d4a2323..bc3338a 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FormatParser.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FormatParser.java
@@ -30,9 +30,11 @@
private static final ImmutableSet<Character> ILLEGAL_IN_FIELD =
ImmutableSet.of('.', '[', ']', ',');
+ private final Environment environment;
private final Location location;
- public FormatParser(Location location) {
+ public FormatParser(Environment environment, Location location) {
+ this.environment = environment;
this.location = location;
}
@@ -91,7 +93,7 @@
History history,
StringBuilder output)
throws EvalException {
- BasePrinter printer = Printer.getPrinter(output);
+ BasePrinter printer = Printer.getPrinter(environment, output);
if (has(chars, pos + 1, '{')) {
// Escaped brace -> output and move to char after right brace
printer.append("{");
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index 6caf43f..3ab4e4d 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -29,7 +29,6 @@
import com.google.devtools.build.lib.skylarkinterface.SkylarkInterfaceUtils;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
import com.google.devtools.build.lib.syntax.EvalException.EvalExceptionWithJavaCause;
-import com.google.devtools.build.lib.syntax.Printer.BasePrinter;
import com.google.devtools.build.lib.syntax.Runtime.NoneType;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.Preconditions;
@@ -271,7 +270,7 @@
@Override
public String toString() {
- BasePrinter printer = Printer.getPrinter();
+ Printer.LengthLimitedPrinter printer = new Printer.LengthLimitedPrinter();
if (obj != null) {
printer.append(obj.toString()).append(".");
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
index 5c5905b..e549699 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
@@ -963,7 +963,7 @@
Location loc,
Environment env)
throws EvalException {
- return new FormatParser(loc)
+ return new FormatParser(env, loc)
.format(
self,
args.getImmutableList(),
@@ -1619,12 +1619,13 @@
doc =
"Converts any object to string. This is useful for debugging."
+ "<pre class=\"language-python\">str(\"ab\") == \"ab\"</pre>",
- parameters = {@Param(name = "x", doc = "The object to convert.")}
+ parameters = {@Param(name = "x", doc = "The object to convert.")},
+ useEnvironment = true
)
private static final BuiltinFunction str =
new BuiltinFunction("str") {
- public String invoke(Object x) {
- return Printer.getPrinter().str(x).toString();
+ public String invoke(Object x, Environment env) {
+ return Printer.getPrinter(env).str(x).toString();
}
};
@@ -1634,12 +1635,13 @@
doc =
"Converts any object to a string representation. This is useful for debugging.<br>"
+ "<pre class=\"language-python\">str(\"ab\") == \\\"ab\\\"</pre>",
- parameters = {@Param(name = "x", doc = "The object to convert.")}
+ parameters = {@Param(name = "x", doc = "The object to convert.")},
+ useEnvironment = true
)
private static final BuiltinFunction repr =
new BuiltinFunction("repr") {
- public String invoke(Object x) {
- return Printer.getPrinter().repr(x).toString();
+ public String invoke(Object x, Environment env) {
+ return Printer.getPrinter(env).repr(x).toString();
}
};
@@ -2104,11 +2106,14 @@
public Runtime.NoneType invoke(
String sep, SkylarkList<?> starargs, Location loc, Environment env)
throws EvalException {
- String msg = starargs.stream().map(Printer::str).collect(joining(sep));
+ String msg =
+ starargs
+ .stream()
+ .map((Object o) -> Printer.getPrinter(env).str(o).toString())
+ .collect(joining(sep));
// As part of the integration test "skylark_flag_test.sh", if the
// "--internal_skylark_flag_test_canary" flag is enabled, append an extra marker string to
- // the
- // output.
+ // the output.
if (env.getSemantics().skylarkFlagTestCanary) {
msg += "<== skylark flag test ==>";
}
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 0a58177..12cc792 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
@@ -17,7 +17,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.skylarkinterface.SkylarkPrintableValue;
import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
@@ -51,22 +50,53 @@
public static final int SUGGESTED_CRITICAL_LIST_ELEMENTS_STRING_LENGTH = 32;
/**
- * Creates an instance of BasePrinter that wraps an existing buffer.
- * @param buffer an Appendable
- * @return new BasePrinter
+ * Creates an instance of {@link BasePrinter} that wraps an existing buffer.
+ *
+ * @param buffer an {@link Appendable}
+ * @return new {@link BasePrinter}
*/
- public static BasePrinter getPrinter(Appendable buffer) {
+ static BasePrinter getPrinter(Appendable buffer) {
return new BasePrinter(buffer);
}
/**
- * Creates an instance of BasePrinter with an empty buffer.
- * @return new BasePrinter
+ * Creates an instance of {@link BasePrinter} with an empty buffer.
+ *
+ * @return new {@link BasePrinter}
*/
public static BasePrinter getPrinter() {
return getPrinter(new StringBuilder());
}
+ /**
+ * Creates an instance of BasePrinter with a given buffer.
+ *
+ * @param env {@link Environment}
+ * @param buffer an {@link Appendable}
+ * @return new BasePrinter
+ */
+ static BasePrinter getPrinter(Environment env, Appendable buffer) {
+ if (env.getSemantics().incompatibleDescriptiveStringRepresentations) {
+ return new BasePrinter(buffer);
+ } else {
+ return new LegacyPrinter(buffer);
+ }
+ }
+
+ /**
+ * Creates an instance of BasePrinter with an empty buffer.
+ *
+ * @param env {@link Environment}
+ * @return new BasePrinter
+ */
+ static BasePrinter getPrinter(Environment env) {
+ if (env.getSemantics().incompatibleDescriptiveStringRepresentations) {
+ return new BasePrinter();
+ } else {
+ return new LegacyPrinter();
+ }
+ }
+
private Printer() {}
// These static methods proxy to the similar methods of BasePrinter
@@ -111,7 +141,7 @@
@Nullable String singletonTerminator,
int maxItemsToPrint,
int criticalItemsStringLength) {
- return getPrinter()
+ return new LengthLimitedPrinter()
.printAbbreviatedList(
list,
before,
@@ -164,7 +194,7 @@
boolean isTuple,
int maxItemsToPrint,
int criticalItemsStringLength) {
- return getPrinter()
+ return new LengthLimitedPrinter()
.printAbbreviatedList(list, isTuple, maxItemsToPrint, criticalItemsStringLength)
.toString();
}
@@ -256,158 +286,26 @@
}
}
- /**
- * Helper class for {@code Appendable}s that want to limit the length of their input.
- *
- * <p>Instances of this class act as a proxy for one {@code Appendable} object and decide whether
- * the given input (or parts of it) can be written to the underlying {@code Appendable}, depending
- * on whether the specified maximum length has been met or not.
- */
- private static final class LengthLimitedAppendable implements Appendable {
-
- private static final ImmutableSet<Character> SPECIAL_CHARS =
- ImmutableSet.of(',', ' ', '"', '\'', ':', '(', ')', '[', ']', '{', '}');
-
- private static final Pattern ARGS_PATTERN = Pattern.compile("<\\d+ more arguments>");
-
- private final Appendable original;
- private int limit;
- private boolean ignoreLimit;
- private boolean previouslyShortened;
-
- private LengthLimitedAppendable(Appendable original, int limit) {
- this.original = original;
- this.limit = limit;
- }
-
- private static LengthLimitedAppendable create(Appendable original, int limit) {
- // We don't want to overwrite the limit if original is already an instance of this class.
- return (original instanceof LengthLimitedAppendable)
- ? (LengthLimitedAppendable) original : new LengthLimitedAppendable(original, limit);
- }
-
- @Override
- public Appendable append(CharSequence csq) throws IOException {
- if (ignoreLimit || hasOnlySpecialChars(csq)) {
- // Don't update limit.
- original.append(csq);
- previouslyShortened = false;
- } else {
- int length = csq.length();
- if (length <= limit) {
- limit -= length;
- original.append(csq);
- } else {
- original.append(csq, 0, limit);
- // We don't want to append multiple ellipses.
- if (!previouslyShortened) {
- original.append("...");
- }
- appendTrailingSpecialChars(csq, limit);
- previouslyShortened = true;
- limit = 0;
- }
- }
- return this;
- }
-
- /**
- * Appends any trailing "special characters" (e.g. brackets, quotation marks) in the given
- * sequence to the output buffer, regardless of the limit.
- *
- * <p>For example, let's look at foo(['too long']). Without this method, the shortened result
- * would be foo(['too...) instead of the prettier foo(['too...']).
- *
- * <p>If the input string was already shortened and contains "<x more arguments>", this part
- * will also be appended.
- */
- // TODO(bazel-team): Given an input list
- //
- // [1, 2, 3, [10, 20, 30, 40, 50, 60], 4, 5, 6]
- //
- // the inner list gets doubly mangled as
- //
- // [1, 2, 3, [10, 20, 30, 40, <2 more argu...<2 more arguments>], <3 more arguments>]
- private void appendTrailingSpecialChars(CharSequence csq, int limit) throws IOException {
- int length = csq.length();
- Matcher matcher = ARGS_PATTERN.matcher(csq);
- // We assume that everything following the "x more arguments" part has to be copied, too.
- int start = matcher.find() ? matcher.start() : length;
- // Find the left-most non-arg char that has to be copied.
- for (int i = start - 1; i > limit; --i) {
- if (isSpecialChar(csq.charAt(i))) {
- start = i;
- } else {
- break;
- }
- }
- if (start < length) {
- original.append(csq, start, csq.length());
- }
- }
-
- /**
- * Returns whether the given sequence denotes characters that are not part of the value of an
- * argument.
- *
- * <p>Examples are brackets, braces and quotation marks.
- */
- private boolean hasOnlySpecialChars(CharSequence csq) {
- for (int i = 0; i < csq.length(); ++i) {
- if (!isSpecialChar(csq.charAt(i))) {
- return false;
- }
- }
- return true;
- }
-
- private boolean isSpecialChar(char c) {
- return SPECIAL_CHARS.contains(c);
- }
-
- @Override
- public Appendable append(CharSequence csq, int start, int end) throws IOException {
- return this.append(csq.subSequence(start, end));
- }
-
- @Override
- public Appendable append(char c) throws IOException {
- return this.append(String.valueOf(c));
- }
-
- public boolean hasHitLimit() {
- return limit <= 0;
- }
-
- public void enforceLimit() {
- ignoreLimit = false;
- }
-
- public void ignoreLimit() {
- ignoreLimit = true;
- }
-
- @Override
- public String toString() {
- return original.toString();
- }
- }
-
/** Actual class that implements Printer API */
- public static final class BasePrinter implements SkylarkPrinter {
+ public static class BasePrinter implements SkylarkPrinter {
// Methods of this class should not recurse through static methods of Printer
- private final Appendable buffer;
+ protected final Appendable buffer;
/**
* Creates a printer instance.
*
- * @param buffer the Appendable to which to print the representation
+ * @param buffer the {@link Appendable} to which to print the representation
*/
- private BasePrinter(Appendable buffer) {
+ protected BasePrinter(Appendable buffer) {
this.buffer = buffer;
}
+ /** Creates a printer instance with a new StringBuilder. */
+ protected BasePrinter() {
+ this.buffer = new StringBuilder();
+ }
+
@Override
public String toString() {
return buffer.toString();
@@ -421,8 +319,8 @@
* @return the buffer, in fluent style
*/
public BasePrinter str(Object o) {
- if (o instanceof SkylarkPrintableValue) {
- ((SkylarkPrintableValue) o).str(this);
+ if (o instanceof SkylarkValue) {
+ ((SkylarkValue) o).str(this);
return this;
}
@@ -496,9 +394,9 @@
* Write a properly escaped Skylark representation of a string to a buffer.
*
* @param s the string a representation of which to repr.
- * @return the Appendable, in fluent style.
+ * @return this printer.
*/
- private BasePrinter writeString(String s) {
+ protected BasePrinter writeString(String s) {
this.append(SKYLARK_QUOTATION_MARK);
int len = s.length();
for (int i = 0; i < len; i++) {
@@ -544,7 +442,7 @@
* @param singletonTerminator null or a string to print after the list if it is a singleton The
* singleton case is notably relied upon in python syntax to distinguish a tuple of size one
* such as ("foo",) from a merely parenthesized object such as ("foo").
- * @return the BasePrinter.
+ * @return this printer.
*/
@Override
public BasePrinter printList(
@@ -553,48 +451,9 @@
String separator,
String after,
@Nullable String singletonTerminator) {
- return printAbbreviatedList(list, before, separator, after, singletonTerminator, -1, -1);
- }
- /**
- * Print a list of object representations.
- *
- * <p>The length of the output will be limited when both {@code maxItemsToPrint} and {@code
- * criticalItemsStringLength} have values greater than zero.
- *
- * @param list the list of objects to repr (each as with repr)
- * @param before a string to print before the list
- * @param separator a separator to print between each object
- * @param after a string to print after the list
- * @param singletonTerminator null or a string to print after the list if it is a singleton The
- * singleton case is notably relied upon in python syntax to distinguish a tuple of size one
- * such as ("foo",) from a merely parenthesized object such as ("foo").
- * @param maxItemsToPrint the maximum number of elements to be printed.
- * @param criticalItemsStringLength a soft limit for the total string length of all arguments.
- * 'Soft' means that this limit may be exceeded because of formatting.
- * @return the BasePrinter.
- */
- public BasePrinter printAbbreviatedList(
- Iterable<?> list,
- String before,
- String separator,
- String after,
- @Nullable String singletonTerminator,
- int maxItemsToPrint,
- int criticalItemsStringLength) {
this.append(before);
- int len = 0;
- // Limits the total length of the string representation of the elements, if specified.
- if (maxItemsToPrint > 0 && criticalItemsStringLength > 0) {
- len =
- appendListElements(
- LengthLimitedAppendable.create(buffer, criticalItemsStringLength),
- list,
- separator,
- maxItemsToPrint);
- } else {
- len = appendListElements(list, separator);
- }
+ int len = appendListElements(list, separator);
if (singletonTerminator != null && len == 1) {
this.append(singletonTerminator);
}
@@ -620,70 +479,21 @@
}
/**
- * Tries to append the given elements to the specified {@link Appendable} until specific limits
- * are reached.
- *
- * @return the number of appended elements.
- */
- private int appendListElements(
- LengthLimitedAppendable appendable,
- Iterable<?> list,
- String separator,
- int maxItemsToPrint) {
- boolean printSeparator = false; // don't print the separator before the first element
- boolean skipArgs = false;
- int items = Iterables.size(list);
- int len = 0;
- // We don't want to print "1 more arguments", hence we don't skip arguments if there is only
- // one above the limit.
- int itemsToPrint = (items - maxItemsToPrint == 1) ? items : maxItemsToPrint;
- appendable.enforceLimit();
- for (Object o : list) {
- // We don't want to print "1 more arguments", even if we hit the string limit.
- if (len == itemsToPrint || (appendable.hasHitLimit() && len < items - 1)) {
- skipArgs = true;
- break;
- }
- if (printSeparator) {
- this.append(separator);
- }
- Printer.getPrinter(appendable).repr(o);
- printSeparator = true;
- len++;
- }
- appendable.ignoreLimit();
- if (skipArgs) {
- this.append(separator);
- this.append(String.format("<%d more arguments>", items - len));
- }
- return len;
- }
-
- /**
* Print a Skylark list or tuple of object representations
*
* @param list the contents of the list or tuple
* @param isTuple if true the list will be formatted with parentheses and with a trailing comma
- * in case of one-element tuples.
- * @param maxItemsToPrint the maximum number of elements to be printed.
- * @param criticalItemsStringLength a soft limit for the total string length of all arguments.
- * 'Soft' means that this limit may be exceeded because of formatting.
- * @return the Appendable, in fluent style.
+ * in case of one-element tuples. 'Soft' means that this limit may be exceeded because of
+ * formatting.
+ * @return this printer.
*/
- public BasePrinter printAbbreviatedList(
- Iterable<?> list, boolean isTuple, int maxItemsToPrint, int criticalItemsStringLength) {
- if (isTuple) {
- return this.printAbbreviatedList(list, "(", ", ", ")", ",",
- maxItemsToPrint, criticalItemsStringLength);
- } else {
- return this.printAbbreviatedList(list, "[", ", ", "]", null,
- maxItemsToPrint, criticalItemsStringLength);
- }
- }
-
@Override
public BasePrinter printList(Iterable<?> list, boolean isTuple) {
- return this.printAbbreviatedList(list, isTuple, -1, -1);
+ if (isTuple) {
+ return this.printList(list, "(", ", ", ")", ",");
+ } else {
+ return this.printList(list, "[", ", ", "]", null);
+ }
}
/**
@@ -743,9 +553,9 @@
if (a >= argLength) {
throw new MissingFormatWidthException(
"not enough arguments for format pattern "
- + this.repr(pattern)
+ + Printer.repr(pattern)
+ ": "
- + this.repr(Tuple.copyOf(arguments)));
+ + Printer.repr(Tuple.copyOf(arguments)));
}
Object argument = arguments.get(a++);
switch (directive) {
@@ -755,7 +565,7 @@
continue;
} else {
throw new MissingFormatWidthException(
- "invalid argument " + this.repr(argument) + " for format pattern %d");
+ "invalid argument " + Printer.repr(argument) + " for format pattern %d");
}
case 'r':
this.repr(argument);
@@ -791,5 +601,258 @@
Printer.append(buffer, s);
return this;
}
+
+ BasePrinter append(CharSequence sequence, int start, int end) {
+ return this.append(sequence.subSequence(start, end));
+ }
+ }
+
+ /** A version of BasePrinter that renders object in old style for compatibility reasons. */
+ static final class LegacyPrinter extends BasePrinter {
+ protected LegacyPrinter() {
+ super();
+ }
+
+ protected LegacyPrinter(Appendable buffer) {
+ super(buffer);
+ }
+
+ @Override
+ public LegacyPrinter repr(Object o) {
+ if (o instanceof SkylarkValue) {
+ ((SkylarkValue) o).reprLegacy(this);
+ } else {
+ super.repr(o);
+ }
+ return this;
+ }
+
+ @Override
+ public LegacyPrinter str(Object o) {
+ if (o instanceof SkylarkValue) {
+ ((SkylarkValue) o).strLegacy(this);
+ } else {
+ super.str(o);
+ }
+ return this;
+ }
+ }
+
+ /** A version of {@code BasePrinter} that is able to print abbreviated lists. */
+ public static final class LengthLimitedPrinter extends BasePrinter {
+
+ private static final ImmutableSet<Character> SPECIAL_CHARS =
+ ImmutableSet.of(',', ' ', '"', '\'', ':', '(', ')', '[', ']', '{', '}');
+
+ private static final Pattern ARGS_PATTERN = Pattern.compile("<\\d+ more arguments>");
+
+ // Limits can be set several times recursively and then unset the same amount of times.
+ // But in fact they should be set only the first time and unset only the last time.
+ // To achieve that we need to keep track of the recursion depth.
+ private int recursionDepth;
+ // Current limit of symbols to print in the limited mode (`ignoreLimit = false`).
+ private int limit;
+ private boolean ignoreLimit = true;
+ private boolean previouslyShortened;
+
+ /**
+ * Print a list of object representations.
+ *
+ * <p>The length of the output will be limited when both {@code maxItemsToPrint} and {@code
+ * criticalItemsStringLength} have values greater than zero.
+ *
+ * @param list the list of objects to repr (each as with repr)
+ * @param before a string to print before the list
+ * @param separator a separator to print between each object
+ * @param after a string to print after the list
+ * @param singletonTerminator null or a string to print after the list if it is a singleton The
+ * singleton case is notably relied upon in python syntax to distinguish a tuple of size one
+ * such as ("foo",) from a merely parenthesized object such as ("foo").
+ * @param maxItemsToPrint the maximum number of elements to be printed.
+ * @param criticalItemsStringLength a soft limit for the total string length of all arguments.
+ * 'Soft' means that this limit may be exceeded because of formatting.
+ * @return the BasePrinter.
+ */
+ LengthLimitedPrinter printAbbreviatedList(
+ Iterable<?> list,
+ String before,
+ String separator,
+ String after,
+ @Nullable String singletonTerminator,
+ int maxItemsToPrint,
+ int criticalItemsStringLength) {
+ this.append(before);
+ int len = appendListElements(list, separator, maxItemsToPrint, criticalItemsStringLength);
+ if (singletonTerminator != null && len == 1) {
+ this.append(singletonTerminator);
+ }
+ return this.append(after);
+ }
+
+ /**
+ * Print a Skylark list or tuple of object representations
+ *
+ * @param list the contents of the list or tuple
+ * @param isTuple if true the list will be formatted with parentheses and with a trailing comma
+ * in case of one-element tuples.
+ * @param maxItemsToPrint the maximum number of elements to be printed.
+ * @param criticalItemsStringLength a soft limit for the total string length of all arguments.
+ * 'Soft' means that this limit may be exceeded because of formatting.
+ * @return this printer.
+ */
+ public LengthLimitedPrinter printAbbreviatedList(
+ Iterable<?> list, boolean isTuple, int maxItemsToPrint, int criticalItemsStringLength) {
+ if (isTuple) {
+ return this.printAbbreviatedList(
+ list, "(", ", ", ")", ",", maxItemsToPrint, criticalItemsStringLength);
+ } else {
+ return this.printAbbreviatedList(
+ list, "[", ", ", "]", null, maxItemsToPrint, criticalItemsStringLength);
+ }
+ }
+
+ /**
+ * Tries to append the given elements to the specified {@link Appendable} until specific limits
+ * are reached.
+ *
+ * @return the number of appended elements.
+ */
+ private int appendListElements(
+ Iterable<?> list, String separator, int maxItemsToPrint, int criticalItemsStringLength) {
+ boolean printSeparator = false; // don't print the separator before the first element
+ boolean skipArgs = false;
+ int items = Iterables.size(list);
+ int len = 0;
+ // We don't want to print "1 more arguments", hence we don't skip arguments if there is only
+ // one above the limit.
+ int itemsToPrint = (items - maxItemsToPrint == 1) ? items : maxItemsToPrint;
+ enforceLimit(criticalItemsStringLength);
+ for (Object o : list) {
+ // We don't want to print "1 more arguments", even if we hit the string limit.
+ if (len == itemsToPrint || (hasHitLimit() && len < items - 1)) {
+ skipArgs = true;
+ break;
+ }
+ if (printSeparator) {
+ this.append(separator);
+ }
+ this.repr(o);
+ printSeparator = true;
+ len++;
+ }
+ ignoreLimit();
+ if (skipArgs) {
+ this.append(separator);
+ this.append(String.format("<%d more arguments>", items - len));
+ }
+ return len;
+ }
+
+ @Override
+ public LengthLimitedPrinter append(CharSequence csq) {
+ if (ignoreLimit || hasOnlySpecialChars(csq)) {
+ // Don't update limit.
+ Printer.append(buffer, csq);
+ previouslyShortened = false;
+ } else {
+ int length = csq.length();
+ if (length <= limit) {
+ limit -= length;
+ Printer.append(buffer, csq);
+ } else {
+ Printer.append(buffer, csq, 0, limit);
+ // We don't want to append multiple ellipses.
+ if (!previouslyShortened) {
+ Printer.append(buffer, "...");
+ }
+ appendTrailingSpecialChars(csq, limit);
+ previouslyShortened = true;
+ limit = 0;
+ }
+ }
+ return this;
+ }
+
+ @Override
+ public LengthLimitedPrinter append(char c) {
+ // Use the local `append(sequence)` method so that limits can apply
+ return this.append(String.valueOf(c));
+ }
+
+ /**
+ * Appends any trailing "special characters" (e.g. brackets, quotation marks) in the given
+ * sequence to the output buffer, regardless of the limit.
+ *
+ * <p>For example, let's look at foo(['too long']). Without this method, the shortened result
+ * would be foo(['too...) instead of the prettier foo(['too...']).
+ *
+ * <p>If the input string was already shortened and contains "<x more arguments>", this part
+ * will also be appended.
+ */
+ // TODO(bazel-team): Given an input list
+ //
+ // [1, 2, 3, [10, 20, 30, 40, 50, 60], 4, 5, 6]
+ //
+ // the inner list gets doubly mangled as
+ //
+ // [1, 2, 3, [10, 20, 30, 40, <2 more argu...<2 more arguments>], <3 more arguments>]
+ private LengthLimitedPrinter appendTrailingSpecialChars(CharSequence csq, int limit) {
+ int length = csq.length();
+ Matcher matcher = ARGS_PATTERN.matcher(csq);
+ // We assume that everything following the "x more arguments" part has to be copied, too.
+ int start = matcher.find() ? matcher.start() : length;
+ // Find the left-most non-arg char that has to be copied.
+ for (int i = start - 1; i > limit; --i) {
+ if (isSpecialChar(csq.charAt(i))) {
+ start = i;
+ } else {
+ break;
+ }
+ }
+ if (start < length) {
+ Printer.append(buffer, csq, start, csq.length());
+ }
+ return this;
+ }
+
+ /**
+ * Returns whether the given sequence denotes characters that are not part of the value of an
+ * argument.
+ *
+ * <p>Examples are brackets, braces and quotation marks.
+ */
+ private boolean hasOnlySpecialChars(CharSequence csq) {
+ for (int i = 0; i < csq.length(); ++i) {
+ if (!isSpecialChar(csq.charAt(i))) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ private boolean isSpecialChar(char c) {
+ return SPECIAL_CHARS.contains(c);
+ }
+
+ boolean hasHitLimit() {
+ return limit <= 0;
+ }
+
+ private void enforceLimit(int limit) {
+ ignoreLimit = false;
+ if (recursionDepth == 0) {
+ this.limit = limit;
+ ++recursionDepth;
+ }
+ }
+
+ private void ignoreLimit() {
+ if (recursionDepth > 0) {
+ --recursionDepth;
+ }
+ if (recursionDepth == 0) {
+ ignoreLimit = true;
+ }
+ }
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java
index 097f581..d77fda8 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemanticsOptions.java
@@ -195,4 +195,19 @@
help = "If set to true, arithmetic operations throw an error in case of overflow/underflow."
)
public boolean incompatibleCheckedArithmetic;
+
+ @Option(
+ name = "incompatible_descriptive_string_representations",
+ defaultValue = "false",
+ category = "incompatible changes",
+ documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+ effectTags = {OptionEffectTag.UNKNOWN},
+ metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+ optionUsageRestrictions = OptionUsageRestrictions.UNDOCUMENTED,
+ help =
+ "If set to true, objects are converted to strings by `str` and `repr` functions using the "
+ + "new style representations that are designed to be more descriptive and not to leak "
+ + "information that's not supposed to be exposed."
+ )
+ public boolean incompatibleDescriptiveStringRepresentations;
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index 4f4126e..833cd9c 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -168,7 +168,7 @@
protected ConfigurationFactory configurationFactory;
protected BuildView view;
- private SequencedSkyframeExecutor skyframeExecutor;
+ protected SequencedSkyframeExecutor skyframeExecutor;
protected TimestampGranularityMonitor tsgm;
protected BlazeDirectories directories;
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index 5dde93b..860f1d7 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -509,7 +509,7 @@
"macro()",
"cc_library(name = 'a', srcs = [])");
getConfiguredTarget("//test:a");
- assertContainsEvent("selector({\"//foo:foo\": [\"//bar:bar\"]})");
+ assertContainsEvent("selector({Label(\"//foo:foo\"): [Label(\"//bar:bar\")]})");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkStringRepresentationsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkStringRepresentationsTest.java
new file mode 100644
index 0000000..7c777e2
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkStringRepresentationsTest.java
@@ -0,0 +1,303 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.skylark;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.skylark.util.SkylarkTestCase;
+import com.google.devtools.build.lib.util.Pair;
+import com.google.devtools.build.lib.vfs.ModifiedFileSet;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for string representations of Skylark objects. */
+@RunWith(JUnit4.class)
+public class SkylarkStringRepresentationsTest extends SkylarkTestCase {
+
+ // Different ways to format objects, these suffixes are used in the `prepare_params` function
+ private static final ImmutableList<String> SUFFIXES =
+ ImmutableList.of("_str", "_repr", "_format", "_str_perc", "_repr_perc");
+
+ private Object skylarkLoadingEval(String code) throws Exception {
+ return skylarkLoadingEval(code, "");
+ }
+
+ /**
+ * Evaluates {@code code} in the loading phase in a .bzl file
+ *
+ * @param code The code to execute
+ * @param definition Additional code to define necessary variables
+ */
+ private Object skylarkLoadingEval(String code, String definition) throws Exception {
+ scratch.overwriteFile("eval/BUILD", "load(':eval.bzl', 'eval')", "eval(name='eval')");
+ scratch.overwriteFile(
+ "eval/eval.bzl",
+ definition,
+ String.format("x = %s", code), // Should be placed here to execute during the loading phase
+ "def _impl(ctx):",
+ " return struct(result = x)",
+ "eval = rule(implementation = _impl)");
+ skyframeExecutor.invalidateFilesUnderPathForTesting(
+ reporter,
+ new ModifiedFileSet.Builder()
+ .modify(PathFragment.create("eval/BUILD"))
+ .modify(PathFragment.create("eval/eval.bzl"))
+ .build(),
+ rootDirectory);
+
+ ConfiguredTarget target = getConfiguredTarget("//eval");
+ return target.get("result");
+ }
+
+ /**
+ * Asserts that all 5 different ways to convert an object to a string of {@code expression}
+ * ({@code str}, {@code repr}, {@code '%s'}, {@code '%r'}, {@code '{}'.format} return the correct
+ * {@code representation}. Not applicable for objects that have different {@code str} and {@code
+ * repr} representations.
+ *
+ * @param definition optional definition required to evaluate the {@code expression}
+ * @param expression the expression to evaluate a string representation of
+ * @param representation desired string representation
+ */
+ private void assertStringRepresentation(
+ String definition, String expression, String representation) throws Exception {
+ assertThat(skylarkLoadingEval(String.format("str(%s)", expression), definition))
+ .isEqualTo(representation);
+ assertThat(skylarkLoadingEval(String.format("repr(%s)", expression), definition))
+ .isEqualTo(representation);
+ assertThat(skylarkLoadingEval(String.format("'%%s' %% (%s,)", expression), definition))
+ .isEqualTo(representation);
+ assertThat(skylarkLoadingEval(String.format("'%%r' %% (%s,)", expression), definition))
+ .isEqualTo(representation);
+ assertThat(skylarkLoadingEval(String.format("'{}'.format(%s)", expression), definition))
+ .isEqualTo(representation);
+ }
+
+ private void assertStringRepresentation(String expression, String representation)
+ throws Exception {
+ assertStringRepresentation("", expression, representation);
+ }
+
+ /**
+ * Creates a set of BUILD and .bzl files that gathers objects of many different types available in
+ * Skylark and creates their string representations by calling `str` and `repr` on them. The
+ * strings are available in the configured target for //test/skylark:check
+ */
+ private void generateFilesToTestStrings() throws Exception {
+ // Generate string representations of different Skylark types. Objects are generated in
+ // test/skylark/rules.bzl: the top-level objects dict contains objects
+ // available during the loading phase, and _check_impl(ctx) returns objects that are available
+ // during the analysis phase. prepare_params(objects) converts a list of objects to a list of
+ // their string representations.
+
+ scratch.file(
+ "test/skylark/rules.bzl",
+ "aspect_ctx_provider = provider()",
+ "def prepare_params(objects):",
+ " params = {}",
+ " for k, v in objects.items():",
+ " params[k + '_str'] = str(v)",
+ " params[k + '_repr'] = repr(v)",
+ " params[k + '_format'] = '{}'.format(v)",
+ " params[k + '_str_perc'] = '%s' % (v,)",
+ " params[k + '_repr_perc'] = '%r' % (v,)",
+ " return params",
+ "",
+ "def _impl_aspect(target, ctx):",
+ " return [aspect_ctx_provider(ctx = ctx)]",
+ "my_aspect = aspect(implementation = _impl_aspect)",
+ "",
+ "def _impl(ctx): pass",
+ "dep = rule(implementation = _impl)",
+ "",
+ "def _genfile_impl(ctx):",
+ " ctx.file_action(output = ctx.outputs.my_output, content = 'foo')",
+ "genfile = rule(",
+ " implementation = _genfile_impl,",
+ " outputs = {",
+ " 'my_output': '%{name}.txt',",
+ " },",
+ ")",
+ "",
+ "def _check_impl(ctx):",
+ " objects = {",
+ " 'target': ctx.attr.deps[0],",
+ " 'alias_target': ctx.attr.deps[1],",
+ " 'aspect_target': ctx.attr.asp_deps[0],",
+ " 'input_target': ctx.attr.srcs[0],",
+ " 'output_target': ctx.attr.srcs[1],",
+ " 'rule_ctx': ctx,",
+ " 'aspect_ctx': ctx.attr.asp_deps[0][aspect_ctx_provider].ctx,",
+ " }",
+ " return struct(**prepare_params(objects))",
+ "check = rule(",
+ " implementation = _check_impl,",
+ " attrs = {",
+ " 'deps': attr.label_list(),",
+ " 'asp_deps': attr.label_list(aspects = [my_aspect]),",
+ " 'srcs': attr.label_list(allow_files = True),",
+ " },",
+ ")");
+
+ scratch.file(
+ "test/skylark/BUILD",
+ "load(':rules.bzl', 'check', 'dep', 'genfile')",
+ "",
+ "dep(name = 'foo')",
+ "dep(name = 'bar')",
+ "alias(name = 'foobar', actual = ':foo')",
+ "genfile(name = 'output')",
+ "check(",
+ " name = 'check',",
+ " deps = [':foo', ':foobar'],",
+ " asp_deps = [':bar'],",
+ " srcs = ['input.txt', 'output.txt'],",
+ ")");
+ }
+
+ @Test
+ public void testStringRepresentations_Strings() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
+
+ assertThat(skylarkLoadingEval("str('foo')")).isEqualTo("foo");
+ assertThat(skylarkLoadingEval("'%s' % 'foo'")).isEqualTo("foo");
+ assertThat(skylarkLoadingEval("'{}'.format('foo')")).isEqualTo("foo");
+ assertThat(skylarkLoadingEval("repr('foo')")).isEqualTo("\"foo\"");
+ assertThat(skylarkLoadingEval("'%r' % 'foo'")).isEqualTo("\"foo\"");
+ }
+
+ @Test
+ public void testStringRepresentations_Labels() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
+
+ assertThat(skylarkLoadingEval("str(Label('//foo:bar'))")).isEqualTo("//foo:bar");
+ assertThat(skylarkLoadingEval("'%s' % Label('//foo:bar')")).isEqualTo("//foo:bar");
+ assertThat(skylarkLoadingEval("'{}'.format(Label('//foo:bar'))")).isEqualTo("//foo:bar");
+ assertThat(skylarkLoadingEval("repr(Label('//foo:bar'))")).isEqualTo("Label(\"//foo:bar\")");
+ assertThat(skylarkLoadingEval("'%r' % Label('//foo:bar')")).isEqualTo("Label(\"//foo:bar\")");
+
+ assertThat(skylarkLoadingEval("'{}'.format([Label('//foo:bar')])")).isEqualTo("[Label(\"//foo:bar\")]");
+ }
+
+ @Test
+ public void testStringRepresentations_Primitives() throws Exception {
+ // Strings are tested in a separate test case as they have different str and repr values.
+ setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
+
+ assertStringRepresentation("1543", "1543");
+ assertStringRepresentation("True", "True");
+ assertStringRepresentation("False", "False");
+ }
+
+ @Test
+ public void testStringRepresentations_Containers() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=true");
+
+ assertStringRepresentation("['a', 'b']", "[\"a\", \"b\"]");
+ assertStringRepresentation("('a', 'b')", "(\"a\", \"b\")");
+ assertStringRepresentation("{'a': 'b', 'c': 'd'}", "{\"a\": \"b\", \"c\": \"d\"}");
+ assertStringRepresentation("struct(d = 4, c = 3)", "struct(c = 3, d = 4)");
+ }
+
+ @Test
+ public void testLegacyStringRepresentations_Labels() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=false");
+
+ assertThat(skylarkLoadingEval("str(Label('//foo:bar'))")).isEqualTo("//foo:bar");
+ assertThat(skylarkLoadingEval("'%s' % Label('//foo:bar')")).isEqualTo("//foo:bar");
+ assertThat(skylarkLoadingEval("'{}'.format(Label('//foo:bar'))")).isEqualTo("//foo:bar");
+ assertThat(skylarkLoadingEval("repr(Label('//foo:bar'))")).isEqualTo("\"//foo:bar\"");
+ assertThat(skylarkLoadingEval("'%r' % Label('//foo:bar')")).isEqualTo("\"//foo:bar\"");
+
+ // Also test that str representations (as opposed to repr) also use legacy formatting
+ // They are equivalent for labels, but not equivalent for lists of labels, because
+ // containers always render their items with repr
+ assertThat(skylarkLoadingEval("str([Label('//foo:bar')])")).isEqualTo("[\"//foo:bar\"]");
+ assertThat(skylarkLoadingEval("'{}'.format([Label('//foo:bar')])")).isEqualTo("[\"//foo:bar\"]");
+ assertThat(skylarkLoadingEval("'%s' % [Label('//foo:bar')]")).isEqualTo("[\"//foo:bar\"]");
+ }
+
+ @Test
+ public void testLegacyStringRepresentations_Functions() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=false");
+
+ assertStringRepresentation("all", "<function all>");
+ assertStringRepresentation("def f(): pass", "f", "<function f>");
+ }
+
+ @Test
+ public void testLegacyStringRepresentations_Rules() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=false");
+
+ assertStringRepresentation("native.cc_library", "<function cc_library>");
+ assertStringRepresentation("rule(implementation=str)", "<function rule>");
+ assertStringRepresentation("aspect(implementation=str)", "Aspect:<function str>");
+ }
+
+ @Test
+ public void testLegacyStringRepresentations_Providers() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=false");
+
+ assertStringRepresentation("provider()", "<function <no name>>");
+ assertStringRepresentation("p = provider()", "p(b = 2, a = 1)", "p(a = 1, b = 2)");
+ }
+
+ @Test
+ public void testLegacyStringRepresentations_Select() throws Exception {
+ setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=false");
+
+ assertStringRepresentation(
+ "select({'//foo': ['//bar']}) + select({'//foo2': ['//bar2']})",
+ "selector({\"//foo\": [\"//bar\"]}) + selector({\"//foo2\": [\"//bar2\"]})");
+ }
+
+ @Test
+ public void testLegacyStringRepresentations_Targets() throws Exception {
+ // alias targets in skylark used to leak their memory addresses in string representations,
+ // we don't try to preserve this behaviour as it's harmful.
+ // An example of their legacy representation:
+ // "<com.google.devtools.build.lib.rules.AliasConfiguredTarget@12da9140>"
+
+ setSkylarkSemanticsOptions("--incompatible_descriptive_string_representations=false");
+
+ generateFilesToTestStrings();
+ ConfiguredTarget target = getConfiguredTarget("//test/skylark:check");
+
+
+ ImmutableList<Pair<String, String>> parameters = ImmutableList.of(
+ new Pair<>("rule_ctx", "//test/skylark:check"),
+ new Pair<>("aspect_ctx", "//test/skylark:bar"),
+ new Pair<>("input_target", "InputFileConfiguredTarget(//test/skylark:input.txt)"));
+ for (String suffix : SUFFIXES) {
+ for (Pair<String, String > pair : parameters) {
+ assertThat(target.get(pair.getFirst() + suffix)).isEqualTo(pair.getSecond());
+ }
+ }
+
+ // Legacy representation of several types of objects may contain nondeterministic chunks
+ parameters = ImmutableList.of(
+ new Pair<>("target", "ConfiguredTarget\\(//test/skylark:foo, [0-9a-f]+\\)"),
+ new Pair<>("aspect_target", "ConfiguredTarget\\(//test/skylark:bar, [0-9a-f]+\\)"),
+ new Pair<>("output_target", "ConfiguredTarget\\(//test/skylark:output.txt, [0-9a-f]+\\)"));
+ for (String suffix : SUFFIXES) {
+ for (Pair<String, String > pair : parameters) {
+ assertThat((String) target.get(pair.getFirst() + suffix)).matches(pair.getSecond());
+ }
+ }
+ }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
index 5e9391f..0e1bdab 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
@@ -47,8 +47,8 @@
* <p>If a test uses this method, it allows potential subclasses to run the very same test in a
* different mode in subclasses
*/
- protected ModalTestCase newTest() {
- return new BuildTest();
+ protected ModalTestCase newTest(String... skylarkOptions) {
+ return new BuildTest(skylarkOptions);
}
@Test
@@ -601,7 +601,12 @@
return new SkylarkValue() {
@Override
public void repr(SkylarkPrinter printer) {
- printer.append("str marker");
+ printer.append("<str marker>");
+ }
+
+ @Override
+ public void reprLegacy(SkylarkPrinter printer) {
+ printer.append("<str legacy marker>");
}
@Override
@@ -615,36 +620,45 @@
return new Object() {
@Override
public String toString() {
- return "unknown object";
+ return "<unknown object>";
}
};
}
@Test
public void testPercOnObject() throws Exception {
- newTest()
+ newTest("--incompatible_descriptive_string_representations=true")
.update("obj", createObjWithStr())
- .testStatement("'%s' % obj", "str marker");
+ .testStatement("'%s' % obj", "<str marker>");
+ newTest("--incompatible_descriptive_string_representations=false")
+ .update("obj", createObjWithStr())
+ .testStatement("'%s' % obj", "<str legacy marker>");
newTest()
.update("unknown", createUnknownObj())
- .testStatement("'%s' % unknown", "unknown object");
+ .testStatement("'%s' % unknown", "<unknown object>");
}
@Test
public void testPercOnObjectList() throws Exception {
- newTest()
+ newTest("--incompatible_descriptive_string_representations=true")
.update("obj", createObjWithStr())
- .testStatement("'%s %s' % (obj, obj)", "str marker str marker");
+ .testStatement("'%s %s' % (obj, obj)", "<str marker> <str marker>");
+ newTest("--incompatible_descriptive_string_representations=false")
+ .update("obj", createObjWithStr())
+ .testStatement("'%s %s' % (obj, obj)", "<str legacy marker> <str legacy marker>");
newTest()
.update("unknown", createUnknownObj())
- .testStatement("'%s %s' % (unknown, unknown)", "unknown object unknown object");
+ .testStatement("'%s %s' % (unknown, unknown)", "<unknown object> <unknown object>");
}
@Test
public void testPercOnObjectInvalidFormat() throws Exception {
- newTest()
+ newTest("--incompatible_descriptive_string_representations=true")
.update("obj", createObjWithStr())
- .testIfExactError("invalid argument str marker for format pattern %d", "'%d' % obj");
+ .testIfExactError("invalid argument <str marker> for format pattern %d", "'%d' % obj");
+ newTest("--incompatible_descriptive_string_representations=false")
+ .update("obj", createObjWithStr())
+ .testIfExactError("invalid argument <str marker> for format pattern %d", "'%d' % obj");
}
@Test
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 fdf4327..f1e5691 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
@@ -21,6 +21,8 @@
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter;
+import com.google.devtools.build.lib.skylarkinterface.SkylarkValue;
import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
import java.util.Arrays;
@@ -55,7 +57,7 @@
assertThat(Printer.repr(Runtime.NONE)).isEqualTo("None");
assertThat(Printer.str(Label.parseAbsolute("//x"))).isEqualTo("//x:x");
- assertThat(Printer.repr(Label.parseAbsolute("//x"))).isEqualTo("\"//x:x\"");
+ assertThat(Printer.repr(Label.parseAbsolute("//x"))).isEqualTo("Label(\"//x:x\")");
List<?> list = MutableList.of(null, "foo", "bar");
List<?> tuple = Tuple.of("foo", "bar");
@@ -215,6 +217,14 @@
assertThat(Printer.str(list)).isEqualTo(String.format("[%s]", Joiner.on(", ").join(list)));
}
+ @Test
+ public void testLegacyPrinter() throws Exception {
+ assertThat(new Printer.LegacyPrinter().str(createObjWithStr()).toString())
+ .isEqualTo("<str legacy marker>");
+ assertThat(new Printer.LegacyPrinter().repr(createObjWithStr()).toString())
+ .isEqualTo("<repr legacy marker>");
+ }
+
private String printListWithLimit(List<?> list) {
return printList(list, Printer.SUGGESTED_CRITICAL_LIST_ELEMENTS_COUNT,
Printer.SUGGESTED_CRITICAL_LIST_ELEMENTS_STRING_LENGTH);
@@ -224,4 +234,33 @@
return Printer.printAbbreviatedList(
list, "[", ", ", "]", "", criticalElementsCount, criticalStringLength);
}
+
+ private SkylarkValue createObjWithStr() {
+ return new SkylarkValue() {
+ @Override
+ public void repr(SkylarkPrinter printer) {
+ printer.append("<repr marker>");
+ }
+
+ @Override
+ public void reprLegacy(SkylarkPrinter printer) {
+ printer.append("<repr legacy marker>");
+ }
+
+ @Override
+ public void str(SkylarkPrinter printer) {
+ printer.append("<str marker>");
+ }
+
+ @Override
+ public void strLegacy(SkylarkPrinter printer) {
+ printer.append("<str legacy marker>");
+ }
+
+ @Override
+ public boolean isImmutable() {
+ return false;
+ }
+ };
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index 256985c..b5c6a70 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -55,8 +55,8 @@
* Skylark context
*/
@Override
- protected ModalTestCase newTest() {
- return new SkylarkTest();
+ protected ModalTestCase newTest(String... skylarkOptions) {
+ return new SkylarkTest(skylarkOptions);
}
@Immutable
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java b/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java
index 19665c6..70604c6 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java
@@ -579,11 +579,15 @@
* A class that runs all tests in Build mode
*/
protected class BuildTest extends ModalTestCase {
- public BuildTest() {}
+ private final String[] skylarkOptions;
+
+ public BuildTest(String... skylarkOptions) {
+ this.skylarkOptions = skylarkOptions;
+ }
@Override
protected void run(Testable testable) throws Exception {
- enableBuildMode();
+ enableBuildMode(skylarkOptions);
testable.run();
}
}
