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

Simplify FunctionK.lift macro #3402

Merged
merged 4 commits into from
Dec 6, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ package cats
package arrow

import scala.language.experimental.macros
import scala.reflect.macros.blackbox.Context
import scala.reflect.macros.blackbox

private[arrow] class FunctionKMacroMethods {
protected type τ[F[_], G[_]]

/**
* Lifts function `f` of `F[A] => G[A]` into a `FunctionK[F, G]`.
Expand All @@ -27,76 +28,61 @@ private[arrow] class FunctionKMacroMethods {
*/
def lift[F[_], G[_]](f: (F[α] => G[α]) forSome { type α }): FunctionK[F, G] =
macro FunctionKMacros.lift[F, G]

/**
* Lifts function `f` of `F[A] => G[A]` into a `FunctionK[F, G]`.
*
* {{{
* def headOption[A](list: List[A]): Option[A] = list.headOption
* val lifted = FunctionK.liftFunction[List, Option](headOption)
* }}}
*
* Note: The weird `τ[F, G]` parameter is there to compensate for
* the lack of polymorphic function types in Scala 2.
*/
def liftFunction[F[_], G[_]](f: F[τ[F, G]] => G[τ[F, G]]): FunctionK[F, G] =
new FunctionK[F, G] {
def apply[A](fa: F[A]): G[A] = f.asInstanceOf[F[A] => G[A]](fa)
}
}

