com-lihaoyi / scalasql

Scala ORM to query SQL databases from Scala via concise, type-safe, and familiar case classes and collection operations. Connects to Postgres, MySql, H2, and Sqlite out of the box
195 stars 22 forks source link

Scala 3 support #11

Closed mrdziuban closed 6 months ago

mrdziuban commented 6 months ago

Closes #2

Macros

Just like @KuceraMartin in #3, I ran into scala/scala3#19493 and scala/scala3#19436 when trying to resolve a TypeMapper by importing from a DialectTypeMappers. As a workaround, I introduced additional implicit defs in the TableMapper companion object that instead rely on an implicit instance of DialectTypeMappers, i.e. in a macro:

// bad, causes a compiler crash
// TableMacro.scala
(dialect: DialectTypeMappers) => {
  import dialect.*
  summonInline[TypeMapper[t]]
}

// good
// TypeMapper.scala
implicit def stringFromDialectTypeMappers(implicit d: DialectTypeMappers): TypeMapper[String] = d.StringType

// TableMacro.scala
(dialect: DialectTypeMappers) => {
  given d: DialectTypeMappers = dialect
  summonInline[TypeMapper[t]]
}

Supporting changes

In addition to building out the macros in Scala 3, the following changes were necessary:

  1. Update the generated code to ensure defs aren't too far to the left -- this is to silence Scala 3 warnings
  2. Convert CharSequences to Strings explicitly -- see the error the Scala 3 compiler reported here
  3. Remove try block without a corresponding catch -- see the warning the Scala 3 compiler reported here
  4. Add types to implicit definitions
  5. Mark renderSql as private[scalasql] instead of protected -- see the error the Scala 3 compiler reported here
  6. Use Scala 3.4 -- this is a little unfortunate since it's not the LTS but it's necessary for the Scala 3 macros to match on higher kinded types like this. This type of match doesn't work in Scala 3.3
  7. Replace _ wildcards with ? -- this is to silence Scala 3 warnings
  8. Replace Foo with Bar in types with Foo & Bar -- this is to silence Scala 3 warnings
  9. Add the -Xsource:3 compiler option for Scala 2 -- this is necessary to use the language features mentioned in points 7 and 8
  10. Add a number of type annotations to method overrides -- this is to silence warnings reported by the Scala 2 compiler as a result of enabling -Xsource:3. All of the warnings relate to the inferred type of the method changing between Scala 2 and 3
mrdziuban commented 6 months ago

Sorry I forgot to update the docs, just pushed that

lihaoyi commented 6 months ago

Left one small comment.

One slightly bigger ask: to demonstrate the Scala 3 stuff working, could you take the test/src/examples folder and add a test/src-3/examples/ folder containing a Scala3Example.scala exercising the Scala 3 syntax and show it working?

I pinged the #metaprogramming discord to see if I can find anyone to help review the Scala 3 macro stuff. It looks fine to me, but I'm not an expert. If nobody steps up to help review, I'd be happy to merge as-is given tests are passing

KuceraMartin commented 6 months ago

Hi @mrdziuban, amazing that you found this workaround! I had a brief look at the code and it looks good to me – pretty similar to what I was trying to do in my PR otherwise.

One question: do you really need to enforce V[_[_]] <: Product? I didn't notice it being used anywhere, and the Scala 2 macro doesn't do it so it's a small incompatibility.

mrdziuban commented 6 months ago

One slightly bigger ask: to demonstrate the Scala 3 stuff working, could you take the test/src/examples folder and add a test/src-3/examples/ folder containing a Scala3Example.scala exercising the Scala 3 syntax and show it working?

Yeah I can take a look at this. By Scala 3 syntax do you mean using things like fewer braces?

One question: do you really need to enforce V[_[_]] <: Product?

Yeah, it's necessary because TableQueryable has a <: Product constraint. Without the constraint in the macro, there's a compile error in the construct0 param here

-- [E007] Type Mismatch Error: /Users/matt/scalasql/scalasql/query/src-3/TableMacro.scala:85:14
85 |              Apply(
   |              ^
   |Found:    scala.quoted.Expr[V[scalasql.core.Sc]]
   |Required: scala.quoted.Expr[Product]
   |
   |where:    V is a type in method applyImpl with bounds <: [_[_$2]] =>> Any
86 |                TypeApply(
87 |                  Select.unique(New(TypeIdent(TypeRepr.of[V].typeSymbol)), "<init>"),
88 |                  List(TypeTree.of[Sc])
89 |                ),
90 |                constructorValueParams.zipWithIndex.map { case (param, i) =>
91 |                  val paramTpe = paramType(param)
92 |                  val tpe = subParam(paramTpe, TypeRepr.of[Sc]).asType
93 |                  val tpe2 = subParam(paramTpe, TypeRepr.of[SqlExpr]).asType
94 |                  (tpe, tpe2) match {
95 |                    case ('[t], '[t2]) => '{ queryable[t2, t](${ Expr(i) }).construct(args) }.asTerm
96 |                  }
97 |                }
98 |              ).asExprOf[V[Sc]]
   |
   | longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: /Users/matt/scalasql/scalasql/query/src-3/TableMacro.scala:169:57
169 |    '{ new Table.Metadata[V]($queryables, $walkLabels0, $queryable, $vExpr0) }
    |                                                         ^^^^^^^^^
    |Found:    (queryable :
    |  scala.quoted.Expr[(() => Seq[String], scalasql.core.DialectTypeMappers,
    |    scalasql.query.Table.Metadata.QueryableProxy) =>
    |    scalasql.query.Table.Internal.TableQueryable[V[scalasql.core.Expr²], Product
    |       & V[scalasql.core.Sc]]
    |  ]
    |)
    |Required: scala.quoted.Expr[(() => Seq[String], scalasql.core.DialectTypeMappers,
    |  scalasql.query.Table.Metadata.QueryableProxy) =>
    |  scalasql.core.Queryable[V[scalasql.core.Expr²], V[scalasql.core.Sc]]]
    |
    |where:    Expr  is a class in package scala.quoted
    |          Expr² is a trait in package scalasql.core
    |          V     is a type in method applyImpl with bounds <: [_[_$2]] =>> Any
    |
    | longer explanation available when compiling with `-explain`
lihaoyi commented 6 months ago

Yeah just an example without the braces should be enough to sanity check that things work on Scala 3 (and that we didn't accidentally mix up the scala versions in Mill or something)

mrdziuban commented 6 months ago

👍 done, I ported the H2Example

lihaoyi commented 6 months ago

Thanks @mrdziuban !