diff options
17 files changed, 700 insertions, 59 deletions
diff --git a/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkBuiltinMethodDoc.java b/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkBuiltinMethodDoc.java index bcb52923f9..4a2c5f8bad 100644 --- a/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkBuiltinMethodDoc.java +++ b/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkBuiltinMethodDoc.java @@ -13,10 +13,8 @@ // limitations under the License. package com.google.devtools.build.docgen.skylark; -import com.google.devtools.build.lib.skylarkinterface.Param; import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature; import com.google.devtools.build.lib.syntax.BaseFunction; -import java.util.ArrayList; import java.util.List; /** @@ -33,8 +31,12 @@ public final class SkylarkBuiltinMethodDoc extends SkylarkMethodDoc { this.module = module; this.annotation = annotation; this.fieldClass = fieldClass; - this.params = new ArrayList<>(); - processParams(); + this.params = + SkylarkDocUtils.determineParams( + this, + annotation.parameters(), + annotation.extraPositionals(), + annotation.extraKeywords()); } public SkylarkSignature getAnnotation() { @@ -78,20 +80,4 @@ public final class SkylarkBuiltinMethodDoc extends SkylarkMethodDoc { public List<SkylarkParamDoc> getParams() { return params; } - - private void processParams() { - processParams(adjustedParameters(annotation)); - if (!annotation.extraPositionals().name().isEmpty()) { - this.params.add(new SkylarkParamDoc(this, annotation.extraPositionals())); - } - if (!annotation.extraKeywords().name().isEmpty()) { - this.params.add(new SkylarkParamDoc(this, annotation.extraKeywords())); - } - } - - private void processParams(Param[] params) { - for (Param param : params) { - this.params.add(new SkylarkParamDoc(this, param)); - } - } } diff --git a/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkDocUtils.java b/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkDocUtils.java index ac73f65596..18300683af 100644 --- a/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkDocUtils.java +++ b/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkDocUtils.java @@ -14,7 +14,9 @@ package com.google.devtools.build.docgen.skylark; +import com.google.common.collect.ImmutableList; import com.google.devtools.build.docgen.DocgenConsts; +import com.google.devtools.build.lib.skylarkinterface.Param; /** A utility class for the documentation generator. */ public final class SkylarkDocUtils { @@ -30,4 +32,26 @@ public final class SkylarkDocUtils { .replace("$BE_ROOT", DocgenConsts.BeDocsRoot) .replace("$DOC_EXT", DocgenConsts.documentationExtension); } + + /** + * Returns a list of parameter documentation elements for a given method doc and the method's + * parameters. + */ + static ImmutableList<SkylarkParamDoc> determineParams( + SkylarkMethodDoc methodDoc, + Param[] userSuppliedParams, + Param extraPositionals, + Param extraKeywords) { + ImmutableList.Builder<SkylarkParamDoc> paramsBuilder = ImmutableList.builder(); + for (Param param : userSuppliedParams) { + paramsBuilder.add(new SkylarkParamDoc(methodDoc, param)); + } + if (!extraPositionals.name().isEmpty()) { + paramsBuilder.add(new SkylarkParamDoc(methodDoc, extraPositionals)); + } + if (!extraKeywords.name().isEmpty()) { + paramsBuilder.add(new SkylarkParamDoc(methodDoc, extraKeywords)); + } + return paramsBuilder.build(); + } } diff --git a/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkJavaMethodDoc.java b/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkJavaMethodDoc.java index fe92bdfe90..60ca32e093 100644 --- a/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkJavaMethodDoc.java +++ b/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkJavaMethodDoc.java @@ -42,11 +42,9 @@ public final class SkylarkJavaMethodDoc extends SkylarkMethodDoc { : callable.name(); this.method = method; this.callable = callable; - ImmutableList.Builder<SkylarkParamDoc> paramsBuilder = ImmutableList.builder(); - for (Param param : callable.parameters()) { - paramsBuilder.add(new SkylarkParamDoc(this, param)); - } - this.params = paramsBuilder.build(); + this.params = + SkylarkDocUtils.determineParams( + this, callable.parameters(), callable.extraPositionals(), callable.extraKeywords()); } public Method getMethod() { diff --git a/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkMethodDoc.java b/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkMethodDoc.java index 116fd4a528..b617803612 100644 --- a/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkMethodDoc.java +++ b/src/main/java/com/google/devtools/build/docgen/skylark/SkylarkMethodDoc.java @@ -72,6 +72,12 @@ public abstract class SkylarkMethodDoc extends SkylarkDoc { } argList.add(formatParameter(param)); } + if (!annotation.extraPositionals().name().isEmpty()) { + argList.add("*" + annotation.extraPositionals().name()); + } + if (!annotation.extraKeywords().name().isEmpty()) { + argList.add("**" + annotation.extraKeywords().name()); + } return Joiner.on(", ").join(argList); } diff --git a/src/main/java/com/google/devtools/build/lib/skylarkinterface/Param.java b/src/main/java/com/google/devtools/build/lib/skylarkinterface/Param.java index 8abb3448b9..8f10c72850 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkinterface/Param.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkinterface/Param.java @@ -86,8 +86,14 @@ public @interface Param { /** * If true, the parameter may be specified as a named parameter. For example for an integer named - * parameter {@code foo} of a method {@code bar}, then the method call will look like - * {@code bar(foo=1)}. + * parameter {@code foo} of a method {@code bar}, then the method call will look like {@code + * bar(foo=1)}. + * + * <p>If false, then {@link #positional} must be true (otherwise there is no way to reference the + * parameter via an argument). + * + * <p>If this parameter represents the 'extra positionals' (args) or 'extra keywords' (kwargs) + * element of a method, this field has no effect. */ boolean named() default false; @@ -97,7 +103,13 @@ public @interface Param { * {@code bar(1)}. If {@link #named()} is {@code false}, then this will be the only way to call * {@code bar}. * + * <p>If false, then {@link #named} must be true (otherwise there is no way to reference the + * parameter via an argument) + * * <p>Positional arguments should come first. + * + * <p>If this parameter represents the 'extra positionals' (args) or 'extra keywords' (kwargs) + * element of a method, this field has no effect. */ boolean positional() default true; diff --git a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkCallable.java b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkCallable.java index aed52245f9..92ef0a3a87 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkCallable.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkinterface/SkylarkCallable.java @@ -27,12 +27,13 @@ import java.lang.annotation.Target; * <ul> * <li>The method must be public. * <li>If structField=true, there must be zero user-supplied parameters. - * <li>Method parameters must be supplied in the following order: - * <pre>method([positionals][other user-args] + * <li>The underlying java method's parameters must be supplied in the following order: + * <pre>method([positionals]*[named args]*(extra positionals list)(extra kwargs) * (Location)(FuncallExpression)(Envrionment)(SkylarkSemantics))</pre> - * where Location, FuncallExpression, Environment, and SkylarkSemantics are supplied by the - * interpreter if and only if useLocation, useAst, useEnvironment, and useSkylarkSemantics are - * specified, respectively. + * where (extra positionals list) is a SkylarkList if extraPositionals is defined, (extra + * kwargs) is a SkylarkDict if extraKeywords is defined, and Location, FuncallExpression, + * Environment, and SkylarkSemantics are supplied by the interpreter if and only if + * useLocation, useAst, useEnvironment, and useSkylarkSemantics are specified, respectively. * <li>The number of method parameters much match the number of annotation-declared parameters * plus the number of interpreter-supplied parameters. * </ul> @@ -82,6 +83,34 @@ public @interface SkylarkCallable { Param[] parameters() default {}; /** + * Defines a catch-all list for additional unspecified positional parameters. + * + * <p>If this is left as default, it is an error for the caller to pass more positional arguments + * than are explicitly allowed by the method signature. If this is defined, all additional + * positional arguments are passed as elements of a {@link SkylarkList} to the method. + * + * <p>See python's <code>*args</code> (http://thepythonguru.com/python-args-and-kwargs/). + * + * <p>(If this is defined, the annotated method signature must contain a corresponding SkylarkList + * parameter. See the interface-level javadoc for details.) + */ + Param extraPositionals() default @Param(name = ""); + + /** + * Defines a catch-all dictionary for additional unspecified named parameters. + * + * <p>If this is left as default, it is an error for the caller to pass any named arguments not + * explicitly declared by the method signature. If this is defined, all additional named arguments + * are passed as elements of a {@link SkylarkDict} to the method. + * + * <p>See python's <code>**kwargs</code> (http://thepythonguru.com/python-args-and-kwargs/). + * + * <p>(If this is defined, the annotated method signature must contain a corresponding SkylarkDict + * parameter. See the interface-level javadoc for details.) + */ + Param extraKeywords() default @Param(name = ""); + + /** * Set it to true if the Java method may return <code>null</code> (which will then be converted to * <code>None</code>). If not set and the Java method returns null, an error will be raised. */ diff --git a/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java b/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java index 8057b89f0d..a39acba89a 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessor.java @@ -68,6 +68,9 @@ import javax.tools.Diagnostic; public final class SkylarkCallableProcessor extends AbstractProcessor { private Messager messager; + private static final String SKYLARK_LIST = "com.google.devtools.build.lib.syntax.SkylarkList<?>"; + private static final String SKYLARK_DICT = + "com.google.devtools.build.lib.syntax.SkylarkDict<?,?>"; private static final String LOCATION = "com.google.devtools.build.lib.events.Location"; private static final String AST = "com.google.devtools.build.lib.syntax.FuncallExpression"; private static final String ENVIRONMENT = "com.google.devtools.build.lib.syntax.Environment"; @@ -100,7 +103,7 @@ public final class SkylarkCallableProcessor extends AbstractProcessor { try { verifyDocumented(methodElement, annotation); - verifyNotStructFieldWithInvalidExtraParams(methodElement, annotation); + verifyNotStructFieldWithParams(methodElement, annotation); verifyParamSemantics(methodElement, annotation); verifyNumberOfParameters(methodElement, annotation); verifyExtraInterpreterParams(methodElement, annotation); @@ -121,15 +124,19 @@ public final class SkylarkCallableProcessor extends AbstractProcessor { } } - private void verifyNotStructFieldWithInvalidExtraParams( + private void verifyNotStructFieldWithParams( ExecutableElement methodElement, SkylarkCallable annotation) throws SkylarkCallableProcessorException { if (annotation.structField()) { - if (annotation.useAst() || annotation.useEnvironment() || annotation.useAst()) { + if (annotation.useAst() + || annotation.useEnvironment() + || annotation.useAst() + || !annotation.extraPositionals().name().isEmpty() + || !annotation.extraKeywords().name().isEmpty()) { throw new SkylarkCallableProcessorException( methodElement, "@SkylarkCallable-annotated methods with structField=true may not also specify " - + "useAst, useEnvironment, or useLocation"); + + "useAst, useEnvironment, useLocation, extraPositionals, or extraKeywords"); } } } @@ -255,6 +262,32 @@ public final class SkylarkCallableProcessor extends AbstractProcessor { // TODO(cparsons): Matching by class name alone is somewhat brittle, but due to tangled // dependencies, it is difficult for this processor to depend directy on the expected // classes here. + if (!annotation.extraPositionals().name().isEmpty()) { + if (!SKYLARK_LIST.equals(methodSignatureParams.get(currentIndex).asType().toString())) { + throw new SkylarkCallableProcessorException( + methodElement, + String.format( + "Expected parameter index %d to be the %s type, matching extraPositionals, " + + "but was %s", + currentIndex, + SKYLARK_LIST, + methodSignatureParams.get(currentIndex).asType().toString())); + } + currentIndex++; + } + if (!annotation.extraKeywords().name().isEmpty()) { + if (!SKYLARK_DICT.equals(methodSignatureParams.get(currentIndex).asType().toString())) { + throw new SkylarkCallableProcessorException( + methodElement, + String.format( + "Expected parameter index %d to be the %s type, matching extraKeywords, " + + "but was %s", + currentIndex, + SKYLARK_DICT, + methodSignatureParams.get(currentIndex).asType().toString())); + } + currentIndex++; + } if (annotation.useLocation()) { if (!LOCATION.equals(methodSignatureParams.get(currentIndex).asType().toString())) { throw new SkylarkCallableProcessorException( @@ -306,6 +339,8 @@ public final class SkylarkCallableProcessor extends AbstractProcessor { private int numExpectedExtraInterpreterParams(SkylarkCallable annotation) { int numExtraInterpreterParams = 0; + numExtraInterpreterParams += annotation.extraPositionals().name().isEmpty() ? 0 : 1; + numExtraInterpreterParams += annotation.extraKeywords().name().isEmpty() ? 0 : 1; numExtraInterpreterParams += annotation.useLocation() ? 1 : 0; numExtraInterpreterParams += annotation.useAst() ? 1 : 0; numExtraInterpreterParams += annotation.useEnvironment() ? 1 : 0; 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 ef8db702ad..c9f3aee01d 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 @@ -33,6 +33,7 @@ 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.Runtime.NoneType; +import com.google.devtools.build.lib.syntax.SkylarkList.Tuple; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.StringUtilities; import java.io.IOException; @@ -492,10 +493,16 @@ public final class FuncallExpression extends Expression { Map<String, Object> kwargs, MethodDescriptor method, Environment environment) { + SkylarkCallable callable = method.getAnnotation(); ImmutableList.Builder<Object> builder = ImmutableList.builder(); + ImmutableList.Builder<Object> extraArgsBuilder = ImmutableList.builder(); + ImmutableMap.Builder<String, Object> extraKwargsBuilder = ImmutableMap.builder(); + boolean acceptsExtraArgs = !callable.extraPositionals().name().isEmpty(); + boolean acceptsExtraKwargs = !callable.extraKeywords().name().isEmpty(); Class<?>[] javaMethodSignatureParams = method.getMethod().getParameterTypes(); - SkylarkCallable callable = method.getAnnotation(); int numExtraInterpreterParams = 0; + numExtraInterpreterParams += acceptsExtraArgs ? 1 : 0; + numExtraInterpreterParams += acceptsExtraKwargs ? 1 : 0; numExtraInterpreterParams += callable.useLocation() ? 1 : 0; numExtraInterpreterParams += callable.useAst() ? 1 : 0; numExtraInterpreterParams += callable.useEnvironment() ? 1 : 0; @@ -535,7 +542,6 @@ public final class FuncallExpression extends Expression { // Then process parameters specified in callable.parameters() Set<String> keys = new LinkedHashSet<>(kwargs.keySet()); - int paramIndex = mandatoryPositionals; // Positional parameters are always enumerated before non-positional parameters, // And default-valued positional parameters are always enumerated after other positional // parameters. These invariants are validated by the SkylarkCallable annotation processor. @@ -543,13 +549,8 @@ public final class FuncallExpression extends Expression { SkylarkType type = getType(param); Object value = null; - if (argIndex < args.size()) { // Positional arguments remain. + if (argIndex < args.size() && param.positional()) { // Positional args and params remain. value = args.get(argIndex); - if (!param.positional()) { - return ArgumentListConversionResult.fromError( - String.format("expected no more than %s positional arguments, but got %s", - paramIndex, args.size())); - } if (!type.contains(value)) { return ArgumentListConversionResult.fromError( String.format( @@ -561,7 +562,7 @@ public final class FuncallExpression extends Expression { String.format("got multiple values for keyword argument '%s'", param.name())); } argIndex++; - } else { // No more positional arguments. + } else { // No more positional arguments, or no more positional parameters. if (param.named() && keys.remove(param.name())) { // Param specified by keyword argument. value = kwargs.get(param.name()); if (!type.contains(value)) { @@ -583,22 +584,41 @@ public final class FuncallExpression extends Expression { String.format("parameter '%s' cannot be None", param.name())); } builder.add(value); - paramIndex++; } if (argIndex < args.size()) { - return ArgumentListConversionResult.fromError( - String.format("expected no more than %s positional arguments, but got %s", - paramIndex, args.size())); + if (acceptsExtraArgs) { + for (; argIndex < args.size(); argIndex++) { + extraArgsBuilder.add(args.get(argIndex)); + } + } else { + return ArgumentListConversionResult.fromError( + String.format( + "expected no more than %s positional arguments, but got %s", + argIndex, args.size())); + } } if (!keys.isEmpty()) { - return ArgumentListConversionResult.fromError( - String.format("unexpected keyword%s %s", - keys.size() > 1 ? "s" : "", - Joiner.on(",").join(Iterables.transform(keys, s -> "'" + s + "'")))); + if (acceptsExtraKwargs) { + for (String key : keys) { + extraKwargsBuilder.put(key, kwargs.get(key)); + } + } else { + return ArgumentListConversionResult.fromError( + String.format( + "unexpected keyword%s %s", + keys.size() > 1 ? "s" : "", + Joiner.on(",").join(Iterables.transform(keys, s -> "'" + s + "'")))); + } } - // Then add any skylark-info arguments (for example the Environment). + // Then add any skylark-interpreter arguments (for example kwargs or the Environment). + if (acceptsExtraArgs) { + builder.add(Tuple.copyOf(extraArgsBuilder.build())); + } + if (acceptsExtraKwargs) { + builder.add(SkylarkDict.copyOf(environment, extraKwargsBuilder.build())); + } if (callable.useLocation()) { builder.add(getLocation()); } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java index a2dc39a377..cbc653a087 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java @@ -68,9 +68,16 @@ public class SkylarkSignatureProcessor { parameters.add(parameter); } - return getSignatureForCallable(name, documented, parameters.build(), annotation.parameters(), - /*extraPositionals=*/null, - /*extraKeywords=*/null, /*defaultValues=*/null, paramDoc, enforcedTypesList); + return getSignatureForCallable( + name, + documented, + parameters.build(), + annotation.parameters(), + annotation.extraPositionals(), + annotation.extraKeywords(), + /*defaultValues=*/ null, + paramDoc, + enforcedTypesList); } diff --git a/src/test/java/com/google/devtools/build/docgen/SkylarkDocumentationTest.java b/src/test/java/com/google/devtools/build/docgen/SkylarkDocumentationTest.java index d57aa760e9..4c1eb448ec 100644 --- a/src/test/java/com/google/devtools/build/docgen/SkylarkDocumentationTest.java +++ b/src/test/java/com/google/devtools/build/docgen/SkylarkDocumentationTest.java @@ -30,6 +30,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.Environment; +import com.google.devtools.build.lib.syntax.SkylarkDict; import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.syntax.SkylarkList.MutableList; import com.google.devtools.build.lib.syntax.SkylarkList.Tuple; @@ -167,6 +168,61 @@ public class SkylarkDocumentationTest extends SkylarkTestCase { } } + /** MockClassF */ + @SkylarkModule(name = "MockClassF", doc = "MockClassF") + private static class MockClassF { + @SkylarkCallable( + doc = "MockClassF#test", + parameters = { + @Param(name = "a", named = false, positional = true), + @Param(name = "b", named = true, positional = true), + @Param(name = "c", named = true, positional = false), + @Param(name = "d", named = true, positional = false, defaultValue = "1"), + }, + extraPositionals = @Param(name = "myArgs") + ) + public Integer test(int a, int b, int c, int d, SkylarkList<?> args) { + return 0; + } + } + + /** MockClassG */ + @SkylarkModule(name = "MockClassG", doc = "MockClassG") + private static class MockClassG { + @SkylarkCallable( + doc = "MockClassG#test", + parameters = { + @Param(name = "a", named = false, positional = true), + @Param(name = "b", named = true, positional = true), + @Param(name = "c", named = true, positional = false), + @Param(name = "d", named = true, positional = false, defaultValue = "1"), + }, + extraKeywords = @Param(name = "myKwargs") + ) + public Integer test(int a, int b, int c, int d, SkylarkDict<?, ?> kwargs) { + return 0; + } + } + + /** MockClassH */ + @SkylarkModule(name = "MockClassH", doc = "MockClassH") + private static class MockClassH { + @SkylarkCallable( + doc = "MockClassH#test", + parameters = { + @Param(name = "a", named = false, positional = true), + @Param(name = "b", named = true, positional = true), + @Param(name = "c", named = true, positional = false), + @Param(name = "d", named = true, positional = false, defaultValue = "1"), + }, + extraPositionals = @Param(name = "myArgs"), + extraKeywords = @Param(name = "myKwargs") + ) + public Integer test(int a, int b, int c, int d, SkylarkList<?> args, SkylarkDict<?, ?> kwargs) { + return 0; + } + } + /** MockClassWithContainerReturnValues */ @SkylarkModule(name = "MockClassWithContainerReturnValues", doc = "MockClassWithContainerReturnValues") @@ -240,6 +296,57 @@ public class SkylarkDocumentationTest extends SkylarkTestCase { } @Test + public void testSkylarkCallableParametersAndArgs() throws Exception { + Map<String, SkylarkModuleDoc> objects = collect(MockClassF.class); + assertThat(objects).hasSize(1); + assertThat(objects).containsKey("MockClassF"); + SkylarkModuleDoc moduleDoc = objects.get("MockClassF"); + assertThat(moduleDoc.getDocumentation()).isEqualTo("MockClassF"); + assertThat(moduleDoc.getMethods()).hasSize(1); + SkylarkMethodDoc methodDoc = moduleDoc.getMethods().iterator().next(); + assertThat(methodDoc.getDocumentation()).isEqualTo("MockClassF#test"); + assertThat(methodDoc.getSignature()) + .isEqualTo( + "<a class=\"anchor\" href=\"int.html\">int</a> " + + "MockClassF.test(a, b, *, c, d=1, *myArgs)"); + assertThat(methodDoc.getParams()).hasSize(5); + } + + @Test + public void testSkylarkCallableParametersAndKwargs() throws Exception { + Map<String, SkylarkModuleDoc> objects = collect(MockClassG.class); + assertThat(objects).hasSize(1); + assertThat(objects).containsKey("MockClassG"); + SkylarkModuleDoc moduleDoc = objects.get("MockClassG"); + assertThat(moduleDoc.getDocumentation()).isEqualTo("MockClassG"); + assertThat(moduleDoc.getMethods()).hasSize(1); + SkylarkMethodDoc methodDoc = moduleDoc.getMethods().iterator().next(); + assertThat(methodDoc.getDocumentation()).isEqualTo("MockClassG#test"); + assertThat(methodDoc.getSignature()) + .isEqualTo( + "<a class=\"anchor\" href=\"int.html\">int</a> " + + "MockClassG.test(a, b, *, c, d=1, **myKwargs)"); + assertThat(methodDoc.getParams()).hasSize(5); + } + + @Test + public void testSkylarkCallableParametersAndArgsAndKwargs() throws Exception { + Map<String, SkylarkModuleDoc> objects = collect(MockClassH.class); + assertThat(objects).hasSize(1); + assertThat(objects).containsKey("MockClassH"); + SkylarkModuleDoc moduleDoc = objects.get("MockClassH"); + assertThat(moduleDoc.getDocumentation()).isEqualTo("MockClassH"); + assertThat(moduleDoc.getMethods()).hasSize(1); + SkylarkMethodDoc methodDoc = moduleDoc.getMethods().iterator().next(); + assertThat(methodDoc.getDocumentation()).isEqualTo("MockClassH#test"); + assertThat(methodDoc.getSignature()) + .isEqualTo( + "<a class=\"anchor\" href=\"int.html\">int</a> " + + "MockClassH.test(a, b, *, c, d=1, *myArgs, **myKwargs)"); + assertThat(methodDoc.getParams()).hasSize(6); + } + + @Test public void testSkylarkCallableOverriding() throws Exception { Map<String, SkylarkModuleDoc> objects = collect(MockClassE.class); assertThat(objects).hasSize(1); diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessorTest.java b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessorTest.java index 69a2725e7d..016fe30704 100644 --- a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessorTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/SkylarkCallableProcessorTest.java @@ -74,7 +74,29 @@ public final class SkylarkCallableProcessorTest { .failsToCompile() .withErrorContaining( "@SkylarkCallable-annotated methods with structField=true may not also specify " - + "useAst, useEnvironment, or useLocation"); + + "useAst, useEnvironment, useLocation, extraPositionals, or extraKeywords"); + } + + @Test + public void testStructFieldWithExtraArgs() throws Exception { + assertAbout(javaSource()) + .that(getFile("StructFieldWithExtraArgs.java")) + .processedWith(new SkylarkCallableProcessor()) + .failsToCompile() + .withErrorContaining( + "@SkylarkCallable-annotated methods with structField=true may not also specify " + + "useAst, useEnvironment, useLocation, extraPositionals, or extraKeywords"); + } + + @Test + public void testStructFieldWithExtraKeywords() throws Exception { + assertAbout(javaSource()) + .that(getFile("StructFieldWithExtraKeywords.java")) + .processedWith(new SkylarkCallableProcessor()) + .failsToCompile() + .withErrorContaining( + "@SkylarkCallable-annotated methods with structField=true may not also specify " + + "useAst, useEnvironment, useLocation, extraPositionals, or extraKeywords"); } @Test @@ -220,4 +242,27 @@ public final class SkylarkCallableProcessorTest { .withErrorContaining( "Positional-only parameter 'two' is specified after one or more named parameters"); } + + @Test + public void testExtraKeywordsOutOfOrder() throws Exception { + assertAbout(javaSource()) + .that(getFile("ExtraKeywordsOutOfOrder.java")) + .processedWith(new SkylarkCallableProcessor()) + .failsToCompile() + .withErrorContaining( + "Expected parameter index 1 to be the " + + "com.google.devtools.build.lib.syntax.SkylarkDict<?,?> type, matching " + + "extraKeywords, but was java.lang.String"); + } + + @Test + public void testExtraPositionalsMissing() throws Exception { + assertAbout(javaSource()) + .that(getFile("ExtraPositionalsMissing.java")) + .processedWith(new SkylarkCallableProcessor()) + .failsToCompile() + .withErrorContaining( + "@SkylarkCallable annotated method has 3 parameters, but annotation declared " + + "1 user-supplied parameters and 3 extra interpreter parameters."); + } } diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/ExtraKeywordsOutOfOrder.java b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/ExtraKeywordsOutOfOrder.java new file mode 100644 index 0000000000..31498c0eef --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/ExtraKeywordsOutOfOrder.java @@ -0,0 +1,41 @@ +// Copyright 2018 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.processor.testsources; + +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.syntax.Environment; +import com.google.devtools.build.lib.syntax.SkylarkDict; + +/** + * Test case for a SkylarkCallable method which specifies extraKeywords, but specifies the argument + * out of order. + */ +public class ExtraKeywordsOutOfOrder { + + @SkylarkCallable( + name = "extra_kwargs_out_of_order", + documented = false, + parameters = {@Param(name = "one")}, + extraKeywords = @Param(name = "kwargs"), + useLocation = true, + useEnvironment = true + ) + public String threeArgMethod( + SkylarkDict<?, ?> kwargs, String one, Location location, Environment environment) { + return "bar"; + } +} diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/ExtraPositionalsMissing.java b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/ExtraPositionalsMissing.java new file mode 100644 index 0000000000..4fbb044e2b --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/ExtraPositionalsMissing.java @@ -0,0 +1,38 @@ +// Copyright 2018 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.processor.testsources; + +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.syntax.Environment; + +/** + * Test case for a SkylarkCallable method which specifies extraPositionals, but omits that argument. + */ +public class ExtraPositionalsMissing { + + @SkylarkCallable( + name = "extra_positionals_missing", + documented = false, + parameters = {@Param(name = "one")}, + extraPositionals = @Param(name = "args"), + useLocation = true, + useEnvironment = true + ) + public String threeArgMethod(String one, Location location, Environment environment) { + return "bar"; + } +} diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/GoldenCase.java b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/GoldenCase.java index 53baa0145c..1b0e136fea 100644 --- a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/GoldenCase.java +++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/GoldenCase.java @@ -20,6 +20,8 @@ import com.google.devtools.build.lib.skylarkinterface.ParamType; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.syntax.Environment; import com.google.devtools.build.lib.syntax.FuncallExpression; +import com.google.devtools.build.lib.syntax.SkylarkDict; +import com.google.devtools.build.lib.syntax.SkylarkList; import com.google.devtools.build.lib.syntax.SkylarkSemantics; /** @@ -152,4 +154,48 @@ public class GoldenCase { Location location) { return "baz"; } + + @SkylarkCallable( + name = "two_arg_method_with_params_and_info_and_kwargs", + documented = false, + parameters = { + @Param(name = "one", type = String.class, named = true), + @Param(name = "two", type = Integer.class, named = true), + }, + extraKeywords = @Param(name = "kwargs"), + useAst = true, + useLocation = true, + useEnvironment = true, + useSkylarkSemantics = true + ) + public String twoArgMethodWithParamsAndInfoAndKwargs( + String one, + Integer two, + SkylarkDict<?, ?> kwargs, + Location location, + FuncallExpression ast, + Environment environment, + SkylarkSemantics skylarkSemantics) { + return "blep"; + } + + @SkylarkCallable( + name = "two_arg_method_with_env_and_args_and_kwargs", + documented = false, + parameters = { + @Param(name = "one", type = String.class, named = true), + @Param(name = "two", type = Integer.class, named = true), + }, + extraPositionals = @Param(name = "args"), + extraKeywords = @Param(name = "kwargs"), + useEnvironment = true + ) + public String twoArgMethodWithParamsAndInfoAndKwargs( + String one, + Integer two, + SkylarkList<?> args, + SkylarkDict<?, ?> kwargs, + Environment environment) { + return "yar"; + } } diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/StructFieldWithExtraArgs.java b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/StructFieldWithExtraArgs.java new file mode 100644 index 0000000000..8c8c5081ce --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/StructFieldWithExtraArgs.java @@ -0,0 +1,33 @@ +// Copyright 2018 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.processor.testsources; + +import com.google.devtools.build.lib.skylarkinterface.Param; +import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; +import com.google.devtools.build.lib.syntax.SkylarkList; + +/** Test case which verifies a struct field method cannot specify extraArgs. */ +public class StructFieldWithExtraArgs { + + @SkylarkCallable( + name = "struct_field_method_with_extra_args", + documented = false, + structField = true, + extraPositionals = @Param(name = "args") + ) + public String structFieldMethodWithExtraArgs(SkylarkList<?> args) { + return "Cat."; + } +} diff --git a/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/StructFieldWithExtraKeywords.java b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/StructFieldWithExtraKeywords.java new file mode 100644 index 0000000000..9055172465 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/skylarkinterface/processor/testsources/StructFieldWithExtraKeywords.java @@ -0,0 +1,33 @@ +// Copyright 2018 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.processor.testsources; + +import com.google.devtools.build.lib.skylarkinterface.Param; +import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; +import com.google.devtools.build.lib.syntax.SkylarkDict; + +/** Test case which verifies a struct field method cannot specify extraArgs. */ +public class StructFieldWithExtraKeywords { + + @SkylarkCallable( + name = "struct_field_method_with_extra_kwargs", + documented = false, + structField = true, + extraKeywords = @Param(name = "kwargs") + ) + public String structFieldMethodWithExtraKeywords(SkylarkDict<?, ?> args) { + return "Dog."; + } +} 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 3804a72d53..7b86d6cad4 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 @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.syntax; import static com.google.common.truth.Truth.assertThat; +import static java.util.stream.Collectors.joining; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; @@ -163,7 +164,7 @@ public class SkylarkEvaluationTest extends EvaluationTest { return ImmutableMap.of("a", ImmutableList.of("b", "c")); } - + @SkylarkCallable( name = "with_params", documented = false, @@ -365,6 +366,82 @@ public class SkylarkEvaluationTest extends EvaluationTest { return NativeProvider.STRUCT.create(builder.build(), "no native callable '%s'"); } + @SkylarkCallable( + name = "with_args_and_env", + documented = false, + parameters = { + @Param(name = "pos1", type = Integer.class), + @Param(name = "pos2", defaultValue = "False", type = Boolean.class), + @Param(name = "named", type = Boolean.class, positional = false, named = true), + }, + extraPositionals = @Param(name = "args"), + useEnvironment = true + ) + public String withArgsAndEnv( + Integer pos1, boolean pos2, boolean named, SkylarkList<?> args, Environment env) { + String argsString = + "args(" + args.stream().map(Printer::debugPrint).collect(joining(", ")) + ")"; + return "with_args_and_env(" + + pos1 + + ", " + + pos2 + + ", " + + named + + ", " + + argsString + + ", " + + env.isGlobal() + + ")"; + } + + @SkylarkCallable( + name = "with_kwargs", + documented = false, + parameters = { + @Param(name = "pos", defaultValue = "False", type = Boolean.class), + @Param(name = "named", type = Boolean.class, positional = false, named = true), + }, + extraKeywords = @Param(name = "kwargs") + ) + public String withKwargs(boolean pos, boolean named, SkylarkDict<?, ?> kwargs) + throws EvalException { + String kwargsString = + "kwargs(" + + kwargs + .getContents(String.class, Object.class, "kwargs") + .entrySet() + .stream() + .map(entry -> entry.getKey() + "=" + entry.getValue()) + .collect(joining(", ")) + + ")"; + return "with_kwargs(" + pos + ", " + named + ", " + kwargsString + ")"; + } + + @SkylarkCallable( + name = "with_args_and_kwargs", + documented = false, + parameters = { + @Param(name = "foo", named = true, positional = true, type = String.class), + }, + extraPositionals = @Param(name = "args"), + extraKeywords = @Param(name = "kwargs") + ) + public String withArgsAndKwargs(String foo, SkylarkList<?> args, SkylarkDict<?, ?> kwargs) + throws EvalException { + String argsString = + "args(" + args.stream().map(Printer::debugPrint).collect(joining(", ")) + ")"; + String kwargsString = + "kwargs(" + + kwargs + .getContents(String.class, Object.class, "kwargs") + .entrySet() + .stream() + .map(entry -> entry.getKey() + "=" + entry.getValue()) + .collect(joining(", ")) + + ")"; + return "with_args_and_kwargs(" + foo + ", " + argsString + ", " + kwargsString + ")"; + } + @Override public String toString() { return "<mock>"; @@ -1108,6 +1185,107 @@ public class SkylarkEvaluationTest extends EvaluationTest { } @Test + public void testJavaFunctionWithExtraArgsAndEnv() throws Exception { + new SkylarkTest() + .update("mock", new Mock()) + .setUp("b = mock.with_args_and_env(1, True, 'extraArg1', 'extraArg2', named=True)") + .testLookup("b", "with_args_and_env(1, true, true, args(extraArg1, extraArg2), true)"); + + // Use an args list. + new SkylarkTest() + .update("mock", new Mock()) + .setUp( + "myargs = ['extraArg2']", + "b = mock.with_args_and_env(1, True, 'extraArg1', named=True, *myargs)") + .testLookup("b", "with_args_and_env(1, true, true, args(extraArg1, extraArg2), true)"); + } + + @Test + public void testJavaFunctionWithExtraKwargs() throws Exception { + new SkylarkTest() + .update("mock", new Mock()) + .setUp("b = mock.with_kwargs(True, extraKey1=True, named=True, extraKey2='x')") + .testLookup("b", "with_kwargs(true, true, kwargs(extraKey1=true, extraKey2=x))"); + + // Use a kwargs dict. + new SkylarkTest() + .update("mock", new Mock()) + .setUp( + "mykwargs = {'extraKey2':'x', 'named':True}", + "b = mock.with_kwargs(True, extraKey1=True, **mykwargs)") + .testLookup("b", "with_kwargs(true, true, kwargs(extraKey1=true, extraKey2=x))"); + } + + @Test + public void testJavaFunctionWithArgsAndKwargs() throws Exception { + // Foo is used positionally + new SkylarkTest() + .update("mock", new Mock()) + .setUp("b = mock.with_args_and_kwargs('foo', 'bar', 'baz', extraKey1=True, extraKey2='x')") + .testLookup( + "b", "with_args_and_kwargs(foo, args(bar, baz), kwargs(extraKey1=true, extraKey2=x))"); + + // Use an args list and a kwargs dict + new SkylarkTest() + .update("mock", new Mock()) + .setUp( + "mykwargs = {'extraKey1':True}", + "myargs = ['baz']", + "b = mock.with_args_and_kwargs('foo', 'bar', extraKey2='x', *myargs, **mykwargs)") + .testLookup( + "b", "with_args_and_kwargs(foo, args(bar, baz), kwargs(extraKey2=x, extraKey1=true))"); + + // Foo is used by name + new SkylarkTest() + .update("mock", new Mock()) + .setUp("b = mock.with_args_and_kwargs(foo='foo', extraKey1=True)") + .testLookup("b", "with_args_and_kwargs(foo, args(), kwargs(extraKey1=true))"); + + // Empty args and kwargs. + new SkylarkTest() + .update("mock", new Mock()) + .setUp("b = mock.with_args_and_kwargs('foo')") + .testLookup("b", "with_args_and_kwargs(foo, args(), kwargs())"); + } + + @Test + public void testProxyMethodsObjectWithArgsAndKwargs() throws Exception { + // Foo is used positionally + new SkylarkTest() + .update("mock", new Mock()) + .setUp( + "m = mock.proxy_methods_object()", + "b = m.with_args_and_kwargs('foo', 'bar', 'baz', extraKey1=True, extraKey2='x')") + .testLookup( + "b", "with_args_and_kwargs(foo, args(bar, baz), kwargs(extraKey1=true, extraKey2=x))"); + + // Use an args list and a kwargs dict + new SkylarkTest() + .update("mock", new Mock()) + .setUp( + "mykwargs = {'extraKey1':True}", + "myargs = ['baz']", + "m = mock.proxy_methods_object()", + "b = m.with_args_and_kwargs('foo', 'bar', extraKey2='x', *myargs, **mykwargs)") + .testLookup( + "b", "with_args_and_kwargs(foo, args(bar, baz), kwargs(extraKey2=x, extraKey1=true))"); + + // Foo is used by name + new SkylarkTest() + .update("mock", new Mock()) + .setUp( + "m = mock.proxy_methods_object()", + "b = m.with_args_and_kwargs(foo='foo', extraKey1=True)") + .testLookup("b", "with_args_and_kwargs(foo, args(), kwargs(extraKey1=true))"); + + // Empty args and kwargs. + new SkylarkTest() + .update("mock", new Mock()) + .setUp("m = mock.proxy_methods_object()", "b = m.with_args_and_kwargs('foo')") + .testLookup("b", "with_args_and_kwargs(foo, args(), kwargs())"); + } + + @Test public void testStructAccessOfMethod() throws Exception { new SkylarkTest() .update("mock", new Mock()) @@ -1591,7 +1769,10 @@ public class SkylarkEvaluationTest extends EvaluationTest { "struct_field_callable", "value_of", "voidfunc", + "with_args_and_env", + "with_args_and_kwargs", "with_extra", + "with_kwargs", "with_params", "with_params_and_extra"); } |