Emit migration warnings under `-Xsource:3` as fatal warnings, not errors; `-Xmigration` turns off erroring #10439

som-snytt · 2023-06-22T19:58:56Z

This allows toning down / silencing Scala 3 migration warnings using -Wconf or @nowarn.

Scala 3 migration warnings are emitted as "fatal warnings" under category scala3-migration. Without -Xsource:3, they continue to be reported as deprecations.

Adding -Xmigration makes the migration warnings non-fatal (same effect as -Wconf:cat=scala3-migration:w).

Add a new warning for the case when an inferred type differs between Scala 2 and 3. Example:

trait A { def f: Object }
trait B extends A { def f = "" } // under -Xsource:3, inferred Object instead of String

Benign syntax enabled by -Xsource:3 does not warn, such as relaxed import syntax.

Fixes scala/bug#12798

som-snytt · 2023-06-22T20:39:43Z

Remember to update the "override parens" checks, which still just error under -Xsource:3.

Summary: instead of deprecation and then error under -Xsource:3, all warnings prefer to warn only, and warn with category migration under -Xmigration. Previously, some parser changes were "syntax errors" which fail early.

This change means projects can use -Xsource:3 and opt in to errors with -Wconf:cat=migration:e -Xmigration.

test/files/neg/t5265b.check

test/files/neg/for-comprehension-val.check

src/compiler/scala/tools/reflect/FastStringInterpolator.scala

lrytz

A new warning category looks good to me, it allows making Scala 3 migration warnings fatal without -Werror (which is what 2.13 currently does under -Xsource:3).

-Wconf:cat=migration:e -Xmigration -Xsource:3 is bit much 🙂 Maybe -Xsource:3 can be enough?

What's the value in -Xmigration changing the category in which these new warnings are emitted (deprecations vs Scala 3 migration)?
Can we have a postSetHook such that -Xsource:3 enables -Wconf:cat=scala3migration:e?

There should be a test to ensure @nowarn works (it does):

➜ sandbox qs -Wconf:cat=migration:e -Xmigration -Xsource:3
Welcome to Scala 2.13.12-20230623-225806-c850aeb (OpenJDK 64-Bit Server VM, Java 17.0.6).
Type in expressions for evaluation. Or try :help.

scala> @annotation.nowarn def f { println() }
def f: Unit

src/compiler/scala/tools/nsc/ast/parser/Parsers.scala

som-snytt · 2023-07-01T12:07:35Z

I felt bad that -Xmigration is ignored. In Scala 3, -source 3.0-migration is an intermediate step, although it means "allow old forms". We don't have -Xsource:3-migration as a 3rd state. In the sense of "mitigating migration pain", it would leave migration messages as warnings, but also they are not summarized as deprecations, so there is some beneficial pain. By that logic, omitting -Xmigration under -Xsource:3 would -Wconf:cat=migration:e.

That isn't where I wound up in this PR when I tossed a dart, or a couple of darts. One is embedded in the wall over by the pinball machine.

Summarizing again, all messages affected by -Xsource:3 should error by default or warn under -Xmigration, using cat=migration (and not cat=deprecation!). Some behaviors already warn at 2.13.

The scary part is "error by default". Seth will have to pin "Did you add -Xmigration?" at the top of the discord chat room. And distribute t-shirts that say -Xmigration, which sounds like a superhero movie about crossing borders.

src/compiler/scala/tools/nsc/ast/parser/Parsers.scala

lrytz

Now the PR does this: if -Xsource:3 and the user didn't specify `cat=migration", set a default:

if -Xmigration is set, ~~cat=migration:error~~ cat=migration:warning
else ~~:warning~~ :error

~~IMO it's still unnecessarily complicated. We have -Wconf and a specific category for Scala 3 migration warnings, I'd leave -Xmigration out of the picture.~~

~~-Xmigration is an often forgotten child and maybe not as useful as it could be, but it has a sort of defined functionality in combination with the annoation.~~ [edit: -Xmigration as a shortcut to demote migration errors seems fine]

