diff options
author | 2017-09-20 14:35:11 +0200 | |
---|---|---|
committer | 2017-09-20 16:37:06 +0200 | |
commit | 17214ac78ffaec369d5d5bafe62a39730473cfaa (patch) | |
tree | 783cb524169b1ef2dfe2634be6bb5e52b00e2eae | |
parent | 30adfbf30b509bdcf5befaf2abd77addb89c8e69 (diff) |
Check parameter types for methods when multiple types are allowed.
Fixes #3714
RELNOTES: None.
PiperOrigin-RevId: 169382686
3 files changed, 71 insertions, 7 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 f3019e471a..fbb4ef11ce 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 @@ -25,6 +25,7 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.skylarkinterface.Param; +import com.google.devtools.build.lib.skylarkinterface.ParamType; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.SkylarkInterfaceUtils; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; @@ -388,11 +389,23 @@ public final class FuncallExpression extends Expression { } private static SkylarkType getType(Param param) { - SkylarkType type = - param.generic1() != Object.class - ? SkylarkType.of(param.type(), param.generic1()) - : SkylarkType.of(param.type()); - return type; + if (param.allowedTypes().length > 0) { + Preconditions.checkState(Object.class.equals(param.type())); + SkylarkType result = SkylarkType.BOTTOM; + for (ParamType paramType : param.allowedTypes()) { + SkylarkType t = paramType.generic1() != Object.class + ? SkylarkType.of(paramType.type(), paramType.generic1()) + : SkylarkType.of(paramType.type()); + result = SkylarkType.Union.of(result, t); + } + return result; + } else { + SkylarkType type = + param.generic1() != Object.class + ? SkylarkType.of(param.type(), param.generic1()) + : SkylarkType.of(param.type()); + return type; + } } /** 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 0add5edade..983539a49a 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 @@ -297,7 +297,8 @@ public class SkylarkRuleImplementationFunctionsTest extends SkylarkTestCase { public void testCreateSpawnActionArgumentsBadExecutable() throws Exception { checkErrorContains( createRuleContext("//foo:foo"), - "expected file or string for executable but got int instead", + "Cannot convert parameter 'executable' to type File or string, in method " + + "run(list inputs, list outputs, list arguments, int executable) of 'actions'", "ruleContext.actions.run(", " inputs = ruleContext.files.srcs,", " outputs = ruleContext.files.srcs,", 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 1febef33f9..175aedad67 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 @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.packages.NativeInfo; import com.google.devtools.build.lib.packages.NativeProvider; import com.google.devtools.build.lib.skylarkinterface.Param; +import com.google.devtools.build.lib.skylarkinterface.ParamType; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature; @@ -155,6 +156,18 @@ public class SkylarkEvaluationTest extends EvaluationTest { positional = false, named = true ), + @Param( + name = "multi", + allowedTypes = { + @ParamType(type = String.class), + @ParamType(type = Integer.class), + @ParamType(type = SkylarkList.class, generic1 = Integer.class), + }, + defaultValue = "None", + noneable = true, + positional = false, + named = true + ) } ) public String withParams( @@ -164,7 +177,8 @@ public class SkylarkEvaluationTest extends EvaluationTest { boolean named, boolean optionalNamed, Object nonNoneable, - Object noneable) { + Object noneable, + Object multi) { return "with_params(" + pos1 + ", " @@ -177,6 +191,8 @@ public class SkylarkEvaluationTest extends EvaluationTest { + optionalNamed + ", " + nonNoneable.toString() + + (noneable != Runtime.NONE ? ", " + noneable : "") + + (multi != Runtime.NONE ? ", " + multi : "") + ")"; } @@ -744,6 +760,21 @@ public class SkylarkEvaluationTest extends EvaluationTest { .testLookup("b", "with_params(1, true, false, true, false, a)"); new SkylarkTest() .update("mock", new Mock()) + .setUp("b = mock.with_params(1, True, named=True, multi=1)") + .testLookup("b", "with_params(1, true, false, true, false, a, 1)"); + new SkylarkTest() + .update("mock", new Mock()) + .setUp("b = mock.with_params(1, True, named=True, multi='abc')") + .testLookup("b", "with_params(1, true, false, true, false, a, abc)"); + + new SkylarkTest() + .update("mock", new Mock()) + .setUp("b = mock.with_params(1, True, named=True, multi=[1,2,3])") + .testLookup("b", "with_params(1, true, false, true, false, a, [1, 2, 3])"); + + + new SkylarkTest() + .update("mock", new Mock()) .setUp("") .testIfExactError( "parameter 'named' has no default value, in method with_params(int, bool) of 'Mock'", @@ -781,6 +812,25 @@ public class SkylarkEvaluationTest extends EvaluationTest { "parameter 'nonNoneable' cannot be None, in method with_params(int, bool, bool, " + "bool named, bool optionalNamed, NoneType nonNoneable) of 'Mock'", "mock.with_params(1, True, True, named=True, optionalNamed=False, nonNoneable=None)"); + + new SkylarkTest() + .update("mock", new Mock()) + .setUp("") + .testIfExactError( + "Cannot convert parameter 'multi' to type string or int or sequence of ints or" + + " NoneType, in method with_params(int, bool, bool named, bool multi) of 'Mock'", + "mock.with_params(1, True, named=True, multi=False)" + ); + + // We do not enforce list item parameter type constraints. + // Test for this behavior. + new SkylarkTest() + .update("mock", new Mock()) + .setUp("b = mock.with_params(1, True, named=True, multi=['a', 'b'])") + .testLookup( + "b", "with_params(1, true, false, true, false, a, [\"a\", \"b\"])" + ); + } @Test |