Skip to content

Commit

Permalink
GROOVY-6882
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Nov 21, 2020
1 parent 542e5f1 commit e37db10
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 4 deletions.
2 changes: 1 addition & 1 deletion ...re.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/StaticCompilationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public void testCompileStatic1() {
"1. ERROR in Main.groovy (at line 5)\n" +
"\tls.add('abc')\n" +
"\t^^^^^^^^^^^^^\n" +
"Groovy:[Static type checking] - Cannot call java.util.ArrayList <Integer>#add(java.lang.Integer) with arguments [java.lang.String] \n" +
"Groovy:[Static type checking] - Cannot find matching method java.util.ArrayList#add(java.lang.String). Please check if the declared type is correct and if the method exists.\n" +
"----------\n");
}

Expand Down
36 changes: 35 additions & 1 deletion ...ovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/TypeCheckedTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public void testTypeChecked2() {
"1. ERROR in Foo.groovy (at line 6)\n" +
"\tls.add(\'abc\')\n" +
"\t^^^^^^^^^^^^^\n" +
"Groovy:[Static type checking] - Cannot call java.util.ArrayList <Integer>#add(java.lang.Integer) with arguments [java.lang.String] \n" +
"Groovy:[Static type checking] - Cannot find matching method java.util.ArrayList#add(java.lang.String). Please check if the declared type is correct and if the method exists.\n" +
"----------\n");
}

Expand Down Expand Up @@ -214,6 +214,40 @@ public void testTypeChecked9() {
runNegativeTest(sources, "");
}

@Test
public void testTypeChecked6882() {
//@formatter:off
String[] sources = {
"Main.groovy",
"class B {\n" +
" void m() {\n" +
" print 'B'\n" +
" }\n" +
"}\n" +
"@groovy.transform.TypeChecked\n" +
"class C extends B {\n" +
" @Override\n" +
" void m() {\n" +
" print 'C'\n" +
" }\n" +
" void test() {\n" +
" def x = new Runnable() {\n" +
" @Override\n" +
" void run() {\n" +
" m()\n" + // Reference to method is ambiguous. Cannot choose between [void C#m(), void B#m()]
" }\n" +
" }\n" +
" x.run()\n" +
" m()\n" +
" }\n" +
"}\n" +
"new C().test()\n",
};
//@formatter:on

runConformTest(sources, "CC");
}