It migt also be confusing that the warning category changes depending on -Xsource; a Scala 3 migration warning can remain in the Scala 3 migration category also under -Xsoruce:2. This might in fact be useful. [edit: they are deprecations now, let's not change it]

lrytz · 2023-07-03T13:26:28Z

src/compiler/scala/tools/nsc/Reporting.scala

@@ -353,11 +364,13 @@ object Reporting {

    object JavaSource extends WarningCategory; add(JavaSource)

+    object Migration extends WarningCategory; add(Migration)


Scala3Migration?

Just update the name? Also, I haven't thought about behavior when going from 2.12 to -Xsource:3, but maybe there is nothing special to do.

On 2.13 we only have migration warnings for going to 3.

I don't think we have any warnings on 2.12 about migrating to 3? 2.13 is required as an intermediate step.

I'd say Migration would be fine if there wasn't already OtherMigration. The Wconfs get a bit more noisy with Scala3Migration but that might be a good thing.

som-snytt · 2023-07-03T14:02:26Z

It's the other way: -Xsource:3 induces error config, but adding -Xmigration makes it warn instead.

But you're saying, just let them use -Wconf:cat=migration:w if that is what they want.

And your other idea is to make current deprecations also migrations. My only qualm is that it is noisier.

BTW, I am glad to tweak as much as necessary for ergonomics.

som-snytt · 2023-07-03T14:06:27Z

Oh wait the part where it configures -Wconf for you is really cute! I have to kill it?

lrytz · 2023-07-03T19:03:05Z

Ah, I got it wrong. Then keeping -Xmigration to demote to warnings is fine with me, though it's not very obvious to find out, or to understand when you come across it in a build.

I see some tests use -Werror -Xsource:3 -Xmigration, that confused me a bit I guess.

I overlooked the advantage of summarized deprecations, so I agree let's not change that, they are deprecations currently.

I see the synthetic -Wconf:cat=migrtion is always added to the end, so user-provided filters should take precedence.

So yeah, LGTM, we're getting there!

som-snytt · 2023-07-03T19:20:39Z

OK I'm awake again, I can ignore a few edits from earlier, but I'll wait a bit before making any rash decisions.

lrytz · 2023-07-03T19:27:22Z

The time I take to cook some crêpes and put the kids to bed is enough for you to get in your sleep and be back at compiler hacking...

Restore override behavior, partest ++ on update Tweak Unicode escapes warning

Simplify a few compiler flags in tests

lrytz · 2023-07-04T07:22:44Z

src/compiler/scala/tools/nsc/typechecker/Namers.scala

+            val pts = pt.toString
+            val leg = legacy.toString
+            val help = if (pts != leg) s" instead of $leg" else ""
+            runReporting.warning(tree.pos, s"under -Xsource:3, inferred $pts$help", WarningCategory.Migration, tree.symbol)


SethTisue · 2023-07-04T19:47:18Z

This is a great improvement.

lrytz · 2023-07-17T09:12:02Z

Let's add a disclaimer to fatal-by-default warnings. People are not used to the fact that error: elephant messages can be toned down / silenced.

SethTisue · 2023-07-17T20:55:22Z

Yeah, I think we all have the associations pretty strongly in our minds that "error" = "compiler literally cannot continue", so some education will be needed here: in the error messages, in the PR description, and in the 2.13.12 release notes.

som-snytt · 2023-07-17T21:33:35Z

I created a ticket scala/bug#12831

prepare Scala 2.13.12 - scala/scala#10439

kubukoz · 2023-10-16T23:23:26Z

src/compiler/scala/tools/nsc/typechecker/Unapplies.scala

@@ -154,6 +156,10 @@ trait Unapplies extends ast.TreeDSL {
    val mods =
      if (applyShouldInheritAccess(inheritedMods))
        (caseMods | (inheritedMods.flags & PRIVATE)).copy(privateWithin = inheritedMods.privateWithin)
+          .tap { mods =>
+            if (currentRun.isScala3 && mods != caseMods)
+              runReporting.warning(cdef.pos, "constructor modifiers are assumed by synthetic `apply` method", WarningCategory.Scala3Migration, cdef.symbol)


question: what's the solution to this warning? I'm a bit confused by the message...

discord is pretty good for questions. There is a ticket to improve this warning.

@som-snytt scala/bug#12883 , or do you have some other ticket in mind?

I didn't want to create needless cross-reference noise... Probably this sounds like a "cross" reference.

scala-jenkins added this to the 2.13.12 milestone Jun 22, 2023

som-snytt force-pushed the issue/12798-xsource3-migration branch from 88537a6 to 8ed0075 Compare June 23, 2023 03:20

som-snytt commented Jun 23, 2023

View reviewed changes

test/files/neg/t5265b.check Show resolved Hide resolved

som-snytt commented Jun 23, 2023

View reviewed changes

test/files/neg/for-comprehension-val.check Show resolved Hide resolved

som-snytt commented Jun 23, 2023

View reviewed changes

src/compiler/scala/tools/reflect/FastStringInterpolator.scala Outdated Show resolved Hide resolved

som-snytt force-pushed the issue/12798-xsource3-migration branch from 8ed0075 to c850aeb Compare June 23, 2023 22:58

som-snytt marked this pull request as ready for review June 23, 2023 22:59

SethTisue added prio:hi high priority (used only by core team, only near release time) release-notes worth highlighting in next release notes labels Jun 27, 2023

lrytz reviewed Jun 29, 2023

View reviewed changes

src/compiler/scala/tools/nsc/ast/parser/Parsers.scala Show resolved Hide resolved

som-snytt commented Jul 1, 2023

View reviewed changes

src/compiler/scala/tools/nsc/ast/parser/Parsers.scala Outdated Show resolved Hide resolved

som-snytt force-pushed the issue/12798-xsource3-migration branch from c850aeb to ce9b345 Compare July 1, 2023 16:13

lrytz reviewed Jul 3, 2023

View reviewed changes

Use migration category for various warnings

c290271

Restore override behavior, partest ++ on update Tweak Unicode escapes warning

som-snytt force-pushed the issue/12798-xsource3-migration branch from 93ac153 to c290271 Compare July 3, 2023 19:34

Rename warning category migration to scala3-migration

1d4d907

Simplify a few compiler flags in tests

lrytz approved these changes Jul 4, 2023

View reviewed changes

lrytz changed the title ~~Use migration category for various warnings~~ Emit migration warnings under -Xsource:3 as fatal warnigns, not errors Jul 4, 2023

lrytz merged commit 0a460e8 into scala:2.13.x Jul 4, 2023
4 checks passed

som-snytt changed the title ~~Emit migration warnings under -Xsource:3 as fatal warnigns, not errors~~ Emit migration warnings under -Xsource:3 as fatal warnings, not errors Jul 4, 2023

som-snytt deleted the issue/12798-xsource3-migration branch July 4, 2023 12:55

lrytz mentioned this pull request Jul 4, 2023

Add explicit result type to some non-private methods #10444

Merged

som-snytt mentioned this pull request Jul 17, 2023

Distinguish warnings configured to error from "compiler-native" errors scala/bug#12831

Closed

SethTisue changed the title ~~Emit migration warnings under -Xsource:3 as fatal warnings, not errors~~ Emit migration warnings under -Xsource:3 as fatal warnings, not errors; -Xmigration disables fatality Aug 23, 2023

SethTisue changed the title ~~Emit migration warnings under -Xsource:3 as fatal warnings, not errors; -Xmigration disables fatality~~ Emit migration warnings under -Xsource:3 as fatal warnings, not errors; -Xmigration avoids fatality Aug 23, 2023

SethTisue changed the title ~~Emit migration warnings under -Xsource:3 as fatal warnings, not errors; -Xmigration avoids fatality~~ Emit migration warnings under -Xsource:3 as fatal warnings, not errors; -Xmigration disables fatality Aug 23, 2023

SethTisue mentioned this pull request Aug 26, 2023

Turn more -Xsource:3 errors into fatal warnings #10511

Merged

nafg added a commit to slick/slick that referenced this pull request Sep 10, 2023

add explicit type (#2825)

dd92938

prepare Scala 2.13.12 - scala/scala#10439

This was referenced Sep 11, 2023

add explicit type scala/scala-collection-laws#106

Merged

add explicit type lightbend/mima#784

Closed

problem with -Xsource:3 and Scala 2.13.12 scalapb/ScalaPB#1576

Closed

SethTisue mentioned this pull request Sep 19, 2023

Scala 2.13.12 release notes draft scala/scala-dev#850

Closed

This was referenced Oct 1, 2023

add explicit type scalatra/scalatra#1580

Merged

add explicit type scalatra/scalatra#1581

Merged

add "-Xsource:3" option nulab/scala-oauth2-provider#161

Merged

xuwei-k mentioned this pull request Oct 14, 2023

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

Merged

7 tasks

kubukoz reviewed Oct 16, 2023

View reviewed changes

xuwei-k mentioned this pull request Dec 30, 2023

add explicit type scopt/scopt#348

Merged

som-snytt mentioned this pull request Feb 28, 2024

Case companions no longer extend FunctionN under -Xsource:3-cross, breaking binary compatibility scala/bug#12961

Closed

som-snytt changed the title ~~Emit migration warnings under -Xsource:3 as fatal warnings, not errors; -Xmigration disables fatality~~ Emit migration warnings under -Xsource:3 as fatal warnings, not errors; -Xmigration turns off erroring Feb 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Emit migration warnings under `-Xsource:3` as fatal warnings, not errors; `-Xmigration` turns off erroring #10439

Emit migration warnings under `-Xsource:3` as fatal warnings, not errors; `-Xmigration` turns off erroring #10439

som-snytt commented Jun 22, 2023 •

edited by lrytz

som-snytt commented Jun 22, 2023

lrytz left a comment •

edited

som-snytt commented Jul 1, 2023

lrytz left a comment •

edited

lrytz Jul 3, 2023

som-snytt Jul 3, 2023

lrytz Jul 3, 2023

som-snytt commented Jul 3, 2023

som-snytt commented Jul 3, 2023

lrytz commented Jul 3, 2023

som-snytt commented Jul 3, 2023

lrytz commented Jul 3, 2023

lrytz Jul 4, 2023

SethTisue commented Jul 4, 2023

lrytz commented Jul 17, 2023

SethTisue commented Jul 17, 2023 •

edited

som-snytt commented Jul 17, 2023

kubukoz Oct 16, 2023

som-snytt Oct 16, 2023

SethTisue Oct 17, 2023

som-snytt Oct 17, 2023

		@@ -353,11 +364,13 @@ object Reporting {

		object JavaSource extends WarningCategory; add(JavaSource)

		object Migration extends WarningCategory; add(Migration)

Emit migration warnings under -Xsource:3 as fatal warnings, not errors; -Xmigration turns off erroring #10439

Emit migration warnings under -Xsource:3 as fatal warnings, not errors; -Xmigration turns off erroring #10439

Conversation

som-snytt commented Jun 22, 2023 • edited by lrytz

som-snytt commented Jun 22, 2023

lrytz left a comment • edited

Choose a reason for hiding this comment

som-snytt commented Jul 1, 2023

lrytz left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

som-snytt commented Jul 3, 2023

som-snytt commented Jul 3, 2023

lrytz commented Jul 3, 2023

som-snytt commented Jul 3, 2023

lrytz commented Jul 3, 2023

Choose a reason for hiding this comment

SethTisue commented Jul 4, 2023

lrytz commented Jul 17, 2023

SethTisue commented Jul 17, 2023 • edited

som-snytt commented Jul 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Emit migration warnings under `-Xsource:3` as fatal warnings, not errors; `-Xmigration` turns off erroring #10439

Emit migration warnings under `-Xsource:3` as fatal warnings, not errors; `-Xmigration` turns off erroring #10439

som-snytt commented Jun 22, 2023 •

edited by lrytz

lrytz left a comment •

edited

lrytz left a comment •

edited

SethTisue commented Jul 17, 2023 •

edited