playframework / anorm

The Anorm database library
https://playframework.github.io/anorm/
Apache License 2.0
240 stars 75 forks source link

Macro.parser match may not be exhaustive using a case class with an ADT #568

Closed KapStorm closed 1 year ago

KapStorm commented 1 year ago

Anorm Version (2.5.x / etc)

Scala 3.3 2.7.0

Operating System (Ubuntu 15.10 / MacOS 10.10 / Windows 10)

Ubuntu 22.04.2

JDK (Oracle 1.8.0_72, OpenJDK 1.8.x, Azul Zing)

openjdk 11.0.16

Library Dependencies

"com.beachape" %% "enumeratum" % "1.7.2"

Expected Behavior

Parse SQL Rows to UserToken using Macro.parser

Actual Behavior

Macro.parser give us a match may not be exhaustive warning that make our CI to fail. This is caused by the "token_type" UserTokenType ADT using beachape Enumeratum. We also tried with Scala 3 enums and gives the same result

[warn] ./src/AnormIssue.scala:52:3: match may not be exhaustive.
[warn] 
[warn] It would fail on pattern case: anorm.~(_, _)
[warn]   Macro.parser[UserToken]("user_token_id", "token", "token_type", "created_at", "expires_at", "user_id")
[warn]   ^^^

Reproducible Test Case

In our project https://github.com/wiringbits/scala-webapp-template/blob/50d08b94103d4b604961c9f02639b826ccf9afd9/server/src/main/scala/net/wiringbits/repositories/daos/package.scala#L50-L58

Using scala-cli

//> using scala 3.3

//> using lib "org.playframework.anorm::anorm::2.7.0"
//> using lib "com.beachape::enumeratum::1.7.2"

import anorm.*
import enumeratum.EnumEntry.Uppercase
import enumeratum.*

import java.time.Instant
import java.util.UUID

sealed trait UserTokenType extends EnumEntry with Uppercase

object UserTokenType extends Enum[UserTokenType] {

  override def values: IndexedSeq[UserTokenType] = findValues

  case object EmailVerification extends UserTokenType
  case object ResetPassword extends UserTokenType
}

case class UserToken(
    id: UUID,
    token: String,
    tokenType: UserTokenType,
    createdAt: Instant,
    expiresAt: Instant,
    userId: UUID
)

implicit val userTokenTypeParser: Column[UserTokenType] = {
  Column.columnToString.mapResult { string =>
    UserTokenType
      .withNameInsensitiveOption(string)
      .toRight(SqlRequestError(new RuntimeException(s"Invalid user token type: $string")))
  }
}

implicit val userTokenParser: RowParser[UserToken] = {
  Macro.parser[UserToken]("user_token_id", "token", "token_type", "created_at", "expires_at", "user_id")
}
cchantep commented 1 year ago

Have you rather tried anorm-enumeratum ?

KapStorm commented 1 year ago

There isn't a Scala 3 anorm-enumeratum https://mvnrepository.com/artifact/org.playframework.anorm/anorm-enumeratum, also we already have the Codec for the ADT so it shouldn't be a problem. I did another case with Scala 2 ADT style and it gives the same warnings.

I tried with only the tokenType ADT field inside the case class (case class UserToken(tokeType: UserTokenType)) and it works correctly, but if I add another field (any field type) it gives the warnings

Scala 2 ADT style

sealed trait UserTokenType

object UserTokenType extends UserTokenType {
  case object EmailVerification extends UserTokenType
  case object ResetPassword extends UserTokenType
}

case class UserToken(
    id: UUID,
    tokenType: UserTokenType
)

implicit val userTokenTypeParser: Column[UserTokenType] = {
  Column.columnToString
    .mapResult {
      case "EmailVerification" => Right(UserTokenType.EmailVerification)
      case "ResetPassword" => Right(UserTokenType.ResetPassword)
      case string => Left(SqlRequestError(new RuntimeException(s"Invalid user token type: $string")))
    }
}

implicit val userTokenParser: RowParser[UserToken] = {
  Macro.parser[UserToken]("user_token_id", "token_type")
}
[warn] ./src/HelloWorld.scala:36:3: match may not be exhaustive.
[warn] 
[warn] It would fail on pattern case: anorm.~(_, _)
[warn]   Macro.parser[UserToken]("user_token_id", "token_type")
[warn]   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
cchantep commented 1 year ago

