enumeratum icon indicating copy to clipboard operation
enumeratum copied to clipboard

Support Scala3

Open cchantep opened this issue 3 years ago • 3 comments

  • Update the build & CI
  • Port the EnumMacros and ValueEnumMacros
  • Refactor coreJVMTests

cchantep avatar Sep 05 '22 16:09 cchantep

Codecov Report

Merging #349 (f826d23) into master (e61f1b6) will decrease coverage by 4.60%. The diff coverage is 92.24%.

@@            Coverage Diff             @@
##           master     #349      +/-   ##
==========================================
- Coverage   90.37%   85.76%   -4.61%     
==========================================
  Files          63       63              
  Lines         530      527       -3     
  Branches        8       24      +16     
==========================================
- Hits          479      452      -27     
- Misses         51       75      +24     
Impacted Files Coverage Δ
...in/scala/enumeratum/values/ArgonautValueEnum.scala 100.00% <ø> (ø)
...umeratum-core/src/main/scala/enumeratum/Enum.scala 100.00% <ø> (ø)
...e/src/main/scala/enumeratum/values/ValueEnum.scala 100.00% <ø> (ø)
...atum-json4s/src/main/scala/enumeratum/Json4s.scala 100.00% <ø> (ø)
...on4s/src/main/scala/enumeratum/values/Json4s.scala 100.00% <ø> (ø)
...in/scala/enumeratum/values/PlayFormValueEnum.scala 100.00% <ø> (ø)
...eratum-quill/src/main/scala/enumeratum/Quill.scala 0.00% <0.00%> (ø)
...quill/src/main/scala/enumeratum/values/Quill.scala 0.00% <0.00%> (ø)
.../main/scala/enumeratum/values/QuillValueEnum.scala 0.00% <0.00%> (ø)
...enumeratum/values/ReactiveMongoBsonValueEnum.scala 33.33% <ø> (ø)
... and 25 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov-commenter avatar Sep 07 '22 11:09 codecov-commenter

Codecov Report

Merging #349 (c907f6c) into master (e61f1b6) will decrease coverage by 4.50%.

The decrease is due to BSONValueHandler no longer being covered, but on the other side, as deprecated, it's no longer used at all ...

cchantep avatar Sep 10 '22 13:09 cchantep

Just leaving a kite that I have not forgotten this PR and will review once the dust settles down from my travelling ..

lloydmeta avatar Sep 18 '22 12:09 lloydmeta

This is one of the most important PR that unblocks our projects for scala-3 migration

Thanks @cchantep and @lloydmeta for all the work 👍 !

mushtaq avatar Nov 23 '22 10:11 mushtaq

Finally finished making my way through. Overall, LGTM

I was testing this PR to see if I can migrate my existing usage of enumeratum to the scala-3 version and I found a few issues. I will try to post them in separate comments below. @cchantep @lloydmeta

mushtaq avatar Nov 26 '22 05:11 mushtaq

Issue-1

//> using scala "3.2.1"
//> using repository "jitpack"
//> using lib "com.github.mushtaq.enumeratum::enumeratum:be1c8d6"

import enumeratum.{Enum, EnumEntry}
import scala.collection.immutable

sealed trait Base extends EnumEntry

class Derived extends Base

object Base extends Enum[Base] {
  override def values: immutable.IndexedSeq[Base] = findValues // error in compilation

  case object E1 extends Base
  case object E2  extends Base
}

mushtaq avatar Nov 26 '22 05:11 mushtaq

Issue-2

//> using scala "3.2.1"
//> using repository "jitpack"
//> using lib "com.github.mushtaq.enumeratum::enumeratum:be1c8d6"


import enumeratum.{Enum, EnumEntry}
import scala.collection.immutable

sealed trait Base extends EnumEntry

object Base extends Enum[Base] {
  class Derived extends Base

  override def values: immutable.IndexedSeq[Base] = findValues

  case object E1 extends Derived
  case object E2  extends Derived
}

object Demo {
  def main(args: Array[String]): Unit = {
    println(Base.values) // prints empty vector
  }
}

mushtaq avatar Nov 26 '22 05:11 mushtaq

These are self contained scala-cli scripts which work with scala 2.13 and current published version of enumeratum.

The artifact referred in the script is jitpack published from a fork of this PR

mushtaq avatar Nov 26 '22 05:11 mushtaq

@mushtaq Cannot reproduce Issue 1, check your env.

Issue 1:

sbt:enumeratum> console
Welcome to Scala 3.2.1-RC1 (9.0.1, Java Java HotSpot(TM) 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.
                                                                                                                            
scala> :paste
Unknown command: ":paste", run ":help" for a list of commands
                                                                                                                            
scala> import enumeratum.{Enum, EnumEntry}
     | import scala.collection.immutable
     | 
     | sealed trait Base extends EnumEntry
     | 
     | class Derived extends Base
     | 
     | object Base extends Enum[Base] {
     |   override def values: immutable.IndexedSeq[Base] = findValues // error in compilation
     | 
     |   case object E1 extends Base
     |   case object E2  extends Base
     | }
// defined trait Base
// defined class Derived
// defined object Base
                                                                                                                            
scala> 
                                                                                                                            
scala> Base.values
val res0: IndexedSeq[Base] = Vector(E1, E2)

cchantep avatar Nov 26 '22 13:11 cchantep

@cchantep Have you tried putting the test in .scala file?

Using either REPL or a .sc file will not reproduce the issue.

To make it easy, I put the issues in gists. You can run them like so:

$ scala-cli https://gist.github.com/mushtaq/16705977494d73a3d921f53ffe5c7a32

Compiling project (Scala 3.2.1, JVM)
[error] 16705977494d73a3d921f53ffe5c7a32-main/issue1.scala:13:53: 
The enum (i.e. the class containing the case objects and the call to `findValues`) must be an object
Error compiling project (Scala 3.2.1, JVM)
Compilation failed


$ scala-cli https://gist.github.com/mushtaq/d643b306e22464ab13e637d3971da3aa

Vector()

mushtaq avatar Nov 26 '22 13:11 mushtaq

@mushtaq In REPL or in a .scala file, in SBT the issue 1 is not reproducible.

cchantep avatar Nov 26 '22 18:11 cchantep

Interesting!

I am Scala 3.2.1 with Java-17 on MacOS. Maybe something to do with that combination.

Can you please share what is the output of this command on your machine:

$ scala-cli https://gist.github.com/mushtaq/16705977494d73a3d921f53ffe5c7a32

mushtaq avatar Nov 27 '22 03:11 mushtaq

@mushtaq Rather test it SBT/Scala REPL. For me, the use in scala-cli is not a priority, and maybe related to Scala 3 support by scala-cli (so out of scope). You can eventually create a separate issue, after more investigation.

cchantep avatar Nov 27 '22 14:11 cchantep

Issue 2 is fine:

  • https://github.com/lloydmeta/enumeratum/pull/349/files#diff-a40c38d0bac17eaa7298efcbc480d866cf8e9ccf79890664d4f4d426cb3d294bR309
  • https://github.com/lloydmeta/enumeratum/pull/349/files#diff-3df4b038e79dec11acb469e7e0eea76a30cf1a96e52a7f0541682975239df600R44

cchantep avatar Nov 27 '22 14:11 cchantep

Issue 2 is fine:

Thanks!

mushtaq avatar Nov 27 '22 15:11 mushtaq

@mushtaq Rather test it SBT/Scala REPL.

Sure, here is the github repo with sbt to reproduce the issue-1.

sbt compile fails with the same message:

[error] -- Error: /Users/mushtaqahmed/projects/scala3/enumeraturm-compilation-issue/src/main/scala/Main.scala:9:52
[error] 9 |  override def values: immutable.IndexedSeq[Base] = findValues // error in compilation
[error]   |                                                    ^^^^^^^^^^
[error]   |The enum (i.e. the class containing the case objects and the call to `findValues`) must be an object

mushtaq avatar Nov 27 '22 16:11 mushtaq

Issue-1 is fine

cchantep avatar Nov 27 '22 19:11 cchantep

Issue-1 is fine

I published and verified both issues with your changes. Issue-1 is indeed fixed, thank you @cchantep !

Unfortunately, issue-2 is yet not fixed. If you take a pull from the sbt repo and run sbt 'runMain issue2.Demo', it still prints Vector() 😔

mushtaq avatar Nov 28 '22 14:11 mushtaq

@mushtaq Issue is fixed and covered by the indicated test

cchantep avatar Nov 28 '22 15:11 cchantep