Port tests to Dotty #3552

LukaJCB · 2020-08-06T04:50:50Z

No description provided.

LukaJCB · 2020-08-06T04:51:45Z

.scalafmt.conf

@@ -10,3 +10,4 @@ docstrings = JavaDoc
 newlines.afterCurlyLambda = preserve
 docstrings.style = Asterisk
 docstrings.oneline = unfold
+project.excludeFilters = [ "core/src/main/scala-3.x" ]


scalafmt can't deal with dotty syntax yet.

LukaJCB · 2020-08-06T04:52:22Z

core/src/main/scala-3.x/src/main/scala/cats/syntax/MonadOps.scala

+
+import cats.{Monad, Alternative}
+
+final class MonadOps[F[_], A](private val fa: F[A]) extends AnyVal {


Had to split this out due to scala/scala3#9480

Doesn't this flip the order of parameters though? That could be moderately source-incompatible. More importantly, it's inconsistent with the other syntax classes.

I actually don't know how dotty compiles this down. Is explicitly still a thing? Do you have a suggestion on how to fix this given the limitation above?

I would summon @smarter actually if I hit this. Probably worth looking at the bytecode quickly to see what the parameter order is. Can we simply put the using block second?

It's source-compatible (to explicitly pass parameters to a using block, you have to write using at use-site in front of the block too). I suggested putting the using block first because doing the implicit search requires instantiating type variables which in turns means that in increment.whileM_(inspect(i => i > 4)).run(3) we can figure out that i needs to be Int, see scala/scala3#9480 (comment) for the full explanation.

That would increase the maintenance burden though. But if you did want to rewrite all the ops classes for dotty, then you might as well use extension syntax which would look more natural:

extension [F[_], A](fa: F[A])(using M: Monad[F]) { def whileM_(p: F[Boolean]): F[Unit] = ??? def untilM(...) }

I'm just concerned that we're going to end up hitting something like this again in the future, but once we release on Dotty, we can't futz with the binary compatibility any further.

Well, you can break BC every time we break BC which is every six weeks until 3.0 is out probably, but afterwards it gets tricky yeah :) (though I guess you can always play trick with something like a package-protected overload to preserve BC?)

Are we promising binary compatibility for dotty artifacts? Should we? (We certainly don't test it yet)

I don't think we're promising BC until Scala 3 goes final. Once it does though, we 100% should be promising it.

LukaJCB · 2020-08-06T04:53:18Z

core/src/main/scala/cats/Invariant.scala

@@ -122,7 +122,6 @@ object Invariant extends ScalaVersionSpecificInvariantInstances with InvariantIn
  implicit def catsFlatMapForSortedMap[K]: FlatMap[SortedMap[K, *]] =
    cats.instances.sortedMap.catsStdInstancesForSortedMap[K]
  implicit def catsBimonadForFunction0: Bimonad[Function0] = cats.instances.function.catsStdBimonadForFunction0
-  implicit def catsMonadForFunction1[I]: Monad[I => *] = cats.instances.function.catsStdMonadForFunction1[I]


Moved this down because dotty thinks Monad[List] and Monad[Function1[Int, *]] are ambiguous

LukaJCB · 2020-08-06T04:53:35Z

core/src/main/scala/cats/Invariant.scala

  implicit def catsCommutativeFlatMapForTuple2[X](implicit X: CommutativeSemigroup[X]): CommutativeFlatMap[(X, *)] =
    cats.instances.tuple.catsStdCommutativeFlatMapForTuple2[X]
+
+  implicit def catsApplicativeForArrow[F[_, _], A](implicit F: Arrow[F]): Applicative[F[A, *]] =


Same with this one

core/src/main/scala/cats/Traverse.scala

LukaJCB · 2020-08-06T04:54:22Z

tests/src/test/scala-2.13+/cats/tests/ScalaVersionSpecific.scala

@@ -10,6 +10,7 @@ import cats.syntax.parallel._
 import cats.syntax.traverse._
 import cats.syntax.eq._
 import org.scalacheck.Prop._
+import cats.catsInstancesForId


Id instances are no longer automatically in scope in dotty.

LukaJCB · 2020-08-06T04:55:11Z

tests/src/test/scala/cats/tests/IndexedStateTSuite.scala

@@ -506,7 +509,7 @@ class IndexedStateTSuite extends CatsSuite {
    Alternative[IndexedStateT[ListWrapper, Int, Int, *]]
    Applicative[IndexedStateT[ListWrapper, Int, Int, *]]
    Apply[IndexedStateT[ListWrapper, Int, Int, *]]
-    Functor[IndexedStateT[ListWrapper, Int, Int, *]]
+    //Functor[IndexedStateT[ListWrapper, Int, Int, *]]


Ambiguous here because both G and F are Functors

LukaJCB · 2020-08-06T04:55:27Z

tests/src/test/scala/cats/tests/OptionTSuite.scala

@@ -536,7 +536,7 @@ class OptionTSuite extends CatsSuite {
    Traverse[OptionT[List, *]]

    implicit val T: Traverse[ListWrapper] = ListWrapper.traverse
-    implicit val M: Monad[ListWrapper] = ListWrapper.monad
+    //implicit val M: Monad[ListWrapper] = ListWrapper.monad


LukaJCB · 2020-08-06T04:56:01Z

tests/src/test/scala/cats/tests/ParallelSuite.scala

@@ -88,34 +83,37 @@ class ParallelSuite extends CatsSuite with ApplicativeErrorForEitherTest with Sc
    }
  }

-  type ListTuple2[A, B] = List[(A, B)]
+  case class ListTuple2[A, B](value: List[(A, B)])


Had to turn this into a case class for it to work

LukaJCB · 2020-08-06T04:56:27Z

tests/src/test/scala/cats/tests/ReducibleSuite.scala

@@ -131,14 +131,14 @@ class ReducibleSuiteAdditional extends CatsSuite {
    val n = 100000
    val xs = NES(0, Stream.from(1))

-    assert(xs.reduceMapM(i => if (i < n) Right(i) else Left(i)) === Left(n))
+    assert(xs.reduceMapM(i => if (i < n) i.asRight[Int] else i.asLeft[Int]) === Left(n))


See scala/scala3#9479

…dotty-tests

LukaJCB · 2020-08-31T19:53:46Z

tests/src/test/scala/cats/tests/ApplicativeErrorSuite.scala

@@ -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]]"), "")


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

