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

remove "-Xmigration" from main scalacOptions. add explicit type #12029

Merged
merged 3 commits into from
Mar 13, 2024

Conversation

xuwei-k
Copy link
Contributor

@xuwei-k xuwei-k commented Oct 14, 2023

Pull Request Checklist

Helpful things

Fixes

Purpose

remove "-Xmigration" from main code and add explicit types. for avoid different return types Scala 2 and 3.

Background Context

"-Xmigration" suppress some warnings. But I think this is not good idea.

please see following example and scala/scala pull request.

Scala 3

Welcome to Scala 3.3.1 (11.0.20.1, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.
                                                                                                                                             
scala> trait A { def x: Any } ; class B extends A { def x = "x" }
// defined trait A
// defined class B
                                                                                                                                             
scala> new B().x // not String !. different type with Scala 2.13
val res0: Any = x

no option

Welcome to Scala 2.13.12 (OpenJDK 64-Bit Server VM, Java 11.0.20.1).
Type in expressions for evaluation. Or try :help.

scala> trait A { def x: Any } ; class B extends A { def x = "x" }
trait A
class B

scala> new B().x
val res0: String = x

-Xsource:3

$ scala -Xsource:3
Welcome to Scala 2.13.12 (OpenJDK 64-Bit Server VM, Java 11.0.20.1).
Type in expressions for evaluation. Or try :help.

scala> trait A { def x: Any } ; class B extends A { def x = "x" }
                                                        ^
       error: under -Xsource:3, inferred Any instead of String [quickfixable]
       Scala 3 migration messages are errors under -Xsource:3. Use -Wconf / @nowarn to filter them or add -Xmigration to demote them to warnings.
       Applicable -Wconf / @nowarn filters for this fatal warning: msg=<part of the message>, cat=scala3-migration, site=B.x

scala> trait A { def x: Any } ; class B extends A { def x: String = "x" }
trait A
class B

-Xsource:3 and -Xmigration

$ scala -Xsource:3 -Xmigration
Welcome to Scala 2.13.12 (OpenJDK 64-Bit Server VM, Java 11.0.20.1).
Type in expressions for evaluation. Or try :help.

scala> trait A { def x: Any } ; class B extends A { def x = "x" }
                                                        ^
       warning: under -Xsource:3, inferred Any instead of String [quickfixable]
trait A
class B

scala> def f: Seq[Int] = Nil // unnecessary migration message!!!
              ^
       warning: type Seq in package scala has changed semantics in version 2.13.0:
       scala.Seq is now scala.collection.immutable.Seq instead of scala.collection.Seq
def f: Seq[Int]

References

@mkurz
Copy link
Member

mkurz commented Mar 12, 2024

@xuwei-k can you please rebase? Thanks!

Copy link
Member

@mkurz mkurz left a comment

Choose a reason for hiding this comment

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

Thanks LGTM!

However, I pushed two commits:

@mkurz mkurz merged commit 1f8ed62 into playframework:main Mar 13, 2024
29 checks passed
@mkurz mkurz added the not backported to 2.9.x If we ever release a 2.9.x major version this should backported. label Mar 13, 2024
case _ =>
Seq.empty
}
},
Copy link
Member

Choose a reason for hiding this comment

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

The -Xmigration flag for the test scope eventually got removed in

I adjusted all the test finally (necessary for upgrading to Scala 2.13.14 without adding any Wconf filters, see #12576 (comment))

@mkurz
Copy link
Member

mkurz commented May 2, 2024

Backported this changes (together with removing -Xmigration from tests) to 2.9.x and 3.0.x:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not backported to 2.9.x If we ever release a 2.9.x major version this should backported.
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants