From 7aafa60ee417485c3b98bd0caadfe14fa70e5fae Mon Sep 17 00:00:00 2001 From: Stephan Herrmann Date: Thu, 6 Jun 2024 21:53:58 +0200 Subject: [PATCH] stricter analysis: test if removing the cast would change overload resolution. revert some test changes which actually witness an impact on overloading --- .../internal/compiler/ast/MessageSend.java | 36 +++++++++++++------ .../regression/AmbiguousMethodTest.java | 8 ++--- .../tests/compiler/regression/CastTest.java | 36 +++++++++++++++++++ .../compiler/regression/GenericTypeTest.java | 9 ++--- .../compiler/regression/MethodVerifyTest.java | 7 +--- 5 files changed, 66 insertions(+), 30 deletions(-) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MessageSend.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MessageSend.java index de0d20234da..9bc7180ee50 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MessageSend.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MessageSend.java @@ -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; @@ -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) { @@ -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) diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AmbiguousMethodTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AmbiguousMethodTest.java index c649fce42d2..6d926945a20 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AmbiguousMethodTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AmbiguousMethodTest.java @@ -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" + diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CastTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CastTest.java index 3844b650c93..58bae8fc451 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CastTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/CastTest.java @@ -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; } 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 f34fd50a0d5..2b0f686803c 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 @@ -22771,7 +22771,7 @@ public void test0706() { " void foo() {\n" + " BB bb = new BB();\n" + " bb.test();\n" + - " ((AA) bb).test();\n" + + " ((AA) bb).test();\n" + // cast relevant for disambiguation " }\n" + "}\n" + "class AA { AA test() {return null;} }\n" + @@ -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) bb).test();\n" + - " ^^^^^^^^^^^^^\n" + - "Unnecessary cast from BB to AA\n" + - "----------\n" + - "3. ERROR in X.java (at line 9)\n" + + "2. ERROR in X.java (at line 9)\n" + " class BB extends AA { BB test() {return null;} }\n" + " ^^^^^^\n" + "Name clash: The method test() of type BB has the same erasure as test() of type AA but does not override it\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 ddd2e676209..5bfe0dab08d 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 @@ -3076,7 +3076,7 @@ public void test043c() { // ambiguous message sends because of substitution from " void test(E e) { e.id(Integer.valueOf(111)); }\n" + " void test(M m) {\n" + " m.id(Integer.valueOf(111));\n" + - " ((E) m).id(Integer.valueOf(111));\n" + + " ((E) m).id(Integer.valueOf(111));\n" + // cast needed for disambiguation " }\n" + " void test(N n) { n.id(Integer.valueOf(111)); }\n" + "}\n" + @@ -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\n" + - "----------\n" + - "2. WARNING in X.java (at line 5)\n" + - " ((E) m).id(Integer.valueOf(111));\n" + - " ^^^^^^^^^^^^^^^^^^^^^^^^^\n" + - "Unnecessary cast from M to E\n" + "----------\n" // reference to id is ambiguous, both method id(A) in C and method id(B) in M match );