Merge pull request #4933 from oowekyala/issue4902-bad-intersection

[java] Fix problem with type inference with enums sharing a common interface

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/Lub.java

@@ -4,10 +4,11 @@
 
 package net.sourceforge.pmd.lang.java.types;
 
+import static net.sourceforge.pmd.util.CollectionUtil.setOf;
+
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.LinkedHashSet;
@@ -319,34 +320,48 @@ final class Lub {
             return mostSpecific.iterator().next();
         }
 
-        List<JTypeMirror> bounds = new ArrayList<>(mostSpecific);
+        List<JTypeMirror> bounds = new ArrayList<>(mostSpecific.size());
+        bounds.add(null); // first element will be replaced with primary bound
 
         JTypeMirror primaryBound = null;
 
-        for (int i = 0; i < bounds.size(); i++) {
-            JTypeMirror ci = bounds.get(i);
-
+        for (JTypeMirror ci : mostSpecific) {
             if (isExclusiveIntersectionBound(ci)) {
                 // either Ci is an array, or Ci is a class, or Ci is a type var (possibly captured)
                 // Ci is not unresolved
                 if (primaryBound == null) {
                     primaryBound = ci;
-                    // move primary bound first
-                    Collections.swap(bounds, 0, i);
+                } else if (ci.isArray() && primaryBound.isArray()) {
+                    // A[] & B[] = (A & B)[]
+                    // Note that since we're after mostSpecific, we already know
+                    // that A is unrelated to B. Therefore if both B and A are classes,
+                    // then A & B cannot exist and so (A & B)[] similarly does not exist.
+
+                    JTypeMirror componentGlb = glb(ts, setOf(((JArrayType) ci).getComponentType(),
+                                                             ((JArrayType) primaryBound).getComponentType()));
+                    primaryBound = ts.arrayType(componentGlb);
                 } else {
                     throw new IllegalArgumentException(
                         "Bad intersection, unrelated class types " + ci + " and " + primaryBound + " in " + types
                     );
                 }
+            } else {
+                bounds.add(ci);
             }
         }
 
+
         if (primaryBound == null) {
-            if (bounds.size() == 1) {
-                return bounds.get(0);
-            }
             primaryBound = ts.OBJECT;
         }
+        bounds.set(0, primaryBound); // set the primary bound
+        if (primaryBound == ts.OBJECT) {
+            // if primary bound is object, it does not appear in the bounds
+            bounds = bounds.subList(1, bounds.size());
+        }
+        if (bounds.size() == 1) {
+            return bounds.get(0); // not an intersection
+        }
 
         return new JIntersectionType(ts, primaryBound, bounds);
     }

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeOps.java

@@ -580,6 +580,11 @@ public final class TypeOps {
             return this == UNCHECKED_WARNING;
         }
 
+        /** True if this is {@link #SUBTYPING} or {@link #UNCHECKED_NO_WARNING}. */
+        public boolean withoutWarnings() {
+            return this == SUBTYPING || this == UNCHECKED_NO_WARNING;
+        }
+
         // package:
 
 
