Refactor migration warnings (#19102)

Now we have a single way to define migration warning versions and a
single place to see all the deprecations we still need to address.

Author: nicolasstucki

compiler/src/dotty/tools/dotc/config/MigrationVersion.scala

@@ -0,0 +1,42 @@
+package dotty.tools
+package dotc
+package config
+
+import SourceVersion.*
+import Feature.*
+import core.Contexts.Context
+
+class MigrationVersion(val warnFrom: SourceVersion, val errorFrom: SourceVersion):
+  assert(warnFrom.ordinal <= errorFrom.ordinal)
+  def needsPatch(using Context): Boolean =
+    sourceVersion.isMigrating && sourceVersion.isAtLeast(errorFrom)
+
+object MigrationVersion:
+
+  val Scala2to3 = MigrationVersion(`3.0`, `3.0`)
+
+  val OverrideValParameter = MigrationVersion(`3.0`, future)
+
+  // we tighten for-comprehension without `case` to error in 3.4,
+  // but we keep pat-defs as warnings for now ("@unchecked"),
+  // until we propose an alternative way to assert exhaustivity to the typechecker.
+  val ForComprehensionPatternWithoutCase = MigrationVersion(`3.2`,  `3.4`)
+  val ForComprehensionUncheckedPathDefs = MigrationVersion(`3.2`,  future)
+
+  val NonLocalReturns = MigrationVersion(`3.2`, future)
+
+  val AscriptionAfterPattern = MigrationVersion(`3.3`, future)
+
+  val AlphanumericInfix = MigrationVersion(`3.4`, future)
+  val RemoveThisQualifier = MigrationVersion(`3.4`, future)
+  val UninitializedVars = MigrationVersion(`3.4`, future)
+  val VarargSpliceAscription = MigrationVersion(`3.4`, future)
+  val WildcardType = MigrationVersion(`3.4`, future)
+  val WithOperator = MigrationVersion(`3.4`, future)
+  val FunctionUnderscore = MigrationVersion(`3.4`, future)
+
+  val ImportWildcard = MigrationVersion(future, future)
+  val ImportRename = MigrationVersion(future, future)
+  val ParameterEnclosedByParenthesis = MigrationVersion(future, future)
+
+end MigrationVersion

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

@@ -33,6 +33,7 @@ import config.Feature
 import config.Feature.{sourceVersion, migrateTo3, globalOnlyImports}
 import config.SourceVersion.*
 import config.SourceVersion
+import dotty.tools.dotc.config.MigrationVersion
 
 object Parsers {
 
@@ -462,8 +463,8 @@ object Parsers {
         case t @ Typed(Ident(_), _) =>
           report.errorOrMigrationWarning(
             em"parentheses are required around the parameter of a lambda${rewriteNotice()}",
-            in.sourcePos(), from = `3.0`)
-          if sourceVersion.isMigrating then
+            in.sourcePos(), MigrationVersion.Scala2to3)
+          if MigrationVersion.Scala2to3.needsPatch then
             patch(source, t.span.startPos, "(")
             patch(source, t.span.endPos, ")")
           convertToParam(t, mods) :: Nil
@@ -1314,8 +1315,8 @@ object Parsers {
                     |or enclose in braces '{$name} if you want a quoted expression.
                     |For now, you can also `import language.deprecated.symbolLiterals` to accept
                     |the idiom, but this possibility might no longer be available in the future.""",
-                in.sourcePos(), from = `3.0`)
-              if sourceVersion.isMigrating then
+                in.sourcePos(), MigrationVersion.Scala2to3)
+              if MigrationVersion.Scala2to3.needsPatch then
                 patch(source, Span(in.offset, in.offset + 1), "Symbol(\"")
                 patch(source, Span(in.charOffset - 1), "\")")
             atSpan(in.skipToken()) { SymbolLit(in.strVal) }
@@ -1412,8 +1413,8 @@ object Parsers {
             em"""This opening brace will start a new statement in Scala 3.
                 |It needs to be indented to the right to keep being treated as
                 |an argument to the previous expression.${rewriteNotice()}""",
-            in.sourcePos(), from = `3.0`)
-          if sourceVersion.isMigrating then
+            in.sourcePos(), MigrationVersion.Scala2to3)
+          if MigrationVersion.Scala2to3.needsPatch then
             patch(source, Span(in.offset), "  ")
 
     def possibleTemplateStart(isNew: Boolean = false): Unit =
@@ -1776,12 +1777,11 @@ object Parsers {
           t
         else
           val withSpan = Span(withOffset, withOffset + 4)
-          report.gradualErrorOrMigrationWarning(
+          report.errorOrMigrationWarning(
             DeprecatedWithOperator(rewriteNotice(`3.4-migration`)),
             source.atSpan(withSpan),
-            warnFrom = `3.4`,
-            errorFrom = future)
-          if sourceVersion.isMigrating && sourceVersion.isAtLeast(`3.4-migration`) then
+            MigrationVersion.WithOperator)
+          if MigrationVersion.WithOperator.needsPatch then
             patch(source, withSpan, "&")
           atSpan(startOffset(t)) { makeAndType(t, withType()) }
       else t
@@ -1882,12 +1882,11 @@ object Parsers {
           Ident(tpnme.USCOREkw).withSpan(Span(start, in.lastOffset, start))
         else
           if !inTypeMatchPattern then
-            report.gradualErrorOrMigrationWarning(
+            report.errorOrMigrationWarning(
               em"`_` is deprecated for wildcard arguments of types: use `?` instead${rewriteNotice(`3.4-migration`)}",
               in.sourcePos(),
-              warnFrom = `3.4`,
-              errorFrom = future)
-            if sourceVersion.isMigrating && sourceVersion.isAtLeast(`3.4-migration`) then
+              MigrationVersion.WildcardType)
+            if MigrationVersion.WildcardType.needsPatch then
               patch(source, Span(in.offset, in.offset + 1), "?")
           end if
           val start = in.skipToken()
@@ -2111,7 +2110,7 @@ object Parsers {
       else if in.token == VIEWBOUND then
         report.errorOrMigrationWarning(
           em"view bounds `<%' are no longer supported, use a context bound `:' instead",
-          in.sourcePos(), from = `3.0`)
+          in.sourcePos(), MigrationVersion.Scala2to3)
         atSpan(in.skipToken()) {
           Function(Ident(pname) :: Nil, toplevelTyp())
         } :: contextBounds(pname)
@@ -2260,15 +2259,15 @@ object Parsers {
         report.errorOrMigrationWarning(
           em"""`do <body> while <cond>` is no longer supported,
               |use `while <body> ; <cond> do ()` instead.${rewriteNotice()}""",
-          in.sourcePos(), from = `3.0`)
+          in.sourcePos(), MigrationVersion.Scala2to3)
         val start = in.skipToken()
         atSpan(start) {
           val body = expr()
           if (isStatSep) in.nextToken()
           val whileStart = in.offset
           accept(WHILE)
           val cond = expr()
-          if sourceVersion.isMigrating then
+          if MigrationVersion.Scala2to3.needsPatch then
             patch(source, Span(start, start + 2), "while ({")
             patch(source, Span(whileStart, whileStart + 5), ";")
             cond match {
@@ -2370,18 +2369,17 @@ object Parsers {
           val isVarargSplice = location.inArgs && followingIsVararg()
           in.nextToken()
           if isVarargSplice then
-            report.gradualErrorOrMigrationWarning(
+            report.errorOrMigrationWarning(
               em"The syntax `x: _*` is no longer supported for vararg splices; use `x*` instead${rewriteNotice(`3.4-migration`)}",
               in.sourcePos(uscoreStart),
-              warnFrom = `3.4`,
-              errorFrom = future)
-            if sourceVersion.isMigrating && sourceVersion.isAtLeast(`3.4-migration`) then
+              MigrationVersion.VarargSpliceAscription)
+            if MigrationVersion.VarargSpliceAscription.needsPatch then
               patch(source, Span(t.span.end, in.lastOffset), "*")
           else if opStack.nonEmpty then
             report.errorOrMigrationWarning(
               em"""`_*` can be used only for last argument of method application.
                   |It is no longer allowed in operands of infix operations.""",
-              in.sourcePos(uscoreStart), from = `3.0`)
+              in.sourcePos(uscoreStart), MigrationVersion.Scala2to3)
           else
             syntaxError(SeqWildcardPatternPos(), uscoreStart)
           Typed(t, atSpan(uscoreStart) { Ident(tpnme.WILDCARD_STAR) })
@@ -2448,13 +2446,12 @@ object Parsers {
             report.errorOrMigrationWarning(
               em"This syntax is no longer supported; parameter needs to be enclosed in (...)${rewriteNotice(`future-migration`)}",
               source.atSpan(Span(start, in.lastOffset)),
-              from = future)
+              MigrationVersion.ParameterEnclosedByParenthesis)
             in.nextToken()
             val t = infixType()
-            if (sourceVersion == `future-migration`) {
+            if MigrationVersion.ParameterEnclosedByParenthesis.needsPatch then
               patch(source, Span(start), "(")
               patch(source, Span(in.lastOffset), ")")
-            }
             t
           }
           else TypeTree()
@@ -2958,15 +2955,13 @@ object Parsers {
       if in.isColon then
         val isVariableOrNumber = isVarPattern(p) || p.isInstanceOf[Number]
         if !isVariableOrNumber then
-          report.gradualErrorOrMigrationWarning(
+          report.errorOrMigrationWarning(
             em"""Type ascriptions after patterns other than:
                 |  * variable pattern, e.g. `case x: String =>`
                 |  * number literal pattern, e.g. `case 10.5: Double =>`
                 |are no longer supported. Remove the type ascription or move it to a separate variable pattern.""",
             in.sourcePos(),
-            warnFrom = `3.3`,
-            errorFrom = future
-          )
+            MigrationVersion.AscriptionAfterPattern)
         in.nextToken()
         ascription(p, location)
       else p
@@ -3147,13 +3142,12 @@ object Parsers {
           else mods.withPrivateWithin(ident().toTypeName)
         }
         if mods1.is(Local) then
-          report.gradualErrorOrMigrationWarning(
+          report.errorOrMigrationWarning(
               em"""The [this] qualifier will be deprecated in the future; it should be dropped.
                   |See: https://docs.scala-lang.org/scala3/reference/dropped-features/this-qualifier.html${rewriteNotice(`3.4-migration`)}""",
               in.sourcePos(),
-              warnFrom = `3.4`,
-              errorFrom = future)
-          if sourceVersion.isMigrating && sourceVersion.isAtLeast(`3.4-migration`) then
+              MigrationVersion.RemoveThisQualifier)
+          if MigrationVersion.RemoveThisQualifier.needsPatch then
             patch(source, Span(startOffset, in.lastOffset), "")
         mods1
       }
