Skip to content

Commit

Permalink
[Issue-195] Improve error message in case a Factory instance is mis…
Browse files Browse the repository at this point in the history
…sing when transforming between collections (#198)

Closes #195
  • Loading branch information
arainko authored Aug 22, 2024
2 parents 0c23a3d + 749ca91 commit 60098fb
Show file tree
Hide file tree
Showing 14 changed files with 82 additions and 21 deletions.
4 changes: 2 additions & 2 deletions docs/total_transformations/configuring_transformations.md
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ Docs.printCode(
card
.into[PaymentBand]
.transform(
Field.computed(_.color, card => if (card.digits % 2 == 0) "green" else "yellow")
Field.computed(_.color, card => if card.digits % 2 == 0 then "green" else "yellow")
)
```
@:choice(generated)
Expand All @@ -252,7 +252,7 @@ Docs.printCode(
card
.into[PaymentBand]
.transform(
Field.computed(_.color, card => if (card.digits % 2 == 0) "green" else "yellow")
Field.computed(_.color, card => if card.digits % 2 == 0 then "green" else "yellow")
)
)
```
Expand Down
2 changes: 1 addition & 1 deletion docs/transformation_rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ case class Positive private (value: Int)

object Positive {
given Transformer.Fallible[[a] =>> Either[String, a], Int, Positive] =
int => if (int < 0) Left("Lesser or equal to 0") else Right(Positive(int))
int => if int < 0 then Left("Lesser or equal to 0") else Right(Positive(int))
}

Mode.FailFast.either[String].locally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ private[ducktape] object Configuration {

Varargs
.unapply(configs)
.get // TODO: Make it nicer
.getOrElse(report.errorAndAbort("All of the transformation configs need to be inlined", configs))
.map(expr => parser.applyOrElse(expr.asTerm, fallback))
.toList
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,4 +117,13 @@ private[ducktape] object ErrorMessage {
|You can make this work if you supply a deprioritized instance of Mode.FailFast for the same wrapper type.""".stripMargin
}

final case class CollectionFactoryNotFound(struct: Structure.Collection, explanation: String) extends ErrorMessage {
val side: Side = Side.Dest
val span: Span | None.type = None
def render(using Quotes): String = {
s"""Couldn't derive a transformation between collections due to a missing instance of scala.Factory[${struct.paramStruct.tpe.repr.show}, ${struct.tpe.repr.show}].
|Implicit search failure explanation: $explanation""".stripMargin
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ private[ducktape] object FallibilityRefiner {
case BetweenNonOptionOption(source, dest, plan) =>
recurse(plan)

case BetweenCollections(source, dest, plan) =>
case BetweenCollections(source, dest, _, plan) =>
recurse(plan)

case BetweenFallibleNonFallible(source, dest, plan) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,10 @@ private[ducktape] object FalliblePlanInterpreter {
Value.Wrapped('{ ${ F.value }.map(${ recurse(plan, source, F).wrapped(F, Type.of[dest]) }, Some.apply) })
}

case Plan.BetweenCollections(source, dest, plan) =>
case Plan.BetweenCollections(source, dest, collFactory, plan) =>
(dest.tpe, source.paramStruct.tpe, dest.paramStruct.tpe) match {
case ('[destCollTpe], '[srcElem], '[destElem]) =>
// TODO: Make it nicer, move this into Planner since we cannot be sure that a factory exists
val factory = Expr.summon[Factory[destElem, destCollTpe]].get
val factory = collFactory.asExprOf[Factory[destElem, destCollTpe]]
factory match {
case '{
type dest <: Iterable[`destElem`]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,12 @@ private[ducktape] object Function {
)(using Quotes) = {
import quotes.reflect.*

val func = fromExpr(function).getOrElse(report.errorAndAbort(s"Couldn't construct a function TODO ERROR MESSAGE"))
val func = fromExpr(function).getOrElse(
report.errorAndAbort(
s"Couldn't encode a function as a type. Please make sure you're passing in an eta-expanded method.",
function
)
)
func.encodeAsType(initial)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package io.github.arainko.ducktape.internal
import io.github.arainko.ducktape.*
import io.github.arainko.ducktape.internal.*

import scala.collection.Factory
import scala.collection.immutable.VectorMap
import scala.quoted.*
import scala.reflect.TypeTest
Expand Down Expand Up @@ -149,6 +150,7 @@ private[ducktape] object Plan {
case class BetweenCollections[+E <: Erroneous, +F <: Fallible](
source: Structure.Collection,
dest: Structure.Collection,
factory: Expr[Factory[?, ?]],
plan: Plan[E, F]
) extends Plan[E, F]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ private[ducktape] object PlanConfigurer {
current: Plan[Erroneous, F]
): Plan[Erroneous, F] =
current match {
case parent @ BetweenCollections(_, _, plan) =>
case parent @ BetweenCollections(_, _, _, plan) =>
parent.copy(plan = recurse(plan, tail, parent, config))

case parent @ BetweenOptions(_, _, plan) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,11 @@ private[ducktape] object PlanInterpreter {
'{ Some(${ transformation(sourceValue) }) }
}

case Plan.BetweenCollections(source, dest, plan) =>
case Plan.BetweenCollections(source, dest, collFactory, plan) =>
(dest.tpe, source.paramStruct.tpe, dest.paramStruct.tpe) match {
case ('[destCollTpe], '[srcElem], '[destElem]) =>
val sourceValue = value.asExprOf[Iterable[srcElem]]
// TODO: Make it nicer, move this into Planner since we cannot be sure that a facotry exists
val factory = Expr.summon[Factory[destElem, destCollTpe]].get
val factory = collFactory.asExprOf[Factory[destElem, destCollTpe]]
def transformation(value: Expr[srcElem])(using Quotes): Expr[destElem] = recurse(plan, value).asExprOf[destElem]
'{ $sourceValue.map(src => ${ transformation('src) }).to($factory) }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ private[ducktape] trait PlanTraverser[A] {
recurse(plan :: next, foldOver(p, accumulator))
case p @ Plan.BetweenNonOptionOption(_, _, plan) =>
recurse(plan :: next, foldOver(p, accumulator))
case p @ Plan.BetweenCollections(_, _, plan) =>
case p @ Plan.BetweenCollections(_, _, _, plan) =>
recurse(plan :: next, foldOver(p, accumulator))
case plan: Plan.BetweenSingletons =>
recurse(next, foldOver(plan, accumulator))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import io.github.arainko.ducktape.internal.Context.{ PossiblyFallible, Total }
import io.github.arainko.ducktape.internal.Plan.{ Derived, UserDefined }
import io.github.arainko.ducktape.internal.Summoner.UserDefined.{ FallibleTransformer, TotalTransformer }

import scala.collection.Factory
import scala.collection.immutable.VectorMap
import scala.quoted.*
import scala.util.boundary
Expand Down Expand Up @@ -83,12 +84,24 @@ private[ducktape] object Planner {
recurse(source, underlying)
)

case (source @ Collection(_, _, srcParamStruct)) -> (dest @ Collection(_, _, destParamStruct)) =>
Plan.BetweenCollections(
source,
dest,
recurse(srcParamStruct, destParamStruct)
)
case (source @ Collection(_, _, srcParamStruct)) -> (dest @ Collection('[destColl], _, destParamStruct @ Structure('[destElem]))) =>
Implicits.search(TypeRepr.of[Factory[destElem, destColl]]) match {
case success: ImplicitSearchSuccess =>
Plan.BetweenCollections(
source,
dest,
success.tree.asExprOf[Factory[destElem, destColl]],
recurse(srcParamStruct, destParamStruct)
)
case failure: ImplicitSearchFailure =>
Plan.Error(
source,
dest,
ErrorMessage.CollectionFactoryNotFound(dest, failure.explanation),
None
)

}

case (source: Product, dest: Product) =>
planProductTransformation(source, dest)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import scala.deriving.Mirror
import scala.quoted.*
import scala.reflect.TypeTest

private[ducktape] sealed trait Structure derives Debug {
private[ducktape] sealed trait Structure extends scala.Product derives Debug {
def tpe: Type[?]

def path: Path

def _1: Type[?] = tpe

final def force: Structure =
this match {
case lzy: Lazy => lzy.struct.force
Expand All @@ -24,6 +26,8 @@ private[ducktape] sealed trait Structure derives Debug {
}

private[ducktape] object Structure {
def unapply(struct: Structure): Structure = struct

def toplevelAny(using Quotes) = Structure.Ordinary(Type.of[Any], Path.empty(Type.of[Any]))

def toplevelNothing(using Quotes) = Structure.Ordinary(Type.of[Nothing], Path.empty(Type.of[Nothing]))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package io.github.arainko.ducktape.issues

import io.github.arainko.ducktape.*

class Issue195Suite extends DucktapeSuite {
test("missing Factory instances are reported in a nice way") {

assertFailsToCompileContains {
"""
import scala.collection.immutable.SortedSet
import io.github.arainko.ducktape.to as convertTo
import io.github.arainko.ducktape.Transformer
final class MyClassA
final class MyClassB
given Transformer[MyClassA, MyClassB] = ???
val x: SortedSet[MyClassA] = ???
val y: SortedSet[MyClassB] = x.convertTo[SortedSet[MyClassB]]
"""
}(
// TODO: this is weird, the type in the explanation is different in CI ('java.util.Comparator[MyClassB]') as opposed to running it locally ('scala.math.Ordering.AsComparable[MyClassB]')
"""Couldn't derive a transformation between collections due to a missing instance of scala.Factory[MyClassB, scala.collection.immutable.SortedSet[MyClassB]].
Implicit search failure explanation: no implicit values were found that match type"""
)

}
}

0 comments on commit 60098fb

Please sign in to comment.