Cats: Consider adding scalafmt

Created on 16 Jul 2016  ·  12Comments  ·  Source: typelevel/cats

As @mpilquist pointed out in #1195, scalafmt can both check that javadoc-style comments are used _and_ automatically format them for you. We may want to consider incorporating it into the project in some way.

build in progress

Most helpful comment

Both of these issues have been fixed now. I'm hoping to get a release out this weekend. If you stumble into other issue please let me know :)

All 12 comments

I'm happy to answer any questions. I am also open to add new flags to capture the Typelevel coding style.

@olafurpg thank you, that's a very kind offer!

I'm not sure that I'll get to this right away, but I definitely want to look into it. And maybe someone else will beat me to it :)

@olafurpg I just gave scalafmt a spin on the cats codebase. To start off, I just used all of the default parameters except used --javaDocs since we've settled on that convention. I've had a skepticism with code formatters in the past, but I'm delighted to say that I was really pleased with the results! You've done an excellent job.

There are only 2 items that I've noticed so far that I'm not entirely fond of, and it isn't obvious to me which rules should be changed to make them behave differently (or whether they even can be changed in isolation). Since you offered to answer questions, I'm throwing them out here before digging too deeply myself, but I also don't expect you to bend over backwards to help out. Also please let me know if it would be preferable to open scalafmt issues for these items.

Item 1: _[_]

The first is the more problematic one, because it collides with one of our scalastyle rules (no space before left brackets). Here is an example scalafmt diff:

-trait TransLift[MT[_[_], _]] {
+trait TransLift[MT[_ [_], _]] {

Item 2: self references

The second one isn't too big of a deal, but it seems a little inconsistent.

Here is an example scalafmt diff that seems entirely reasonable to me:

-private[cats] trait ComposedInvariant[F[_], G[_]] extends Invariant[λ[α => F[G[α]]]] { outer =>
+private[cats] trait ComposedInvariant[F[_], G[_]]
+    extends Invariant[λ[α => F[G[α]]]] { outer =>
   def F: Invariant[F]
   def G: Invariant[G]

However, in this other case, the outer => gets moved down to a new line, aligned with the inside of the defs. In general the pattern seems to be that the self-reference is inlined on extends lines but not on with lines.

-private[cats] trait ComposedFunctor[F[_], G[_]] extends Functor[λ[α => F[G[α]]]] with ComposedInvariant[F, G] { outer =>
+private[cats] trait ComposedFunctor[F[_], G[_]]
+    extends Functor[λ[α => F[G[α]]]]
+    with ComposedInvariant[F, G] {
+  outer =>
   def F: Functor[F]
   def G: Functor[G]

Any tips for changing the behavior of either of these?

I'm delighted to say that I was really pleased with the results!

That makes me happy to hear, thanks!

Item 1: _[_]

This is a bug, there should be no space there. I've opened an issue https://github.com/olafurpg/scalafmt/issues/375

Item 2: self references

I agree that scalafmt should be more consistent there. According to my expectations, the outer => in the second example should stay on the same line as the opening {. I've opened an issue https://github.com/olafurpg/scalafmt/issues/376

Thanks a lot for going through the diff. I will attend these issue when I find the time. However, I won't be able to look at it right away since I'm starting a new job this week.

@olafurpg fantastic. Thanks!

Both of these issues have been fixed now. I'm hoping to get a release out this weekend. If you stumble into other issue please let me know :)

Thanks a bunch, @olafurpg!

I'm +1 to this.

My opinion is broadly irrelevant… but I'm :+1: for it as well. Though, the default parameters do a _ton_ of vertical alignment that I'm engaged on a personal crusade against.

My feeling is that even if we don't all get the style we want, we get a consistent style and we can stop having style discussions.

That said cats does not seem as crippled by this as some projects/groups.

This is is done and so can be closed?

closed by #2546

Was this page helpful?
0 / 5 - 0 ratings

Related issues

alexandru picture alexandru  ·  4Comments

diesalbla picture diesalbla  ·  4Comments

fosskers picture fosskers  ·  3Comments

davidabrahams picture davidabrahams  ·  3Comments

peterneyens picture peterneyens  ·  5Comments