Fix/ exponential compile time in builder chains

.github/workflows/ci.yml

@@ -11,6 +11,9 @@ jobs:
     ci:
         name: Compile and test CaseComplete
         runs-on: ubuntu-latest
+        # A LongChainSpec regression fails by hanging, so cap the job instead of burning
+        # GitHub's 360-minute default.
+        timeout-minutes: 15
         steps:
             - uses: actions/checkout@v4
             - name: Setup JDK 21, Scala, SBT

build.sbt

@@ -23,7 +23,7 @@ developers := List(
   )
 )
 
-version := "0.2.2"
+version := "0.3.0"
 
 scalaVersion := "3.3.8"
 

src/main/scala/io/github/stivens/CaseComplete/CaseComplete.scala

@@ -11,17 +11,12 @@ object CaseComplete {
     CaseCompleteBuilder.apply[SOURCE_TYPE, TARGET_TYPE]
 }
 
-/**
- * Implementation of CaseComplete that stores handlers in a Map and evaluates them in sorted order.
- * 
- * This class is used internally by the CaseCompleteBuilder to create the final CaseComplete instance
- * after all field handlers have been registered.
- */
 private[casecomplete] class CaseCompleteImpl[SOURCE_TYPE <: Product, TARGET_TYPE](
     handlers: Map[String, SOURCE_TYPE => TARGET_TYPE]
 ) extends CaseComplete[SOURCE_TYPE, TARGET_TYPE] {
+  private val sortedHandlers: List[SOURCE_TYPE => TARGET_TYPE] =
+    handlers.toList.sortBy(_._1).map(_._2)
+
   def eval(source: SOURCE_TYPE): List[TARGET_TYPE] =
-    handlers.toList
-      .sortBy { case (fieldName, _) => fieldName }
-      .map { case (_, handler) => handler(source) }
+    sortedHandlers.map(_(source))
 }

src/test/scala/externaluser/ExternalAccessSpec.scala

@@ -0,0 +1,71 @@
+package externaluser
+
+import io.github.stivens.casecomplete.CaseComplete
+import org.scalatest.funspec.AnyFunSpec
+import testsupport.CompileErrorAssertions
+
+case class Filter(a: Option[String], b: Option[String])
+
+/**
+ * Deliberately outside `io.github.stivens.casecomplete` -- the only vantage point where
+ * `private[casecomplete]` differs from public. Pins both directions: generated code reaches the
+ * package-private members, users cannot.
+ */
+class ExternalAccessSpec extends AnyFunSpec with CompileErrorAssertions {
+
+  describe("a builder used from outside the library's package") {
+
+    it("should compile and evaluate a chain") {
+      val handler = CaseComplete
+        .build[Filter, Option[String]]
+        .usingNonEmpty(_.a)(value => f"a = $value")
+        .ignoring(_.b)
+        .compile
+
+      assert(handler.eval(Filter(a = Some("x"), b = None)).flatten == List("a = x"))
+    }
+
+    it("should not let a field be marked handled without a handler") {
+      assertInaccessible(
+        """
+        CaseComplete.build[Filter, Option[String]]
+          .using(_.a)(identity)
+          .markHandled[("b", "a")]
+          .compile
+        """,
+        "markHandled"
+      )
+    }
+
+    it("should not let a builder be constructed directly") {
+      assertInaccessible(
+        """
+        new io.github.stivens.casecomplete.macros.CaseCompleteBuilder[Filter, Option[String], ("a", "b")](Map.empty)
+        """,
+        "CaseCompleteBuilder"
+      )
+    }
+
+    it("should not let a handler be registered under a forged field name") {
+      assertInaccessible(
+        """
+        CaseComplete.build[Filter, Option[String]]
+          .using(_.a)(identity)
+          .addHandler[("b", "a")]("b", _ => None)
+          .compile
+        """,
+        "addHandler"
+      )
+    }
+
+    it("should not expose the handler map") {
+      assertInaccessible(
+        """CaseComplete.build[Filter, Option[String]].handlers""",
+        "handlers"
+      )
+    }
+  }
+
+  private inline def assertInaccessible(inline code: String, member: String): Unit =
+    assertErrorContains(code, member, "cannot be accessed")
+}

src/test/scala/io/github/stivens/CaseComplete/CaseCompleteSpec.scala

@@ -1,10 +1,11 @@
 package io.github.stivens.casecomplete
 
 import org.scalatest.funspec.AnyFunSpec
+import testsupport.CompileErrorAssertions
 
 import java.time.Year
 
