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

Scala 3 #143

Merged
merged 14 commits into from
May 25, 2021
Merged

Scala 3 #143

merged 14 commits into from
May 25, 2021

Conversation

tpolecat
Copy link
Member

@tpolecat tpolecat commented May 4, 2021

resolves #104

This adds a cross-build for Scala 3.0.0.

  • Removed sbt-gsp (which doesn't work with Scala 3) and inlined the necessary settings.
  • Added version-dependent Circe dependencies, with 0.13 for Scala 2 and 0.14 for Scala 3. They are not compatible and forcing Scala 2 people to use Circe 0.14 would likely break things. We can revisit this when things settle down a bit.
  • Removed the dependency on circe-optics, which doesn't exist for Scala 3. Workaround is JsonExtractors and a bit of ACursor bumbling in the tests.
  • The inability to have a term of type Codec[_] forced me to write ExistentialCodec which pushes the type parameter down into a type member. The Anti-Aux pattern.
  • generic is ported as-is, with all sources under scala-2 an publishing disabled for Scala 3. We can add Scala 3 support in a followup PR.

TODO:

  • enable demo (http4s is available now)

val q = parseSelections(sels, None, fragments)
val vs = vds.map {
case VariableDefinition(nme, tpe, _) => UntypedVarDef(nme.value, tpe, None)
def parseOperation(op: Operation, fragments: Map[String, FragmentDefinition]): Result[UntypedOperation] = {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scala 3 warns that this single-case match would only fail with case null => so I collapsed it to a destructuring assignment. There are a few more of these.

import io.circe.Json
import io.circe.JsonObject

object JsonExtractor {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because circe-optics doesn't exist for Scala 3 [yet].

@@ -21,7 +21,7 @@ import QueryInterpreter.mkErrorResult
* we need to be able to construct where clauses from predicates over fields/attributes), so
* these cannot be arbitrary functions `Cursor => Boolean`.
*/
trait Term[T] extends Product with Serializable {
trait Term[T] extends Product with Serializable { // fun fact: making this covariant crashes Scala 3
Copy link
Member Author

@tpolecat tpolecat May 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because Term[+T] crashes Scala 3 there are some upcasts to Term[Any] later on.

c.flatListPath(path).map(_.map { case ScalarFocus(f: T @unchecked) => f })
c.flatListPath(path).map(_.map {
case ScalarFocus(f: T @unchecked) => f
case _ => sys.error("impossible")
Copy link
Member Author

@tpolecat tpolecat May 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exhaustiveness checker is a lot better so I needed to add a few of these. All these cases should be impossible but maybe the error messages could be better. What do you think?

@@ -170,7 +170,7 @@ class QueryInterpreter[F[_]](mapping: Mapping[F]) {
case Ior.Right(elem) => (errors, elem :: elems)
case Ior.Both(errs, elem) => (errs.toChain ++ errors, elem :: elems)
}
}).map(_.reverse)
}).fmap(_.reverse)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functor .map syntax doesn't work for tuples in Scala 3 because there's polymorphic map on Tuple now.

@@ -630,7 +630,7 @@ case class NullableType(
*
* @see https://facebook.github.io/graphql/draft/#sec-The-__Field-Type
*/
case class Field private(
case class Field(
Copy link
Member Author

@tpolecat tpolecat May 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're calling this from a place that Scala 2 allows but Scala 3 doesn't. I don't think it needs to be private anyway.

// For license information see LICENSE or https://opensource.org/licenses/BSD-3-Clause

package compiler

import cats.Id
import cats.implicits._
import cats.catsInstancesForId
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scala 3 can't find instances for cats.Id but it's not clear why. This fixes it.


abstract class DoobieMapping[F[_]: Sync](
val transactor: Transactor[F],
val monitor: DoobieMonitor[F]
) extends SqlMapping[F] {

type Codec[A] = (Meta[A], Boolean)
type Encoder[A] = (Put[A], Boolean)
type Encoder[-A] = (Put[A], Boolean) @uncheckedVariance
Copy link
Member Author

@tpolecat tpolecat May 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See SqlModule for more on this.

@@ -190,7 +190,7 @@ trait SqlMapping[F[_]] extends CirceMapping[F] with SqlModule[F] { self =>
final class MappedQuery(
table: String,
columns: List[ColumnRef],
metas: List[(Boolean, Codec[_])],
metas: List[(Boolean, ExistentialCodec)],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ExistentialCodec is defined in SqlModule, see below.

@@ -14,8 +14,24 @@ trait SqlModule[F[_]] {
/** The type of a codec that reads and writes column values of type `A`. */
type Codec[A]

/** A codec that has forgotten its type argument. */
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codec[_] isn't a type in Scala 3 so we have to push the existential down into a type member.

/** The type of an encoder that writes column values of type `A`. */
type Encoder[A]
type Encoder[-A]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Encoder is invariant there are one million pages of type errors in MappedQuery. Scala 2 is happy to crush stuff to Any after a while but Scala 3 keeps up and doesn't like what's happening in calls to loop(...).

"io.circe" %% "circe-core" % circeVersion,
"io.circe" %% "circe-optics" % circeOpticsVersion,
"io.circe" %% "circe-parser" % circeVersion
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These dependencies all arrive transitively so I removed them as a simplification. Same in a few places below.

@@ -193,7 +193,7 @@ Here the root of the result is "Luke Skywalker" and we loop back to Luke through
The data type is not itself recursive, instead friend are identified by their ids. When traversing through the list of
friends the `resolveFriends` method is used to locate the next `Character` value to visit.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea why this is being rendered in red.

@@ -5,6 +5,7 @@ package edu.gemini.grackle
package circetests

import cats.Id
import cats.catsInstancesForId
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These instances are unavailable in Scala 3. I don't know why.

@@ -189,7 +189,7 @@ object GraphQLParser {
QueryShorthand.widen[Ast.OperationDefinition] | Operation.widen

lazy val QueryShorthand: Parser[Ast.OperationDefinition.QueryShorthand] =
SelectionSet.map(Ast.OperationDefinition.QueryShorthand)
SelectionSet.map(Ast.OperationDefinition.QueryShorthand.apply)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scala 3 warns that apply-insertion is deprecated. I guess the companions no longer extend function types?

@tpolecat tpolecat marked this pull request as ready for review May 7, 2021 15:06
@tpolecat tpolecat requested a review from milessabin May 7, 2021 15:06
@tpolecat tpolecat changed the title WIP: Scala 3 Scala 3 May 7, 2021
Copy link
Member

@milessabin milessabin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@tpolecat
Copy link
Member Author

Waiting on test-containers, which is having some issues. testcontainers/testcontainers-scala#177

@tpolecat
Copy link
Member Author

test-containers is fixed, now waiting on cats-testkit-scalatest

@tpolecat
Copy link
Member Author

(updated to 3.0.0 final)

@tpolecat
Copy link
Member Author

Squashing since most commits don't build.

@tpolecat tpolecat merged commit fd9f78a into main May 25, 2021
@tpolecat tpolecat deleted the scala-3 branch May 25, 2021 17:23
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 this pull request may close these issues.

Scala 3 support
2 participants