@@ -1733,8 +1738,15 @@ public final class TypeOps {
         vLoop:
         for (JTypeMirror v : set) {
             for (JTypeMirror w : set) {
-                if (!w.equals(v) && !hasUnresolvedSymbol(w) && isSubtypePure(w, v).bySubtyping()) {
-                    continue vLoop;
+                if (!w.equals(v) && !hasUnresolvedSymbol(w)) {
+                    Convertibility isConvertible = isSubtypePure(w, v);
+                    if (isConvertible.bySubtyping()
+                        // This last case covers unchecked conversion. It is made antisymmetric by the
+                        // test for a symbol. eg |G| <~> G<?> so it would fail.
+                        // However, |G| ~> S if |G| <: |S|, so we should consider |G| more specific than S.
+                        || isConvertible.withoutWarnings() && !Objects.equals(w.getSymbol(), v.getSymbol())) {
+                        continue vLoop;
+                    }
                 }
             }
             result.add(v);

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeSystem.java

@@ -697,12 +697,20 @@ public final class TypeSystem {
      * <li>The intersection has a single component that is a
      * class, array, or type variable. If all components are interfaces,
      * then that component is {@link #OBJECT}.
+     * <li>If several components are arrays, then their components
+     * are intersected: {@code A[] & B[] = (A & B)[]}
      * </ul>
      *
      * <p>If after these transformations, only a single component remains,
      * then that is the returned type. Otherwise a {@link JIntersectionType}
      * is created. Note that the intersection may be unsatisfiable (eg {@code A[] & Runnable}),
-     * but we don't attempt to minimize this to {@link #NULL_TYPE}.
+     * but we don't attempt to minimize this to {@link #NULL_TYPE}. Similarly,
+     * we do not attempt to minimize valid intersections. For instance {@code List<?> & Collection<Number>}
+     * can technically be minimized to {@code List<Number>}, but doing this
+     * requires inference of a fitting parameterization in general, which is
+     * complex, and not necessary in the internal tasks where intersection types are
+     * useful. In fact intersection types are precisely useful because they are
+     * simple to build.
      *
      * <p>See also JLS§4.9 (Intersection types).
      *

pmd-java/src/test/java/net/sourceforge/pmd/lang/java/types/testdata/LubTestData.java

@@ -46,5 +46,19 @@ public class LubTestData {
     public static class GenericSub2<T> extends GenericSuper<T> implements I2<I3>, I4 {
     }
 
+    public interface EnumSuperItf {
+    }
 
+    public enum Enum1 implements EnumSuperItf {
+    }
+
+    public enum Enum2 implements EnumSuperItf {
+    }
+
+    // unrelated
+    public static class C1 {
+    }
+
+    public static class C2 {
+    }
 }

pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/GlbTest.kt

@@ -7,11 +7,13 @@ package net.sourceforge.pmd.lang.java.types
 import io.kotest.assertions.throwables.shouldThrow
 import io.kotest.core.spec.style.FunSpec
 import io.kotest.matchers.collections.shouldContainExactly
+import io.kotest.matchers.collections.shouldContainExactlyInAnyOrder
 import io.kotest.matchers.nulls.shouldBeNull
 import io.kotest.matchers.shouldBe
 import io.kotest.property.checkAll
-import net.sourceforge.pmd.lang.test.ast.shouldBeA
 import net.sourceforge.pmd.lang.java.symbols.internal.asm.createUnresolvedAsmSymbol
+import net.sourceforge.pmd.lang.java.types.testdata.LubTestData
+import net.sourceforge.pmd.lang.test.ast.shouldBeA
 
 /**
  * Tests "the greatest lower bound" (glb).
@@ -71,6 +73,32 @@ class GlbTest : FunSpec({
 
             }
 
+            test("Test GLB of arrays") {
+
+                glb(ts.SERIALIZABLE.toArray(), t_ArrayList.toArray()) shouldBe t_ArrayList.toArray()
+                glb(t_ArrayList.toArray(), ts.SERIALIZABLE.toArray()) shouldBe t_ArrayList.toArray()
+                glb(t_List.toArray(), `t_List{?}`.toArray()) shouldBe `t_List{?}`.toArray()
+
+            }
+
+
+            test("Test GLB of arrays of unrelated type") {
+
+                // C1 & C2 does not exist as they are both unrelated classes
+                shouldThrow<IllegalArgumentException> {
+                    glb(LubTestData.C1::class.decl, LubTestData.C2::class.decl)
+                }
+
+                // C1[] & C2[] = (C1 & C2)[] equally does not exist
+                shouldThrow<IllegalArgumentException> {
+                    glb(LubTestData.C1::class.decl.toArray(), LubTestData.C2::class.decl.toArray())
+                }
+
+                // but C1[] & I1[] = (C1 & I1)[] exists because I1 is an interface.
+                glb(LubTestData.C1::class.decl.toArray(), LubTestData.I1::class.decl.toArray())
+                    .shouldBe((LubTestData.C1::class.decl * LubTestData.I1::class.decl).toArray())
+            }
+
 
             test("Test lub of zero types") {
                 shouldThrow<IllegalArgumentException> {
@@ -89,28 +117,60 @@ class GlbTest : FunSpec({
                 }
             }
 
+            test("test corner case") {
+
+                TypeOps.mostSpecific(
+                    setOf(
+                        t_Collection[`?` extends t_Number],
+                        `t_List{?}`,
+                        t_List
+                    )
+                ) shouldBe setOf(t_Collection[`?` extends t_Number], `t_List{?}`)
+            }
+
             test("Test GLB corner cases") {
 
+                // note: intersections are not minimized, or reduced to the null type if they are unsatisfiable.
+                // note also that in this test we test the components explicitly instead of using the DSL,
+                // because the DSL operator to create intersections actually calls GLB.
+
                 glb(t_Iterable[`?` extends t_Number], t_Iterable[t_String]).shouldBeA<JIntersectionType> {
                     it.components.shouldContainExactly(t_Iterable[`?` extends t_Number], t_Iterable[t_String])
                 }
                 glb(`t_ArrayList{Integer}`, ts.NULL_TYPE) shouldBe ts.NULL_TYPE
                 glb(`t_ArrayList{Integer}`, t_Iterable[`?` extends t_Number], t_Iterable[t_String]).shouldBeA<JIntersectionType> {
-                    it.components.shouldContainExactly(`t_ArrayList{Integer}`, t_Iterable[t_String])
+                    it.components.shouldContainExactlyInAnyOrder(`t_ArrayList{Integer}`, t_Iterable[t_String])
                 }
 
                 glb(`t_List{? extends Number}`, `t_Collection{Integer}`).shouldBeA<JIntersectionType> {
-                    it.components.shouldContainExactly(`t_List{? extends Number}`, `t_Collection{Integer}`)
+                    it.components.shouldContainExactlyInAnyOrder(`t_List{? extends Number}`, `t_Collection{Integer}`)
                 }
 
                 glb(t_List.toArray(), t_Iterable).shouldBeA<JIntersectionType> {
-                    it.components.shouldContainExactly(t_List.toArray(), t_Iterable)
+                    it.components.shouldContainExactlyInAnyOrder(t_List.toArray(), t_Iterable)
                     it.inducedClassType.shouldBeNull()
                 }
                 glb(`t_List{? extends Number}`, `t_Collection{Integer}`, `t_ArrayList{Integer}`) shouldBe `t_ArrayList{Integer}`
                 glb(`t_List{? extends Number}`, `t_List{String}`, `t_Enum{JPrimitiveType}`).shouldBeA<JIntersectionType> {
-                    it.components.shouldContainExactly(`t_Enum{JPrimitiveType}`, `t_List{String}`, `t_List{? extends Number}`)
+                    it.components.shouldContainExactlyInAnyOrder(`t_Enum{JPrimitiveType}`, `t_List{String}`, `t_List{? extends Number}`)
                 }
+
+                glb(
+                    t_Collection[`?` extends t_Number],
+                    `t_List{?}`,
+                    t_List
+                ).shouldBeA<JIntersectionType> {
+                    it.components.shouldContainExactlyInAnyOrder(t_Collection[`?` extends t_Number], `t_List{?}`)
+                }
+
+                glb(t_List, `t_List{?}`) shouldBe `t_List{?}`
+                glb(
+                    t_Collection[`?` extends t_Number],
+                    `t_List{?}`
+                ).shouldBeA<JIntersectionType> {
+                    it.components.shouldContainExactlyInAnyOrder(t_Collection[`?` extends t_Number], `t_List{?}`)
+                }
+
             }
 
             test("Test GLB with unresolved things") {

pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/LubTest.kt

@@ -140,6 +140,22 @@ class LubTest : FunSpec({
                 lub(t_List[Sub1::class.decl], t_List[Sub2::class.decl]) shouldBe result
 
             }
+
+            test("Test lub of related arrays") {
+
+                // the component type
+                lub(
+                    Enum1::class.decl,
+                    Enum2::class.decl,
+                ) shouldBe t_Enum[`?` extends t_Enum[`?`] * EnumSuperItf::class.decl] * EnumSuperItf::class.decl
+
+                // let's try arrays
+                lub(
+                    Enum1::class.decl.toArray(),
+                    Enum2::class.decl.toArray(),
+                ) shouldBe (t_Enum * EnumSuperItf::class.decl).toArray()
+
+            }
         }
     }
 

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

@@ -7,13 +7,13 @@
 package net.sourceforge.pmd.lang.java.types.internal.infer
 
 import io.kotest.matchers.shouldBe
+import net.sourceforge.pmd.lang.java.ast.*
+import net.sourceforge.pmd.lang.java.symbols.JConstructorSymbol
+import net.sourceforge.pmd.lang.java.types.*
 import net.sourceforge.pmd.lang.test.ast.NodeSpec
 import net.sourceforge.pmd.lang.test.ast.shouldBe
 import net.sourceforge.pmd.lang.test.ast.shouldBeA
 import net.sourceforge.pmd.lang.test.ast.shouldMatchN
-import net.sourceforge.pmd.lang.java.ast.*
-import net.sourceforge.pmd.lang.java.symbols.JConstructorSymbol
-import net.sourceforge.pmd.lang.java.types.*
 import java.util.*
 
 /**
@@ -384,4 +384,37 @@ class Foo {
     }
 
 
+    parserTest("#4902 bad intersection") {
+
+        val (acu, spy) = parser.parseWithTypeInferenceSpy("""
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+public class BadIntersection {
+
+  interface Animal { }
+  enum Bird implements Animal { PARROT, CHICKEN }
+  enum Fish implements Animal { GOLDFISH, MACKEREL }
+
+  private static List<Animal> combineAnimals() {
+    return Stream.of(
+            Bird.values(),
+            Fish.values()
+        )
+        .flatMap(Arrays::stream)
+        .collect(Collectors.toList());
+  }
+}
+        """.trimIndent())
+
+        val (_, t_Animal) = acu.declaredTypeSignatures()
+
+        spy.shouldBeOk {
+            acu.firstMethodCall() shouldHaveType java.util.List::class[t_Animal]
+        }
+    }
+
+
 })