Merge pull request #4560 from oowekyala/issue4557-unnecessary-import-ambiguous

[java] Fix FP of UnnecessaryImport

docs/pages/release_notes.md

@@ -514,6 +514,7 @@ Language specific fixes:
     * [#4487](https://github.com/pmd/pmd/issues/4487): \[java] UnnecessaryConstructor: false-positive with @<!-- -->Inject and @<!-- -->Autowired
     * [#4511](https://github.com/pmd/pmd/issues/4511): \[java] LocalVariableCouldBeFinal shouldn't report unused variables
     * [#4512](https://github.com/pmd/pmd/issues/4512): \[java] MethodArgumentCouldBeFinal shouldn't report unused parameters
+    * [#4557](https://github.com/pmd/pmd/issues/4557): \[java] UnnecessaryImport FP with static imports of overloaded methods
 * java-design
     * [#1014](https://github.com/pmd/pmd/issues/1014): \[java] LawOfDemeter: False positive with lambda expression
     * [#1605](https://github.com/pmd/pmd/issues/1605): \[java] LawOfDemeter: False positive for standard UTF-8 charset name

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/internal/infer/PhaseOverloadSet.java

@@ -167,6 +167,23 @@ final class PhaseOverloadSet extends OverloadSet<MethodCtDecl> {
             JMethodSig sfun = TypeOps.findFunctionalInterfaceMethod(si);
             JMethodSig tfun = TypeOps.findFunctionalInterfaceMethod(ti);
             if (sfun == null || tfun == null) {
+                if (phase.canBox()) {
+                    JTypeMirror stdExprTy = ei.getStandaloneType();
+                    if (stdExprTy != null
+                        // there is a boxing or unboxing conversion happening
+                        && stdExprTy.isPrimitive() != si.isPrimitive()
+                        && stdExprTy.isPrimitive() != ti.isPrimitive()) {
+                        // si or ti is more specific if it only involves
+                        // the boxing/unboxing conversion, without widening
+                        // afterwards.
+                        if (stdExprTy.box().equals(si.box())) {
+                            return true;
+                        } else if (stdExprTy.box().equals(ti.box())) {
+                            return false;
+                        }
+                    }
+                }
+
                 infer.checkConvertibleOrDefer(ctx, si, ti, ei, phase, site);
                 continue;
             }

pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/OverloadResolutionTest.kt

@@ -5,6 +5,7 @@
 
 package net.sourceforge.pmd.lang.java.types.internal.infer
 
+import io.kotest.matchers.should
 import io.kotest.matchers.shouldBe
 import net.sourceforge.pmd.lang.ast.test.shouldBe
 import net.sourceforge.pmd.lang.ast.test.shouldMatchN
@@ -283,7 +284,7 @@ class OverloadResolutionTest : ProcessorTestSpec({
                     void foo(long i) {
                         foo('c');
                     }
-                    
+
                     void foo(String other) {}
                 }
             """)
@@ -307,10 +308,77 @@ class OverloadResolutionTest : ProcessorTestSpec({
         }
     }
 
+    parserTest("#4557 two overloads with boxed types") {
+
+        val acu = parser.parse(
+            """
+            package p;
+
+            import static p.Static.assertThat;
+
+            class Klass {
+                static {
+                    // This is assertThat(Integer)
+                    // Integer is more specific than Long because int -> Integer
+                    // only involves boxing, while int -> Long needs widening and
+                    // then boxing.
+                    assertThat(1);
+                }
+            }
+            class Static {
+
+                public static Object assertThat(Integer actual) {
+                    return null;
+                }
+
+                public static Object assertThat(Long actual) {
+                    return null;
+                }
+
+            }
+            """.trimIndent()
+        )
+
+        val fooM = acu.methodDeclarations().firstOrThrow()
+        val call = acu.firstMethodCall()
+
+        call.overloadSelectionInfo.should {
+            it.isFailed shouldBe false
+            it.methodType.symbol shouldBe fooM.symbol
+        }
+    }
+    parserTest("Two overloads with boxed types, widening required, ambiguous") {
+
+        val (acu, spy) = parser.parseWithTypeInferenceSpy(
+            """
+            class Static {
+                static {
+                    // ambiguous: 1 is int, and neither Double nor Long is more
+                    // specific because they both involve boxing + widening
+                    assertThat(1);
+                }
+
+                public static Object assertThat(Double actual) {
+                    return null;
+                }
+
+                public static Object assertThat(Long actual) {
+                    return null;
+                }
+
+            }
+            """.trimIndent()
+        )
+
+        val call = acu.firstMethodCall()
+        spy.shouldBeAmbiguous(call)
+    }
+
 
     parserTest("Overload selection must identify fallbacks if any") {
 
-        val acu = parser.parse("""
+        val acu = parser.parse(
+            """
 import java.util.Arrays;
 import java.util.stream.Collectors;
 import java.lang.reflect.Type;

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryImport.xml

@@ -1143,4 +1143,29 @@ public class Sample {
             }
             ]]></code>
     </test-code>
+    <test-code>
+        <description>#4557 - UnnecessaryImport FP with static imports of overloaded methods</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+            package p;
+
+            import static p.Static.assertThat;
+
+            class Klass {
+                static {
+                    assertThat(1); // This is assertThat(Integer)
+                }
+            }
+            class Static {
+
+                public static Object assertThat(Integer actual) {
+                    return null;
+                }
+
+                public static Object assertThat(Long actual) {
+                    return null;
+                }
+            }
+            ]]></code>
+    </test-code>
 </test-data>