Skip to content

Commit

Permalink
stricter analysis: test if removing the cast would change overload
Browse files Browse the repository at this point in the history
resolution.

revert some test changes which actually witness an impact on overloading
  • Loading branch information
stephan-herrmann committed Jun 6, 2024
1 parent 82ec822 commit 7aafa60
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
import org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment;
import org.eclipse.jdt.internal.compiler.lookup.MethodBinding;
import org.eclipse.jdt.internal.compiler.lookup.MethodScope;
import org.eclipse.jdt.internal.compiler.lookup.MethodVerifier;
import org.eclipse.jdt.internal.compiler.lookup.MissingTypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.ParameterizedGenericMethodBinding;
import org.eclipse.jdt.internal.compiler.lookup.ParameterizedMethodBinding;
Expand Down Expand Up @@ -966,17 +967,10 @@ public TypeBinding resolveType(BlockScope scope) {
return null;
}

if (this.receiver instanceof CastExpression) {
// this check was suppressed while resolving receiver, check now based on this.binding.declaringClass
TypeBinding uncastedReceiverType = ((CastExpression)this.receiver).expression.resolvedType;
if (uncastedReceiverType != null
&& uncastedReceiverType.isCompatibleWith(this.binding.declaringClass)
&& !(uncastedReceiverType.isRawType() && this.binding.declaringClass.isParameterizedType()))
{
if (!scope.environment().usesNullTypeAnnotations() || !NullAnnotationMatching.analyse(this.actualReceiverType, uncastedReceiverType, -1).isAnyMismatch()) {
scope.problemReporter().unnecessaryCast((CastExpression) this.receiver);
}
}
if (this.receiver instanceof CastExpression castedRecevier) {
// this check was suppressed while resolving receiver, check now based on the resolved method
if (isUnnecessaryReceiverCast(scope, castedRecevier.expression.resolvedType))
scope.problemReporter().unnecessaryCast(castedRecevier);
}

if (compilerOptions.isAnnotationBasedNullAnalysisEnabled) {
Expand Down Expand Up @@ -1098,6 +1092,26 @@ public TypeBinding resolveType(BlockScope scope) {
: null;
}

protected boolean isUnnecessaryReceiverCast(BlockScope scope, TypeBinding uncastedReceiverType) {
if (uncastedReceiverType == null || !uncastedReceiverType.isCompatibleWith(this.binding.declaringClass)) {
return false;
}
if (uncastedReceiverType.isRawType() && this.binding.declaringClass.isParameterizedType()) {
return false;
}
MethodBinding otherMethod = scope.getMethod(uncastedReceiverType, this.selector, this.argumentTypes, this);
if (!otherMethod.isValidBinding()) {
return false;
}
if (scope.environment().usesNullTypeAnnotations()
&& NullAnnotationMatching.analyse(this.actualReceiverType, uncastedReceiverType, -1).isAnyMismatch()) {
return false;
}
return otherMethod == this.binding
|| MethodVerifier.doesMethodOverride(this.binding, otherMethod, scope.environment())
|| MethodVerifier.doesMethodOverride(otherMethod, this.binding, scope.environment());
}

protected TypeBinding handleNullnessCodePatterns(BlockScope scope, TypeBinding returnType) {
// j.u.s.Stream.filter() may modify nullness of stream elements:
if (this.binding.isWellknownMethod(TypeConstants.JAVA_UTIL_STREAM__STREAM, TypeConstants.FILTER)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2444,16 +2444,12 @@ public void test041() {
"public abstract class X extends J implements I {\n" +
" void bar() {\n" +
" String s = ((I) this).foo(0.0f);\n" +
" s = this.foo(0.0f);\n" + // without cast a different overload is selected, returning String
" }\n" +
"}"
},
"----------\n" +
"1. WARNING in X.java (at line 9)\n" +
" String s = ((I) this).foo(0.0f);\n" +
" ^^^^^^^^^^\n" +
"Unnecessary cast from X to I\n" +
"----------\n" +
"2. ERROR in X.java (at line 9)\n" +
"1. ERROR in X.java (at line 9)\n" +
" String s = ((I) this).foo(0.0f);\n" +
" ^^^^^^^^^^^^^^^^^^^^\n" +
"Type mismatch: cannot convert from Object to String\n" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3777,6 +3777,42 @@ The method bar() is undefined for the type Object
runner.runNegativeTest();
}

public void testGH2470_overload() {
Runner runner = new Runner();
runner.customOptions = getCompilerOptions();
runner.customOptions.put(CompilerOptions.OPTION_ReportUnnecessaryTypeCheck, CompilerOptions.ERROR);
runner.testFiles = new String[] {
"p1/C1.java",
"""
package p1;
import p2.C2;
public class C1 {
void m(String s) {
System.out.print("String.");
}
public void m(CharSequence s) {
System.out.print("CharSequence.");
}
public static void main(String[] args) {
C1 c = new C2();
((C2)c).m("hallo");
c.m("hallo");
}
}
""",
"p2/C2.java",
"""
package p2;
import p1.C1;
public class C2 extends C1 {
}
"""
};
runner.expectedOutputString = "CharSequence.String.";
runner.runConformTest();
}

public static Class testClass() {
return CastTest.class;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22771,7 +22771,7 @@ public void test0706() {
" void foo() {\n" +
" BB bb = new BB();\n" +
" bb.<Object>test();\n" +
" ((AA<CC>) bb).test();\n" +
" ((AA<CC>) bb).test();\n" + // cast relevant for disambiguation
" }\n" +
"}\n" +
"class AA<T> { AA<Object> test() {return null;} }\n" +
Expand All @@ -22784,12 +22784,7 @@ public void test0706() {
" ^^^^\n" +
"The method test() is ambiguous for the type BB\n" +
"----------\n" +
"2. WARNING in X.java (at line 5)\n" +
" ((AA<CC>) bb).test();\n" +
" ^^^^^^^^^^^^^\n" +
"Unnecessary cast from BB to AA<CC>\n" +
"----------\n" +
"3. ERROR in X.java (at line 9)\n" +
"2. ERROR in X.java (at line 9)\n" +
" class BB extends AA<CC> { <U> BB test() {return null;} }\n" +
" ^^^^^^\n" +
"Name clash: The method test() of type BB has the same erasure as test() of type AA<T> but does not override it\n" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3076,7 +3076,7 @@ public void test043c() { // ambiguous message sends because of substitution from
" void test(E<Integer,Integer> e) { e.id(Integer.valueOf(111)); }\n" +
" void test(M<Integer,Integer> m) {\n" +
" m.id(Integer.valueOf(111));\n" +
" ((E<Integer, Integer>) m).id(Integer.valueOf(111));\n" +
" ((E<Integer, Integer>) m).id(Integer.valueOf(111));\n" + // cast needed for disambiguation
" }\n" +
" void test(N<Integer> n) { n.id(Integer.valueOf(111)); }\n" +
"}\n" +
Expand All @@ -3091,11 +3091,6 @@ public void test043c() { // ambiguous message sends because of substitution from
" m.id(Integer.valueOf(111));\n" +
" ^^\n" +
"The method id(Integer) is ambiguous for the type M<Integer,Integer>\n" +
"----------\n" +
"2. WARNING in X.java (at line 5)\n" +
" ((E<Integer, Integer>) m).id(Integer.valueOf(111));\n" +
" ^^^^^^^^^^^^^^^^^^^^^^^^^\n" +
"Unnecessary cast from M<Integer,Integer> to E<Integer,Integer>\n" +
"----------\n"
// reference to id is ambiguous, both method id(A) in C<java.lang.Integer> and method id(B) in M<java.lang.Integer,java.lang.Integer> match
);
Expand Down

0 comments on commit 7aafa60

Please sign in to comment.