@@ -3542,8 +3536,8 @@ object Parsers {
           report.errorOrMigrationWarning(
             em"`_` is no longer supported for a wildcard $exprName; use `*` instead${rewriteNotice(`future-migration`)}",
             in.sourcePos(),
-            from = future)
-          if sourceVersion == `future-migration` then
+            MigrationVersion.ImportWildcard)
+          if MigrationVersion.ImportWildcard.needsPatch then
             patch(source, Span(in.offset, in.offset + 1), "*")
         ImportSelector(atSpan(in.skipToken()) { Ident(nme.WILDCARD) })
 
@@ -3562,8 +3556,8 @@ object Parsers {
             report.errorOrMigrationWarning(
               em"The $exprName renaming `a => b` is no longer supported ; use `a as b` instead${rewriteNotice(`future-migration`)}",
               in.sourcePos(),
-              from = future)
-            if sourceVersion == `future-migration` then
+              MigrationVersion.ImportRename)
+            if MigrationVersion.ImportRename.needsPatch then
               patch(source, Span(in.offset, in.offset + 2),
                   if testChar(in.offset - 1, ' ') && testChar(in.offset + 2, ' ') then "as"
                   else " as ")
@@ -3674,13 +3668,12 @@ object Parsers {
           subExpr() match
             case rhs0 @ Ident(name) if placeholderParams.nonEmpty && name == placeholderParams.head.name
                 && !tpt.isEmpty && mods.is(Mutable) && lhs.forall(_.isInstanceOf[Ident]) =>
-              report.gradualErrorOrMigrationWarning(
+              report.errorOrMigrationWarning(
                 em"""`= _` has been deprecated; use `= uninitialized` instead.
                         |`uninitialized` can be imported with `scala.compiletime.uninitialized`.${rewriteNotice(`3.4-migration`)}""",
                 in.sourcePos(rhsOffset),
-                warnFrom = `3.4`,
-                errorFrom = future)
-              if sourceVersion.isMigrating && sourceVersion.isAtLeast(`3.4-migration`) then
+                MigrationVersion.UninitializedVars)
+              if MigrationVersion.UninitializedVars.needsPatch then
                 patch(source, Span(rhsOffset, rhsOffset + 1), "scala.compiletime.uninitialized")
               placeholderParams = placeholderParams.tail
               atSpan(rhs0.span) { Ident(nme.WILDCARD) }
@@ -3722,9 +3715,10 @@ object Parsers {
           else ": Unit "  // trailing space ensures that `def f()def g()` works.
         if migrateTo3 then
           report.errorOrMigrationWarning(
-            em"Procedure syntax no longer supported; `$toInsert` should be inserted here",
-            in.sourcePos(), from = `3.0`)
-          patch(source, Span(in.lastOffset), toInsert)
+            em"Procedure syntax no longer supported; `$toInsert` should be inserted here${rewriteNotice()}",
+            in.sourcePos(), MigrationVersion.Scala2to3)
+          if MigrationVersion.Scala2to3.needsPatch then
+            patch(source, Span(in.lastOffset), toInsert)
           true
         else
           false
@@ -4150,7 +4144,7 @@ object Parsers {
           if (in.token == LBRACE || in.token == COLONeol) {
             report.errorOrMigrationWarning(
               em"`extends` must be followed by at least one parent",
-              in.sourcePos(), from = `3.0`)
+              in.sourcePos(), MigrationVersion.Scala2to3)
             Nil
           }
           else constrApps()

compiler/src/dotty/tools/dotc/parsing/Scanners.scala

@@ -19,6 +19,7 @@ import rewrites.Rewrites.patch
 import config.Feature
 import config.Feature.{migrateTo3, fewerBracesEnabled}
 import config.SourceVersion.{`3.0`, `3.0-migration`}
+import config.MigrationVersion
 import reporting.{NoProfile, Profile, Message}
 
 import java.util.Objects
@@ -257,8 +258,8 @@ object Scanners {
           report.errorOrMigrationWarning(
             em"$what is now a keyword, write `$what` instead of $what to keep it as an identifier${rewriteNotice("This", `3.0-migration`)}",
             sourcePos(),
-            from = `3.0`)
-          if sourceVersion.isMigrating then
+            MigrationVersion.Scala2to3)
+          if MigrationVersion.Scala2to3.needsPatch then
             patch(source, Span(offset), "`")
             patch(source, Span(offset + identifier.length), "`")
           IDENTIFIER
@@ -470,7 +471,7 @@ object Scanners {
             em"""$what starts with an operator;
                 |it is now treated as a continuation of the $previous,
                 |not as a separate statement.""",
-            sourcePos(), from = `3.0`)
+            sourcePos(), MigrationVersion.Scala2to3)
         true
       }
 

