Bug 388795 - [compiler] detection of name clash depends on order of
super interfaces
diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/GenericTypeTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/GenericTypeTest.java
index 7275093..f1d2111 100644
--- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/GenericTypeTest.java
+++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/GenericTypeTest.java
@@ -7,7 +7,9 @@
*
* Contributors:
* IBM Corporation - initial API and implementation
- * Stephan Herrmann - Contribution for bug 383690 - [compiler] location of error re uninitialized final field should be aligned
+ * Stephan Herrmann - Contributions for
+ * bug 383690 - [compiler] location of error re uninitialized final field should be aligned
+ * bug 388795 - [compiler] detection of name clash depends on order of super interfaces
*******************************************************************************/
package org.eclipse.jdt.core.tests.compiler.regression;
@@ -42809,7 +42811,7 @@
"4. ERROR in X.java (at line 13)\n" +
" public interface CombinedSubInterface extends SubInterface, OtherSubInterface {}\n" +
" ^^^^^^^^^^^^^^^^^^^^\n" +
- "The return types are incompatible for the inherited methods X.SuperInterface.and(X.SuperInterface), X.OtherSubInterface.and(X.SuperInterface), X.SubInterface.and(X.SuperInterface)\n" +
+ "The return types are incompatible for the inherited methods X.OtherSubInterface.and(X.SuperInterface), X.SubInterface.and(X.SuperInterface)\n" +
"----------\n" +
"5. WARNING in X.java (at line 15)\n" +
" public interface OtherSubInterface extends SuperInterface {\n" +
diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/MethodVerifyTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/MethodVerifyTest.java
index 53f8c5b..079991d 100644
--- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/MethodVerifyTest.java
+++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/MethodVerifyTest.java
@@ -7,6 +7,8 @@
*
* Contributors:
* IBM Corporation - initial API and implementation
+ * Stephan Herrmann - Contribution for
+ * bug 388795 - [compiler] detection of name clash depends on order of super interfaces
*******************************************************************************/
package org.eclipse.jdt.core.tests.compiler.regression;
@@ -2491,32 +2493,27 @@
"Name clash: The method foo(A<String>) of type K has the same erasure as foo(A) of type I but does not override it\n" +
"----------\n" :
"----------\n" +
- "1. ERROR in X.java (at line 2)\n" +
- " abstract class Y implements J, I { }\n" +
- " ^\n" +
- "Name clash: The method foo(A<String>) of type J has the same erasure as foo(A) of type I but does not override it\n" +
- "----------\n" +
- "2. WARNING in X.java (at line 4)\n" +
+ "1. WARNING in X.java (at line 4)\n" +
" class YYY implements J, I { public void foo(A a) {} }\n" +
" ^\n" +
"A is a raw type. References to generic type A<T> should be parameterized\n" +
"----------\n" +
- "3. WARNING in X.java (at line 5)\n" +
+ "2. WARNING in X.java (at line 5)\n" +
" class XXX implements I, J { public void foo(A a) {} }\n" +
" ^\n" +
"A is a raw type. References to generic type A<T> should be parameterized\n" +
"----------\n" +
- "4. WARNING in X.java (at line 6)\n" +
+ "3. WARNING in X.java (at line 6)\n" +
" class ZZZ implements K { public void foo(A a) {} }\n" +
" ^\n" +
"A is a raw type. References to generic type A<T> should be parameterized\n" +
"----------\n" +
- "5. WARNING in X.java (at line 7)\n" +
+ "4. WARNING in X.java (at line 7)\n" +
" interface I { void foo(A a); }\n" +
" ^\n" +
"A is a raw type. References to generic type A<T> should be parameterized\n" +
"----------\n" +
- "6. ERROR in X.java (at line 9)\n" +
+ "5. ERROR in X.java (at line 9)\n" +
" interface K extends I { void foo(A<String> a); }\n" +
" ^^^^^^^^^^^^^^^^\n" +
"Name clash: The method foo(A<String>) of type K has the same erasure as foo(A) of type I but does not override it\n" +
@@ -9087,7 +9084,7 @@
"1. ERROR in X.java (at line 1)\n" +
" public abstract class X implements J, K {}\n" +
" ^\n" +
- "The return types are incompatible for the inherited methods I.foo(Number), K.foo(Number), J.foo(Number)\n" +
+ "The return types are incompatible for the inherited methods K.foo(Number), J.foo(Number)\n" +
"----------\n" +
"2. WARNING in X.java (at line 6)\n" +
" XX foo(Number n);\n" +
@@ -13378,6 +13375,11 @@
" Zork z;\n" +
" ^^^^\n" +
"Zork cannot be resolved to a type\n" +
+ "----------\n" +
+ "5. ERROR in X.java (at line 7)\n" +
+ " class ChildX<Z> extends X<Z>{}\n" +
+ " ^^^^^^\n" +
+ "Duplicate methods named forAccountSet with the parameters (List<R>) and (List) are defined by the type X<Z>\n" +
"----------\n":
"----------\n" +
"1. ERROR in X.java (at line 3)\n" +
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java
index fee31e9..ccb933b 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java
@@ -11,6 +11,7 @@
* bug 186342 - [compiler][null] Using annotations for null checking
* bug 365519 - editorial cleanup after bug 186342 and bug 365387
* bug 388281 - [compiler][null] inheritance of null annotations as an option
+ * bug 388795 - [compiler] detection of name clash depends on order of super interfaces
*******************************************************************************/
package org.eclipse.jdt.internal.compiler.lookup;
@@ -33,6 +34,9 @@
super(environment);
}
boolean areMethodsCompatible(MethodBinding one, MethodBinding two) {
+ return areMethodsCompatible(one, two, false);
+}
+boolean areMethodsCompatible(MethodBinding one, MethodBinding two, boolean allowReverse) {
// use the original methods to test compatibility, but do not check visibility, etc
one = one.original();
two = one.findOriginalInheritedMethod(two);
@@ -40,7 +44,7 @@
if (two == null)
return false; // method's declaringClass does not inherit from inheritedMethod's
- return isParameterSubsignature(one, two);
+ return isParameterSubsignature(one, two) || (allowReverse && isParameterSubsignature(two, one));
}
boolean areReturnTypesCompatible(MethodBinding one, MethodBinding two) {
if (one.returnType == two.returnType) return true;
@@ -461,6 +465,12 @@
int inheritedLength = inherited.length;
MethodBinding[] matchingInherited = new MethodBinding[inheritedLength];
MethodBinding[] foundMatch = new MethodBinding[inheritedLength]; // null is no match, otherwise value is matching currentMethod
+
+ // skip tracks inherited methods which can be safely ignored for one of these reasons:
+ // - methods that have matched other inherited methods
+ // either because they match the same currentMethod or match each other
+ // - methods that are overridden by a current method
+ boolean[] skip = new boolean[inheritedLength];
if (current != null) {
for (int i = 0, length1 = current.length; i < length1; i++) {
MethodBinding currentMethod = current[i];
@@ -469,6 +479,8 @@
MethodBinding inheritedMethod = computeSubstituteMethod(inherited[j], currentMethod);
if (inheritedMethod != null) {
if (foundMatch[j] == null && isSubstituteParameterSubsignature(currentMethod, inheritedMethod)) {
+ // already checked compatibility, do visibility etc. also indicate overriding? If so ignore inheritedMethod further downstream
+ skip[j] = couldMethodOverride(currentMethod, inheritedMethod);
matchingInherited[++index] = inheritedMethod;
foundMatch[j] = currentMethod;
} else {
@@ -492,9 +504,6 @@
}
}
- // skip tracks which inherited methods have matched other inherited methods
- // either because they match the same currentMethod or match each other
- boolean[] skip = new boolean[inheritedLength];
for (int i = 0; i < inheritedLength; i++) {
MethodBinding matchMethod = foundMatch[i];
if (matchMethod == null && current != null && this.type.isPublic()) { // current == null case handled already.
@@ -522,24 +531,28 @@
// This elimination used to happen rather eagerly in computeInheritedMethods step
// itself earlier. (https://bugs.eclipse.org/bugs/show_bug.cgi?id=302358)
if (inheritedMethod.declaringClass != otherInheritedMethod.declaringClass) {
- if (otherInheritedMethod.declaringClass.isInterface()) {
+ if (otherInheritedMethod.declaringClass.isInterface() && !inheritedMethod.declaringClass.isInterface()) {
if (isInterfaceMethodImplemented(otherInheritedMethod, inheritedMethod, otherInheritedMethod.declaringClass)) {
skip[j] = true;
continue;
}
- } else if (areMethodsCompatible(inheritedMethod, otherInheritedMethod)) {
+ } else if (areMethodsCompatible(inheritedMethod, otherInheritedMethod, true)) {
skip[j] = true;
continue;
}
}
otherInheritedMethod = computeSubstituteMethod(otherInheritedMethod, inheritedMethod);
if (otherInheritedMethod != null) {
- if (isSubstituteParameterSubsignature(inheritedMethod, otherInheritedMethod)) {
- if (index == -1)
- matchingInherited[++index] = inheritedMethod;
- if (foundMatch[j] == null)
- matchingInherited[++index] = otherInheritedMethod;
- skip[j] = true;
+ if (((!inheritedMethod.isAbstract() || otherInheritedMethod.isAbstract()) // if (abstract(inherited) => abstract(other)) check if inherited overrides other
+ && isSubstituteParameterSubsignature(inheritedMethod, otherInheritedMethod))
+ || ((!otherInheritedMethod.isAbstract() || inheritedMethod.isAbstract()) // if (abstract(other) => abstract(inherited)) check if other overrides inherited
+ && isSubstituteParameterSubsignature(otherInheritedMethod, inheritedMethod)))
+ {
+ if (index == -1)
+ matchingInherited[++index] = inheritedMethod;
+ if (foundMatch[j] == null)
+ matchingInherited[++index] = otherInheritedMethod;
+ skip[j] = true;
} else if (matchMethod == null && foundMatch[j] == null) {
checkInheritedMethods(inheritedMethod, otherInheritedMethod);
}
@@ -659,7 +672,6 @@
ReferenceBinding[] interfaces = typeVariable.superInterfaces;
if (interfaceLength != interfaces.length)
return inheritedMethod; // not a match
- // TODO (kent) another place where we expect the superinterfaces to be in the exact same order
next : for (int j = 0; j < interfaceLength; j++) {
TypeBinding superType = Scope.substitute(substitute, inheritedTypeVariable.superInterfaces[j]);
for (int k = 0; k < interfaceLength; k++)
