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

Port tests to Dotty #3552

Closed
wants to merge 14 commits into from
5 changes: 2 additions & 3 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ isTravisBuild in Global := sys.env.contains("TRAVIS")

val scalaCheckVersion = "1.14.3"

val munitVersion = "0.7.10"
val munitVersion = "0.7.12"

val disciplineVersion = "1.0.3"

val disciplineScalatestVersion = "2.0.0"
val disciplineMunitVersion = "0.2.3"
val disciplineMunitVersion = "0.2.4"

val kindProjectorVersion = "0.11.0"

Expand Down Expand Up @@ -154,7 +154,6 @@ lazy val includeGeneratedSrc: Setting[_] = {

lazy val disciplineDependencies = Seq(
libraryDependencies ++= Seq(
("org.scalacheck" %%% "scalacheck" % scalaCheckVersion).withDottyCompat(scalaVersion.value),
"org.typelevel" %%% "discipline-core" % disciplineVersion
)
)
Expand Down
4 changes: 2 additions & 2 deletions free/src/main/scala/cats/free/Free.scala
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,12 @@ sealed abstract class Free[S[_], A] extends Product with Serializable {
final def foldStep[B](
onPure: A => B,
onSuspend: S[A] => B,
onFlatMapped: ((S[X], X => Free[S, A]) forSome { type X }) => B
onFlatMapped: ((S[Any], Any => Free[S, A])) => B
Copy link
Member

Choose a reason for hiding this comment

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

Isn't foldStep public? This would be a rather drastic change if so. I would prefer to see this made package-private (thus, bincompat) and have a 2.0-specific enrichment which uses forSome in this skolem rank, while a 3.0-specific enrichment uses a higher-rank universal to achieve the same thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is bincompat due to type erasure, but I believe having separate sources here is probably a good idea 👍

Copy link
Member

Choose a reason for hiding this comment

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

It's definitely bincompat, I was mostly just concerned about the loss of type safety in a public-facing API.

): B =
this.step match {
case Pure(a) => onPure(a)
case Suspend(a) => onSuspend(a)
case FlatMapped(Suspend(fa), f) => onFlatMapped((fa, f))
case FlatMapped(Suspend(fa), f) => onFlatMapped((fa.asInstanceOf[S[Any]], f.asInstanceOf[Any => Free[S, A]]))
case _ => sys.error("FlatMapped should be right associative after step")
}

Expand Down
1 change: 1 addition & 0 deletions free/src/test/scala/cats/free/FreeApplicativeSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import cats.syntax.eq._
import org.scalacheck.Prop._
import cats.tests.CatsSuite
import org.scalacheck.{Arbitrary, Gen}
import cats._

class FreeApplicativeSuite extends CatsSuite {
import FreeApplicativeSuite._
Expand Down
2 changes: 1 addition & 1 deletion free/src/test/scala/cats/free/FreeSuite.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package cats.free

import cats.{:<:, Foldable, Functor, Id, Monad, Traverse}
import cats._
import cats.arrow.FunctionK
import cats.data.EitherK
import cats.instances.all._
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class ApplicativeErrorSuite extends CatsSuite {
assert(compileErrors("e2.attemptNarrow[Num]").nonEmpty)

val e3: Either[List[T[String]], Unit] = List(Str).asLeft[Unit]
assert(compileErrors("e3.attemptNarrow[List[Str.type]]").nonEmpty)
//assertEquals(compileErrors("e3.attemptNarrow[List[Str.type]]"), "")
Copy link
Member Author

Choose a reason for hiding this comment

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

This is an odd one, because when I try it in the console it gives me an error but munit returns an empty string here

}

test("attemptT syntax creates an EitherT") {
Expand Down
25 changes: 0 additions & 25 deletions tests/src/test/scala/cats/tests/ExtraRegressionSuite.scala

This file was deleted.

2 changes: 1 addition & 1 deletion tests/src/test/scala/cats/tests/RegressionSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class RegressionSuite extends CatsSuite with ScalaVersionSpecificRegressionSuite

test("#500: foldMap - traverse consistency") {
assert(
List(1, 2, 3).traverse(i => Const.of[List[Int]](List(i))).getConst == List(1, 2, 3).foldMap(List(_))
List(1, 2, 3).traverse(i => Const[List[Int], List[Int]](List(i))).getConst == List(1, 2, 3).foldMap(List(_))
Copy link
Member Author

Choose a reason for hiding this comment

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

Dotty was inferring Int => Int, I'll try to reach out to the dotty team on how to get this working

)
}

Expand Down