Attachment 125679 Details for Bug 264881 – Proposed patch and testcase

[patch] Proposed patch and testcase

patch.txt (text/plain), 10.67 KB, created by Kent Johnson

on 2009-02-13 15:03:09 EST

(hide)

Description:

Filename:

MIME Type:

Creator: Kent Johnson

Created: 2009-02-13 15:03:09 EST

Size: 10.67 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core
>Index: compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java,v
>retrieving revision 1.96
>diff -u -r1.96 MethodVerifier15.java
>--- compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java	23 Jan 2009 19:50:08 -0000	1.96
>+++ compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java	13 Feb 2009 19:52:54 -0000
>@@ -124,18 +124,10 @@
> 
> 		// so the parameters are equal and the return type is compatible b/w the currentMethod & the substituted inheritedMethod
> 		MethodBinding originalInherited = abstractMethod.original();
>-		if (originalInherited.returnType != concreteMethod.returnType) {
>-			if (abstractMethod.returnType.leafComponentType().isParameterizedTypeWithActualArguments()) {
>-				if (concreteMethod.returnType.leafComponentType().isRawType())
>-					problemReporter().unsafeReturnTypeOverride(concreteMethod, originalInherited, this.type);
>-			} else if (abstractMethod.hasSubstitutedReturnType() && originalInherited.returnType.leafComponentType().isTypeVariable()) {
>-				if (((TypeVariableBinding) originalInherited.returnType.leafComponentType()).declaringElement == originalInherited) { // see 81618 - type variable from inherited method
>-					TypeBinding currentReturnType = concreteMethod.returnType.leafComponentType();
>-					if (!currentReturnType.isTypeVariable() || ((TypeVariableBinding) currentReturnType).declaringElement != concreteMethod)
>-						problemReporter().unsafeReturnTypeOverride(concreteMethod, originalInherited, this.type);
>-				}
>-			}
>-		}
>+		if (originalInherited.returnType != concreteMethod.returnType)
>+			if (!isAcceptableReturnTypeOverride(concreteMethod, abstractMethod))
>+				problemReporter().unsafeReturnTypeOverride(concreteMethod, originalInherited, this.type);
>+
> 		// check whether bridge method is already defined above for interface methods
> 		if (originalInherited.declaringClass.isInterface()) {
> 			if ((concreteMethod.declaringClass == this.type.superclass && this.type.superclass.isParameterizedType())
>@@ -150,20 +142,9 @@
> 
> 	// so the parameters are equal and the return type is compatible b/w the currentMethod & the substituted inheritedMethod
> 	MethodBinding originalInherited = inheritedMethod.original();
>-	if (originalInherited.returnType != currentMethod.returnType) {
>-//		if (currentMethod.returnType.needsUncheckedConversion(inheritedMethod.returnType)) {
>-//			problemReporter(currentMethod).unsafeReturnTypeOverride(currentMethod, originalInherited, this.type);
>-		if (inheritedMethod.returnType.leafComponentType().isParameterizedTypeWithActualArguments() 
>-				&& currentMethod.returnType.leafComponentType().isRawType()) {
>+	if (originalInherited.returnType != currentMethod.returnType)
>+		if (!isAcceptableReturnTypeOverride(currentMethod, inheritedMethod))
> 			problemReporter(currentMethod).unsafeReturnTypeOverride(currentMethod, originalInherited, this.type);
>-		} else if (inheritedMethod.hasSubstitutedReturnType() && originalInherited.returnType.leafComponentType().isTypeVariable()) {
>-			if (((TypeVariableBinding) originalInherited.returnType.leafComponentType()).declaringElement == originalInherited) { // see 81618 - type variable from inherited method
>-				TypeBinding currentReturnType = currentMethod.returnType.leafComponentType();
>-				if (!currentReturnType.isTypeVariable() || ((TypeVariableBinding) currentReturnType).declaringElement != currentMethod)
>-					problemReporter(currentMethod).unsafeReturnTypeOverride(currentMethod, originalInherited, this.type);
>-			}
>-		}
>-	}
> 
> 	if (this.type.addSyntheticBridgeMethod(originalInherited, currentMethod.original()) != null) {
> 		for (int i = 0, l = allInheritedMethods == null ? 0 : allInheritedMethods.length; i < l; i++) {
>@@ -586,21 +567,6 @@
> public boolean doesMethodOverride(MethodBinding method, MethodBinding inheritedMethod) {
> 	return couldMethodOverride(method, inheritedMethod) && areMethodsCompatible(method, inheritedMethod);
> }
>-boolean hasGenericParameter(MethodBinding method) {
>-	if (method.genericSignature() == null) return false;
>-
>-	// may be only the return type that is generic, need to check parameters
>-	TypeBinding[] params = method.parameters;
>-	for (int i = 0, l = params.length; i < l; i++) {
>-		TypeBinding param = params[i].leafComponentType();
>-		if (param instanceof ReferenceBinding) {
>-			int modifiers = ((ReferenceBinding) param).modifiers;
>-			if ((modifiers & ExtraCompilerModifiers.AccGenericSignature) != 0)
>-				return true;
>-		}
>-	}
>-	return false;
>-}
> boolean doTypeVariablesClash(MethodBinding one, MethodBinding substituteTwo) {
> 	// one has type variables and substituteTwo did not pass bounds check in computeSubstituteMethod()
> 	return one.typeVariables != Binding.NO_TYPE_VARIABLES && !(substituteTwo instanceof ParameterizedGenericMethodBinding);
>@@ -674,6 +640,43 @@
> 	}
> 	return copy;
> }
>+boolean hasGenericParameter(MethodBinding method) {
>+	if (method.genericSignature() == null) return false;
>+
>+	// may be only the return type that is generic, need to check parameters
>+	TypeBinding[] params = method.parameters;
>+	for (int i = 0, l = params.length; i < l; i++) {
>+		TypeBinding param = params[i].leafComponentType();
>+		if (param instanceof ReferenceBinding) {
>+			int modifiers = ((ReferenceBinding) param).modifiers;
>+			if ((modifiers & ExtraCompilerModifiers.AccGenericSignature) != 0)
>+				return true;
>+		}
>+	}
>+	return false;
>+}
>+boolean isAcceptableReturnTypeOverride(MethodBinding currentMethod, MethodBinding inheritedMethod) {
>+	// called when currentMethod's return type is compatible with inheritedMethod's return type
>+
>+	if (inheritedMethod.declaringClass.isRawType())
>+		return true; // since the inheritedMethod comes from a raw type, the return type is always acceptable
>+
>+	TypeBinding currentReturnType = currentMethod.returnType.leafComponentType();
>+	switch (currentReturnType.kind()) {
>+	 	case Binding.RAW_TYPE :
>+			return !inheritedMethod.returnType.leafComponentType().isParameterizedTypeWithActualArguments();
>+	 	case Binding.TYPE_PARAMETER :
>+	 		if (currentReturnType == inheritedMethod.returnType.leafComponentType())
>+	 			return true;
>+	 		//$FALL-THROUGH$
>+		default :
>+			MethodBinding originalInherited = inheritedMethod.original();
>+			TypeBinding originalInheritedReturnType = originalInherited.returnType.leafComponentType();
>+			if (originalInheritedReturnType.isTypeVariable() && ((TypeVariableBinding) originalInheritedReturnType).declaringElement == originalInherited)
>+				return false;
>+			return true;
>+	}
>+}
> // caveat: returns false if a method is implemented that needs a bridge method
> boolean isInterfaceMethodImplemented(MethodBinding inheritedMethod, MethodBinding existingMethod, ReferenceBinding superType) {
> 	if (inheritedMethod.original() != inheritedMethod && existingMethod.declaringClass.isInterface())
>@@ -731,6 +734,8 @@
> 	return method.typeVariables == Binding.NO_TYPE_VARIABLES;
> }
> boolean isUnsafeReturnTypeOverride(MethodBinding currentMethod, MethodBinding inheritedMethod) {
>+	// called when currentMethod's return type is NOT compatible with inheritedMethod's return type
>+
> 	// JLS 3 §8.4.5: more are accepted, with an unchecked conversion
> 	if (currentMethod.returnType == inheritedMethod.returnType.erasure()) {
> 		TypeBinding[] currentParams = currentMethod.parameters;
>#P org.eclipse.jdt.core.tests.compiler
>Index: src/org/eclipse/jdt/core/tests/compiler/regression/MethodVerifyTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/MethodVerifyTest.java,v
>retrieving revision 1.174
>diff -u -r1.174 MethodVerifyTest.java
>--- src/org/eclipse/jdt/core/tests/compiler/regression/MethodVerifyTest.java	23 Jan 2009 19:50:07 -0000	1.174
>+++ src/org/eclipse/jdt/core/tests/compiler/regression/MethodVerifyTest.java	13 Feb 2009 19:52:56 -0000
>@@ -9383,4 +9383,64 @@
> 		"----------\n"
> 	);
> }
>+//https://bugs.eclipse.org/bugs/show_bug.cgi?id=264881
>+public void test184() {
>+	this.runNegativeTest(
>+		new String[] {
>+			"X.java",
>+			"class A {\n" +
>+			"	<T extends Number> U a() { return null; }\n" +
>+			"	<T> T x() { return null; }\n" +
>+			"	<T extends Number> T y() { return null; }\n" +
>+			"	<T extends Integer> T z() { return null; }\n" +
>+			"}\n" +
>+			"class B extends A<Double> {\n" +
>+			"	@Override Double a() { return 1.0; }\n" +
>+			"	@Override Integer x() { return 1; }\n" +
>+			"	@Override Integer y() { return 1; }\n" +
>+			"	@Override Integer z() { return 1; }\n" +
>+			"}\n" +
>+			"class C extends A {\n" +
>+			"	@Override Double a() { return 1.0; }\n" +
>+			"	@Override Integer x() { return 1; }\n" +
>+			"	@Override Integer y() { return 1; }\n" +
>+			"	@Override Integer z() { return 1; }\n" +
>+			"}\n" +
>+			"class M {\n" +
>+			"	<T extends M> Object m(Class<T> c) { return null; }\n" +
>+			"	<T extends M> Object n(Class<T> c) { return null; }\n" +
>+			"}\n" +
>+			"class N<V> extends M {\n" +
>+			"	@Override <T extends M> T m(Class<T> c) { return null; }\n" +
>+			"	@Override <T extends M> V n(Class<T> c) { return null; }\n" +
>+			"}"
>+		},
>+		"----------\n" + 
>+		"1. WARNING in X.java (at line 5)\n" + 
>+		"	<T extends Integer> T z() { return null; }\n" + 
>+		"	 ^^^^^^^\n" + 
>+		"The type parameter T should not be bounded by the final type Integer. Final types cannot be further extended\n" + 
>+		"----------\n" + 
>+		"2. WARNING in X.java (at line 9)\n" + 
>+		"	@Override Integer x() { return 1; }\n" + 
>+		"	 ^^^^^^^\n" + 
>+		"Type safety: The return type Integer for x() from the type B needs unchecked conversion to conform to T from the type A\n" + 
>+		"----------\n" + 
>+		"3. WARNING in X.java (at line 10)\n" + 
>+		"	@Override Integer y() { return 1; }\n" + 
>+		"	 ^^^^^^^\n" + 
>+		"Type safety: The return type Integer for y() from the type B needs unchecked conversion to conform to T from the type A\n" + 
>+		"----------\n" + 
>+		"4. WARNING in X.java (at line 11)\n" + 
>+		"	@Override Integer z() { return 1; }\n" + 
>+		"	 ^^^^^^^\n" + 
>+		"Type safety: The return type Integer for z() from the type B needs unchecked conversion to conform to T from the type A\n" + 
>+		"----------\n" + 
>+		"5. WARNING in X.java (at line 13)\n" + 
>+		"	class C extends A {\n" + 
>+		"	 ^\n" + 
>+		"A is a raw type. References to generic type A should be parameterized\n" + 
>+		"----------\n"
>+	);
>+}
> }

Actions: View | Diff

Attachments on bug 264881: 125679 | 125685