diff options
| -rw-r--r-- | src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java | 50 | ||||
| -rw-r--r-- | src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java | 42 |
2 files changed, 65 insertions, 27 deletions
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 1e49c91330..e5aa180426 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,6 +29,7 @@ import com.google.devtools.build.lib.skylarkinterface.Param; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.syntax.EvalException.EvalExceptionWithJavaCause; +import com.google.devtools.build.lib.syntax.Runtime.NoneType; import com.google.devtools.build.lib.syntax.compiler.ByteCodeMethodCalls; import com.google.devtools.build.lib.syntax.compiler.ByteCodeUtils; import com.google.devtools.build.lib.syntax.compiler.DebugInfo; @@ -436,54 +437,61 @@ public final class FuncallExpression extends Expression { ImmutableList.Builder<Object> builder = ImmutableList.builder(); Class<?>[] params = method.getMethod().getParameterTypes(); SkylarkCallable callable = method.getAnnotation(); - int i = 0; - int nbPositional = callable.mandatoryPositionals(); - if (nbPositional < 0) { + int mandatoryPositionals = callable.mandatoryPositionals(); + if (mandatoryPositionals < 0) { if (callable.parameters().length > 0) { - nbPositional = 0; + mandatoryPositionals = 0; } else { - nbPositional = params.length; + mandatoryPositionals = params.length; } } - if (nbPositional > args.size() || args.size() > nbPositional + callable.parameters().length) { + if (mandatoryPositionals > args.size() + || args.size() > mandatoryPositionals + callable.parameters().length) { return null; } // First process the legacy positional parameters - for (Class<?> param : params) { - Object value = args.get(i); - if (!param.isAssignableFrom(value.getClass())) { - return null; - } - builder.add(value); - i++; - if (nbPositional >= 0 && i >= nbPositional) { - // Stops for specified parameters instead. - break; + int i = 0; + if (mandatoryPositionals > 0) { + for (Class<?> param : params) { + Object value = args.get(i); + if (!param.isAssignableFrom(value.getClass())) { + return null; + } + builder.add(value); + i++; + if (mandatoryPositionals >= 0 && i >= mandatoryPositionals) { + // Stops for specified parameters instead. + break; + } } } // Then the parameters specified in callable.parameters() Set<String> keys = new HashSet<>(kwargs.keySet()); for (Param param : callable.parameters()) { SkylarkType type = getType(param); + Object value = null; if (i < args.size()) { - if (!param.positional() || !type.contains(args.get(i))) { + value = args.get(i); + if (!param.positional() || !type.contains(value)) { return null; // next positional argument is not of the good type } - builder.add(args.get(i)); i++; } else if (param.named() && keys.remove(param.name())) { // Named parameters - Object value = kwargs.get(param.name()); + value = kwargs.get(param.name()); if (!type.contains(value)) { return null; // type does not match } - builder.add(value); } else { // Use default value if (param.defaultValue().isEmpty()) { return null; // no default value } - builder.add(SkylarkSignatureProcessor.getDefaultValue(param, null)); + value = SkylarkSignatureProcessor.getDefaultValue(param, null); + } + builder.add(value); + if (!param.noneable() && value instanceof NoneType) { + return null; // cannot be None } } if (i < args.size() || !keys.isEmpty()) { 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 7061bf6311..8f0a6f0c2f 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 @@ -124,11 +124,32 @@ public class SkylarkEvaluationTest extends EvaluationTest { defaultValue = "False", positional = false, named = true - ) + ), + @Param( + name = "nonNoneable", + type = Object.class, + defaultValue = "\"a\"", + positional = false, + named = true + ), + @Param( + name = "noneable", + type = Object.class, + defaultValue = "None", + noneable = true, + positional = false, + named = true + ), } ) public String withParams( - Integer pos1, boolean pos2, boolean posOrNamed, boolean named, boolean optionalNamed) { + Integer pos1, + boolean pos2, + boolean posOrNamed, + boolean named, + boolean optionalNamed, + Object nonNoneable, + Object noneable) { return "with_params(" + pos1 + ", " @@ -139,6 +160,8 @@ public class SkylarkEvaluationTest extends EvaluationTest { + named + ", " + optionalNamed + + ", " + + nonNoneable.toString() + ")"; } @@ -647,7 +670,7 @@ public class SkylarkEvaluationTest extends EvaluationTest { new SkylarkTest() .update("mock", new Mock()) .setUp("b = mock.with_params(1, True, named=True)") - .testLookup("b", "with_params(1, true, false, true, false)"); + .testLookup("b", "with_params(1, true, false, true, false, a)"); new SkylarkTest() .update("mock", new Mock()) .setUp("") @@ -662,21 +685,28 @@ public class SkylarkEvaluationTest extends EvaluationTest { new SkylarkTest() .update("mock", new Mock()) .setUp("b = mock.with_params(1, True, True, named=True)") - .testLookup("b", "with_params(1, true, true, true, false)"); + .testLookup("b", "with_params(1, true, true, true, false, a)"); new SkylarkTest() .update("mock", new Mock()) .setUp("b = mock.with_params(1, True, named=True, posOrNamed=True)") - .testLookup("b", "with_params(1, true, true, true, false)"); + .testLookup("b", "with_params(1, true, true, true, false, a)"); new SkylarkTest() .update("mock", new Mock()) .setUp("b = mock.with_params(1, True, named=True, posOrNamed=True, optionalNamed=True)") - .testLookup("b", "with_params(1, true, true, true, true)"); + .testLookup("b", "with_params(1, true, true, true, true, a)"); new SkylarkTest() .update("mock", new Mock()) .setUp("") .testIfExactError( "Type Mock has no function with_params(int, bool, bool named, bool posOrNamed, int n)", "mock.with_params(1, True, named=True, posOrNamed=True, n=2)"); + new SkylarkTest() + .update("mock", new Mock()) + .setUp("") + .testIfExactError( + "Type Mock has no function with_params(int, bool, bool, bool named, bool optionalNamed," + + " NoneType nonNoneable)", + "mock.with_params(1, True, True, named=True, optionalNamed=False, nonNoneable=None)"); } @Test |