workaround for a possible scala bug in show for value class.

[kailuowang]

fixes #1802

[codecov-io]

Codecov Report

Merging #1804 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1804      +/-   ##
==========================================
+ Coverage   94.88%   94.88%   +<.01%     
==========================================
  Files         241      241              
  Lines        4147     4148       +1     
  Branches      107       97      -10     
==========================================
+ Hits         3935     3936       +1     
  Misses        212      212

Impacted Files	Coverage Δ
core/src/main/scala/cats/Show.scala	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8bc6354...e07aa6b. Read the comment docs.

[edmundnoble]

Looks alright to me. I might consider naming it cshow because it's not really private - user-defined functions that require a ContravariantShow will need it, not sure if anyone will use that though.

[dwijnand]

Minimised and reported upstream at scala/bug#10458, if you want to leave a reference in the code.

[kailuowang]

Thanks very much! @dwijnand

[kailuowang]

Now I run into https://issues.scala-lang.org/browse/SI-6260 which is not fixed on 2.10.6
@dwijnand any suggestions?

[dwijnand]

Drop 2.10 support?

Otherwise I suggest seeing if simulacrum can be taught to see the show inherited from ContravariantShow.

[johnynek]

should this be final?

[kailuowang]

I moved away from this solution.

[kailuowang]

I can't get simulacrum to recoganize the super class. So i decided to hand roll the type class boilerplate by simulacrum

[dwijnand]

SI-6260 is scala/bug#6260

[kailuowang]

so I got rid of the simulacrum and thus the duplicated show in Show, but I still can't get around SI-6260 in scala 2.10. I only see 2 options going forward

1. leave everything as is, i.e. close this PR. The work around isn't that terrible, instead of creating the anonymous class instance of Show, use Show.show
2. use the current fix, but give up on 2.10, which means I probably need to remove the regression test to get build passing.

What do you guys think? @johnynek @julien-truffaut @sellout @edmundnoble @dwijnand

[dwijnand]

3. use the current fix, but restrict the test to 2.11+ to avoid SI-6260.

[kailuowang]

@dwijnand what would be the best way to restrict the test to 2.11+? I can think of using sbt-buildinfo to make scala version available in test code. Is there any other better ways?

[dwijnand]

unmanagedSourceDirectories in Test ++= {
  val ss = (scalaSource in Test).value
  if (CrossVersion.partialVersion(scalaVersion.value) exists (_._2 >= 11))
    Seq(file(ss.getPath + "-2.11+"))
  else
    Nil
}

> show crossScalaVersions
[info] * 2.10.6
[info] * 2.11.11
[info] * 2.12.3

> +show test:unmanagedSourceDirectories
[info] Setting version to 2.10.6
[info] * /s/t-test211/src/test/scala-2.10
[info] * /s/t-test211/src/test/scala
[info] * /s/t-test211/src/test/java

[info] Setting version to 2.11.11
[info] * /s/t-test211/src/test/scala-2.11
[info] * /s/t-test211/src/test/scala
[info] * /s/t-test211/src/test/java
[info] * /s/t-test211/src/test/scala-2.11+

[info] Setting version to 2.12.3
[info] * /s/t-test211/src/test/scala-2.12
[info] * /s/t-test211/src/test/scala
[info] * /s/t-test211/src/test/java
[info] * /s/t-test211/src/test/scala-2.11+

[kailuowang]

thanks @dwijnand in scalajs enabled projects the scalaSource is
.jvm/src/test/scala, which means that the settings you gave added /cats/tests/.jvm/src/test/scala-2.11+ instead of
/cats/tests/src/test/scala-2.11+. I am not sure if this test should be jvm specific.
I wonder if there is a key pointing to /cats/tests/src/test/scala to use instead of scalaSource?

[dwijnand]

Hmm, that makes things more complicated :)

unmanagedSourceDirectories in Test ++= {
  val bd = baseDirectory.value
  if (CrossVersion.partialVersion(scalaVersion.value) exists (_._2 >= 11))
    CrossType.Pure.sharedSrcDir(bd, "test").toList map (f => file(f.getPath + "-2.11+"))
  else
    Nil
}

> show tJVM/test:unmanagedSourceDirectories
[info] * /s/t-test211/.jvm/src/test/scala-2.12
[info] * /s/t-test211/.jvm/src/test/scala
[info] * /s/t-test211/.jvm/src/test/java
[info] * /s/t-test211/src/test/scala-2.12
[info] * /s/t-test211/src/test/scala
[info] * /s/t-test211/src/test/scala-2.11+

> show tJS/test:unmanagedSourceDirectories
[info] * /s/t-test211/.js/src/test/scala-2.12
[info] * /s/t-test211/.js/src/test/scala
[info] * /s/t-test211/.js/src/test/java
[info] * /s/t-test211/src/test/scala-2.12
[info] * /s/t-test211/src/test/scala
[info] * /s/t-test211/src/test/scala-2.11+

[kailuowang]

Thanks! @dwijnand it would've taken me weeks to figure out. fix pushed.

[edmundnoble]

Does this mean that 2.10 users won't be able to use Show?

[kailuowang]

@edmundnoble , it means that they will use Show.show when writing a Show instance for a value class.
And that's mostly due to scala/bug#6260 than anything else.