Anyway, for enumeratum (so Scala 2) it's better to use the recommended module.

For other warnings, make sur to have the compiler options such as -Ywarn-macros:after or -Wmacros:after to avoid false errors.

irumiha commented 1 year ago

Looks like the issue is not about Enumeratum. Here's a minimal reproducer:

//> using scala 3.3.0
//> using lib "org.playframework.anorm::anorm::2.7.0"

import anorm.*

case class Failing(
  name: String,  
  another: Boolean,
)
object Failing:
  val rowParser: RowParser[Failing] =
    Macro.namedParser[Failing](Macro.ColumnNaming.SnakeCase)

Also fails with the indexedParser.

cchantep commented 1 year ago

Try 2.7.1-0ca1cde7-SNAPSHOT

irumiha commented 1 year ago

Try 2.7.1-0ca1cde7-SNAPSHOT

Works for me! :+1:

cchantep commented 1 year ago

Seems Scala 3.x re-introduce issue in reflection/meta about pattern exhaustivity check.

boggye commented 10 months ago

Hi - I have the same issue - I am using anorm 2.7.0. Here is the code:

case class GradingField(
    field_id: Int,
    field_name: String,
    grading_field_group_id: Int,
    order_index: Int,
    add_by_default: Boolean,
    min_value: BigDecimal,
    max_value: BigDecimal,
    created_dt: ZonedDateTime,
    last_modified_dt: ZonedDateTime
)
val gradingFieldRowParser: RowParser[GradingField] = Macro.parser[GradingField](
  "field_id",
  "field_name",
  "grading_field_group_id",
  "order_index",
  "add_by_default",
  "min_value",
  "max_value",
  "created_dt",
  "last_modified_dt"
)

The warning I get is:

[warn] -- [E029] Pattern Match Exhaustivity Warning: GradingField.scala:23:79 
[warn]  23 |val gradingFieldRowParser: RowParser[GradingField] = Macro.parser[GradingField](
[warn]     |                                                     ^
[warn]     |                          match may not be exhaustive.
[warn]     |
[warn]     |                          It would fail on pattern case: anorm.~(_, _)
[warn]  24 |  "field_id",
[warn]  25 |  "field_name",
[warn]  26 |  "grading_field_group_id",
[warn]  27 |  "order_index",
[warn]  28 |  "add_by_default",
[warn]  29 |  "min_value",
[warn]  30 |  "max_value",
[warn]  31 |  "created_dt",
[warn]  32 |  "last_modified_dt"
[warn]  33 |)
[warn]     |---------------------------------------------------------------------------
[warn]     |Inline stack trace
[warn]     |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
[warn]     |This location contains code that was inlined from RowParserImpl.scala:314
[warn]      ---------------------------------------------------------------------------
[warn]     |
[warn]     | longer explanation available when compiling with `-explain`
[warn] one warning found

What do I have to do? The latest version on maven is 2.7.0: https://mvnrepository.com/artifact/org.playframework.anorm/anorm

Thanks

boggye commented 10 months ago

Try 2.7.1-0ca1cde7-SNAPSHOT

Works for me! 👍

@irumiha How did you try this? Did you build it yourself? Thank you

irumiha commented 10 months ago

@irumiha How did you try this? Did you build it yourself? Thank you

You need to use the Sonatype snapshots repository. It's not easy to search for the latest version. I don't remember what I did back then but there is a list of runs for the publish action here on GitHub: https://github.com/playframework/anorm/actions/workflows/publish.yml. Look at the logs for the "Publish artifacts" and find out the version being published. For example, this scala-cli snippet compiles without warnings:

//> using scala 3.3.1
//> using repository sonatype:snapshots
//> using lib "org.playframework.anorm::anorm::2.7.1-416e8ceb-SNAPSHOT"

import anorm.*

case class Failing(
  name: String,  
  another: Boolean,
)
object Failing:
  val rowParser: RowParser[Failing] =
    Macro.namedParser[Failing](Macro.ColumnNaming.SnakeCase)