Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Issue-195] Improve error message in case a Factory instance is missing when transforming between collections #198

Merged
merged 2 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"""
)

}
}