From 09dd662efd02b462d037fed8351c5b3fc3ce0f39 Mon Sep 17 00:00:00 2001 From: arostovtsev Date: Mon, 28 Jan 2019 12:04:40 -0800 Subject: [PATCH] Automated rollback of commit d3dd9d1b3cd238467143becb8665ea252fda8e0d. *** Reason for rollback *** Breaks lots of tests with "method 'getattr' returns an object of invalid type SingletonSet" error; see b/123520324 *** Original change description *** Convert MethodLibrary's SkylarkSignature methods to SkylarkCallable Note that converted parameter @Param annotations need legacyNamed = true (and in some places, noneable = True) to retain compatibility with the bugginess of SkylarkSignature. This facilitates followup to clean up these methods / parameters -- given the refactor, we can enforce some of these parameters are positional-only (subject to an --incompatible flag) Progress toward #5010 RELNOTES: None. PiperOrigin-RevId: 231264758 --- .../devtools/build/docgen/ApiExporter.java | 10 - .../build/lib/syntax/BuiltinCallable.java | 7 +- .../build/lib/syntax/FuncallExpression.java | 2 +- .../build/lib/syntax/MethodLibrary.java | 1019 +++++++++-------- .../build/lib/syntax/ParamDescriptor.java | 6 +- ...kylarkRuleImplementationFunctionsTest.java | 4 +- .../SkylarkStringRepresentationsTest.java | 4 +- .../build/lib/syntax/MethodLibraryTest.java | 11 +- .../build/lib/syntax/RuntimeTest.java | 10 + .../lib/syntax/SkylarkEvaluationTest.java | 7 +- .../lib/syntax/SkylarkNestedSetTest.java | 11 +- src/test/skylark/testdata/all_any.sky | 4 +- src/test/skylark/testdata/int_constructor.sky | 2 +- src/test/skylark/testdata/int_function.sky | 4 +- src/test/skylark/testdata/reversed.sky | 2 +- .../devtools/skylark/skylint/Environment.java | 8 +- 16 files changed, 594 insertions(+), 517 deletions(-) diff --git a/src/main/java/com/google/devtools/build/docgen/ApiExporter.java b/src/main/java/com/google/devtools/build/docgen/ApiExporter.java index 0063a5362244a5..de5d6829ebb8f4 100644 --- a/src/main/java/com/google/devtools/build/docgen/ApiExporter.java +++ b/src/main/java/com/google/devtools/build/docgen/ApiExporter.java @@ -27,7 +27,6 @@ import com.google.devtools.build.lib.skylarkinterface.SkylarkInterfaceUtils; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.syntax.BaseFunction; -import com.google.devtools.build.lib.syntax.BuiltinCallable; import com.google.devtools.build.lib.syntax.FuncallExpression; import com.google.devtools.build.lib.syntax.FunctionSignature; import com.google.devtools.build.lib.syntax.MethodDescriptor; @@ -94,15 +93,6 @@ private static void appendGlobals(Builtins.Builder builtins, Map Value.Builder value = Value.newBuilder(); if (obj instanceof BaseFunction) { value = collectFunctionInfo((BaseFunction) obj); - } else if (obj instanceof BuiltinCallable) { - BuiltinCallable builtinCallable = (BuiltinCallable) obj; - MethodDescriptor descriptor = - builtinCallable.getMethodDescriptor(SkylarkSemantics.DEFAULT_SEMANTICS); - value = - collectFunctionInfo( - descriptor.getName(), - SkylarkSignatureProcessor.getSignatureForCallable( - descriptor.getName(), descriptor, null, null)); } else { value.setName(entry.getKey()); } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinCallable.java b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinCallable.java index 93faa0fe11f4b6..1b8a6ed20c3e40 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/BuiltinCallable.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/BuiltinCallable.java @@ -42,7 +42,8 @@ public Object call( FuncallExpression ast, Environment env) throws EvalException, InterruptedException { - MethodDescriptor methodDescriptor = getMethodDescriptor(env.getSemantics()); + MethodDescriptor methodDescriptor = + FuncallExpression.getMethod(env.getSemantics(), obj.getClass(), methodName); // TODO(cparsons): Profiling should be done at the MethodDescriptor level. try (SilentCloseable c = @@ -54,10 +55,6 @@ public Object call( } } - public MethodDescriptor getMethodDescriptor(SkylarkSemantics semantics) { - return FuncallExpression.getMethod(semantics, obj.getClass(), methodName); - } - @Override public void repr(SkylarkPrinter printer) { 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 0fecadb9229e84..4cacee4041f929 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 @@ -647,7 +647,7 @@ private EvalException unexpectedKeywordArgumentException( String.format( "unexpected keyword%s %s", unexpectedKeywords.size() > 1 ? "s" : "", - Joiner.on(", ").join(Iterables.transform(unexpectedKeywords, s -> "'" + s + "'"))), + Joiner.on(",").join(Iterables.transform(unexpectedKeywords, s -> "'" + s + "'"))), method, objClass); } 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 4e7787ca4c4893..8b6e12c1fc913c 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 @@ -26,10 +26,9 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.skylarkinterface.Param; -import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; -import com.google.devtools.build.lib.skylarkinterface.SkylarkGlobalLibrary; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; +import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature; import com.google.devtools.build.lib.syntax.EvalUtils.ComparisonException; import com.google.devtools.build.lib.syntax.SkylarkList.MutableList; import com.google.devtools.build.lib.syntax.SkylarkList.Tuple; @@ -44,11 +43,13 @@ import javax.annotation.Nullable; /** A helper class containing built in functions for the Skylark language. */ -@SkylarkGlobalLibrary public class MethodLibrary { - @SkylarkCallable( + private MethodLibrary() {} + + @SkylarkSignature( name = "min", + returnType = Object.class, doc = "Returns the smallest one of all given arguments. " + "If only one argument is provided, it must be a non-empty iterable. " @@ -59,16 +60,22 @@ public class MethodLibrary { @Param(name = "args", type = SkylarkList.class, doc = "The elements to be checked."), useLocation = true, useEnvironment = true) - public Object min(SkylarkList args, Location loc, Environment env) throws EvalException { - try { - return findExtreme(args, EvalUtils.SKYLARK_COMPARATOR.reverse(), loc, env); - } catch (ComparisonException e) { - throw new EvalException(loc, e); - } - } + private static final BuiltinFunction min = + new BuiltinFunction("min") { + @SuppressWarnings("unused") // Accessed via Reflection. + public Object invoke(SkylarkList args, Location loc, Environment env) + throws EvalException { + try { + return findExtreme(args, EvalUtils.SKYLARK_COMPARATOR.reverse(), loc, env); + } catch (ComparisonException e) { + throw new EvalException(loc, e); + } + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "max", + returnType = Object.class, doc = "Returns the largest one of all given arguments. " + "If only one argument is provided, it must be a non-empty iterable." @@ -79,13 +86,18 @@ public Object min(SkylarkList args, Location loc, Environment env) throws Eva @Param(name = "args", type = SkylarkList.class, doc = "The elements to be checked."), useLocation = true, useEnvironment = true) - public Object max(SkylarkList args, Location loc, Environment env) throws EvalException { - try { - return findExtreme(args, EvalUtils.SKYLARK_COMPARATOR, loc, env); - } catch (ComparisonException e) { - throw new EvalException(loc, e); - } - } + private static final BuiltinFunction max = + new BuiltinFunction("max") { + @SuppressWarnings("unused") // Accessed via Reflection. + public Object invoke(SkylarkList args, Location loc, Environment env) + throws EvalException { + try { + return findExtreme(args, EvalUtils.SKYLARK_COMPARATOR, loc, env); + } catch (ComparisonException e) { + throw new EvalException(loc, e); + } + } + }; /** Returns the maximum element from this list, as determined by maxOrdering. */ private static Object findExtreme( @@ -101,8 +113,9 @@ private static Object findExtreme( } } - @SkylarkCallable( + @SkylarkSignature( name = "all", + returnType = Boolean.class, doc = "Returns true if all elements evaluate to True or if the collection is empty. " + "Elements are converted to boolean using the bool function." @@ -112,17 +125,22 @@ private static Object findExtreme( @Param( name = "elements", type = Object.class, - doc = "A string or a collection of elements.", - legacyNamed = true) + doc = "A string or a collection of elements.") }, useLocation = true, useEnvironment = true) - public Boolean all(Object collection, Location loc, Environment env) throws EvalException { - return !hasElementWithBooleanValue(collection, false, loc, env); - } + private static final BuiltinFunction all = + new BuiltinFunction("all") { + @SuppressWarnings("unused") // Accessed via Reflection. + public Boolean invoke(Object collection, Location loc, Environment env) + throws EvalException { + return !hasElementWithBooleanValue(collection, false, loc, env); + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "any", + returnType = Boolean.class, doc = "Returns true if at least one element evaluates to True. " + "Elements are converted to boolean using the bool function." @@ -132,14 +150,18 @@ public Boolean all(Object collection, Location loc, Environment env) throws Eval @Param( name = "elements", type = Object.class, - doc = "A string or a collection of elements.", - legacyNamed = true) + doc = "A string or a collection of elements.") }, useLocation = true, useEnvironment = true) - public Boolean any(Object collection, Location loc, Environment env) throws EvalException { - return hasElementWithBooleanValue(collection, true, loc, env); - } + private static final BuiltinFunction any = + new BuiltinFunction("any") { + @SuppressWarnings("unused") // Accessed via Reflection. + public Boolean invoke(Object collection, Location loc, Environment env) + throws EvalException { + return hasElementWithBooleanValue(collection, true, loc, env); + } + }; private static boolean hasElementWithBooleanValue( Object collection, boolean value, Location loc, Environment env) throws EvalException { @@ -152,29 +174,35 @@ private static boolean hasElementWithBooleanValue( return false; } - @SkylarkCallable( + // supported list methods + @SkylarkSignature( name = "sorted", + returnType = MutableList.class, doc = "Sort a collection. Elements should all belong to the same orderable type, they are " + "sorted by their value (in ascending order). " + "It is an error if elements are not comparable (for example int with string)." + "
sorted([3, 5, 4]) == [3, 4, 5]
", - parameters = { - @Param(name = "self", type = Object.class, doc = "This collection.", legacyNamed = true) - }, + parameters = {@Param(name = "self", type = Object.class, doc = "This collection.")}, useLocation = true, useEnvironment = true) - public MutableList sorted(Object self, Location loc, Environment env) throws EvalException { - try { - return MutableList.copyOf( - env, EvalUtils.SKYLARK_COMPARATOR.sortedCopy(EvalUtils.toCollection(self, loc, env))); - } catch (EvalUtils.ComparisonException e) { - throw new EvalException(loc, e); - } - } + private static final BuiltinFunction sorted = + new BuiltinFunction("sorted") { + public MutableList invoke(Object self, Location loc, Environment env) + throws EvalException { + try { + return MutableList.copyOf( + env, + EvalUtils.SKYLARK_COMPARATOR.sortedCopy(EvalUtils.toCollection(self, loc, env))); + } catch (EvalUtils.ComparisonException e) { + throw new EvalException(loc, e); + } + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "reversed", + returnType = MutableList.class, doc = "Returns a list that contains the elements of the original sequence in reversed order." + "
reversed([3, 5, 4]) == [4, 5, 3]
", @@ -182,121 +210,143 @@ public MutableList sorted(Object self, Location loc, Environment env) throws @Param( name = "sequence", type = Object.class, - doc = "The sequence to be reversed (string, list or tuple).", - legacyNamed = true), + doc = "The sequence to be reversed (string, list or tuple).") }, useLocation = true, useEnvironment = true) - public MutableList reversed(Object sequence, Location loc, Environment env) - throws EvalException { - // We only allow lists and strings. - if (sequence instanceof SkylarkDict) { - throw new EvalException(loc, "Argument to reversed() must be a sequence, not a dictionary."); - } else if (sequence instanceof NestedSet || sequence instanceof SkylarkNestedSet) { - throw new EvalException(loc, "Argument to reversed() must be a sequence, not a depset."); - } - ArrayDeque tmpList = new ArrayDeque<>(); - for (Object element : EvalUtils.toIterable(sequence, loc, env)) { - tmpList.addFirst(element); - } - return MutableList.copyOf(env, tmpList); - } + private static final BuiltinFunction reversed = + new BuiltinFunction("reversed") { + @SuppressWarnings("unused") // Accessed via Reflection. + public MutableList invoke(Object sequence, Location loc, Environment env) + throws EvalException { + // We only allow lists and strings. + if (sequence instanceof SkylarkDict) { + throw new EvalException( + loc, "Argument to reversed() must be a sequence, not a dictionary."); + } else if (sequence instanceof NestedSet || sequence instanceof SkylarkNestedSet) { + throw new EvalException( + loc, "Argument to reversed() must be a sequence, not a depset."); + } + ArrayDeque tmpList = new ArrayDeque<>(); + for (Object element : EvalUtils.toIterable(sequence, loc, env)) { + tmpList.addFirst(element); + } + return MutableList.copyOf(env, tmpList); + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "tuple", + returnType = Tuple.class, doc = "Converts a collection (e.g. list, tuple or dictionary) to a tuple." + "
tuple([1, 2]) == (1, 2)\n"
               + "tuple((2, 3, 2)) == (2, 3, 2)\n"
               + "tuple({5: \"a\", 2: \"b\", 4: \"c\"}) == (5, 2, 4)