@Test
public void testTypeChecked7333() {
//@formatter:off
Expand Down
26 changes: 25 additions & 1 deletion ...rg.codehaus.groovy25/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1103,7 +1103,7 @@ Person foo(B i){...}
Person p = foo(b)
*/

Map<GenericsType, GenericsType> declaringAndActualGenericsTypeMap = GenericsUtils.makeDeclaringAndActualGenericsTypeMap(declaringClassForDistance, actualReceiverForDistance);
Map<GenericsType, GenericsType> declaringAndActualGenericsTypeMap = GenericsUtils.makeDeclaringAndActualGenericsTypeMapOfExactType(declaringClassForDistance, actualReceiverForDistance);
Parameter[] params = makeRawTypes(safeNode.getParameters(), declaringAndActualGenericsTypeMap);
int dist = measureParametersAndArgumentsDistance(params, safeArgs);
if (dist >= 0) {
Expand Down Expand Up @@ -1238,7 +1238,12 @@ private static Collection<MethodNode> removeCovariantsAndInterfaceEquivalents(Co
MethodNode two = list.get(j);
if (toBeRemoved.contains(two)) continue;
if (one.getParameters().length == two.getParameters().length) {
/* GRECLIPSE edit -- GROOVY-6882, GROOVY-6970
if (areOverloadMethodsInSameClass(one, two)) {
*/
ClassNode oneDC = one.getDeclaringClass(), twoDC = two.getDeclaringClass();
if (oneDC == twoDC) {
// GRECLIPSE end
if (ParameterUtils.parametersEqual(one.getParameters(), two.getParameters())) {
removeMethodWithSuperReturnType(toBeRemoved, one, two);
} else {
Expand All @@ -1247,10 +1252,25 @@ private static Collection<MethodNode> removeCovariantsAndInterfaceEquivalents(Co
// in that case, Java marks the Object version as synthetic
removeSyntheticMethodIfOne(toBeRemoved, one, two);
}
/* GRECLIPSE edit -- GROOVY-6882, GROOVY-6970
} else if (areEquivalentInterfaceMethods(one, two)) {
// GROOVY-6970 choose between equivalent interface methods
removeMethodInSuperInterface(toBeRemoved, one, two);
}
*/
} else if (!oneDC.equals(twoDC)) {
if (ParameterUtils.parametersEqual(one.getParameters(), two.getParameters())) {
// GROOVY-6882, GROOVY-6970: drop overridden or interface equivalent method
if (twoDC.isInterface() ? oneDC.implementsInterface(twoDC)
: oneDC.isDerivedFrom(twoDC)) {
toBeRemoved.add(two);
} else if (oneDC.isInterface() ? twoDC.isInterface()
: twoDC.isDerivedFrom(oneDC)) {
toBeRemoved.add(one);
}
}
}
// GRECLIPSE end
}
}
}
Expand All @@ -1260,6 +1280,7 @@ private static Collection<MethodNode> removeCovariantsAndInterfaceEquivalents(Co
return result;
}

/* GRECLIPSE edit
private static void removeMethodInSuperInterface(List<MethodNode> toBeRemoved, MethodNode one, MethodNode two) {
ClassNode oneDC = one.getDeclaringClass();
ClassNode twoDC = two.getDeclaringClass();
Expand All @@ -1276,6 +1297,7 @@ private static boolean areEquivalentInterfaceMethods(MethodNode one, MethodNode
&& two.getDeclaringClass().isInterface()
&& ParameterUtils.parametersEqual(one.getParameters(), two.getParameters());
}
*/

private static void removeSyntheticMethodIfOne(List<MethodNode> toBeRemoved, MethodNode one, MethodNode two) {
if (one.isSynthetic() && !two.isSynthetic()) {
Expand All @@ -1302,9 +1324,11 @@ private static boolean isCovariant(ClassNode left, ClassNode right) {
return left.isDerivedFrom(right) || left.implementsInterface(right);
}

/* GRECLIPSE edit
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
Expand Down
26 changes: 25 additions & 1 deletion ...rg.codehaus.groovy30/src/org/codehaus/groovy/transform/stc/StaticTypeCheckingSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1039,7 +1039,7 @@ Person foo(B i){...}
Person p = foo(b)
*/

Map<GenericsType, GenericsType> declaringAndActualGenericsTypeMap = GenericsUtils.makeDeclaringAndActualGenericsTypeMap(declaringClassForDistance, actualReceiverForDistance);
Map<GenericsType, GenericsType> declaringAndActualGenericsTypeMap = GenericsUtils.makeDeclaringAndActualGenericsTypeMapOfExactType(declaringClassForDistance, actualReceiverForDistance);
Parameter[] params = makeRawTypes(safeNode.getParameters(), declaringAndActualGenericsTypeMap);
int dist = measureParametersAndArgumentsDistance(params, safeArgs);
if (dist >= 0) {
Expand Down Expand Up @@ -1162,7 +1162,12 @@ private static Collection<MethodNode> removeCovariantsAndInterfaceEquivalents(fi
MethodNode two = list.get(j);
if (toBeRemoved.contains(two)) continue;
if (one.getParameters().length == two.getParameters().length) {
/* GRECLIPSE edit -- GROOVY-6882, GROOVY-6970
if (areOverloadMethodsInSameClass(one, two)) {
*/
ClassNode oneDC = one.getDeclaringClass(), twoDC = two.getDeclaringClass();
if (oneDC == twoDC) {
// GRECLIPSE end
if (ParameterUtils.parametersEqual(one.getParameters(), two.getParameters())) {
removeMethodWithSuperReturnType(toBeRemoved, one, two);
} else {
Expand All @@ -1171,10 +1176,25 @@ private static Collection<MethodNode> removeCovariantsAndInterfaceEquivalents(fi
// in that case, Java marks the Object version as synthetic
removeSyntheticMethodIfOne(toBeRemoved, one, two);
}
/* GRECLIPSE edit -- GROOVY-6882, GROOVY-6970
} else if (areEquivalentInterfaceMethods(one, two)) {
// GROOVY-6970: choose between equivalent interface methods
removeMethodInSuperInterface(toBeRemoved, one, two);
}
*/
} else if (!oneDC.equals(twoDC)) {
if (ParameterUtils.parametersEqual(one.getParameters(), two.getParameters())) {
// GROOVY-6882, GROOVY-6970: drop overridden or interface equivalent method
if (twoDC.isInterface() ? oneDC.implementsInterface(twoDC)
: oneDC.isDerivedFrom(twoDC)) {
toBeRemoved.add(two);
} else if (oneDC.isInterface() ? twoDC.isInterface()
: twoDC.isDerivedFrom(oneDC)) {
toBeRemoved.add(one);
}
}
}
// GRECLIPSE end
}
}
}
Expand All @@ -1184,6 +1204,7 @@ private static Collection<MethodNode> removeCovariantsAndInterfaceEquivalents(fi
return result;
}

/* GRECLIPSE edit
private static void removeMethodInSuperInterface(final List<MethodNode> toBeRemoved, final MethodNode one, final MethodNode two) {
ClassNode oneDC = one.getDeclaringClass();
ClassNode twoDC = two.getDeclaringClass();
Expand All @@ -1200,6 +1221,7 @@ private static boolean areEquivalentInterfaceMethods(final MethodNode one, final
&& two.getDeclaringClass().isInterface()
&& ParameterUtils.parametersEqual(one.getParameters(), two.getParameters()));
}
*/

private static void removeSyntheticMethodIfOne(final List<MethodNode> toBeRemoved, final MethodNode one, final MethodNode two) {
if (one.isSynthetic() && !two.isSynthetic()) {
Expand All @@ -1226,9 +1248,11 @@ private static boolean isCovariant(final ClassNode left, final ClassNode right)
return (left.isDerivedFrom(right) || left.implementsInterface(right));
}

/* GRECLIPSE edit
private static boolean areOverloadMethodsInSameClass(final MethodNode one, final MethodNode two) {
return (one.getName().equals(two.getName()) && one.getDeclaringClass() == two.getDeclaringClass());
}
*/

/**
* Given a receiver and a method node, parameterize the method arguments using
Expand Down

0 comments on commit e37db10

Please sign in to comment.