Use Java 14+ pattern matching in instanceof wherever applicable + m…

…ove `com.virtuslab.qual.{gitmachete => subtyping.gitmachete}`
VirtusLab · Mar 30, 2023 · e40d1da · e40d1da
1 parent 80b19a5
commit e40d1da
Show file tree

Hide file tree

Showing 14 changed files with 65 additions and 23 deletions.
diff --git a/qual/src/main/java/com/virtuslab/qual/internal/package-info.java b/qual/src/main/java/com/virtuslab/qual/internal/package-info.java
diff --git a/qual/src/main/java/com/virtuslab/qual/subtyping/gitmachete/backend/api/ConfirmedLocal.java b/qual/src/main/java/com/virtuslab/qual/subtyping/gitmachete/backend/api/ConfirmedLocal.java
@@ -7,7 +7,7 @@
 
 import org.checkerframework.framework.qual.SubtypeOf;
 
-import com.virtuslab.qual.internal.SubtypingTop;
+import com.virtuslab.qual.subtyping.internal.SubtypingTop;
 
 /**
  * Used to annotate a type of a {@code com.virtuslab.gitmachete.backend.api.IBranchReference} object

diff --git a/qual/src/main/java/com/virtuslab/qual/subtyping/gitmachete/backend/api/ConfirmedNonRoot.java b/qual/src/main/java/com/virtuslab/qual/subtyping/gitmachete/backend/api/ConfirmedNonRoot.java
@@ -7,7 +7,7 @@
 
 import org.checkerframework.framework.qual.SubtypeOf;
 
-import com.virtuslab.qual.internal.SubtypingTop;
+import com.virtuslab.qual.subtyping.internal.SubtypingTop;
 
 /**
  * Used to annotate a type of a {@code com.virtuslab.gitmachete.backend.api.IManagedBranchSnapshot} object

diff --git a/qual/src/main/java/com/virtuslab/qual/subtyping/gitmachete/backend/api/ConfirmedRemote.java b/qual/src/main/java/com/virtuslab/qual/subtyping/gitmachete/backend/api/ConfirmedRemote.java
@@ -7,7 +7,7 @@
 
 import org.checkerframework.framework.qual.SubtypeOf;
 
-import com.virtuslab.qual.internal.SubtypingTop;
+import com.virtuslab.qual.subtyping.internal.SubtypingTop;
 
 /**
  * Used to annotate a type of a {@code com.virtuslab.gitmachete.backend.api.IBranchReference} object

diff --git a/qual/src/main/java/com/virtuslab/qual/subtyping/gitmachete/backend/api/ConfirmedRoot.java b/qual/src/main/java/com/virtuslab/qual/subtyping/gitmachete/backend/api/ConfirmedRoot.java
@@ -7,7 +7,7 @@
 
 import org.checkerframework.framework.qual.SubtypeOf;
 
-import com.virtuslab.qual.internal.SubtypingTop;
+import com.virtuslab.qual.subtyping.internal.SubtypingTop;
 
 /**
  * Used to annotate a type of a {@code com.virtuslab.gitmachete.backend.api.IManagedBranchSnapshot} object

diff --git a/...m/virtuslab/qual/subtyping/gitmachete/frontend/graph/api/elements/ConfirmedGraphEdge.java b/...m/virtuslab/qual/subtyping/gitmachete/frontend/graph/api/elements/ConfirmedGraphEdge.java
@@ -7,7 +7,7 @@
 
 import org.checkerframework.framework.qual.SubtypeOf;
 
-import com.virtuslab.qual.internal.SubtypingTop;
+import com.virtuslab.qual.subtyping.internal.SubtypingTop;
 
 /**
  * Used to annotate a type of a {@code com.virtuslab.gitmachete.frontend.graph.api.elements.IGraphElement} object

diff --git a/...m/virtuslab/qual/subtyping/gitmachete/frontend/graph/api/elements/ConfirmedGraphNode.java b/...m/virtuslab/qual/subtyping/gitmachete/frontend/graph/api/elements/ConfirmedGraphNode.java
@@ -7,7 +7,7 @@
 
 import org.checkerframework.framework.qual.SubtypeOf;
 
-import com.virtuslab.qual.internal.SubtypingTop;
+import com.virtuslab.qual.subtyping.internal.SubtypingTop;
 
 /**
  * Used to annotate a type of a {@code com.virtuslab.gitmachete.frontend.graph.api.elements.IGraphElement} object

diff --git a/...com/virtuslab/qual/subtyping/gitmachete/frontend/graph/api/items/ConfirmedBranchItem.java b/...com/virtuslab/qual/subtyping/gitmachete/frontend/graph/api/items/ConfirmedBranchItem.java
@@ -7,7 +7,7 @@
 
 import org.checkerframework.framework.qual.SubtypeOf;
 
-import com.virtuslab.qual.internal.SubtypingTop;
+import com.virtuslab.qual.subtyping.internal.SubtypingTop;
 
 /**
  * Used to annotate a type of a {@code com.virtuslab.gitmachete.frontend.graph.api.items.IBranchItem} object

diff --git a/...com/virtuslab/qual/subtyping/gitmachete/frontend/graph/api/items/ConfirmedCommitItem.java b/...com/virtuslab/qual/subtyping/gitmachete/frontend/graph/api/items/ConfirmedCommitItem.java
@@ -7,7 +7,7 @@
 
 import org.checkerframework.framework.qual.SubtypeOf;
 
-import com.virtuslab.qual.internal.SubtypingTop;
+import com.virtuslab.qual.subtyping.internal.SubtypingTop;
 
 /**
  * Used to annotate a type of a {@code com.virtuslab.gitmachete.frontend.graph.api.items.IBranchItem} object

diff --git a/...tuslab/qual/internal/SubtypingBottom.java → ...l/subtyping/internal/SubtypingBottom.java b/...tuslab/qual/internal/SubtypingBottom.java → ...l/subtyping/internal/SubtypingBottom.java
@@ -1,4 +1,4 @@
-package com.virtuslab.qual.internal;
+package com.virtuslab.qual.subtyping.internal;
 
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
@@ -21,7 +21,7 @@
 /**
  * There needs to be single subtyping hierarchy with single bottom and top annotation.
  * We could theoretically create a separate hierarchy with a dedicated top and bottom type
- * for each pair of annotations from {@link com.virtuslab.qual.gitmachete}.* packages,
+ * for each pair of annotations from {@link com.virtuslab.qual.subtyping}.* packages,
  * but then <a href="https://checkerframework.org/manual/#subtyping-checker">Subtyping Checker</a>
  * would raise an error about multiple top/bottom types.
  */

