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

Doctests of package objects are in the wrong package #120

Closed
fthomas opened this issue Aug 25, 2018 · 7 comments · Fixed by #121
Closed

Doctests of package objects are in the wrong package #120

fthomas opened this issue Aug 25, 2018 · 7 comments · Fixed by #121

Comments

@fthomas
Copy link
Collaborator

fthomas commented Aug 25, 2018

I tried to upgrade sbt-doctest from 0.8.0 to 0.9.1 in refined but the Doctest of this package object fails to compile because it is now in a different package.

Here is the Doctest generated by 0.8.0:

package eu.timepit.refined

object packageDoctest
    extends _root_.org.scalacheck.Properties("package.scala") {

  def sbtDoctestTypeEquals[A](a1: => A)(a2: => A): _root_.scala.Unit = ()
  def sbtDoctestReplString(any: _root_.scala.Any): _root_.scala.Predef.String = {
    val s = _root_.scala.runtime.ScalaRunTime.replStringOf(any, 1000).init
    if (s.headOption == Some('\n')) s.tail else s
  }

  include(new _root_.org.scalacheck.Properties("package.scala:9: W") {
    val d: W.`3.14`.T = 3.14

    val s: W.`"abc"`.T = "abc"
  })

}

and this is what 0.9.1 generates:

package eu.timepit

object packageDoctest
    extends _root_.org.scalacheck.Properties("package.scala") {

  def sbtDoctestTypeEquals[A](a1: => A)(a2: => A): _root_.scala.Unit = {
    val _ = () => (a1, a2)
  }
  def sbtDoctestReplString(any: _root_.scala.Any): _root_.scala.Predef.String = {
    val s = _root_.scala.runtime.ScalaRunTime.replStringOf(any, 1000).init
    if (s.headOption == Some('\n')) s.tail else s
  }

  include(new _root_.org.scalacheck.Properties("package.scala:9: W") {
    val d: W.`3.14`.T = 3.14

    val s: W.`"abc"`.T = "abc"
  })

}

The difference between these two that to causes the compilation failure is in the first line:

--- packageDoctest_0.8.0.scala	2018-08-24 22:59:56.000000000 +0200
+++ packageDoctest_0.9.1.scala	2018-08-24 22:53:34.000000000 +0200
@@ -1,4 +1,4 @@
-package eu.timepit.refined
+package eu.timepit

I think the behaviour of 0.8.0 with regards to package objects should be restored.

@jozic
Copy link
Collaborator

jozic commented Aug 25, 2018

this was probably introduced by me when I migrated to scalameta. Will try to fix it

jozic added a commit to jozic/sbt-doctest that referenced this issue Aug 26, 2018
tkawachi added a commit that referenced this issue Aug 27, 2018
members of package objects got correct package name (fixes #120)
@fthomas
Copy link
Collaborator Author

fthomas commented Aug 27, 2018

Thanks for the quick fix and release, @jozic and @tkawachi. Your fix worked but it seems that I'm now hitting another wall. The build log suggests that there is a conflict between sbt-scalafmt and sbt-doctest wrt to Scalameta. I've no idea what is going on there.

@jozic
Copy link
Collaborator

jozic commented Aug 28, 2018

looks like sbt-scalafmt also brings scalameta, but super old one (at least version wise) - 1.7.0. Sbt-doctest is also a plugin, so i believe they share classpath
i guess we have two ways to solve this

  • rollback changes in sbt-doctest (sounds gross, as i think scalameta is the way to go for all tools of this sort)
  • ask scalafmt maintainers to release latest stable version with latest stable scalameta (I will open a issue in scalafmt)

cc @olafurpg

any other ideas?

@olafurpg
Copy link
Contributor

It's a shame that sbt forces all plugins on the same classpath. Even if everyone use the latest scalameta version and we never introduce a binary breaking change again you will still encounter runtime classpath exceptions when parsing files with XML literals due to sbt/zinc#546

I'm afraid that the only robust way to bring in a dependency like Scalameta into an sbt plugin is to fetch the artifacts of your library and classload. Here is a write-up about the problem https://docs.google.com/document/d/1Y1MVMjVQ8P25YEI3uvh86gg3n61WZng0hr1qRUS_S6I/edit# We use this approach in sbt-scalafix https://github.com/scalacenter/sbt-scalafix/blob/b66445a78d1335d166e09cbce56ff07c636613e0/src/main/scala/scalafix/sbt/ScalafixPlugin.scala#L186-L191

The steps are roughly

I'm happy to give more guidance on this approach. It may seem like a lot of ceremony but I personally found it very helpful to write and document the interfaces module. Having spent almost 3 years writing sbt plugins that use scalameta I believe this is the only way to do it correctly.

@jozic
Copy link
Collaborator

jozic commented Aug 28, 2018

@fthomas can you try using scalafmt v1.6.0-RC4 as suggested by @olafurpg in scalameta/scalafmt#1263?

@fthomas
Copy link
Collaborator Author

fthomas commented Aug 28, 2018

@jozic Yes, that worked. fthomas/refined#559 is now green.

@jozic
Copy link
Collaborator

jozic commented Aug 28, 2018

great! I guess this can be a workaround for now.

@fthomas @tkawachi
Do you guys think it's worth to update README explaining that scalameta in plugins classpath has to be 3.7.x+ and as such sbt-scalafmt (if used) should be 1.6.x?

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

Successfully merging a pull request may close this issue.

3 participants