Make Quill compatible with Scala 2.12.2 #770

mosyp · 2017-05-10T15:08:36Z

Fixes #231

Problem

Build fails against 2.12

Solution

Make Quill compatible with 2.12.2, more in todo list

adjust travis build
adjust new flags for scalac 2.12.2
fix failing tests (deprecations, not exhaustive match etc.)
workaround for bug intoduced with 2.12.2 with macros untypechek (manual creating of private[this] param with different name to accessor)
Unit test all changes
Update README.md if applicable
Add [WIP] to the pull request title if it's work in progress
Squash commits that aren't meaningful changes
Run sbt scalariformFormat test:scalariformFormat to make sure that the source files are formatted

@getquill/maintainers

mosyp · 2017-05-10T16:13:19Z

I've been trying to add some dummy abstract methods to Quoted / make it abstract class and compile it without -Xsource:2.11 and both without success.

fwbrasil · 2017-05-10T18:50:19Z

@mentegy what's the error message?

mosyp

@getquill/maintainers
After doing some research on this it seems like current workaround with "-Xsource:2.11" does not work for us. We need to figure out how to fix such errors introduced with 2.12

mosyp · 2017-05-10T18:37:28Z

quill-core/src/test/scala/io/getquill/quotation/QuotationSpec.scala

@@ -842,7 +844,7 @@ class QuotationSpec extends Spec {
      }
      "abritrary" in {
        val q = quote(lift(String.valueOf(1)))
-        q.liftings.`java.this.lang.String.valueOf(1)`.value mustEqual String.valueOf(1)
+        //q.liftings.`java.lang.String.valueOf(1)`.value mustEqual String.valueOf(1)


this is required by 2.11 and fail without, opposite to 2.12 which fails with.
How should we handle that? Keep scala-2.1* different test directories or something like that. Any ideas?

mosyp · 2017-05-10T18:39:11Z

build.sbt

+    CrossVersion.partialVersion(scalaVersion.value) match {
+      case Some((2, 12)) => Seq("-Xsource:2.11") ++ orig.map {
+        case "-Xlint" => "-Xlint:-unused,_"
+        case "-Ywarn-unused-import" => "-Ywarn-unused:imports,-patvars"


We want to keep only unused import flags as in 2.11

mosyp · 2017-05-10T18:47:59Z

build.sbt

+  scalacOptions := {
+    val orig = scalacOptions.value
+    CrossVersion.partialVersion(scalaVersion.value) match {
+      case Some((2, 12)) => Seq("-Xsource:2.11") ++ orig.map {


We have a problem with that. I have put this flag only to core module and it didn't work. Imho, forcing users to add this flag into their builds doesn't make sense.

mosyp · 2017-05-10T18:48:53Z

build/build.sh

 		sbt coverageOff publish
 	else
-		sbt clean coverage test tut coverageReport coverageAggregate checkUnformattedFiles
+		sbt clean coverage +test tut coverageReport coverageAggregate checkUnformattedFiles


seems like this didn't work also, I don't see any 2.12 appearances in travis logs

mosyp · 2017-05-10T18:49:54Z

quill-core/src/test/scala/io/getquill/quotation/QuotationSpec.scala

@@ -724,6 +725,7 @@ class QuotationSpec extends Spec {
          quote(unquote(q)).ast match {
            case Function(params, Tuple(values)) =>
              values mustEqual params
+            case _ => require(false, "Should not happen")


match is not exhaustive

mosyp · 2017-05-10T18:52:12Z

quill-core/src/test/scala/io/getquill/quotation/QuotationSpec.scala

-        case class Test(a: String)
-        val t = Test("a")
+        case class TestEntity(a: String)
+        val t = TestEntity("a")


I forgot detailed error however it is connected with suspicious shadowing of generated class name or something like that

mosyp · 2017-05-10T19:04:16Z

@fwbrasil type mismatch, 2 types - with and without stacktrace:

1st type (not stacktrace, pattern of such error described here cross compile to Scala 2.12 #231 (comment))

[error]  quill/quill-core/src/test/scala/io/getquill/dsl/EncodingDslSpec.scala:23: type mismatch;
[error]  found   : io.getquill.testContext.Quoted[io.getquill.testContext.Query[io.getquill.dsl.Address] => io.getquill.testContext.Query[Nothing]]{def ast: io.getquill.ast.Function}
[error]  required: io.getquill.testContext.Quoted[io.getquill.testContext.Query[io.getquill.dsl.Address] => io.getquill.testContext.Query[Nothing]]{def quoted: io.getquill.ast.Function; def ast: io.getquill.ast.Function; def id1252593445(): Unit; val liftings: Object}
[error]   def `inside run - query` = run {

2nd type (has stack trace)

error] /home/misha/Documents/quill/quill-core/src/test/scala/io/getquill/context/ContextMacroSpec.scala:153: exception during macro expansion: 
[error] scala.reflect.macros.TypecheckException: type mismatch;
[error]  found   : io.getquill.testContext.Quoted[io.getquill.testContext.EntityQuery[io.getquill.testContext.TestEntity]]{def ast: io.getquill.ast.Entity}
[error]  required: io.getquill.testContext.Quoted[io.getquill.testContext.EntityQuery[io.getquill.testContext.TestEntity]]{def quoted: io.getquill.ast.Entity; def ast: io.getquill.ast.Entity; def id1703614372(): Unit; val liftings: Object}
...
[error]         at io.getquill.context.ContextMacro.extractAst(ContextMacro.scala:35)
[error]         at io.getquill.context.ContextMacro.extractAst$(ContextMacro.scala:34)
[error]         at io.getquill.context.QueryMacro.extractAst(QueryMacro.scala:8)

UPD. seems like stack trace is thrown only for dynamic queries

mosyp · 2017-05-11T12:29:24Z

@getquill/maintainers Build successfully passes against 2.11/2.12 scala version. Please review.

fwbrasil · 2017-05-12T17:24:18Z

build.sbt

-    if (scalaVersion.value.startsWith("2.12.")) "-Xfatal-warnings" else ""
-  )
-)
+lazy val `scala 2.12 hacks` = Seq(


what's the plan for removing these hacks? Are they due to scala bugs?

this one brings back -Xfatal-warnings for 2.12, but fails only for unused imports rather than all others (params, patvars and so on). So with this new hack we bring the behavior as with 2.11

nice, it's more granular! I'm still curious how we're going to get rid of the hacks, though. Is it something we can fix?

I will just rename it seems like it'll be persistent since scala/scala#5402

@fwbrasil Also look here https://github.com/som-snytt/scala/blob/bad61ce0ff9f460c2f8873c134a7f6bee0a53824/src/compiler/scala/tools/nsc/settings/Warnings.scala#L25-L30
For now we use only imports

fwbrasil · 2017-05-12T17:24:42Z

quill-core/src/test/scala/io/getquill/quotation/QuotationSpec.scala

@@ -857,7 +859,7 @@ class QuotationSpec extends Spec {
      }
      "abritrary" in {
        val q = quote(lift(String.valueOf(1)))
-        q.liftings.`java.this.lang.String.valueOf(1)`.value mustEqual String.valueOf(1)
+        //q.liftings.`java.this.lang.String.valueOf(1)`.value mustEqual String.valueOf(1)


remove comment?

seems like my previous message has been lost with rebase. I'm not sure what should I do here. this must be present in 2.11, but in 2.12 should be removed

you can try to replace String.valueOf(1) with a tree that doesn't have this package naming issue. Maybe a scala object in the stdlib?

fwbrasil · 2017-05-12T17:25:36Z

quill-core/src/test/scala/io/getquill/quotation/QuotationSpec.scala

@@ -726,6 +726,7 @@ class QuotationSpec extends Spec {
          quote(unquote(q)).ast match {
            case Function(params, Tuple(values)) =>
              values mustEqual params
+            case _ => require(false, "Should not happen")


you can use io.getquill.util.Messages.fail

mosyp · 2017-05-12T20:26:28Z

quill-core/src/test/scala/io/getquill/quotation/QuotationSpec.scala

-        q.liftings.`java.this.lang.String.valueOf(1)`.value mustEqual String.valueOf(1)
+        class A { def x = 1 }
+        val q = quote(lift(new A().x))
+        q.liftings.`new A().x`.value mustEqual new A().x


@fwbrasil I hope this works for such test case, since using static calls as before is prohibit fr cross compilation

mosyp · 2017-05-12T20:26:53Z

build.sbt

+  scalacOptions ++= {
+    CrossVersion.partialVersion(scalaVersion.value) match {
+      case Some((2, 11)) => Seq("-Xlint", "-Ywarn-unused-import")
+      case Some((2, 12)) => Seq("-Xlint:-unused,_", "-Ywarn-unused:imports")


Now it should be more convenient

fwbrasil · 2017-05-12T20:30:10Z

This is awesome, thanks! :)

mosyp changed the title ~~Scala2.12 workarounds~~ Scala 2.12 compilation workarounds May 10, 2017

mosyp changed the title ~~Scala 2.12 compilation workarounds~~ [WIP] Scala 2.12 compilation workarounds May 10, 2017

mosyp force-pushed the scala2.12-workarounds branch from ea0ec36 to 3be6274 Compare May 10, 2017 15:19

mosyp force-pushed the scala2.12-workarounds branch 2 times, most recently from 07433dc to 2c1c33c Compare May 10, 2017 18:23

mosyp commented May 10, 2017

View reviewed changes

mosyp changed the title ~~[WIP] Scala 2.12 compilation workarounds~~ [WIP] Make Quill compatible with Scala 2.12.2 May 10, 2017

mosyp force-pushed the scala2.12-workarounds branch from 1c4212d to 2405f81 Compare May 11, 2017 10:35

zifeo mentioned this pull request May 11, 2017

Prepare release 1.2.0 with Scala 2.12 support #771

Merged

mosyp changed the title ~~[WIP] Make Quill compatible with Scala 2.12.2~~ Make Quill compatible with Scala 2.12.2 May 11, 2017

mosyp force-pushed the scala2.12-workarounds branch 4 times, most recently from bfc27c8 to 4452951 Compare May 12, 2017 16:58

fwbrasil reviewed May 12, 2017

View reviewed changes

Make Quill compatible with Scala 2.12.2

2361b5f

mosyp force-pushed the scala2.12-workarounds branch from f1c56ab to 2361b5f Compare May 12, 2017 20:24

mosyp commented May 12, 2017

View reviewed changes

fwbrasil approved these changes May 12, 2017

View reviewed changes

fwbrasil merged commit af48efa into zio:master May 12, 2017

mosyp deleted the scala2.12-workarounds branch May 13, 2017 10:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make Quill compatible with Scala 2.12.2 #770

Make Quill compatible with Scala 2.12.2 #770

mosyp commented May 10, 2017 •

edited

mosyp commented May 10, 2017

fwbrasil commented May 10, 2017

mosyp left a comment •

edited

mosyp May 10, 2017

mosyp May 10, 2017

mosyp May 10, 2017

mosyp May 10, 2017

mosyp May 10, 2017

mosyp May 10, 2017

mosyp commented May 10, 2017 •

edited

mosyp commented May 11, 2017

fwbrasil May 12, 2017

mosyp May 12, 2017

fwbrasil May 12, 2017

mosyp May 12, 2017

mosyp May 12, 2017

fwbrasil May 12, 2017

mosyp May 12, 2017

fwbrasil May 12, 2017

fwbrasil May 12, 2017

mosyp May 12, 2017 •

edited

mosyp May 12, 2017

fwbrasil commented May 12, 2017

Make Quill compatible with Scala 2.12.2 #770

Make Quill compatible with Scala 2.12.2 #770

Conversation

mosyp commented May 10, 2017 • edited

Problem

Solution

mosyp commented May 10, 2017

fwbrasil commented May 10, 2017

mosyp left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mosyp commented May 10, 2017 • edited

mosyp commented May 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mosyp May 12, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fwbrasil commented May 12, 2017

mosyp commented May 10, 2017 •

edited

mosyp left a comment •

edited

mosyp commented May 10, 2017 •

edited

mosyp May 12, 2017 •

edited