LukaJCB · 2020-08-31T19:54:16Z

tests/src/test/scala/cats/tests/ExtraRegressionSuite.scala

-  /**
-   * Placed here to work around scala/bug#6260 on scala 2.10
-   */
-  test("#1802 duplicated method name") {


Deleted this for now, since it was giving me an error and seems like this is pretty old

LukaJCB · 2020-08-31T19:55:10Z

tests/src/test/scala/cats/tests/RegressionSuite.scala

@@ -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(_))


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

codecov-commenter · 2020-09-08T15:13:14Z

Codecov Report

Merging #3552 into master will decrease coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3552      +/-   ##
==========================================
- Coverage   91.33%   91.27%   -0.06%     
==========================================
  Files         386      386              
  Lines        8628     8611      -17     
  Branches      260      269       +9     
==========================================
- Hits         7880     7860      -20     
- Misses        748      751       +3

djspiewak · 2020-09-11T22:55:35Z

core/src/main/scala-3.x/src/main/scala/cats/syntax/MonadOps.scala

+
+import cats.{Monad, Alternative}
+
+final class MonadOps[F[_], A](private val fa: F[A]) extends AnyVal {


Doesn't this flip the order of parameters though? That could be moderately source-incompatible. More importantly, it's inconsistent with the other syntax classes.

djspiewak · 2020-09-11T22:58:24Z

free/src/main/scala/cats/free/Free.scala

-    onFlatMapped: ((S[X], X => Free[S, A]) forSome { type X }) => B
+    onFlatMapped: ((S[Any], Any => Free[S, A])) => B


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.

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

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

LukaJCB · 2020-09-14T18:35:08Z

Seeing a new failure now, opened a bug report at scala/scala3#9793

larsrh · 2020-10-17T08:02:42Z

This appears to be fixed in scala/scala3#9937, which is scheduled for 3.0.0-M1, so I guess we'll have to wait for that.

larsrh · 2020-10-17T08:21:40Z

I just realized that the milestone date is end of year 😬 Maybe let's try this instead: 300a9cd

codecov-io · 2020-10-17T09:09:23Z

Codecov Report

Merging #3552 into master will decrease coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #3552      +/-   ##
==========================================
- Coverage   91.33%   91.29%   -0.05%     
==========================================
  Files         386      386              
  Lines        8628     8611      -17     
  Branches      260      257       -3     
==========================================
- Hits         7880     7861      -19     
- Misses        748      750       +2

smarter · 2020-10-17T10:22:24Z

M1 should be out next week, the milestone date hasn't been updated

larsrh · 2020-10-17T10:32:35Z

I made a new PR that is up-to-date: #3636. Obviously we can continue the discussion there.

LukaJCB added 2 commits August 5, 2020 17:53

WIP dotty 0.26-RC1

9e44cfe

More fixes

d90679d

LukaJCB commented Aug 6, 2020

View reviewed changes

core/src/main/scala/cats/Traverse.scala Outdated Show resolved Hide resolved

LukaJCB commented Aug 6, 2020

View reviewed changes

scalafmt

824deb1

djspiewak self-requested a review August 17, 2020 19:49

LukaJCB added 3 commits August 18, 2020 11:22

Merge branch 'master' into dotty-tests

d68da01

All tests run in 0.27.0-RC1

94f9dde

Merge branch 'dotty-tests' of https://github.com/typelevel/cats into …

610c528

…dotty-tests

LukaJCB commented Aug 31, 2020

View reviewed changes

LukaJCB added 4 commits August 31, 2020 16:08

scalafmt

2f17ada

Update CI build to use latest dotty milestone

7c7b8f7

Don't run coverage check for dotty

3d9b407

Dotty syntax change

8cca359

LukaJCB requested review from travisbrown, barambani and kailuowang September 8, 2020 15:45

Merge branch 'master' into dotty-tests

3d773d4

LukaJCB marked this pull request as ready for review September 8, 2020 15:46

Reshuffle instances to provide binCompat with cats 2.2.0

6ae0881

LukaJCB force-pushed the dotty-tests branch from 18f1233 to 6ae0881 Compare September 9, 2020 17:11

djspiewak reviewed Sep 11, 2020

View reviewed changes

LukaJCB added 2 commits September 14, 2020 12:24

Merge branch 'master' into dotty-tests

d94e07a

Split Free#foldStep into separate source files

8abdf3c

LukaJCB requested a review from djspiewak September 14, 2020 17:29

larsrh mentioned this pull request Oct 17, 2020

Port tests to Dotty (the 2nd) #3636

Merged

larsrh closed this Oct 17, 2020

larsrh deleted the dotty-tests branch October 17, 2020 10:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port tests to Dotty #3552

Port tests to Dotty #3552

LukaJCB commented Aug 6, 2020

LukaJCB Aug 6, 2020

LukaJCB Aug 6, 2020

djspiewak Sep 11, 2020

LukaJCB Sep 13, 2020

djspiewak Sep 13, 2020

smarter Sep 13, 2020

smarter Sep 14, 2020

djspiewak Sep 14, 2020

smarter Sep 14, 2020

LukaJCB Sep 14, 2020

djspiewak Sep 14, 2020

LukaJCB Aug 6, 2020

LukaJCB Aug 6, 2020

LukaJCB Aug 6, 2020

LukaJCB Aug 6, 2020

LukaJCB Aug 6, 2020

LukaJCB Aug 6, 2020

LukaJCB Aug 6, 2020

LukaJCB Aug 31, 2020

LukaJCB Aug 31, 2020

LukaJCB Aug 31, 2020

codecov-commenter commented Sep 8, 2020 •

edited

Loading

djspiewak Sep 11, 2020

djspiewak Sep 11, 2020

LukaJCB Sep 13, 2020

djspiewak Sep 13, 2020

LukaJCB commented Sep 14, 2020

larsrh commented Oct 17, 2020

larsrh commented Oct 17, 2020

codecov-io commented Oct 17, 2020

smarter commented Oct 17, 2020

larsrh commented Oct 17, 2020


		import cats.{Monad, Alternative}

		final class MonadOps[F[_], A](private val fa: F[A]) extends AnyVal {

		onFlatMapped: ((S[X], X => Free[S, A]) forSome { type X }) => B
		onFlatMapped: ((S[Any], Any => Free[S, A])) => B

Port tests to Dotty #3552

Port tests to Dotty #3552

Conversation

LukaJCB commented Aug 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented Sep 8, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LukaJCB commented Sep 14, 2020

larsrh commented Oct 17, 2020

larsrh commented Oct 17, 2020

codecov-io commented Oct 17, 2020

Codecov Report

smarter commented Oct 17, 2020

larsrh commented Oct 17, 2020

codecov-commenter commented Sep 8, 2020 •

edited

Loading