-class CaseCompleteSpec extends AnyFunSpec {
+class CaseCompleteSpec extends AnyFunSpec with CompileErrorAssertions {
   describe("CaseCompleteBuilder") {
     describe("when given a source type and a target type") {
 
@@ -19,36 +20,43 @@ class CaseCompleteSpec extends AnyFunSpec {
         releaseYear_eq = Some(Year.of(1999)),
         rating_gte = Some(7.0)
       )
-      val expectedResult = Set("releaseYear = 1999", "rating >= 7.0")
+      val expectedOrder  = List("rating >= 7.0", "releaseYear = 1999")
+      val expectedResult = expectedOrder.toSet
 
       val buildMovieFilterHandler = CaseComplete.build[MovieFilter, Option[String]]
 
-      it("should properly use all the fields of the source type and compile") {
-        val movieFilterHandler = buildMovieFilterHandler
-          .using(_.title_like)(_.map(title => f"title ILIKE $title"))
-          .using(_.director_eq)(_.map(director => f"director = $director"))
-          .using(_.releaseYear_eq)(_.map(releaseYear => f"releaseYear = $releaseYear"))
-          .using(_.rating_gte)(_.map(rating => f"rating >= $rating"))
-          .compile
+      val movieFilterHandler = buildMovieFilterHandler
+        .using(_.title_like)(_.map(title => f"title ILIKE $title"))
+        .using(_.director_eq)(_.map(director => f"director = $director"))
+        .using(_.releaseYear_eq)(_.map(releaseYear => f"releaseYear = $releaseYear"))
+        .using(_.rating_gte)(_.map(rating => f"rating >= $rating"))
+        .compile
 
+      it("should properly use all the fields of the source type and compile") {
         val evaulated = movieFilterHandler.eval(filter).toSet.flatten
 
         assert(evaulated == expectedResult)
       }
 
       it("should properly use all the non-empty optional fields of the source type and compile") {
-        val movieFilterHandler = buildMovieFilterHandler
+        val nonEmptyHandler = buildMovieFilterHandler
           .usingNonEmpty(_.title_like)(title => f"title ILIKE $title")
           .usingNonEmpty(_.director_eq)(director => f"director = $director")
           .usingNonEmpty(_.releaseYear_eq)(releaseYear => f"releaseYear = $releaseYear")
           .usingNonEmpty(_.rating_gte)(rating => f"rating >= $rating")
           .compile
 
-        val evaulated = movieFilterHandler.eval(filter).toSet.flatten
+        val evaulated = nonEmptyHandler.eval(filter).toSet.flatten
 
         assert(evaulated == expectedResult)
       }
 
+      it("should evaluate handlers in alphabetical order of field name, on every call") {
+        // Repeated so that caching the ordering as something single-use (a view, an iterator) fails here.
+        assert(movieFilterHandler.eval(filter).flatten == expectedOrder)
+        assert(movieFilterHandler.eval(filter).flatten == expectedOrder)
+      }
+
       it("should allow to explicitly ignore a field") {
         val movieFilterHandler = buildMovieFilterHandler
           .ignoring(_.title_like)
@@ -76,21 +84,167 @@ class CaseCompleteSpec extends AnyFunSpec {
       }
 
       object MovieFilter {
-        val empty = MovieFilter()
+        // Explicit `new`: `MovieFilter()` would re-enter this initializer through the companion's apply.
+        val empty = new MovieFilter(None, None, None, None)
       }
 
-      val buildMovieFilterHandler = CaseComplete.build[MovieFilter, Option[String]]
+      val allConstructorFieldsHandled = CaseComplete
+        .build[MovieFilter, Option[String]]
+        .using(_.title_like)(_ => None)
+        .using(_.director_eq)(_ => None)
+        .using(_.releaseYear_eq)(_ => None)
+        .using(_.rating_gte)(_ => None)
 
       it("should not require the extra fields to be handled") {
-        val movieFilterHandler = buildMovieFilterHandler
-          .using(_.title_like)(_ => None)
-          .using(_.director_eq)(_ => None)
-          .using(_.releaseYear_eq)(_ => None)
-          .using(_.rating_gte)(_ => None)
-          .compile
+        allConstructorFieldsHandled.compile
 
         assert(true) // code compiles
       }
+
+      it("should allow the extra fields to be handled") {
+        val movieFilterHandler = allConstructorFieldsHandled
+          .using(_.foo)(Some(_))
+          .compile
+
+        assert(movieFilterHandler.eval(MovieFilter()).flatten == List("bar"))
+      }
+    }
+
+    describe("when validating the chain at compile time") {
+
+      // Positive control for the negative snippet tests below.
+      it("should compile a chain that handles every field") {
+        assertCompiles("""
+          CaseComplete.build[TwoFieldFilter, Option[String]]
+            .using(_.a)(identity)
+            .using(_.b)(identity)
+            .compile
+        """)
+      }
+
+      // These assert the message text, not just failure: the messages exist to be read, and a
+      // failure-only test would not notice them degrading into raw compiler diagnostics.
+      it("should report the unhandled field when one has no handler") {
+        assertErrorContains(
+          """
+          CaseComplete.build[TwoFieldFilter, Option[String]]
+            .using(_.a)(identity)
+            .compile
+          """,
+          "Missing handlers for fields: b"
+        )
+      }
+
+      it("should report the field name when the same field is handled twice") {
+        assertErrorContains(
+          """
+          CaseComplete.build[TwoFieldFilter, Option[String]]
+            .using(_.a)(identity)
+            .using(_.a)(identity)
+          """,
+          "Field 'a' has already been handled"
+        )
+      }
+
+      it("should report the offending expression when the selector is not a plain field access") {
+        assertErrorContains(
+          """
+          CaseComplete.build[TwoFieldFilter, Option[String]]
+            .using(filter => filter.a.map(_.trim))(identity)
+          """,
+          "expected a field selector"
+        )
+      }
+
+      it("should reject a nested selector, which would register the inner field's name against the source type") {
+        assertErrorContains(
+          """
+          CaseComplete.build[NestedFilter, Option[String]]
+            .using(_.a.b)(identity)
+          """,
+          "expected a field selector"
+        )
+      }
+
+      it("should report the field name when a field is ignored and then handled") {
+        assertErrorContains(
+          """
+          CaseComplete.build[TwoFieldFilter, Option[String]]
+            .ignoring(_.a)
+            .using(_.a)(identity)
+          """,
+          "Field 'a' has already been handled"
+        )
+      }
+
+      it("should point at `using` when usingNonEmpty is applied to a non-Option target") {
+        assertErrorContains(
+          """
+          CaseComplete.build[TwoFieldFilter, String]
+            .usingNonEmpty(_.a)(value => value)
+          """,
+          "usingNonEmpty requires the target type to be exactly Option"
+        )
+      }
+
+      it("should reject a target type that is a strict subtype of Option") {
+        assertErrorContains(
+          """
+          CaseComplete.build[TwoFieldFilter, Some[String]]
+            .usingNonEmpty(_.a)(identity)
+          """,
+          "usingNonEmpty requires the target type to be exactly Option"
+        )
+      }
+
+      it("should not count a handled body val toward the completeness of constructor fields") {
+        assertErrorContains(
+          """
+          CaseComplete.build[BodyValFilter, Option[String]]
+            .using(_.derived)(identity)
+            .compile
+          """,
+          "Missing handlers for fields: a"
+        )
+      }
+
+      it("should explain the fix when compile is called on a builder ascribed a widened type") {
+        assertErrorContains(
+          """
+          val b: macros.CaseCompleteBuilder[TwoFieldFilter, Option[String], ?] =
+            CaseComplete.build[TwoFieldFilter, Option[String]]
+              .using(_.a)(identity)
+              .using(_.b)(identity)
+          b.compile
+          """,
+          "the chain's inferred type"
+        )
+      }
+
+      // The validations live in registerField, shared by all entry points; these pin that
+      // `ignoring` and `usingNonEmpty` route through it rather than just `using`.
+      it("should run the shared selector checks for ignoring too") {
+        assertErrorContains(
+          """CaseComplete.build[NestedFilter, Option[String]].ignoring(_.a.b)""",
+          "expected a field selector"
+        )
+      }
+
+      it("should report a bad selector before usingNonEmpty's target-type check") {
+        assertErrorContains(
+          """
+          CaseComplete.build[NestedFilter, String]
+            .usingNonEmpty(_.a.b)(identity)
+          """,
+          "expected a field selector"
+        )
+      }
     }
   }
 }
+
+// Top-level so the type-checking snippets above can name them.
+case class TwoFieldFilter(a: Option[String], b: Option[String])
+case class NestedInner(b: Option[String])
+case class NestedFilter(a: NestedInner, b: Option[String])
+case class BodyValFilter(a: Option[String]) { val derived: Option[String] = a }