Remodel boolean queries to hold last query

core/src/main/scala/pink/cozydev/lucille/Query.scala

@@ -17,6 +17,7 @@
 package pink.cozydev.lucille
 
 import cats.data.NonEmptyList
+import scala.collection.mutable.ListBuffer
 
 /** A trait for all queries */
 sealed trait Query extends Product with Serializable {
@@ -117,25 +118,27 @@ object Query {
     *
     * @param qs the queries to union
     */
-  final case class Or private (qs: NonEmptyList[Query]) extends Query {
+  final case class Or(allButLast: NonEmptyList[Query], last: Query) extends Query {
     def mapLastTerm(f: Query.Term => Query): Or =
-      Or(rewriteLastTerm(qs, f))
+      this.copy(last = last.mapLastTerm(f))
   }
   object Or {
-    def apply(left: Query, right: Query, tail: Query*): Or =
-      Or(NonEmptyList(left, right :: tail.toList))
-
-    def apply(left: Query, right: Query, tail: List[Query]): Or =
-      Or(NonEmptyList(left, right :: tail))
-
-    def fromListUnsafe(queries: List[Query]): Or =
-      queries match {
-        case Nil =>
-          throw new IllegalArgumentException("Cannot create Or query from empty list")
-        case _ :: Nil =>
-          throw new IllegalArgumentException("Cannot create Or query from single element list")
-        case h :: t => Or(NonEmptyList(h, t))
-      }
+    def apply(left: Query, right: Query, tail: Query*): Or = {
+      val bldr = ListBuffer.empty[Query]
+      bldr.sizeHint(2 + tail.size)
+      var last = right
+      tail.foreach { q => bldr += last; last = q }
+      Or(NonEmptyList(left, bldr.result()), last)
+    }
+
+    def apply(left: Query, right: Query, tail: List[Query]): Or = {
+      val bldr = ListBuffer.empty[Query]
+      bldr.sizeHint(2 + tail.size)
+      var last = right
+      tail.foreach { q => bldr += last; last = q }
+      Or(NonEmptyList(left, bldr.result()), last)
+    }
+
   }
 
   /**  An And operator
@@ -144,25 +147,27 @@ object Query {
     *
     * @param qs the queries to intersect
     */
-  final case class And private (qs: NonEmptyList[Query]) extends Query {
+  final case class And(allButLast: NonEmptyList[Query], last: Query) extends Query {
     def mapLastTerm(f: Query.Term => Query): And =
-      And(rewriteLastTerm(qs, f))
+      this.copy(last = last.mapLastTerm(f))
   }
   object And {
-    def apply(left: Query, right: Query, tail: Query*): And =
-      And(NonEmptyList(left, right :: tail.toList))
-
-    def apply(left: Query, right: Query, tail: List[Query]): And =
-      And(NonEmptyList(left, right :: tail))
-
-    def fromListUnsafe(queries: List[Query]): And =
-      queries match {
-        case Nil =>
-          throw new IllegalArgumentException("Cannot create And query from empty list")
-        case _ :: Nil =>
-          throw new IllegalArgumentException("Cannot create And query from single element list")
-        case h :: t => And(NonEmptyList(h, t))
-      }
+    def apply(left: Query, right: Query, tail: Query*): And = {
+      val bldr = ListBuffer.empty[Query]
+      bldr.sizeHint(2 + tail.size)
+      var last = right
+      tail.foreach { q => bldr += last; last = q }
+      And(NonEmptyList(left, bldr.result()), last)
+    }
+
+    def apply(left: Query, right: Query, tail: List[Query]): And = {
+      val bldr = ListBuffer.empty[Query]
+      bldr.sizeHint(2 + tail.size)
+      var last = right
+      tail.foreach { q => bldr += last; last = q }
+      And(NonEmptyList(left, bldr.result()), last)
+    }
+
   }
 
   /** A Not operator
@@ -251,13 +256,4 @@ object Query {
 
   final case class WildCard(ops: NonEmptyList[WildCardOp]) extends TermQuery
 
-  private def rewriteLastTerm(
-      qs: NonEmptyList[Query],
-      f: Query.Term => Query,
-  ): NonEmptyList[Query] =
-    if (qs.size == 1) NonEmptyList.one(qs.head.mapLastTerm(f))
-    else {
-      val newT = qs.tail.init :+ qs.last.mapLastTerm(f)
-      NonEmptyList(qs.head, newT)
-    }
 }

core/src/main/scala/pink/cozydev/lucille/QueryPrinter.scala

@@ -40,8 +40,8 @@ object QueryPrinter {
     def printQ(query: Query): Unit =
       query match {
         case q: TermQuery => strTermQuery(q)
-        case q: Or => printEachNel(q.qs, " OR ")
-        case q: And => printEachNel(q.qs, " AND ")
+        case q: Or => printEach(q.allButLast, q.last, " OR ")
+        case q: And => printEach(q.allButLast, q.last, " AND ")
         case q: Not =>
           sb.append("NOT ")
           printQ(q.q)
@@ -142,6 +142,16 @@ object QueryPrinter {
         sb.append(c)
       }
 
+    def printEach(allButLast: NonEmptyList[Query], last: Query, sep: String): Unit = {
+      printQ(allButLast.head)
+      allButLast.tail.foreach { q =>
+        sb.append(sep)
+        printQ(q)
+      }
+      sb.append(sep)
+      printQ(last)
+    }
+
     printQ(query)
     sb.result()
   }

core/src/main/scala/pink/cozydev/lucille/internal/Op.scala

@@ -18,6 +18,7 @@ package pink.cozydev.lucille.internal
 
 import pink.cozydev.lucille.Query
 import scala.collection.mutable.ListBuffer
+import cats.data.NonEmptyList
 
 private[lucille] sealed trait Op extends Product with Serializable
 
@@ -34,9 +35,14 @@ private[lucille] object Op {
   def associateOps(first: Query, opQs: List[(Op, Query)]): Query =
     opQs match {
       case Nil => first
+      case (onlyOp, secondQ) :: Nil =>
+        onlyOp match {
+          case OR => Query.Or(NonEmptyList.one(first), secondQ)
+          case AND => Query.And(NonEmptyList.one(first), secondQ)
+        }
       case (headOp, headQ) :: remaining =>
-        var currentOp = headOp
-        var currentQ = headQ
+        var lastOp = headOp
+        var lastQ = headQ
 
         // We'll collect queries in 'tempAccumulator' while successive operators are the same type
         // e.g. (OR, q1), (OR, q2), (OR, q3), ...
@@ -47,63 +53,58 @@ private[lucille] object Op {
         // e.g. (OR, q1), (AND, q2), ...
         val bldr = ListBuffer.empty[Query]
 
-        // Iterate through Op-Query pairs, looking "one ahead" to decide how to process 'currentQ'
-        remaining.foreach { case (nextOp, nextQ) =>
-          if (currentOp == nextOp) {
-            // 'nextOp' hasn't changed, keep accumulating
-            tempAccumulator += currentQ
+        // Iterate through Op-Query pairs
+        remaining.foreach { case (currentOp, currentQ) =>
+          if (lastOp == currentOp) {
+            // Op hasn't changed, keep accumulating
+            tempAccumulator += lastQ
           } else {
-            // 'nextOp' is different from 'currentOp', so we're going to collapse the queries we've
-            // accumulated so far into an AND/OR query before continuing.
+            // 'currentOp' is different from 'lastOp'
+            // Collapse accumulated queries so far into an AND/OR query before continuing.
             // How we do that depends on the precedence of the operator we're switching to.
             // AND has higher precedence than OR, so if we are switching from OR to AND, we
-            // collapse before accumulating 'currentQ' and instead add it to the newly cleared
+            // collapse before accumulating 'lastQ' and instead add it to the newly cleared
             // accumulator.
-            nextOp match {
+            currentOp match {
               case AND =>
                 // OR -> AND
-                // e.g. previousQ OR (currentQ AND nextQ)
-                // From OR to AND, collapse now, new AND gets currentQ
+                // e.g. OR (lastQ AND currentQ)
+                // From OR to AND, collapse now, new AND gets lastQ
                 val qs = tempAccumulator.result()
                 tempAccumulator.clear()
                 bldr ++= qs
-                tempAccumulator += currentQ
+                tempAccumulator += lastQ
               case OR =>
                 // AND -> OR
-                // e.g. (previousQ AND currentQ) OR nextQ
-                // From AND to OR, add currentQ before collapsing
-                tempAccumulator += currentQ
-                val qs = tempAccumulator.result()
+                // e.g. (... AND lastQ) OR currentQ
+                // From AND to OR, add lastQ to AND query
+                val qs = NonEmptyList.fromListUnsafe(tempAccumulator.result())
                 tempAccumulator.clear()
-
-                bldr += Query.And.fromListUnsafe(qs)
+                bldr += Query.And(qs, lastQ)
             }
           }
-          // get ready for next iteration
-          currentOp = nextOp
-          currentQ = nextQ
+          // prep for next iteration
+          lastQ = currentQ
+          lastOp = currentOp
         }
+        val qs = tempAccumulator.result()
 
-        // We're done iterating
-        // But because we were looking one ahead, we still have not processed the last 'currentQ'.
-        // It's safe to add 'currentQ' to 'tempAccumulator', it's either already collecting queries
-        // for 'currentOp', or we've just cleared it for a new Op type.
-        tempAccumulator += currentQ
-        val innerQs = tempAccumulator.result()
-        currentOp match {
-          case AND =>
-            // Final OP was an AND, collapse into one AND query, add to 'bldr'
-            bldr += Query.And.fromListUnsafe(innerQs)
-          case OR =>
-            // Final OP was an OR, directly add to 'bldr'.
-            // Safe because all the ANDs have been grouped together.
-            // Wrapping in an OR query would create unnecessary nesting.
-            bldr ++= innerQs
+        bldr.result() match {
+          case Nil =>
+            lastOp match {
+              case OR => Query.Or(NonEmptyList.fromListUnsafe(qs), lastQ)
+              case AND => Query.And(NonEmptyList.fromListUnsafe(qs), lastQ)
+            }
+          case head :: tail =>
+            lastOp match {
+              case OR => Query.Or(NonEmptyList(head, tail ++ qs), lastQ)
+              case AND =>
+                Query.Or(
+                  NonEmptyList(head, tail),
+                  Query.And(NonEmptyList.fromListUnsafe(qs), lastQ),
+                )
+            }
         }
-        val finalQs = bldr.result()
-        // If we only have one query, it must be an AND query, directly return that.
-        // Otherwise we wrap our multiple queries in an OR query.
-        if (finalQs.size == 1) finalQs.head else Query.Or.fromListUnsafe(finalQs)
     }
 
 }