", - parameters = {@Param(name = "x", doc = "The object to convert.", legacyNamed = true)}, + parameters = {@Param(name = "x", doc = "The object to convert.")}, useLocation = true, useEnvironment = true) - public Tuple tuple(Object x, Location loc, Environment env) throws EvalException { - return Tuple.copyOf(EvalUtils.toCollection(x, loc, env)); - } + private static final BuiltinFunction tuple = + new BuiltinFunction("tuple") { + public Tuple invoke(Object x, Location loc, Environment env) throws EvalException { + return Tuple.copyOf(EvalUtils.toCollection(x, loc, env)); + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "list", + returnType = MutableList.class, doc = "Converts a collection (e.g. list, tuple or dictionary) to a list." + "
list([1, 2]) == [1, 2]\n"
               + "list((2, 3, 2)) == [2, 3, 2]\n"
               + "list({5: \"a\", 2: \"b\", 4: \"c\"}) == [5, 2, 4]
", - parameters = {@Param(name = "x", doc = "The object to convert.", legacyNamed = true)}, + parameters = {@Param(name = "x", doc = "The object to convert.")}, useLocation = true, useEnvironment = true) - public MutableList list(Object x, Location loc, Environment env) throws EvalException { - return MutableList.copyOf(env, EvalUtils.toCollection(x, loc, env)); - } + private static final BuiltinFunction list = + new BuiltinFunction("list") { + public MutableList invoke(Object x, Location loc, Environment env) throws EvalException { + return MutableList.copyOf(env, EvalUtils.toCollection(x, loc, env)); + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "len", + returnType = Integer.class, doc = "Returns the length of a string, list, tuple, depset, or dictionary.", - parameters = {@Param(name = "x", doc = "The object to check length of.", legacyNamed = true)}, + parameters = {@Param(name = "x", doc = "The object to check length of.")}, useLocation = true, useEnvironment = true) - public Integer len(Object x, Location loc, Environment env) throws EvalException { - if (env.getSemantics().incompatibleDepsetIsNotIterable() && x instanceof SkylarkNestedSet) { - throw new EvalException( - loc, - EvalUtils.getDataTypeName(x) - + " is not iterable. You may use `len(.to_list())` instead. Use " - + "--incompatible_depset_is_not_iterable=false to temporarily disable this " - + "check."); - } - int l = EvalUtils.size(x); - if (l == -1) { - throw new EvalException(loc, EvalUtils.getDataTypeName(x) + " is not iterable"); - } - return l; - } + private static final BuiltinFunction len = + new BuiltinFunction("len") { + public Integer invoke(Object x, Location loc, Environment env) throws EvalException { + if (env.getSemantics().incompatibleDepsetIsNotIterable() + && x instanceof SkylarkNestedSet) { + throw new EvalException( + loc, + EvalUtils.getDataTypeName(x) + + " is not iterable. You may use `len(.to_list())` instead. Use " + + "--incompatible_depset_is_not_iterable=false to temporarily disable this " + + "check."); + } + int l = EvalUtils.size(x); + if (l == -1) { + throw new EvalException(loc, EvalUtils.getDataTypeName(x) + " is not iterable"); + } + return l; + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "str", + returnType = String.class, doc = "Converts any object to string. This is useful for debugging." + "
str(\"ab\") == \"ab\"\n"
               + "str(8) == \"8\"
", - parameters = { - @Param(name = "x", doc = "The object to convert.", legacyNamed = true, noneable = true) - }) - public String str(Object x) { - return Printer.str(x); - } + parameters = {@Param(name = "x", doc = "The object to convert.")}) + private static final BuiltinFunction str = + new BuiltinFunction("str") { + public String invoke(Object x) { + return Printer.str(x); + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "repr", + returnType = String.class, doc = "Converts any object to a string representation. This is useful for debugging.
" + "
repr(\"ab\") == '\"ab\"'
", - parameters = { - @Param(name = "x", doc = "The object to convert.", legacyNamed = true, noneable = true) - }) - public String repr(Object x) { - return Printer.repr(x); - } + parameters = {@Param(name = "x", doc = "The object to convert.")}) + private static final BuiltinFunction repr = + new BuiltinFunction("repr") { + public String invoke(Object x) { + return Printer.repr(x); + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "bool", + returnType = Boolean.class, doc = "Constructor for the bool type. " + "It returns False if the object is None, False" + ", an empty string (\"\"), the number 0, or an " + "empty collection (e.g. (), []). " + "Otherwise, it returns True.", - parameters = { - @Param(name = "x", doc = "The variable to convert.", legacyNamed = true, noneable = true) - }) - public Boolean bool(Object x) throws EvalException { - return EvalUtils.toBoolean(x); - } - - private final ImmutableMap intPrefixes = - ImmutableMap.of("0b", 2, "0o", 8, "0x", 16); + parameters = {@Param(name = "x", doc = "The variable to convert.")}) + private static final BuiltinFunction bool = + new BuiltinFunction("bool") { + public Boolean invoke(Object x) throws EvalException { + return EvalUtils.toBoolean(x); + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "int", + returnType = Integer.class, doc = "Returns x as an int value." + "
    " @@ -335,7 +385,7 @@ public Boolean bool(Object x) throws EvalException { + "int(\"-0x10\", 0) == -16" + "", parameters = { - @Param(name = "x", type = Object.class, doc = "The string to convert.", legacyNamed = true), + @Param(name = "x", type = Object.class, doc = "The string to convert."), @Param( name = "base", type = Object.class, @@ -344,106 +394,117 @@ public Boolean bool(Object x) throws EvalException { "The base used to interpret a string value; defaults to 10. Must be between 2 " + "and 36 (inclusive), or 0 to detect the base as if x were an " + "integer literal. This parameter must not be supplied if the value is not a " - + "string.", - legacyNamed = true) + + "string.") }, useLocation = true) - public Integer convertToInt(Object x, Object base, Location loc) throws EvalException { - if (x instanceof String) { - if (base == Runtime.UNBOUND) { - base = 10; - } else if (!(base instanceof Integer)) { - throw new EvalException( - loc, "base must be an integer (got '" + EvalUtils.getDataTypeName(base) + "')"); - } - return fromString((String) x, loc, (Integer) base); - } else { - if (base != Runtime.UNBOUND) { - throw new EvalException(loc, "int() can't convert non-string with explicit base"); - } - if (x instanceof Boolean) { - return ((Boolean) x).booleanValue() ? 1 : 0; - } else if (x instanceof Integer) { - return (Integer) x; - } - throw new EvalException(loc, Printer.format("%r is not of type string or int or bool", x)); - } - } + private static final BuiltinFunction int_ = + new BuiltinFunction("int") { + private final ImmutableMap intPrefixes = + ImmutableMap.of("0b", 2, "0o", 8, "0x", 16); - private int fromString(String string, Location loc, int base) throws EvalException { - String stringForErrors = string; + @SuppressWarnings("unused") + public Integer invoke(Object x, Object base, Location loc) throws EvalException { + if (x instanceof String) { + if (base == Runtime.UNBOUND) { + base = 10; + } else if (!(base instanceof Integer)) { + throw new EvalException( + loc, "base must be an integer (got '" + EvalUtils.getDataTypeName(base) + "')"); + } + return fromString((String) x, loc, (Integer) base); + } else { + if (base != Runtime.UNBOUND) { + throw new EvalException(loc, "int() can't convert non-string with explicit base"); + } + if (x instanceof Boolean) { + return ((Boolean) x).booleanValue() ? 1 : 0; + } else if (x instanceof Integer) { + return (Integer) x; + } + throw new EvalException( + loc, Printer.format("%r is not of type string or int or bool", x)); + } + } - boolean isNegative = false; - if (string.isEmpty()) { - throw new EvalException(loc, Printer.format("string argument to int() cannot be empty")); - } - char c = string.charAt(0); - if (c == '+') { - string = string.substring(1); - } else if (c == '-') { - string = string.substring(1); - isNegative = true; - } + private int fromString(String string, Location loc, int base) throws EvalException { + String stringForErrors = string; - String prefix = getIntegerPrefix(string); - String digits; - if (prefix == null) { - // Nothing to strip. Infer base 10 if autodetection was requested (base == 0). - digits = string; - if (base == 0) { - if (string.length() > 1 && string.startsWith("0")) { - // We don't infer the base when input starts with '0' (due - // to confusion between octal and decimal). - throw new EvalException( - loc, - Printer.format( - "cannot infer base for int() when value begins with a 0: %r", stringForErrors)); - } - base = 10; - } - } else { - // Strip prefix. Infer base from prefix if unknown (base == 0), or else verify its - // consistency. - digits = string.substring(prefix.length()); - int expectedBase = intPrefixes.get(prefix); - if (base == 0) { - base = expectedBase; - } else if (base != expectedBase) { - throw new EvalException( - loc, - Printer.format("invalid literal for int() with base %d: %r", base, stringForErrors)); - } - } + boolean isNegative = false; + if (string.isEmpty()) { + throw new EvalException( + loc, Printer.format("string argument to int() cannot be empty")); + } + char c = string.charAt(0); + if (c == '+') { + string = string.substring(1); + } else if (c == '-') { + string = string.substring(1); + isNegative = true; + } - if (base < 2 || base > 36) { - throw new EvalException(loc, "int() base must be >= 2 and <= 36"); - } - try { - // Negate by prepending a negative symbol, rather than by using arithmetic on the - // result, to handle the edge case of -2^31 correctly. - String parseable = isNegative ? "-" + digits : digits; - return Integer.parseInt(parseable, base); - } catch (NumberFormatException | ArithmeticException e) { - throw new EvalException( - loc, - Printer.format("invalid literal for int() with base %d: %r", base, stringForErrors), - e); - } - } + String prefix = getIntegerPrefix(string); + String digits; + if (prefix == null) { + // Nothing to strip. Infer base 10 if autodetection was requested (base == 0). + digits = string; + if (base == 0) { + if (string.length() > 1 && string.startsWith("0")) { + // We don't infer the base when input starts with '0' (due + // to confusion between octal and decimal). + throw new EvalException( + loc, + Printer.format( + "cannot infer base for int() when value begins with a 0: %r", + stringForErrors)); + } + base = 10; + } + } else { + // Strip prefix. Infer base from prefix if unknown (base == 0), or else verify its + // consistency. + digits = string.substring(prefix.length()); + int expectedBase = intPrefixes.get(prefix); + if (base == 0) { + base = expectedBase; + } else if (base != expectedBase) { + throw new EvalException( + loc, + Printer.format( + "invalid literal for int() with base %d: %r", base, stringForErrors)); + } + } - @Nullable - private String getIntegerPrefix(String value) { - value = Ascii.toLowerCase(value); - for (String prefix : intPrefixes.keySet()) { - if (value.startsWith(prefix)) { - return prefix; - } - } - return null; - } + if (base < 2 || base > 36) { + throw new EvalException(loc, "int() base must be >= 2 and <= 36"); + } + try { + // Negate by prepending a negative symbol, rather than by using arithmetic on the + // result, to handle the edge case of -2^31 correctly. + String parseable = isNegative ? "-" + digits : digits; + return Integer.parseInt(parseable, base); + } catch (NumberFormatException | ArithmeticException e) { + throw new EvalException( + loc, + Printer.format("invalid literal for int() with base %d: %r", base, stringForErrors), + e); + } + } - @SkylarkCallable( + @Nullable + private String getIntegerPrefix(String value) { + value = Ascii.toLowerCase(value); + for (String prefix : intPrefixes.keySet()) { + if (value.startsWith(prefix)) { + return prefix; + } + } + return null; + } + }; + + @SkylarkSignature( name = "dict", + returnType = SkylarkDict.class, doc = "Creates a dictionary from an optional positional " + "argument and an optional set of keyword arguments. In the case where the same key " @@ -457,69 +518,80 @@ private String getIntegerPrefix(String value) { defaultValue = "[]", doc = "Either a dictionary or a list of entries. Entries must be tuples or lists with " - + "exactly two elements: key, value.", - legacyNamed = true), + + "exactly two elements: key, value."), }, extraKeywords = @Param(name = "kwargs", doc = "Dictionary of additional entries."), useLocation = true, useEnvironment = true) - public SkylarkDict dict( - Object args, SkylarkDict kwargs, Location loc, Environment env) throws EvalException { - SkylarkDict argsDict = - (args instanceof SkylarkDict) ? (SkylarkDict) args : getDictFromArgs(args, loc, env); - return SkylarkDict.plus(argsDict, kwargs, env); - } + private static final BuiltinFunction dict = + new BuiltinFunction("dict") { + public SkylarkDict invoke( + Object args, SkylarkDict kwargs, Location loc, Environment env) + throws EvalException { + SkylarkDict argsDict = + (args instanceof SkylarkDict) + ? (SkylarkDict) args + : getDictFromArgs(args, loc, env); + return SkylarkDict.plus(argsDict, kwargs, env); + } - private SkylarkDict getDictFromArgs(Object args, Location loc, Environment env) - throws EvalException { - SkylarkDict result = SkylarkDict.of(env); - int pos = 0; - for (Object element : Type.OBJECT_LIST.convert(args, "parameter args in dict()")) { - List pair = convertToPair(element, pos, loc); - result.put(pair.get(0), pair.get(1), loc, env); - ++pos; - } - return result; - } + private SkylarkDict getDictFromArgs( + Object args, Location loc, Environment env) throws EvalException { + SkylarkDict result = SkylarkDict.of(env); + int pos = 0; + for (Object element : Type.OBJECT_LIST.convert(args, "parameter args in dict()")) { + List pair = convertToPair(element, pos, loc); + result.put(pair.get(0), pair.get(1), loc, env); + ++pos; + } + return result; + } - private List convertToPair(Object element, int pos, Location loc) throws EvalException { - try { - List tuple = Type.OBJECT_LIST.convert(element, ""); - int numElements = tuple.size(); - if (numElements != 2) { - throw new EvalException( - loc, - String.format( - "item #%d has length %d, but exactly two elements are required", pos, numElements)); - } - return tuple; - } catch (ConversionException e) { - throw new EvalException(loc, String.format("cannot convert item #%d to a sequence", pos), e); - } - } + private List convertToPair(Object element, int pos, Location loc) + throws EvalException { + try { + List tuple = Type.OBJECT_LIST.convert(element, ""); + int numElements = tuple.size(); + if (numElements != 2) { + throw new EvalException( + location, + String.format( + "item #%d has length %d, but exactly two elements are required", + pos, numElements)); + } + return tuple; + } catch (ConversionException e) { + throw new EvalException( + loc, String.format("cannot convert item #%d to a sequence", pos), e); + } + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "enumerate", + returnType = MutableList.class, doc = "Returns a list of pairs (two-element tuples), with the index (int) and the item from" + " the input list.\n
    "
               + "enumerate([24, 21, 84]) == [(0, 24), (1, 21), (2, 84)]
    \n", - parameters = { - @Param(name = "list", type = SkylarkList.class, doc = "input list.", legacyNamed = true) - }, + parameters = {@Param(name = "list", type = SkylarkList.class, doc = "input list.")}, useEnvironment = true) - public MutableList enumerate(SkylarkList input, Environment env) throws EvalException { - int count = 0; - ArrayList> result = new ArrayList<>(input.size()); - for (Object obj : input) { - result.add(Tuple.of(count, obj)); - count++; - } - return MutableList.wrapUnsafe(env, result); - } + private static final BuiltinFunction enumerate = + new BuiltinFunction("enumerate") { + public MutableList invoke(SkylarkList input, Environment env) throws EvalException { + int count = 0; + ArrayList> result = new ArrayList<>(input.size()); + for (Object obj : input) { + result.add(Tuple.of(count, obj)); + count++; + } + return MutableList.wrapUnsafe(env, result); + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "hash", + returnType = Integer.class, doc = "Return a hash value for a string. This is computed deterministically using the same " + "algorithm as Java's String.hashCode(), namely: " @@ -528,19 +600,17 @@ public MutableList enumerate(SkylarkList input, Environment env) throws Ev // Deterministic hashing is important for the consistency of builds, hence why we // promise a specific algorithm. This is in contrast to Java (Object.hashCode()) and // Python, which promise stable hashing only within a given execution of the program. - parameters = { - @Param( - name = "value", - type = String.class, - doc = "String value to hash.", - legacyNamed = true) - }) - public Integer hash(String value) throws EvalException { - return value.hashCode(); - } + parameters = {@Param(name = "value", type = String.class, doc = "String value to hash.")}) + private static final BuiltinFunction hash = + new BuiltinFunction("hash") { + public Integer invoke(String value) throws EvalException { + return value.hashCode(); + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "range", + returnType = SkylarkList.class, doc = "Creates a list where items go from start to stop, using a " + "step increment. If a single argument is provided, items will " @@ -554,8 +624,7 @@ public Integer hash(String value) throws EvalException { type = Integer.class, doc = "Value of the start element if stop is provided, " - + "otherwise value of stop and the actual start is 0", - legacyNamed = true), + + "otherwise value of stop and the actual start is 0"), @Param( name = "stop_or_none", type = Integer.class, @@ -563,60 +632,62 @@ public Integer hash(String value) throws EvalException { defaultValue = "None", doc = "optional index of the first item not to be included in the resulting " - + "list; generation of the list stops before stop is reached.", - legacyNamed = true), + + "list; generation of the list stops before stop is reached."), @Param( name = "step", type = Integer.class, defaultValue = "1", - doc = "The increment (default is 1). It may be negative.", - legacyNamed = true) + doc = "The increment (default is 1). It may be negative.") }, useLocation = true, useEnvironment = true) - public SkylarkList range( - Integer startOrStop, Object stopOrNone, Integer step, Location loc, Environment env) - throws EvalException { - int start; - int stop; - if (stopOrNone == Runtime.NONE) { - start = 0; - stop = startOrStop; - } else { - start = startOrStop; - stop = Type.INTEGER.convert(stopOrNone, "'stop' operand of 'range'"); - } - if (step == 0) { - throw new EvalException(loc, "step cannot be 0"); - } - return RangeList.of(start, stop, step); - } + private static final BuiltinFunction range = + new BuiltinFunction("range") { + public SkylarkList invoke( + Integer startOrStop, Object stopOrNone, Integer step, Location loc, Environment env) + throws EvalException { + int start; + int stop; + if (stopOrNone == Runtime.NONE) { + start = 0; + stop = startOrStop; + } else { + start = startOrStop; + stop = Type.INTEGER.convert(stopOrNone, "'stop' operand of 'range'"); + } + if (step == 0) { + throw new EvalException(loc, "step cannot be 0"); + } + return RangeList.of(start, stop, step); + } + }; /** Returns true if the object has a field of the given name, otherwise false. */ - @SkylarkCallable( + @SkylarkSignature( name = "hasattr", + returnType = Boolean.class, doc = "Returns True if the object x has an attribute or method of the given " + "name, otherwise False. Example:
    " + "
    hasattr(ctx.attr, \"myattr\")
    ", parameters = { - @Param(name = "x", doc = "The object to check.", legacyNamed = true, noneable = true), - @Param( - name = "name", - type = String.class, - doc = "The name of the attribute.", - legacyNamed = true) + @Param(name = "x", doc = "The object to check."), + @Param(name = "name", type = String.class, doc = "The name of the attribute.") }, useEnvironment = true) - public Boolean hasAttr(Object obj, String name, Environment env) throws EvalException { - if (obj instanceof ClassObject && ((ClassObject) obj).getValue(name) != null) { - return true; - } - // shouldn't this filter things with struct_field = false? - return DotExpression.hasMethod(env.getSemantics(), obj, name); - } + private static final BuiltinFunction hasattr = + new BuiltinFunction("hasattr") { + @SuppressWarnings("unused") + public Boolean invoke(Object obj, String name, Environment env) throws EvalException { + if (obj instanceof ClassObject && ((ClassObject) obj).getValue(name) != null) { + return true; + } + // shouldn't this filter things with struct_field = false? + return DotExpression.hasMethod(env.getSemantics(), obj, name); + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "getattr", doc = "Returns the struct's field of the given name if it exists. If not, it either returns " @@ -627,68 +698,70 @@ public Boolean hasAttr(Object obj, String name, Environment env) throws EvalExce + "
    getattr(ctx.attr, \"myattr\")\n"
               + "getattr(ctx.attr, \"myattr\", \"mydefault\")
    ", parameters = { - @Param( - name = "x", - doc = "The struct whose attribute is accessed.", - legacyNamed = true, - noneable = true), - @Param(name = "name", doc = "The name of the struct attribute.", legacyNamed = true), + @Param(name = "x", doc = "The struct whose attribute is accessed."), + @Param(name = "name", doc = "The name of the struct attribute."), @Param( name = "default", defaultValue = "unbound", doc = "The default value to return in case the struct " - + "doesn't have an attribute of the given name.", - legacyNamed = true, - noneable = true) + + "doesn't have an attribute of the given name.") }, useLocation = true, useEnvironment = true) - public Object getAttr(Object obj, String name, Object defaultValue, Location loc, Environment env) - throws EvalException, InterruptedException { - Object result = DotExpression.eval(obj, name, loc, env); - if (result == null) { - if (defaultValue != Runtime.UNBOUND) { - return defaultValue; - } - throw DotExpression.getMissingFieldException(obj, name, loc, env.getSemantics(), "attribute"); - } - return result; - } + private static final BuiltinFunction getattr = + new BuiltinFunction("getattr") { + @SuppressWarnings("unused") + public Object invoke( + Object obj, String name, Object defaultValue, Location loc, Environment env) + throws EvalException, InterruptedException { + Object result = DotExpression.eval(obj, name, loc, env); + if (result == null) { + if (defaultValue != Runtime.UNBOUND) { + return defaultValue; + } + throw DotExpression.getMissingFieldException( + obj, name, loc, env.getSemantics(), "attribute"); + } + return result; + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "dir", + returnType = MutableList.class, doc = "Returns a list of strings: the names of the attributes and " + "methods of the parameter object.", - parameters = { - @Param(name = "x", doc = "The object to check.", legacyNamed = true, noneable = true) - }, + parameters = {@Param(name = "x", doc = "The object to check.")}, useLocation = true, useEnvironment = true) - public MutableList dir(Object object, Location loc, Environment env) throws EvalException { - // Order the fields alphabetically. - Set fields = new TreeSet<>(); - if (object instanceof ClassObject) { - fields.addAll(((ClassObject) object).getFieldNames()); - } - fields.addAll(Runtime.getBuiltinRegistry().getFunctionNames(object.getClass())); - fields.addAll(FuncallExpression.getMethodNames(env.getSemantics(), object.getClass())); - return MutableList.copyOf(env, fields); - } + private static final BuiltinFunction dir = + new BuiltinFunction("dir") { + public MutableList invoke(Object object, Location loc, Environment env) + throws EvalException { + // Order the fields alphabetically. + Set fields = new TreeSet<>(); + if (object instanceof ClassObject) { + fields.addAll(((ClassObject) object).getFieldNames()); + } + fields.addAll(Runtime.getBuiltinRegistry().getFunctionNames(object.getClass())); + fields.addAll(FuncallExpression.getMethodNames(env.getSemantics(), object.getClass())); + return MutableList.copyOf(env, fields); + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "fail", doc = "Raises an error that cannot be intercepted. It can be used anywhere, " + "both in the loading phase and in the analysis phase.", + returnType = Runtime.NoneType.class, parameters = { @Param( name = "msg", type = Object.class, - doc = "Error to display for the user. The object is converted to a string.", - legacyNamed = true, - noneable = true), + doc = "Error to display for the user. The object is converted to a string."), @Param( name = "attr", type = String.class, @@ -696,20 +769,23 @@ public MutableList dir(Object object, Location loc, Environment env) throws E defaultValue = "None", doc = "The name of the attribute that caused the error. This is used only for " - + "error reporting.", - legacyNamed = true) + + "error reporting.") }, useLocation = true) - public Runtime.NoneType fail(Object msg, Object attr, Location loc) throws EvalException { - String str = Printer.str(msg); - if (attr != Runtime.NONE) { - str = String.format("attribute %s: %s", attr, str); - } - throw new EvalException(loc, str); - } + private static final BuiltinFunction fail = + new BuiltinFunction("fail") { + public Runtime.NoneType invoke(Object msg, Object attr, Location loc) throws EvalException { + String str = Printer.str(msg); + if (attr != Runtime.NONE) { + str = String.format("attribute %s: %s", attr, str); + } + throw new EvalException(loc, str); + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "print", + returnType = Runtime.NoneType.class, doc = "Prints args as debug output. It will be prefixed with the string " + "\"DEBUG\" and the location (file and line number) of this call. The " @@ -733,21 +809,26 @@ public Runtime.NoneType fail(Object msg, Object attr, Location loc) throws EvalE extraPositionals = @Param(name = "args", doc = "The objects to print."), useLocation = true, useEnvironment = true) - public Runtime.NoneType print(String sep, SkylarkList starargs, Location loc, Environment env) - throws EvalException { - String msg = starargs.stream().map(Printer::debugPrint).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. - if (env.getSemantics().internalSkylarkFlagTestCanary()) { - msg += "<== skylark flag test ==>"; - } - env.handleEvent(Event.debug(loc, msg)); - return Runtime.NONE; - } + private static final BuiltinFunction print = + new BuiltinFunction("print") { + public Runtime.NoneType invoke( + String sep, SkylarkList starargs, Location loc, Environment env) + throws EvalException { + String msg = starargs.stream().map(Printer::debugPrint).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. + if (env.getSemantics().internalSkylarkFlagTestCanary()) { + msg += "<== skylark flag test ==>"; + } + env.handleEvent(Event.debug(loc, msg)); + return Runtime.NONE; + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "type", + returnType = String.class, doc = "Returns the type name of its argument. This is useful for debugging and " + "type-checking. Examples:" @@ -761,20 +842,18 @@ public Runtime.NoneType print(String sep, SkylarkList starargs, Location loc, + "
    "
               + "if type(x) == type([]):  # if x is a list"
               + "
    ", - parameters = { - @Param( - name = "x", - doc = "The object to check type of.", - legacyNamed = true, - noneable = true) - }) - public String type(Object object) { - // There is no 'type' type in Skylark, so we return a string with the type name. - return EvalUtils.getDataTypeName(object, false); - } + parameters = {@Param(name = "x", doc = "The object to check type of.")}) + private static final BuiltinFunction type = + new BuiltinFunction("type") { + public String invoke(Object object) { + // There is no 'type' type in Skylark, so we return a string with the type name. + return EvalUtils.getDataTypeName(object, false); + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "depset", + returnType = SkylarkNestedSet.class, doc = "Creates a depset. The direct parameter is a " + "list of direct elements of the depset, and transitive parameter is " @@ -804,16 +883,14 @@ public String type(Object object) { "Deprecated: Either an iterable whose items become the direct elements of " + "the new depset, in left-to-right order, or else a depset that becomes " + "a transitive element of the new depset. In the latter case, " - + "transitive cannot be specified.", - legacyNamed = true), + + "transitive cannot be specified."), @Param( name = "order", type = String.class, defaultValue = "\"default\"", doc = "The traversal strategy for the new depset. See " - + "here for the possible values.", - legacyNamed = true), + + "here for the possible values."), @Param( name = "direct", type = SkylarkList.class, @@ -833,52 +910,55 @@ public String type(Object object) { defaultValue = "None") }, useLocation = true) - public SkylarkNestedSet depset( - Object items, String orderString, Object direct, Object transitive, Location loc) - throws EvalException { - Order order; - try { - order = Order.parse(orderString); - } catch (IllegalArgumentException ex) { - throw new EvalException(loc, ex); - } + private static final BuiltinFunction depset = + new BuiltinFunction("depset") { + public SkylarkNestedSet invoke( + Object items, String orderString, Object direct, Object transitive, Location loc) + throws EvalException { + Order order; + try { + order = Order.parse(orderString); + } catch (IllegalArgumentException ex) { + throw new EvalException(loc, ex); + } - if (transitive == Runtime.NONE && direct == Runtime.NONE) { - // Legacy behavior. - return SkylarkNestedSet.of(order, items, loc); - } + if (transitive == Runtime.NONE && direct == Runtime.NONE) { + // Legacy behavior. + return SkylarkNestedSet.of(order, items, loc); + } - if (direct != Runtime.NONE && !isEmptySkylarkList(items)) { - throw new EvalException( - loc, "Do not pass both 'direct' and 'items' argument to depset constructor."); - } + if (direct != Runtime.NONE && !isEmptySkylarkList(items)) { + throw new EvalException( + loc, "Do not pass both 'direct' and 'items' argument to depset constructor."); + } - // Non-legacy behavior: either 'transitive' or 'direct' were specified. - Iterable directElements; - if (direct != Runtime.NONE) { - directElements = ((SkylarkList) direct).getContents(Object.class, "direct"); - } else { - SkylarkType.checkType(items, SkylarkList.class, "items"); - directElements = ((SkylarkList) items).getContents(Object.class, "items"); - } + // Non-legacy behavior: either 'transitive' or 'direct' were specified. + Iterable directElements; + if (direct != Runtime.NONE) { + directElements = ((SkylarkList) direct).getContents(Object.class, "direct"); + } else { + SkylarkType.checkType(items, SkylarkList.class, "items"); + directElements = ((SkylarkList) items).getContents(Object.class, "items"); + } - Iterable transitiveList; - if (transitive != Runtime.NONE) { - SkylarkType.checkType(transitive, SkylarkList.class, "transitive"); - transitiveList = - ((SkylarkList) transitive).getContents(SkylarkNestedSet.class, "transitive"); - } else { - transitiveList = ImmutableList.of(); - } - SkylarkNestedSet.Builder builder = SkylarkNestedSet.builder(order, loc); - for (Object directElement : directElements) { - builder.addDirect(directElement); - } - for (SkylarkNestedSet transitiveSet : transitiveList) { - builder.addTransitive(transitiveSet); - } - return builder.build(); - } + Iterable transitiveList; + if (transitive != Runtime.NONE) { + SkylarkType.checkType(transitive, SkylarkList.class, "transitive"); + transitiveList = + ((SkylarkList) transitive).getContents(SkylarkNestedSet.class, "transitive"); + } else { + transitiveList = ImmutableList.of(); + } + SkylarkNestedSet.Builder builder = SkylarkNestedSet.builder(order, loc); + for (Object directElement : directElements) { + builder.addDirect(directElement); + } + for (SkylarkNestedSet transitiveSet : transitiveList) { + builder.addTransitive(transitiveSet); + } + return builder.build(); + } + }; private static boolean isEmptySkylarkList(Object o) { return o instanceof SkylarkList && ((SkylarkList) o).isEmpty(); @@ -888,7 +968,7 @@ private static boolean isEmptySkylarkList(Object o) { * Returns a function-value implementing "select" (i.e. configurable attributes) in the specified * package context. */ - @SkylarkCallable( + @SkylarkSignature( name = "select", doc = "select() is the helper function that makes a rule attribute " @@ -896,31 +976,29 @@ private static boolean isEmptySkylarkList(Object o) { + "configurable. See " + "build encyclopedia for details.", parameters = { - @Param( - name = "x", - type = SkylarkDict.class, - doc = "The parameter to convert.", - legacyNamed = true), + @Param(name = "x", type = SkylarkDict.class, doc = "The parameter to convert."), @Param( name = "no_match_error", type = String.class, defaultValue = "''", - doc = "Optional custom error to report if no condition matches.", - legacyNamed = true) + doc = "Optional custom error to report if no condition matches.") }, useLocation = true) - public Object select(SkylarkDict dict, String noMatchError, Location loc) - throws EvalException { - for (Object key : dict.keySet()) { - if (!(key instanceof String)) { - throw new EvalException( - loc, String.format("Invalid key: %s. select keys must be label references", key)); - } - } - return SelectorList.of(new SelectorValue(dict, noMatchError)); - } + private static final BuiltinFunction select = + new BuiltinFunction("select") { + public Object invoke(SkylarkDict dict, String noMatchError, Location loc) + throws EvalException { + for (Object key : dict.keySet()) { + if (!(key instanceof String)) { + throw new EvalException( + loc, String.format("Invalid key: %s. select keys must be label references", key)); + } + } + return SelectorList.of(new SelectorValue(dict, noMatchError)); + } + }; - @SkylarkCallable( + @SkylarkSignature( name = "zip", doc = "Returns a list of tuples, where the i-th tuple contains " @@ -933,32 +1011,36 @@ public Object select(SkylarkDict dict, String noMatchError, Location loc) + "zip([1, 2], [3, 4]) # == [(1, 3), (2, 4)]\n" + "zip([1, 2], [3, 4, 5]) # == [(1, 3), (2, 4)]", extraPositionals = @Param(name = "args", doc = "lists to zip."), + returnType = MutableList.class, useLocation = true, useEnvironment = true) - public MutableList zip(SkylarkList args, Location loc, Environment env) - throws EvalException { - Iterator[] iterators = new Iterator[args.size()]; - for (int i = 0; i < args.size(); i++) { - iterators[i] = EvalUtils.toIterable(args.get(i), loc, env).iterator(); - } - ArrayList> result = new ArrayList<>(); - boolean allHasNext; - do { - allHasNext = !args.isEmpty(); - List elem = Lists.newArrayListWithExpectedSize(args.size()); - for (Iterator iterator : iterators) { - if (iterator.hasNext()) { - elem.add(iterator.next()); - } else { - allHasNext = false; + private static final BuiltinFunction zip = + new BuiltinFunction("zip") { + public MutableList invoke(SkylarkList args, Location loc, Environment env) + throws EvalException { + Iterator[] iterators = new Iterator[args.size()]; + for (int i = 0; i < args.size(); i++) { + iterators[i] = EvalUtils.toIterable(args.get(i), loc, env).iterator(); + } + ArrayList> result = new ArrayList<>(); + boolean allHasNext; + do { + allHasNext = !args.isEmpty(); + List elem = Lists.newArrayListWithExpectedSize(args.size()); + for (Iterator iterator : iterators) { + if (iterator.hasNext()) { + elem.add(iterator.next()); + } else { + allHasNext = false; + } + } + if (allHasNext) { + result.add(Tuple.copyOf(elem)); + } + } while (allHasNext); + return MutableList.wrapUnsafe(env, result); } - } - if (allHasNext) { - result.add(Tuple.copyOf(elem)); - } - } while (allHasNext); - return MutableList.wrapUnsafe(env, result); - } + }; /** Skylark int type. */ @SkylarkModule( @@ -993,6 +1075,17 @@ public static final class BoolModule {} /** Adds bindings for all the builtin functions of this class to the given map builder. */ public static void addBindingsToBuilder(ImmutableMap.Builder builder) { - Runtime.setupSkylarkLibrary(builder, new MethodLibrary()); + for (BaseFunction function : allFunctions) { + builder.put(function.getName(), function); + } + } + + private static final ImmutableList allFunctions = + ImmutableList.of( + all, any, bool, depset, dict, dir, fail, getattr, hasattr, hash, enumerate, int_, len, + list, max, min, print, range, repr, reversed, select, sorted, str, tuple, type, zip); + + static { + SkylarkSignatureProcessor.configureSkylarkFunctions(MethodLibrary.class); } } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ParamDescriptor.java b/src/main/java/com/google/devtools/build/lib/syntax/ParamDescriptor.java index ae60aa32f4755b..de133db656bf65 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/ParamDescriptor.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/ParamDescriptor.java @@ -74,11 +74,7 @@ private ParamDescriptor( this.flagResponsibleForDisable = flagResponsibleForDisable; } - /** - * Returns a {@link ParamDescriptor} representing the given raw {@link Param} annotation and the - * given semantics. - */ - public static ParamDescriptor of(Param param, SkylarkSemantics skylarkSemantics) { + static ParamDescriptor of(Param param, SkylarkSemantics skylarkSemantics) { ImmutableList allowedTypes = Arrays.stream(param.allowedTypes()) .map(ParamTypeDescriptor::of) diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java index a9e9197c49fa36..02aedc746de0ff 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java @@ -1999,9 +1999,7 @@ public void testBuiltInFunctionAsRuleImplementation() throws Exception { "silly_rule(name = 'silly')"); thrown.handleAssertionErrors(); // Compatibility with JUnit 4.11 thrown.expect(AssertionError.class); - thrown.expectMessage( - "expected value of type 'function' for parameter 'implementation', " - + "for call to function rule"); + thrown.expectMessage(" is not of type string or int or bool"); getConfiguredTarget("//test:silly"); } 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 index 280328bfd2583b..be3c0ae8a3c4bc 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkStringRepresentationsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkStringRepresentationsTest.java @@ -267,12 +267,12 @@ public void testStringRepresentations_Functions() throws Exception { @Test public void testStringRepresentations_Rules() throws Exception { assertStringRepresentation("native.cc_library", ""); - assertStringRepresentation("def f(): pass", "rule(implementation=f)", ""); + assertStringRepresentation("rule(implementation=str)", ""); } @Test public void testStringRepresentations_Aspects() throws Exception { - assertStringRepresentation("def f(): pass", "aspect(implementation=f)", ""); + assertStringRepresentation("aspect(implementation=str)", ""); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java index 72b0a737ab1247..a7a74fc3f8a415 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java @@ -335,8 +335,7 @@ public void testDictionaryCreationInvalidPositional() throws Exception { "dict('a')") .testIfErrorContains("cannot convert item #0 to a sequence", "dict(['a'])") .testIfErrorContains("cannot convert item #0 to a sequence", "dict([('a')])") - .testIfErrorContains( - "expected no more than 1 positional arguments, but got 3", "dict((3,4), (3,2), (1,2))") + .testIfErrorContains("too many (3) positional arguments", "dict((3,4), (3,2), (1,2))") .testIfErrorContains( "item #0 has length 3, but exactly two elements are required", "dict([('a', 'b', 'c')])"); @@ -465,8 +464,8 @@ public void testHash() throws Exception { .testStatement("hash('skylark')", "skylark".hashCode()) .testStatement("hash('google')", "google".hashCode()) .testIfErrorContains( - "expected value of type 'string' for parameter 'value', " - + "for call to function hash(value)", + "argument 'value' has type 'NoneType', but should be 'string'\n" + + "in call to builtin function hash(value)", "hash(None)"); } @@ -535,8 +534,8 @@ public void testEnumerate() throws Exception { public void testEnumerateBadArg() throws Exception { new BothModesTest() .testIfErrorContains( - "expected value of type 'sequence' for parameter 'list', " - + "for call to function enumerate(list)", + "argument 'list' has type 'string', but should be 'sequence'\n" + + "in call to builtin function enumerate(list)", "enumerate('a')"); } diff --git a/src/test/java/com/google/devtools/build/lib/syntax/RuntimeTest.java b/src/test/java/com/google/devtools/build/lib/syntax/RuntimeTest.java index 5444ef80790fd1..e2fc1e65d4d36a 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/RuntimeTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/RuntimeTest.java @@ -19,6 +19,7 @@ import com.google.devtools.build.lib.skylarkinterface.SkylarkPrinter; import com.google.devtools.build.lib.skylarkinterface.SkylarkValue; +import java.lang.reflect.Field; import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; @@ -105,4 +106,13 @@ public void checkRegistry_WriteAfterFreezeFails_Function() { .matches("Attempted to register function 'dummyFunc' in namespace '(.*)DummyType' after " + "registry has already been frozen"); } + + @Test + public void checkStaticallyRegistered_Global() throws Exception { + Field lenField = MethodLibrary.class.getDeclaredField("len"); + lenField.setAccessible(true); + Object lenFieldValue = lenField.get(null); + List builtins = Runtime.getBuiltinRegistry().getBuiltins(); + assertThat(builtins).contains(lenFieldValue); + } } 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 39e175c57a1f88..47775502a312c6 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 @@ -2007,10 +2007,9 @@ public void testPrint() throws Exception { @Test public void testPrintBadKwargs() throws Exception { - new SkylarkTest() - .testIfErrorContains( - "unexpected keywords 'end', 'other', for call to function print(sep = \" \", *args)", - "print(end='x', other='y')"); + new SkylarkTest().testIfExactError( + "unexpected keywords 'end', 'other' in call to print(*args, sep: string = \" \")", + "print(end='x', other='y')"); } // Override tests in EvaluationTest incompatible with Skylark diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java index ed522f7141baf3..40e13e437b373e 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkNestedSetTest.java @@ -266,11 +266,12 @@ public void testItemsAndTransitive() throws Exception { @Test public void testTooManyPositionals() throws Exception { - new BothModesTest() - .testIfErrorContains( - "expected no more than 2 positional arguments, but got 3, for call to function " - + "depset(items = [], order = \"default\", direct = None, transitive = None)", - "depset([], 'default', [])"); + new BothModesTest().testIfExactError( + "too many (3) positional arguments in call to " + + "depset(items = [], order: string = \"default\", *, " + + "direct: sequence or NoneType = None, " + + "transitive: sequence of depsets or NoneType = None)", + "depset([], 'default', [])"); } diff --git a/src/test/skylark/testdata/all_any.sky b/src/test/skylark/testdata/all_any.sky index eb521fc2efd2f3..4b0f59ec22b333 100644 --- a/src/test/skylark/testdata/all_any.sky +++ b/src/test/skylark/testdata/all_any.sky @@ -34,9 +34,9 @@ assert_eq(any({1 : None, '' : None}), True) assert_eq(any({None : 1, '' : 2}), False) --- -all(None) ### parameter 'elements' cannot be None, for call to function all(elements) +all(None) ### type 'NoneType' is not iterable --- -any(None) ### parameter 'elements' cannot be None, for call to function any(elements) +any(None) ### type 'NoneType' is not iterable --- any(1) ### type 'int' is not iterable --- diff --git a/src/test/skylark/testdata/int_constructor.sky b/src/test/skylark/testdata/int_constructor.sky index f6ee91ef799b7c..03c43e4d450fa9 100644 --- a/src/test/skylark/testdata/int_constructor.sky +++ b/src/test/skylark/testdata/int_constructor.sky @@ -23,7 +23,7 @@ int('1.5') ### invalid literal for int\(\) with base 10 --- int('ab') ### invalid literal for int\(\) with base 10: "ab" --- -int(None) ### parameter 'x' cannot be None, for call to function int(x, base = unbound) +int(None) ### None is not of type string or int or bool --- int('123', 3) ### invalid literal for int\(\) with base 3: "123" --- diff --git a/src/test/skylark/testdata/int_function.sky b/src/test/skylark/testdata/int_function.sky index 29d066f6c12591..7d6a382a65dd17 100644 --- a/src/test/skylark/testdata/int_function.sky +++ b/src/test/skylark/testdata/int_function.sky @@ -10,10 +10,10 @@ assert_eq(int(True), 1) assert_eq(int(False), 0) --- -int(None) ### parameter 'x' cannot be None, for call to function int(x, base = unbound) +int(None) ### None is not of type string or int or bool --- # This case is allowed in Python but not Skylark -int() ### parameter 'x' has no default value, for call to function int(x, base = unbound) +int() ### insufficient arguments received --- # string, no base diff --git a/src/test/skylark/testdata/reversed.sky b/src/test/skylark/testdata/reversed.sky index bf14c687cddca3..08beba80b73400 100644 --- a/src/test/skylark/testdata/reversed.sky +++ b/src/test/skylark/testdata/reversed.sky @@ -7,7 +7,7 @@ assert_eq(reversed('__test '.elems()), [' ', ' ', 't', 's', 'e', 't', '_', '_'] assert_eq(reversed('bbb'.elems()), ['b', 'b', 'b']) --- -reversed(None) ### parameter 'sequence' cannot be None, for call to function reversed(sequence) +reversed(None) ### type 'NoneType' is not iterable --- reversed(1) ### type 'int' is not iterable --- diff --git a/src/tools/skylark/java/com/google/devtools/skylark/skylint/Environment.java b/src/tools/skylark/java/com/google/devtools/skylark/skylint/Environment.java index f0ae3e7511fa5f..68d18e94972be2 100644 --- a/src/tools/skylark/java/com/google/devtools/skylark/skylint/Environment.java +++ b/src/tools/skylark/java/com/google/devtools/skylark/skylint/Environment.java @@ -14,7 +14,6 @@ package com.google.devtools.skylark.skylint; -import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.packages.BazelLibrary; import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature; import com.google.devtools.build.lib.syntax.ASTNode; @@ -50,12 +49,7 @@ public static Environment defaultBazel() { env.addBuiltin("None"); env.addBuiltin("True"); env.addBuiltin("False"); - env.setupFunctions(BazelLibrary.class); - ImmutableMap.Builder builtinMap = ImmutableMap.builder(); - MethodLibrary.addBindingsToBuilder(builtinMap); - for (String builtinName : builtinMap.build().keySet()) { - env.addBuiltin(builtinName); - } + env.setupFunctions(MethodLibrary.class, BazelLibrary.class); return env; }