Repository: incubator-groovy Updated Branches: refs/heads/master 8f024f601 -> ed1b7d3ae
GROOVY-7434: Groovy should support resolving ambiguous signatures when using ClosureParams Project: http://git-wip-us.apache.org/repos/asf/incubator-groovy/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-groovy/commit/ed1b7d3a Tree: http://git-wip-us.apache.org/repos/asf/incubator-groovy/tree/ed1b7d3a Diff: http://git-wip-us.apache.org/repos/asf/incubator-groovy/diff/ed1b7d3a Branch: refs/heads/master Commit: ed1b7d3ae3322daceee2efda4a74366ae44369ac Parents: 8f024f6 Author: Paul King <pa...@asert.com.au> Authored: Mon May 25 21:49:08 2015 +1000 Committer: Paul King <pa...@asert.com.au> Committed: Tue May 26 19:07:24 2015 +1000 ---------------------------------------------------------------------- .../groovy/transform/stc/ClosureParams.java | 11 ++-- .../stc/ClosureSignatureConflictResolver.java | 54 ++++++++++++++++++++ .../groovy/transform/stc/PickFirstResolver.java | 45 ++++++++++++++++ .../stc/StaticTypeCheckingVisitor.java | 34 +++++++++++- src/spec/doc/core-semantics.adoc | 11 +++- .../stc/ClosureParamTypeResolverSTCTest.groovy | 43 ++++++++++++++++ 6 files changed, 190 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/ed1b7d3a/src/main/groovy/transform/stc/ClosureParams.java ---------------------------------------------------------------------- diff --git a/src/main/groovy/transform/stc/ClosureParams.java b/src/main/groovy/transform/stc/ClosureParams.java index 3cb7e62..e788d44 100644 --- a/src/main/groovy/transform/stc/ClosureParams.java +++ b/src/main/groovy/transform/stc/ClosureParams.java @@ -24,7 +24,7 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; /** - * Parameter annotation aimed at helping the IDEs or the static type checker to infer the + * Parameter annotation aimed at helping IDEs or the static type checker to infer the * parameter types of a closure. Without this annotation, a method signature may look like * this:<p> * <code>public <T,R> List<R> doSomething(List<T> source, Closure<R> consumer)</code> @@ -41,23 +41,24 @@ import java.lang.annotation.Target; * parameter signatures. A typical use case can be found when a closure accepts either a {@link java.util.Map.Entry} * or a (key,value) pair, like the {@link org.codehaus.groovy.runtime.DefaultGroovyMethods#each(java.util.Map, groovy.lang.Closure)} * method.</p> - * <p>For those reasons, the {@link ClosureParams} annotation only takes two arguments: + * <p>For those reasons, the {@link ClosureParams} annotation takes these arguments: * <ul> * <li>{@link ClosureParams#value()} defines a {@link groovy.transform.stc.ClosureSignatureHint} hint class * that the compiler will use to infer the parameter types</li> - * <li>{@link ClosureParams#options()}, a set of options that are passed to the hint when the type is inferred</li> + * <li>{@link ClosureParams#conflictResolutionStrategy()} defines a {@link groovy.transform.stc.ClosureSignatureConflictResolver} resolver + * class that the compiler will use to potentially reduce ambiguities remaining after initial inference calculations</li> + * <li>{@link ClosureParams#options()}, a set of options that are passed to the hint when the type is inferred (and also available to the resolver)</li> * </ul> * </p> * <p>As a result, the previous signature can be written like this:</p> * <code>public <T,R> List<R> doSomething(List<T> source, @ClosureParams(FirstParam.FirstGenericType.class) Closure<R> consumer)</code> * <p>Which uses the {@link FirstParam.FirstGenericType} first generic type of the first argument</p> hint to tell that the only expected * argument type corresponds to the type of the first generic argument type of the first method parameter. - * - * @author Cédric Champeau */ @Target(ElementType.PARAMETER) @Retention(RetentionPolicy.RUNTIME) public @interface ClosureParams { Class<? extends ClosureSignatureHint> value(); + Class<? extends ClosureSignatureConflictResolver> conflictResolutionStrategy() default ClosureSignatureConflictResolver.class; String[] options() default {}; } http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/ed1b7d3a/src/main/groovy/transform/stc/ClosureSignatureConflictResolver.java ---------------------------------------------------------------------- diff --git a/src/main/groovy/transform/stc/ClosureSignatureConflictResolver.java b/src/main/groovy/transform/stc/ClosureSignatureConflictResolver.java new file mode 100644 index 0000000..d727958 --- /dev/null +++ b/src/main/groovy/transform/stc/ClosureSignatureConflictResolver.java @@ -0,0 +1,54 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 groovy.transform.stc; + +import org.codehaus.groovy.ast.ClassNode; +import org.codehaus.groovy.ast.MethodNode; +import org.codehaus.groovy.ast.expr.ClosureExpression; +import org.codehaus.groovy.ast.expr.Expression; +import org.codehaus.groovy.control.CompilationUnit; +import org.codehaus.groovy.control.SourceUnit; + +import java.util.List; + +/** + * If multiple candidate signatures are found after applying type hints, + * a conflict resolver can attempt to resolve the ambiguity. + * + * @since 2.5.0 + */ +public class ClosureSignatureConflictResolver { + /** + * + * @param candidates the list of signatures as determined after applying type hints and performing initial inference calculations + * @param receiver the receiver the method is being called on + * @param arguments the arguments for the closure + * @param closure the closure expression under analysis + * @param methodNode the method for which a {@link groovy.lang.Closure} parameter was annotated with {@link ClosureParams} + * @param sourceUnit the source unit of the file being compiled + * @param compilationUnit the compilation unit of the file being compiled + * @param options the options, corresponding to the {@link ClosureParams#options()} found on the annotation + * @return a non-null list of signatures, where a signature corresponds to an array of class nodes, each of them matching a parameter. A list with more than one element indicates that all ambiguities haven't yet been resolved. + */ + public List<ClassNode[]> resolve(List<ClassNode[]> candidates, ClassNode receiver, Expression arguments, ClosureExpression closure, + MethodNode methodNode, SourceUnit sourceUnit, CompilationUnit compilationUnit, String[] options) { + // do nothing by default + return candidates; + } +} http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/ed1b7d3a/src/main/groovy/transform/stc/PickFirstResolver.java ---------------------------------------------------------------------- diff --git a/src/main/groovy/transform/stc/PickFirstResolver.java b/src/main/groovy/transform/stc/PickFirstResolver.java new file mode 100644 index 0000000..0bebb3e --- /dev/null +++ b/src/main/groovy/transform/stc/PickFirstResolver.java @@ -0,0 +1,45 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 groovy.transform.stc; + +import org.codehaus.groovy.ast.ClassNode; +import org.codehaus.groovy.ast.MethodNode; +import org.codehaus.groovy.ast.expr.ClosureExpression; +import org.codehaus.groovy.ast.expr.Expression; +import org.codehaus.groovy.control.CompilationUnit; +import org.codehaus.groovy.control.SourceUnit; + +import java.util.Collections; +import java.util.List; + +/** + * Returns the first of several candidates found. + * This is useful if several types should be supported but only the first + * should be the default/inferred type. Other options in the list are + * obtained through explicitly typing the parameter(s). + * + * @since 2.5.0 + */ +public class PickFirstResolver extends ClosureSignatureConflictResolver { + @Override + public List<ClassNode[]> resolve(List<ClassNode[]> candidates, ClassNode receiver, Expression arguments, ClosureExpression closure, + MethodNode methodNode, SourceUnit sourceUnit, CompilationUnit compilationUnit, String[] options) { + return Collections.singletonList(candidates.get(0)); + } +} http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/ed1b7d3a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java ---------------------------------------------------------------------- diff --git a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java index 36bb3f7..0c2cfec 100644 --- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -25,6 +25,7 @@ import groovy.lang.Range; import groovy.transform.TypeChecked; import groovy.transform.TypeCheckingMode; import groovy.transform.stc.ClosureParams; +import groovy.transform.stc.ClosureSignatureConflictResolver; import groovy.transform.stc.ClosureSignatureHint; import org.codehaus.groovy.GroovyBugError; import org.codehaus.groovy.ast.ASTNode; @@ -2308,8 +2309,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { for (AnnotationNode annotation : annotations) { Expression hintClass = annotation.getMember("value"); Expression options = annotation.getMember("options"); + Expression resolverClass = annotation.getMember("conflictResolutionStrategy"); if (hintClass instanceof ClassExpression) { - doInferClosureParameterTypes(receiver, arguments, expression, selectedMethod, hintClass, options); + doInferClosureParameterTypes(receiver, arguments, expression, selectedMethod, hintClass, resolverClass, options); } } } else if (isSAMType(param.getOriginType())) { @@ -2404,12 +2406,37 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { return closureSignatures; } + private List<ClassNode[]> resolveWithResolver(List<ClassNode[]> candidates, ClassNode receiver, Expression arguments, final ClosureExpression expression, final MethodNode selectedMethod, final Expression resolverClass, final Expression options) { + // initialize resolver + try { + ClassLoader transformLoader = getTransformLoader(); + @SuppressWarnings("unchecked") + Class<? extends ClosureSignatureConflictResolver> resolver = (Class<? extends ClosureSignatureConflictResolver>) transformLoader.loadClass(resolverClass.getText()); + ClosureSignatureConflictResolver resolverInstance = resolver.newInstance(); + return resolverInstance.resolve( + candidates, + receiver, + arguments, + expression, + selectedMethod instanceof ExtensionMethodNode ? ((ExtensionMethodNode) selectedMethod).getExtensionMethodNode() : selectedMethod, + typeCheckingContext.source, + typeCheckingContext.compilationUnit, + convertToStringArray(options)); + } catch (ClassNotFoundException e) { + throw new GroovyBugError(e); + } catch (InstantiationException e) { + throw new GroovyBugError(e); + } catch (IllegalAccessException e) { + throw new GroovyBugError(e); + } + } + private ClassLoader getTransformLoader() { CompilationUnit compilationUnit = typeCheckingContext.getCompilationUnit(); return compilationUnit!=null?compilationUnit.getTransformLoader():getSourceUnit().getClassLoader(); } - private void doInferClosureParameterTypes(final ClassNode receiver, final Expression arguments, final ClosureExpression expression, final MethodNode selectedMethod, final Expression hintClass, final Expression options) { + private void doInferClosureParameterTypes(final ClassNode receiver, final Expression arguments, final ClosureExpression expression, final MethodNode selectedMethod, final Expression hintClass, Expression resolverClass, final Expression options) { List<ClassNode[]> closureSignatures = getSignaturesFromHint(expression, selectedMethod, hintClass, options); List<ClassNode[]> candidates = new LinkedList<ClassNode[]>(); Parameter[] closureParams = expression.getParameters(); @@ -2452,6 +2479,9 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { } } } + if (candidates.size() > 1 && resolverClass instanceof ClassExpression) { + candidates = resolveWithResolver(candidates, receiver, arguments, expression, selectedMethod, resolverClass, options); + } if (candidates.size()>1) { addError("Ambiguous prototypes for closure. More than one target method matches. Please use explicit argument types.", expression); } http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/ed1b7d3a/src/spec/doc/core-semantics.adoc ---------------------------------------------------------------------- diff --git a/src/spec/doc/core-semantics.adoc b/src/spec/doc/core-semantics.adoc index 9b42fe1..7ddaa17 100644 --- a/src/spec/doc/core-semantics.adoc +++ b/src/spec/doc/core-semantics.adoc @@ -1834,7 +1834,7 @@ is `groovy.transform.stc.FirstParam` which indicated to the type checker that th whose type is the type of the first parameter of the method. In this case, the first parameter of the method is `Person`, so it indicates to the type checker that the first parameter of the closure is in fact a `Person`. -The second argument is optional and named _options_. It's semantics depends on the _type hint_ class. Groovy comes with +A second optional argument is named _options_. It's semantics depend on the _type hint_ class. Groovy comes with various bundled type hints, illustrated in the table below: [cols="1a,1,4a"] @@ -1909,6 +1909,7 @@ include::{projectdir}/src/spec/test/typing/TypeCheckingHintsTest.groovy[tags=typ If there are multiple signatures like in the example above, the type checker will *only* be able to infer the types of the arguments if the arity of each method is different. In the example above, `firstSignature` takes 2 arguments and `secondSignature` takes 1 argument, so the type checker can infer the argument types based on the number of arguments. +But see the optional resolver class attribute discussed next. |`FromString` |Yes @@ -1950,6 +1951,14 @@ In short, the lack of the `@ClosureParams` annotation on a method accepting a `C present (and it can be present in Java sources as well as Groovy sources), then the type checker has *more* information and can perform additional type inference. This makes this feature particularly interesting for framework developers. +A third optional argument is named _conflictResolutionStrategy_. It can reference a class (extending from +`ClosureSignatureConflictResolver`) that can perform additional resolution of parameter types if more than +one are found after initial inference calculations are complete. Groovy comes with the a default type resolver +which does nothing, and another which selects the first signature if multiple are found. The resolver is +only invoked if more than one signature is found and is by design a post processor. Any statements which need +injected typing information must pass one of the parameter signatures determined through type hints. The +resolver then picks among the returned candidate signatures. + ===== `@DelegatesTo` The `@DelegatesTo` annotation is used by the type checker to infer the type of the delegate. It allows the API designer http://git-wip-us.apache.org/repos/asf/incubator-groovy/blob/ed1b7d3a/src/test/groovy/transform/stc/ClosureParamTypeResolverSTCTest.groovy ---------------------------------------------------------------------- diff --git a/src/test/groovy/transform/stc/ClosureParamTypeResolverSTCTest.groovy b/src/test/groovy/transform/stc/ClosureParamTypeResolverSTCTest.groovy new file mode 100644 index 0000000..7cc2e3b --- /dev/null +++ b/src/test/groovy/transform/stc/ClosureParamTypeResolverSTCTest.groovy @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 groovy.transform.stc +/** + * Unit tests for static type checking : closure parameter type resolution + */ +class ClosureParamTypeResolverSTCTest extends StaticTypeCheckingTestCase { + void testInferenceForDGM_CollectUsingExplicitIt() { + assertScript ''' + import groovy.transform.stc.* + + def transform(item, @ClosureParams(value=FromString, conflictResolutionStrategy=PickFirstResolver, options=["Integer", "String"]) Closure condition) { + if (condition.parameterTypes[0].simpleName == 'String') + condition(item instanceof String ? item : item.toString()) + else + condition(item instanceof Integer ? item : item.toString().size()) + } + + assert transform('dog') { String s -> s * 2 } == 'dogdog' + assert transform('dog') { Integer i -> i * 2 } == 6 + assert transform('dog') { it.class.simpleName[0..it] } == 'Inte' + assert transform(35) { String s -> s * 2 } == '3535' + assert transform(35) { Integer i -> i * 2 } == 70 + assert transform(35) { it * 2 } == 70 + ''' + } +}