Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 3AE67200B8B for ; Tue, 20 Sep 2016 05:21:20 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 39A5D160ACC; Tue, 20 Sep 2016 03:21:20 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 2370C160ADC for ; Tue, 20 Sep 2016 05:21:18 +0200 (CEST) Received: (qmail 36885 invoked by uid 500); 20 Sep 2016 03:21:18 -0000 Mailing-List: contact commits-help@groovy.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@groovy.apache.org Delivered-To: mailing list commits@groovy.apache.org Received: (qmail 36862 invoked by uid 99); 20 Sep 2016 03:21:18 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 20 Sep 2016 03:21:18 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id E6F29DFE60; Tue, 20 Sep 2016 03:21:17 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: jwagenleitner@apache.org To: commits@groovy.apache.org Date: Tue, 20 Sep 2016 03:21:17 -0000 Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: [1/2] groovy git commit: GROOVY-7922: Static type checking not strict enough in the presence of ambiguous method matching (closes #422) archived-at: Tue, 20 Sep 2016 03:21:20 -0000 Repository: groovy Updated Branches: refs/heads/GROOVY_2_4_X 369e9826a -> d40f188a8 GROOVY-7922: Static type checking not strict enough in the presence of ambiguous method matching (closes #422) Project: http://git-wip-us.apache.org/repos/asf/groovy/repo Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/32d5d1f0 Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/32d5d1f0 Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/32d5d1f0 Branch: refs/heads/GROOVY_2_4_X Commit: 32d5d1f077a65773c2d78bc9c852f505c20007ea Parents: 369e982 Author: zhangbo Authored: Fri Sep 16 08:29:11 2016 +0800 Committer: John Wagenleitner Committed: Mon Sep 19 19:57:40 2016 -0700 ---------------------------------------------------------------------- .../stc/StaticTypeCheckingSupport.java | 108 +++++++++++-------- src/test/groovy/bugs/Groovy7922Bug.groovy | 42 ++++++++ 2 files changed, 107 insertions(+), 43 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/groovy/blob/32d5d1f0/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java ---------------------------------------------------------------------- diff --git a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java index d881902..bc1680d 100644 --- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java +++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java @@ -35,6 +35,7 @@ import org.codehaus.groovy.ast.expr.MapExpression; import org.codehaus.groovy.ast.expr.VariableExpression; import org.codehaus.groovy.ast.stmt.ReturnStatement; import org.codehaus.groovy.ast.tools.GenericsUtils; +import org.codehaus.groovy.ast.tools.ParameterUtils; import org.codehaus.groovy.ast.tools.WideningCategories; import org.codehaus.groovy.control.CompilationUnit; import org.codehaus.groovy.control.CompilerConfiguration; @@ -887,7 +888,7 @@ public abstract class StaticTypeCheckingSupport { // we want to add one, because there is an interface between // the interface we search for and the interface we are in. if (sub != -1) { - sub+=(i+1); // GROOVY-6970: Make sure we can choose between equivalent methods + sub+=1; } // we are interested in the longest path only max = Math.max(max, sub); @@ -971,7 +972,7 @@ public abstract class StaticTypeCheckingSupport { } List bestChoices = new LinkedList(); int bestDist = Integer.MAX_VALUE; - Collection choicesLeft = removeCovariants(methods); + Collection choicesLeft = removeCovariantsAndInterfaceEquivalents(methods); for (MethodNode candidateNode : choicesLeft) { ClassNode declaringClassForDistance = candidateNode.getDeclaringClass(); ClassNode actualReceiverForDistance = receiver != null ? receiver : candidateNode.getDeclaringClass(); @@ -1106,7 +1107,7 @@ public abstract class StaticTypeCheckingSupport { return raw; } - private static Collection removeCovariants(Collection collection) { + private static Collection removeCovariantsAndInterfaceEquivalents(Collection collection) { if (collection.size()<=1) return collection; List toBeRemoved = new LinkedList(); List list = new LinkedList(new HashSet(collection)); @@ -1116,39 +1117,21 @@ public abstract class StaticTypeCheckingSupport { for (int j=i+1;j toBeRemoved, MethodNode one, MethodNode two) { + ClassNode oneDC=one.getDeclaringClass(); + ClassNode twoDC=two.getDeclaringClass(); + if(oneDC.implementsInterface(twoDC)){ + toBeRemoved.add(two); + }else{ + toBeRemoved.add(one); + } + } + + private static boolean areEquivalentInterfaceMethods(MethodNode one, MethodNode two) { + return one.getName().equals(two.getName()) + && one.getDeclaringClass().isInterface() + && two.getDeclaringClass().isInterface() + && ParameterUtils.parametersEqual(one.getParameters(), two.getParameters()); + } + + private static void removeSyntheticMethodIfOne(List toBeRemoved, MethodNode one, MethodNode two) { + if (one.isSynthetic() && !two.isSynthetic()) { + toBeRemoved.add(one); + } else if (two.isSynthetic() && !one.isSynthetic()) { + toBeRemoved.add(two); + } + } + + private static void removeMethodWithSuperReturnType(List toBeRemoved, MethodNode one, MethodNode two) { + ClassNode oneRT = one.getReturnType(); + ClassNode twoRT = two.getReturnType(); + if (oneRT.isDerivedFrom(twoRT) || oneRT.implementsInterface(twoRT)) { + toBeRemoved.add(two); + } else if (twoRT.isDerivedFrom(oneRT) || twoRT.implementsInterface(oneRT)) { + toBeRemoved.add(one); + } + } + + private static boolean areOverloadMethodsInSameClass(MethodNode one, MethodNode two){ + return one.getName().equals(two.getName()) && one.getDeclaringClass()==two.getDeclaringClass(); + } + /** * Given a receiver and a method node, parameterize the method arguments using * available generic type information. @@ -1376,7 +1398,7 @@ public abstract class StaticTypeCheckingSupport { addMethodLevelDeclaredGenerics(candidateMethod,resolvedMethodGenerics); } // so first we remove hidden generics - for (String key: resolvedMethodGenerics.keySet()) classGTs.remove(key); + for (String key: resolvedMethodGenerics.keySet()) classGTs.remove(key); // then we use the remaining information to refine the given generics applyGenericsConnections(classGTs,resolvedMethodGenerics); // and then start our checks with the receiver @@ -1455,7 +1477,7 @@ public abstract class StaticTypeCheckingSupport { return gt; } - private static boolean compatibleConnections(Map connections, Map resolvedMethodGenerics, Set fixedGenericsPlaceHolders) + private static boolean compatibleConnections(Map connections, Map resolvedMethodGenerics, Set fixedGenericsPlaceHolders) { for (Map.Entry entry : connections.entrySet()) { GenericsType resolved = resolvedMethodGenerics.get(entry.getKey()); @@ -1467,7 +1489,7 @@ public abstract class StaticTypeCheckingSupport { if (!compatibleConnection(resolved,connection)) { if ( !(resolved.isPlaceholder() || resolved.isWildcard()) && !fixedGenericsPlaceHolders.contains(entry.getKey()) && - compatibleConnection(connection,resolved)) + compatibleConnection(connection,resolved)) { // we did for example find T=String and now check against // T=Object, which fails the first compatibleConnection check @@ -1485,9 +1507,9 @@ public abstract class StaticTypeCheckingSupport { private static boolean compatibleConnection(GenericsType resolved, GenericsType connection) { GenericsType gt = connection; if (!connection.isWildcard()) gt = buildWildcardType(connection); - if ( resolved.isPlaceholder() && resolved.getUpperBounds()!=null && - resolved.getUpperBounds().length==1 && !resolved.getUpperBounds()[0].isGenericsPlaceHolder() && - resolved.getUpperBounds()[0].getName().equals("java.lang.Object")) + if ( resolved.isPlaceholder() && resolved.getUpperBounds()!=null && + resolved.getUpperBounds().length==1 && !resolved.getUpperBounds()[0].isGenericsPlaceHolder() && + resolved.getUpperBounds()[0].getName().equals("java.lang.Object")) { return true; } @@ -1546,7 +1568,7 @@ public abstract class StaticTypeCheckingSupport { checkForMorePlaceHolders = checkForMorePlaceHolders || !equalIncludingGenerics(value,newValue); continue; } - GenericsType original = entry.getValue(); + GenericsType original = entry.getValue(); if (!original.isWildcard() && !original.isPlaceholder()) { continue; } @@ -1627,10 +1649,10 @@ public abstract class StaticTypeCheckingSupport { /** * use supplied type to make a connection from usage to declaration - * The method operates in two modes. - * * For type !instanceof target a structural compare will be done + * The method operates in two modes. + * * For type !instanceof target a structural compare will be done * (for example Dummy<T> and List<R> to get T=R) - * * If type equals target, a structural match is done as well + * * If type equals target, a structural match is done as well * (for example Colection<U> and Collection<E> to get U=E) * * otherwise we climb the hierarchy to find a case of type equals target * to then execute the structural match, while applying possibly existing @@ -1732,7 +1754,7 @@ public abstract class StaticTypeCheckingSupport { newTarget.setGenericsTypes(newGTs); return GenericsUtils.extractPlaceholders(newTarget); } - + private static GenericsType[] applyGenericsContext( Map spec, GenericsType[] gts ) { @@ -1924,7 +1946,7 @@ public abstract class StaticTypeCheckingSupport { * specifically by the Groovy compiler. */ private static class ObjectArrayStaticTypesHelper { - public static T getAt(T[] arr, int index) { return null;} + public static T getAt(T[] arr, int index) { return null;} public static void putAt(T[] arr, int index, U object) { } } @@ -1999,7 +2021,7 @@ public abstract class StaticTypeCheckingSupport { scanClassesForDGMMethods(methods, staticExtClasses, true); scanClassesForDGMMethods(methods, instanceExtClasses, false); - + return methods; } http://git-wip-us.apache.org/repos/asf/groovy/blob/32d5d1f0/src/test/groovy/bugs/Groovy7922Bug.groovy ---------------------------------------------------------------------- diff --git a/src/test/groovy/bugs/Groovy7922Bug.groovy b/src/test/groovy/bugs/Groovy7922Bug.groovy new file mode 100644 index 0000000..c71d774 --- /dev/null +++ b/src/test/groovy/bugs/Groovy7922Bug.groovy @@ -0,0 +1,42 @@ +package groovy.bugs; + +import gls.CompilableTestSupport; + +public class Groovy7922Bug extends CompilableTestSupport { + + void testMethodSelection() { + def message = shouldNotCompile ''' + import groovy.transform.CompileStatic + + interface FooA {} + interface FooB {} + class FooAB implements FooA, FooB {} + @CompileStatic + class TestGroovy { + static void test() { println new TestGroovy().foo(new FooAB()) } + def foo(FooB x) { 43 } + def foo(FooA x) { 42 } + } + + TestGroovy.test() + '''; + + assert message.contains("ambiguous") + + shouldCompile ''' + import groovy.transform.CompileStatic + + interface FooA {} + interface FooB {} + class FooAB implements FooA, FooB {} + @CompileStatic + class TestGroovy { + static void test() { println new TestGroovy().foo((FooA)null) } + def foo(FooB x) { 43 } + def foo(FooA x) { 42 } + } + + TestGroovy.test() + ''' + } +}