private[arrow] object FunctionKMacros {

def lift[F[_], G[_]](c: Context)(
def lift[F[_], G[_]](c: blackbox.Context)(
f: c.Expr[(F[α] => G[α]) forSome { type α }]
)(implicit
evF: c.WeakTypeTag[F[_]],
evG: c.WeakTypeTag[G[_]]
): c.Expr[FunctionK[F, G]] =
)(implicit evF: c.WeakTypeTag[F[Any]], evG: c.WeakTypeTag[G[Any]]): c.Expr[FunctionK[F, G]] =
c.Expr[FunctionK[F, G]](new Lifter[c.type](c).lift[F, G](f.tree))
// ^^note: extra space after c.type to appease scalastyle

private[this] class Lifter[C <: Context](val c: C) {
private class Lifter[C <: blackbox.Context](val c: C) {
import c.universe._

def lift[F[_], G[_]](tree: Tree)(implicit
evF: c.WeakTypeTag[F[_]],
evG: c.WeakTypeTag[G[_]]
): Tree =
unblock(tree) match {
case q"($param) => $trans[..$typeArgs](${arg: Ident})" if param.name == arg.name =>
typeArgs
.collect { case tt: TypeTree => tt }
.find(tt => Option(tt.original).isDefined)
.foreach { param =>
c.abort(param.pos,
s"type parameter $param must not be supplied when lifting function $trans to FunctionK"
def lift[F[_], G[_]](tree: Tree)(implicit evF: c.WeakTypeTag[F[Any]], evG: c.WeakTypeTag[G[Any]]): Tree = {
def liftFunction(function: Tree): Tree =
function match {
case q"($param) => $trans[..$typeArgs]($arg)" if param.symbol == arg.symbol =>
for (typeArg @ TypeTree() <- typeArgs) if (typeArg.original != null) {
c.abort(
typeArg.pos,
s"type parameter $typeArg must not be supplied when lifting function $trans to FunctionK"
)
}

val F = punchHole(evF.tpe)
val G = punchHole(evG.tpe)
val F = typeConstructorOf[F[Any]]
val G = typeConstructorOf[G[Any]]
q"${reify(FunctionK)}.liftFunction[$F, $G]($trans(_))"

q"""
new _root_.cats.arrow.FunctionK[$F, $G] {
def apply[A](fa: $F[A]): $G[A] = $trans(fa)
case other =>
c.abort(other.pos, s"Unexpected tree $other when lifting to FunctionK")
}
"""
case other =>
c.abort(other.pos, s"Unexpected tree $other when lifting to FunctionK")
}

private[this] def unblock(tree: Tree): Tree =
tree match {
case Block(Nil, expr) => expr
case _ => tree
}

private[this] def punchHole(tpe: Type): Tree =
tpe match {
case PolyType(undet :: Nil, underlying: TypeRef) =>
val α = TypeName("α")
def rebind(typeRef: TypeRef): Tree =
if (typeRef.sym == undet) tq"$α"
else {
val args = typeRef.args.map {
case ref: TypeRef => rebind(ref)
case arg => tq"$arg"
}
tq"${typeRef.sym}[..$args]"
}
val rebound = rebind(underlying)
tq"""({type λ[$α] = $rebound})#λ"""
case TypeRef(pre, sym, Nil) =>
tq"$sym"
case _ =>
c.abort(c.enclosingPosition, s"Unexpected type $tpe when lifting to FunctionK")
case Block(Nil, expr) => liftFunction(expr)
case Block(stats, expr) => Block(stats, liftFunction(expr))
case other => liftFunction(other)
}
}

private def typeConstructorOf[A: WeakTypeTag]: Type =
weakTypeOf[A].typeConstructor.etaExpand
}

}
1 change: 0 additions & 1 deletion core/src/main/scala/cats/arrow/FunctionK.scala
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,4 @@ object FunctionK extends FunctionKMacroMethods {
* The identity transformation of `F` to `F`
*/
def id[F[_]]: FunctionK[F, F] = new FunctionK[F, F] { def apply[A](fa: F[A]): F[A] = fa }

}
48 changes: 31 additions & 17 deletions tests/src/test/scala/cats/tests/FunctionKSuite.scala
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package cats.tests

import cats.Id
import cats.arrow.FunctionK
import cats.data.EitherK
import cats.data.NonEmptyList
import cats.data.{EitherK, NonEmptyList}
import cats.laws.discipline.arbitrary._
import cats.syntax.eq._
import org.scalacheck.Prop._
import cats.{Applicative, Id}

class FunctionKSuite extends CatsSuite {
type OptionOfNel[+A] = Option[NonEmptyList[A]]

val listToOption = new FunctionK[List, Option] { def apply[A](a: List[A]): Option[A] = a.headOption }
val listToVector = new FunctionK[List, Vector] { def apply[A](a: List[A]): Vector[A] = a.toVector }
Expand All @@ -32,55 +32,55 @@ class FunctionKSuite extends CatsSuite {
test("compose") {
forAll { (list: List[Int]) =>
val listToList = optionToList.compose(listToOption)
assert(listToList(list) === (list.take(1)))
assert(listToList(list) === list.take(1))
}
}

test("andThen") {
forAll { (list: List[Int]) =>
val listToList = listToOption.andThen(optionToList)
assert(listToList(list) === (list.take(1)))
assert(listToList(list) === list.take(1))
}
}

test("id is identity") {
forAll { (list: List[Int]) =>
assert(FunctionK.id[List].apply(list) === (list))
assert(FunctionK.id[List].apply(list) === list)
}
}

test("or") {
val combinedInterpreter = Test1FK.or(Test2FK)
forAll { (a: Int, b: Int) =>
assert(combinedInterpreter(EitherK.left(Test1(a))) === (a))
assert(combinedInterpreter(EitherK.right(Test2(b))) === (b))
assert(combinedInterpreter(EitherK.left(Test1(a))) === a)
assert(combinedInterpreter(EitherK.right(Test2(b))) === b)
}
}

test("and") {
val combinedInterpreter = listToOption.and(listToVector)
forAll { (list: List[Int]) =>
val prod = combinedInterpreter(list)
assert(prod.first === (list.headOption))
assert(prod.second === (list.toVector))
assert(prod.first === list.headOption)
assert(prod.second === list.toVector)
}
}

test("lift simple unary") {
def optionToList[A](option: Option[A]): List[A] = option.toList
val fOptionToList = FunctionK.lift(optionToList _)
forAll { (a: Option[Int]) =>
assert(fOptionToList(a) === (optionToList(a)))
assert(fOptionToList(a) === optionToList(a))
}

val fO2I: FunctionK[Option, Iterable] = FunctionK.lift(Option.option2Iterable _)
forAll { (a: Option[String]) =>
assert(fO2I(a).toList === (Option.option2Iterable(a).toList))
assert(fO2I(a).toList === Option.option2Iterable(a).toList)
}

val fNelFromListUnsafe = FunctionK.lift(NonEmptyList.fromListUnsafe _)
forAll { (a: NonEmptyList[Int]) =>
assert(fNelFromListUnsafe(a.toList) === (NonEmptyList.fromListUnsafe(a.toList)))
assert(fNelFromListUnsafe(a.toList) === NonEmptyList.fromListUnsafe(a.toList))
}
}

Expand All @@ -89,14 +89,29 @@ class FunctionKSuite extends CatsSuite {
def optionToList[A](option: Option[A]): List[A] = option.toList
val fOptionToList = cats.arrow.FunctionK.lift(optionToList _)
forAll { (a: Option[Int]) =>
assert(fOptionToList(a) === (optionToList(a)))
assert(fOptionToList(a) === optionToList(a))
}
}

test("lift compound unary") {
val fNelFromList = FunctionK.lift[List, λ[α => Option[NonEmptyList[α]]]](NonEmptyList.fromList _)
val fNelFromList = FunctionK.lift[List, OptionOfNel](NonEmptyList.fromList)
forAll { (a: List[String]) =>
assert(fNelFromList(a) === (NonEmptyList.fromList(a)))
assert(fNelFromList(a) === NonEmptyList.fromList(a))
}
}

test("lift eta-expanded function") {
val fSomeNel = FunctionK.lift[NonEmptyList, OptionOfNel](Applicative[Option].pure)
forAll { (a: NonEmptyList[Int]) =>
assert(fSomeNel(a) === Some(a))
}
}

test("lift a function directly") {
def headOption[A](list: List[A]): Option[A] = list.headOption
val fHeadOption = FunctionK.liftFunction[List, Option](headOption)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A question for my understanding, could this be

Suggested change
val fHeadOption = FunctionK.liftFunction[List, Option](headOption)
val fHeadOption = FunctionK.lift[List, Option](headOption)

Isn't it forwarding to liftFunction anyway ? Or it's testing the public liftFunction explicitly on purpose ?
Apologies if this doesn't make sense :).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's testing liftFunction explicitly. Do you think we should also try to add a dotty version? It has native polymorphic functions, but we don't provide a way to convert them to FunctionK and I think type inference is still not great.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense, thanks for clarifying. About the dotty version, I guess it's related to #2553 that's still an open issue for what I understand and specifically there is work going on as note here #2553 (comment). We could sync a dotty specific version with that effort.
So what do you thing (you and others) about letting this PR move on as it improves the current situation described here #3400 and might make some user's life easier ? My approval goes in that direction. Of course I happy to do otherwise if the general preference is different.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I agree this can be merged and I can try to add a dotty version in another PR 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brilliant, thank 🙇

forAll { (a: List[Int]) =>
assert(fHeadOption(a) === a.headOption)
}
}

Expand All @@ -105,5 +120,4 @@ class FunctionKSuite extends CatsSuite {
assert(compileErrors("FunctionK.lift(sample[String])").nonEmpty)
assert(compileErrors("FunctionK.lift(sample[Nothing])").nonEmpty)
}

}