diff --git a/...virtuslab/qual/internal/SubtypingTop.java → ...qual/subtyping/internal/SubtypingTop.java b/...virtuslab/qual/internal/SubtypingTop.java → ...qual/subtyping/internal/SubtypingTop.java
@@ -1,4 +1,4 @@
-package com.virtuslab.qual.internal;
+package com.virtuslab.qual.subtyping.internal;
 
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
@@ -11,7 +11,7 @@
 /**
  * There needs to be single subtyping hierarchy with single bottom and top annotation.
  * We could theoretically create a separate hierarchy with a dedicated top and bottom type
- * for each pair of annotations from {@link com.virtuslab.qual.gitmachete}.* packages,
+ * for each pair of annotations from {@link com.virtuslab.qual.subtyping}.* packages,
  * but then <a href="https://checkerframework.org/manual/#subtyping-checker">Subtyping Checker</a>
  * would raise an error about multiple top/bottom types.
  */

diff --git a/qual/src/main/java/com/virtuslab/qual/subtyping/internal/package-info.java b/qual/src/main/java/com/virtuslab/qual/subtyping/internal/package-info.java
@@ -0,0 +1,4 @@
+/**
+ * Module private. Do not use outside {@link com.virtuslab.qual.subtyping}.
+ */
+package com.virtuslab.qual.subtyping.internal;
diff --git a/qual/src/main/java/com/virtuslab/qual/subtyping/package-info.java b/qual/src/main/java/com/virtuslab/qual/subtyping/package-info.java
@@ -1,11 +1,53 @@
 /**
  * Java 17 allows for pattern matching in switch expressions & statements
  * (https://www.baeldung.com/java-switch-pattern-matching).
- * Still, a quick evaluation shows that Subtyping Checker is more convenient:
- * <ol>
- * <li>{@code instanceof} requires introducing a new variable</li>
- * <li>{@code instanceof} only detects one subtype, not the other</li>
- * <li>{@code instanceof} requires sealed classes/interfaces for exclusivity check to work; this is problematic for interfaces like I... coz Base... </li>
- * </ol>
+ * Still, a quick evaluation shows that Subtyping Checker is more convenient.
+ * Let's consider the example of {@code com.virtuslab.gitmachete.backend.api.IManagedBranchSnapshot},
+ * with its two sub-interfaces {@code IRootManagedBranchSnapshot} and {@code INonRootManagedBranchSnapshot}.
+ * <p>
+ * With Subtyping Checker, the following is possible:
+ * <pre>
+ *   if (branch.isRoot()) {
+ *     ... branch.asRoot() ...
+ *   } else {
+ *     ... branch.asNonRoot() ...
+ *   }
+ * </pre>
+ * <p>
+ * With {@code instanceof}, it would look like:
+ * <pre>
+ *   if (branch instanceof IRootManagedBranchSnapshot rootBranch) {
+ *     ... rootBranch ...
+ *   } else {
+ *     ... ((INonRootManagedBranchSnapshot) branch) ...
+ *   }
+ * </pre>
+ * or alternatively
+ * <pre>
+ *   if (branch instanceof IRootManagedBranchSnapshot rootBranch) {
+ *     ... rootBranch ...
+ *   } else if (branch instanceof INonRootManagedBranchSnapshot nonRootBranch) {
+ *     ... nonRootBranch ..
+ *   }
+ * </pre>
+ * <p>
+ * Pattern matching in switch (Java 17 with {@code --enable-preview})
+ * won't work for interfaces that are directly extended by an abstract class
+ * (and not only by sub-interfaces), as is the case with {@code IManagedBranchSnapshot}.
+ * The existence of {@code com.virtuslab.gitmachete.backend.impl.BaseManagedBranchSnapshot} would massively mess up the checks
+ * for sealed interface, which are needed for pattern matching to work properly.
+ * We can either sacrifice sealedness of {@code IManagedBranchSnapshot}:
+ * <pre>
+ *   switch (branch) {
+ *     case IRootManagedBranchSnapshot rootBranch -> ...
+ *     case INonRootManagedBranchSnapshot nonRootBranch -> ...
+ *     // WHOOPS compiler sees this match as non-exhaustive
+ *   }
+ * </pre>
+ * or include {@code BaseManagedBranchSnapshot} in {@code permits} clause for {@code IManagedBranchSnapshot},
+ * which would also cause the compiler to report a non-exhaustive {@code switch} error.
+ * <p>
+ * All things consider, as of Java 17, Subtyping Checker remains a cleaner choice for exhaustive matching on subtypes
+ * than whatever mechanism built into the language.
  */
 package com.virtuslab.qual.subtyping;
diff --git a/src/test/java/com/virtuslab/archunit/ClassStructureTestSuite.java b/src/test/java/com/virtuslab/archunit/ClassStructureTestSuite.java
@@ -131,7 +131,7 @@ public void all_classes_should_be_referenced() {
         .and().resideOutsideOfPackages("com.virtuslab.gitmachete.frontend.defs")
         .and().doNotBelongToAnyOf(classesReferencedFromPluginXmlAttributes)
         // SubtypingBottom is processed by CheckerFramework based on its annotations
-        .and().doNotHaveFullyQualifiedName(com.virtuslab.qual.internal.SubtypingBottom.class.getName())
+        .and().doNotHaveFullyQualifiedName(com.virtuslab.qual.subtyping.internal.SubtypingBottom.class.getName())
         .should(new BeReferencedFromOutsideItself())
         .check(productionClasses);
   }