Make Free/FreeApplicative constructors private

[ceedubs]

There are a couple of reasons this might be a good idea.

Currently it's possible to observe a difference when calling
.map(identity) on a Free instance:

scala> import cats.free._; import cats.implicits._
import cats.free._
import cats.implicits._

scala> def f(x: Free[Option, Int]) = x match {
     |   case Free.Pure(i) => i
     |   case _ => 0
     | }
f: (x: cats.free.Free[Option,Int])Int

scala> val x: Free[Option, Int] = Free.pure(3)
x: cats.free.Free[Option,Int] = Pure(3)

scala> f(x)
res0: Int = 3

scala> f(x.map(identity))
res1: Int = 0

Making these private also frees us up to change the internal
representation in the future without breaking user code.

[adelbertc]

If I remember correctly I made this change before but reverted it - on one hand hiding the constructors ensures lawful-ness.. on the other hand hiding constructors prevents users from writing their own tail-recursive implementation, though perhaps we can say the methods we provide on Free(Applicative) are tail recursive and people should just use those instead? I think @non was the one who convinced me?

[non]

The place where I definite care about this is Streaming, where it's totally plausible that a user might want to write their own tail-recursive methods. I don't have as much sense whether that is an issue here.

[non]

(There's also a legitimate Travis build failure.)

[codecov-io]

Current coverage is 75.99%

Merging #613 into master will decrease coverage by -0.45% as of ec7de90

@@            master    #613   diff @@
======================================
  Files          161     159     -2
  Stmts         2208    2066   -142
  Branches        70      70       
  Methods          0       0       
======================================
- Hit           1688    1570   -118
  Partial          0       0       
+ Missed         520     496    -24

Review entire Coverage Diff as of ec7de90

Powered by Codecov. Updated on successful CI builds.

[ceedubs]

Thanks for the input @adelbertc and @non. I'm inclined to make these private for the reasons stated in my original comment. It's always easier to open them up in the future if people need access to them for custom tail-recursive methods than it is to remove them once they are part of a public API. I would guess that people are more likely to want to write custom tail-recursive methods for Streaming, and we may just have to bite the bullet there. What do you think?

I've also fixed the build error (thanks Travis).

[non]

I'm fine with making these private (and opening them up later if necessary).

I just wanted to make the argument for why some types (e.g. Streaming) should remain public.

[non]

👍

[aryairani]

Squash? :)

[ceedubs]

@refried I can squash if people want me to, but I think per Cats contributor guidelines that would mean closing this PR and opening a new one from a different branch which seems like it might be overkill for just having one extra commit. Happy to do whatever people want, though.

[aryairani]

@ceedubs Oops, I hadn’t read them!

I wasn’t trying to reduce the number of commits in the PR, but reduce the number of non-compiling commits. Maybe it’s not a big deal though?

[adelbertc]

👍