compiler/src/dotty/tools/dotc/report.scala

@@ -9,6 +9,7 @@ import config.SourceVersion
 import ast.*
 import config.Feature.sourceVersion
 import java.lang.System.currentTimeMillis
+import dotty.tools.dotc.config.MigrationVersion
 
 object report:
 
@@ -80,15 +81,11 @@ object report:
     if ctx.settings.YdebugError.value then Thread.dumpStack()
     if ctx.settings.YdebugTypeError.value then ex.printStackTrace()
 
-  def errorOrMigrationWarning(msg: Message, pos: SrcPos, from: SourceVersion)(using Context): Unit =
-    if sourceVersion.isAtLeast(from) then
-      if sourceVersion.isMigrating && sourceVersion.ordinal <= from.ordinal then
-        if ctx.settings.rewrite.value.isEmpty then migrationWarning(msg, pos)
-      else error(msg, pos)
-
-  def gradualErrorOrMigrationWarning(msg: Message, pos: SrcPos, warnFrom: SourceVersion, errorFrom: SourceVersion)(using Context): Unit =
-    if sourceVersion.isAtLeast(errorFrom) then errorOrMigrationWarning(msg, pos, errorFrom)
-    else if sourceVersion.isAtLeast(warnFrom) then warning(msg, pos)
+  def errorOrMigrationWarning(msg: Message, pos: SrcPos, migrationVersion: MigrationVersion)(using Context): Unit =
+    if sourceVersion.isAtLeast(migrationVersion.errorFrom) then
+      if !sourceVersion.isMigrating then error(msg, pos)
+      else if ctx.settings.rewrite.value.isEmpty then migrationWarning(msg, pos)
+    else if sourceVersion.isAtLeast(migrationVersion.warnFrom) then warning(msg, pos)
 
   def restrictionError(msg: Message, pos: SrcPos = NoSourcePosition)(using Context): Unit =
     error(msg.mapMsg("Implementation restriction: " + _), pos)

compiler/src/dotty/tools/dotc/transform/NonLocalReturns.scala

@@ -7,6 +7,7 @@ import MegaPhase.*
 import NameKinds.NonLocalReturnKeyName
 import config.SourceVersion.*
 import Decorators.em
+import dotty.tools.dotc.config.MigrationVersion
 
 object NonLocalReturns {
   import ast.tpd.*
@@ -96,11 +97,10 @@ class NonLocalReturns extends MiniPhase {
 
   override def transformReturn(tree: Return)(using Context): Tree =
     if isNonLocalReturn(tree) then
-      report.gradualErrorOrMigrationWarning(
+      report.errorOrMigrationWarning(
           em"Non local returns are no longer supported; use `boundary` and `boundary.break` in `scala.util` instead",
           tree.srcPos,
-          warnFrom = `3.2`,
-          errorFrom = future)
+          MigrationVersion.NonLocalReturns)
       nonLocalReturnThrow(tree.expr, tree.from.symbol).withSpan(tree.span)
     else tree
 }