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

Automate migration to 1.0 with Scalafix #1791

Closed
gabro opened this issue Aug 6, 2017 · 6 comments
Closed

Automate migration to 1.0 with Scalafix #1791

gabro opened this issue Aug 6, 2017 · 6 comments

Comments

@gabro
Copy link
Contributor

gabro commented Aug 6, 2017

Now that the 1.0 is close, I got a chance to take a look at the breaking changes it introduces and I noticed that the vast majority of them are well within the possibilities of Scalafix.

So, well, why not just automate the migration away?
I started experimenting on it and it's promising! So far I got:

  • removal of Unapply machinery (just a bunch of symbol renames, but, hey, I wanted to start easy)
  • removal of CartesianBuilder syntax (this is trickier, but the rewrite is still around 50 lines of code)

Would you be interested in this?

If so, I think it would be really cool to ship the rewrites with the final release of 1.0.0 so that anyone can access them easily simply by having cats as a dependency. I'll push my work in progress soon, but in the meantime here's some diff I was able to fully automate:

   import cats.instances.option._
-  import cats.syntax.cartesian._
+  import cats.syntax.apply._
   val o1: Option[Int] = Some(42)
   val o2: Option[String] = Some("hello")
   val o3: Option[Int] = Some(2)
-  (o1 |@| o2).map((i: Int, s: String) => i.toString ++ s)
-  (o1 |@| o2).tupled
-  (o1 |@| o2 |@| o3).map(_ + _ + _)
+  (o1, o2).mapN((i: Int, s: String) => i.toString ++ s)
+  (o1, o2).tupled
+  (o1, o2, o3).mapN(_ + _ + _)

   import cats.{Semigroup, Eq}
   import cats.instances.string._
   import cats.instances.double._
   case class Foo(a: String, c: List[Double])

-  (Semigroup[String] |@| Semigroup[List[Double]]).imap(Foo.apply)(Function.unlift(Foo.unapply))
-
-  (Eq[Double] |@| Eq[String]).contramap { (a: Foo) => (2, "bar") }
+  (Semigroup[String], Semigroup[List[Double]]).imapN(Foo.apply)(Function.unlift(Foo.unapply))
+  (Eq[Double], Eq[String]).contramapN { (a: Foo) => (2, "bar") }

A couple of more things to notice:

  • the rewrites are semantic, meaning they're using the semantic information not just the syntactic ones (e.g. when renaming map we're looking for _root_.cats.syntax.CartesianBuilder#CartesianBuilder2.map., not just any map)
  • I'm currently targeting the 0.9.0 -> 1.0.0 migration, although this would probably be easy to extend to prior versions
@kailuowang
Copy link
Contributor

Love the idea. It also makes it easier for people to start contribute to #1762

@gabro
Copy link
Contributor Author

gabro commented Aug 9, 2017

#1793 is now merged!

There are still a lot of low-hanging fruits, but in the meantime it would be great if anyone could try running the rewrites in repos using cats and report feedbacks here.

Instructions here https://github.com/typelevel/cats/blob/master/scalafix/README.md

@kailuowang
Copy link
Contributor

Thanks so much again. I am waiting on this PR #1806 before updating the release note. Then I am thinking we should broadcasting it on twitter, do you mind if I mention your twitter handler?

@gabro
Copy link
Contributor Author

gabro commented Aug 9, 2017

happy to finally contribute back! :) I tweeted it out myself, but I guess some more noise won't hurt ;)

https://twitter.com/gabro27/status/895289070628294656

@mfirry
Copy link

mfirry commented Feb 28, 2018

I've just tried this migration and I get a weird import fix:

-import cats.syntax.cartesian._
+import cats.syntax.semigroupal._import cats.syntax.apply._

@ceedubs
Copy link
Contributor

ceedubs commented Sep 30, 2018

This was mostly resolved by #1793, and at this point Cats 1.0 has been out long enough that I doubt anyone is only now upgrading to it. I'm going to go ahead and close this out.

@ceedubs ceedubs closed this as completed Sep 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants