Clean up and correct

som-snytt · som-snytt · commit 225eaec352f3 · 2025-02-17T09:48:01.000-08:00
diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala
@@ -616,6 +616,8 @@ trait Applications extends Compatibility {
       def infoStr = if methType.isErroneous then "" else i": $methType"
       i"${err.refStr(methRef)}$infoStr"
 
+    private type TreeList[T <: Untyped] = List[Trees.Tree[T]]
+
     /** Re-order arguments to correctly align named arguments
      *  Issue errors in the following situations:
      *
@@ -627,28 +629,43 @@ trait Applications extends Compatibility {
      *          (either named or positional), or
      *        - The formal parameter at the argument position is also mentioned
      *          in a subsequent named parameter.
-     *    - "parameter already instantiated" if a two named arguments have the same name.
+     *    - "parameter already instantiated" if two named arguments have the same name or deprecated alias.
      *    - "does not have parameter" if a named parameter does not mention a formal
      *      parameter name.
      */
-    def reorder[T <: Untyped](args: List[Trees.Tree[T]]): List[Trees.Tree[T]] =
-
-      inline def tailOf[A](list: List[A]): List[A] = if list.isEmpty then list else list.tail // list.drop(1)
-
-      def hasDeprecatedName(pname: Name, other: Name, t: Trees.Tree[T]): Boolean = !ctx.isAfterTyper &&
-        methRef.symbol.paramSymss.flatten.find(_.name == pname).flatMap(_.getAnnotation(defn.DeprecatedNameAnnot)).match
-        case Some(annot) =>
-          val name    = annot.argumentConstantString(0)
-          val version = annot.argumentConstantString(1).filter(!_.isEmpty)
-          val since   = version.map(v => s" (since $v)").getOrElse("")
-          name.map(_.toTermName) match
-          case Some(`other`) =>
-            report.deprecationWarning(em"the parameter name $other is deprecated$since: use $pname instead", t.srcPos)
-          case Some(`pname`) | None =>
-            report.deprecationWarning(em"naming parameter $pname is deprecated$since", t.srcPos)
-          case _ =>
-          true
-        case _ => false
+    def reorder[T <: Untyped](args: TreeList[T]): TreeList[T] =
+
+      extension [A](list: List[A]) inline def dropOne = if list.isEmpty then list else list.tail // aka list.drop(1)
+
+      extension (dna: Annotation)
+        def deprecatedName: Name =
+          dna.argumentConstantString(0).map(_.toTermName).getOrElse(nme.NO_NAME)
+        def since: String =
+          val version = dna.argumentConstantString(1).filter(!_.isEmpty)
+          version.map(v => s" (since $v)").getOrElse("")
+
+      val deprecatedNames: Map[Name, Annotation] =
+        methRef.symbol.paramSymss.find(_.exists(_.isTerm)) match
+        case Some(ps) if ps.exists(_.hasAnnotation(defn.DeprecatedNameAnnot)) =>
+          ps.flatMap: p =>
+            p.getAnnotation(defn.DeprecatedNameAnnot).map(p.name -> _)
+          .toMap
+        case _ => Map.empty
+
+      extension (name: Name)
+        def isMatchedBy(usage: Name): Boolean =
+          name == usage
+          || deprecatedNames.get(name).exists(_.deprecatedName == usage)
+        def checkDeprecationOf(usage: Name, pos: SrcPos): Unit = if !ctx.isAfterTyper then
+          for dna <- deprecatedNames.get(name) do
+            dna.deprecatedName match
+            case `name` | nme.NO_NAME if name == usage =>
+              report.deprecationWarning(em"naming parameter $usage is deprecated${dna.since}", pos)
+            case `usage` =>
+              report.deprecationWarning(em"the parameter name $usage is deprecated${dna.since}: use $name instead", pos)
+            case _ =>
+        def alternative: Name =
+          deprecatedNames.get(name).map(_.deprecatedName).getOrElse(nme.NO_NAME)
 
       /** Reorder the suffix of named args per a list of required names.
        *
@@ -662,55 +679,61 @@ trait Applications extends Compatibility {
        *
        *  1. `(args diff toDrop)` can be reordered to match `pnames`
        *  2. For every `(name -> arg)` in `nameToArg`, `arg` is an element of `args`
+       *
+       *  Recurse over the parameter names looking for named args to use.
+       *  If there are no more parameters or no args fit, process the next arg:
+       *  a named arg may be previously used, or not yet used, or badly named.
        */
-      def handleNamed(pnames: List[Name], args: List[Trees.Tree[T]],
+      def handleNamed(pnames: List[Name], args: TreeList[T],
                       nameToArg: Map[Name, Trees.NamedArg[T]], toDrop: Set[Name],
-                      missingArgs: Boolean): List[Trees.Tree[T]] =
+                      missingArgs: Boolean): TreeList[T] =
         pnames match
-        case pname :: pnames if nameToArg.contains(pname) => // use the named argument for this parameter
-          val arg = nameToArg(pname)
-          hasDeprecatedName(pname, nme.NO_NAME, arg)
+        case pname :: pnames if nameToArg.contains(pname) =>
+          val arg = nameToArg(pname) // use the named argument for this parameter
+          pname.checkDeprecationOf(pname, arg.srcPos)
           arg :: handleNamed(pnames, args, nameToArg - pname, toDrop + pname, missingArgs)
+        case pname :: pnames if nameToArg.contains(pname.alternative) =>
+          val alt = pname.alternative
+          val arg = nameToArg(alt) // use the named argument for this parameter
+          pname.checkDeprecationOf(alt, arg.srcPos)
+          arg :: handleNamed(pnames, args, nameToArg - alt, toDrop + alt, missingArgs)
         case _ =>
           args match
-            case allArgs @ (arg @ NamedArg(aname, _)) :: args =>
-              if toDrop.contains(aname) then // named argument was already picked
-                handleNamed(pnames, args, nameToArg, toDrop - aname, missingArgs)
-              else if pnames.nonEmpty && nameToArg.contains(aname) then
-                val pname = pnames.head
-                if hasDeprecatedName(pname, aname, arg) then // name was deprecated alt, so try again with canonical name
-                  val parg = cpy.NamedArg(arg)(pname, arg.arg).asInstanceOf[Trees.NamedArg[T]]
-                  handleNamed(pnames, parg :: args, nameToArg.removed(aname).updated(pname, parg), toDrop, missingArgs)
-                else // argument for pname is missing, pass an empty tree
-                  genericEmptyTree :: handleNamed(pnames.tail, allArgs, nameToArg, toDrop, missingArgs = true)
-              else // name not (or no longer) available for named arg
-                def msg =
-                  if methodType.paramNames.contains(aname) then
-                    em"parameter $aname of $methString is already instantiated"
-                  else
-                    em"$methString does not have a parameter $aname"
-                fail(msg, arg.asInstanceOf[Arg])
-                arg :: handleNamed(tailOf(pnames), args, nameToArg, toDrop, missingArgs)
-            case arg :: args =>
-              if toDrop.nonEmpty || missingArgs then
-                report.error(i"positional after named argument", arg.srcPos)
-              arg :: handleNamed(tailOf(pnames), args, nameToArg, toDrop, missingArgs) // unnamed argument; pick it
-            case nil => // no more args, continue to pick up any preceding named args
-              if pnames.isEmpty then nil
-              else handleNamed(tailOf(pnames), nil, nameToArg, toDrop, missingArgs)
-
-      /** Skip prefix of positional args, then handleNamed */
-      def handlePositional(pnames: List[Name], args: List[Trees.Tree[T]]): List[Trees.Tree[T]] =
-        args match
-          case (arg @ NamedArg(name, _)) :: args if !pnames.isEmpty && pnames.head == name =>
-            hasDeprecatedName(name, nme.NO_NAME, arg)
-            arg :: handlePositional(pnames.tail, args)
-          case (_: NamedArg) :: _ =>
-            val nameAssocs = args.collect { case arg @ NamedArg(name, _) => name -> arg }
-            handleNamed(pnames, args, nameAssocs.toMap, toDrop = Set.empty, missingArgs = false)
+          case allArgs @ (arg @ NamedArg(aname, _)) :: args =>
+            if toDrop.contains(aname) then
+              // named argument was already picked (using aname), skip it
+              handleNamed(pnames, args, nameToArg, toDrop - aname, missingArgs)
+            else if pnames.nonEmpty && nameToArg.contains(aname) then
+              // argument for pname is missing, pass an empty tree; arg may be used later, so keep it
+              genericEmptyTree :: handleNamed(pnames.tail, allArgs, nameToArg, toDrop, missingArgs = true)
+            else // name not (or no longer) available for named arg
+              def msg =
+                if methodType.paramNames.exists(nm => nm == aname || nm.alternative == aname) then
+                  em"parameter $aname of $methString is already instantiated"
+                else
+                  em"$methString does not have a parameter $aname"
+              fail(msg, arg.asInstanceOf[Arg])
+              arg :: handleNamed(pnames.dropOne, args, nameToArg, toDrop, missingArgs)
           case arg :: args =>
-            arg :: handlePositional(tailOf(pnames), args)
-          case nil => nil
+            if toDrop.nonEmpty || missingArgs then
+              report.error(i"positional after named argument", arg.srcPos)
+            arg :: handleNamed(pnames.dropOne, args, nameToArg, toDrop, missingArgs) // unnamed argument; pick it
+          case nil => // no more args, continue to pick up any preceding named args
+            if pnames.isEmpty then nil
+            else handleNamed(pnames.dropOne, args = nil, nameToArg, toDrop, missingArgs)
+
+      // Skip prefix of positional args, then handleNamed
+      def handlePositional(pnames: List[Name], args: TreeList[T]): TreeList[T] =
+        args match
+        case (arg @ NamedArg(name, _)) :: args if !pnames.isEmpty && pnames.head.isMatchedBy(name) =>
+          pnames.head.checkDeprecationOf(name, arg.srcPos)
+          arg :: handlePositional(pnames.tail, args)
+        case (_: NamedArg) :: _ =>
+          val nameAssocs = args.collect { case arg @ NamedArg(name, _) => name -> arg }
+          handleNamed(pnames, args, nameAssocs.toMap, toDrop = Set.empty, missingArgs = false)
+        case arg :: args =>
+          arg :: handlePositional(pnames.dropOne, args)
+        case nil => nil
 
       handlePositional(methodType.paramNames, args)
     end reorder
diff --git a/tests/neg/i19077.check b/tests/neg/i19077.check
@@ -0,0 +1,17 @@
+-- Error: tests/neg/i19077.scala:5:15 ----------------------------------------------------------------------------------
+5 |  f(1, 2, 3, x = 42) // error
+  |             ^^^^^^
+  |             parameter x of method f: (x: Int, y: Int, z: Int): Int is already instantiated
+-- Error: tests/neg/i19077.scala:6:15 ----------------------------------------------------------------------------------
+6 |  f(1, 2, 3, w = 42) // error
+  |             ^^^^^^
+  |             parameter w of method f: (x: Int, y: Int, z: Int): Int is already instantiated
+-- Error: tests/neg/i19077.scala:7:20 ----------------------------------------------------------------------------------
+7 |  f(1, 2, w = 42, z = 27) // error
+  |                  ^^^^^^
+  |                  parameter z of method f: (x: Int, y: Int, z: Int): Int is already instantiated
+-- Error: tests/neg/i19077.scala:8:20 ----------------------------------------------------------------------------------
+8 |  f(1, 2, z = 42, w = 27) // error
+  |                  ^^^^^^
+  |                  parameter w of method f: (x: Int, y: Int, z: Int): Int is already instantiated
+there was 1 deprecation warning; re-run with -deprecation for details
diff --git a/tests/neg/i19077.scala b/tests/neg/i19077.scala
@@ -0,0 +1,8 @@
+
+def f(@deprecatedName("x") x: Int, @deprecatedName y: Int, @deprecatedName("w") z: Int) = x+y+z
+
+@main def Test =
+  f(1, 2, 3, x = 42) // error
+  f(1, 2, 3, w = 42) // error
+  f(1, 2, w = 42, z = 27) // error
+  f(1, 2, z = 42, w = 27) // error
diff --git a/tests/warn/i19077.check b/tests/warn/i19077.check
@@ -1,24 +1,44 @@
--- Deprecation Warning: tests/warn/i19077.scala:8:6 --------------------------------------------------------------------
-8 |  f(x = 1, 2, 3) // warn
-  |    ^^^^^
-  |    naming parameter x is deprecated
--- Deprecation Warning: tests/warn/i19077.scala:9:9 --------------------------------------------------------------------
-9 |  f(1, y = 2, 3) // warn
-  |       ^^^^^
-  |       naming parameter y is deprecated
--- Deprecation Warning: tests/warn/i19077.scala:10:12 ------------------------------------------------------------------
-10 |  f(1, 2, w = 3) // warn
+-- Deprecation Warning: tests/warn/i19077.scala:26:14 ------------------------------------------------------------------
+26 |  def g = f(y = 42) // warn but omit empty since
+   |            ^^^^^^
+   |            the parameter name y is deprecated: use x instead
+-- Deprecation Warning: tests/warn/i19077.scala:12:6 -------------------------------------------------------------------
+12 |  f(x = 1, 2, 3) // warn
+   |    ^^^^^
+   |    naming parameter x is deprecated
+-- Deprecation Warning: tests/warn/i19077.scala:13:9 -------------------------------------------------------------------
+13 |  f(1, y = 2, 3) // warn
+   |       ^^^^^
+   |       naming parameter y is deprecated
+-- Deprecation Warning: tests/warn/i19077.scala:14:12 ------------------------------------------------------------------
+14 |  f(1, 2, w = 3) // warn
    |          ^^^^^
    |          the parameter name w is deprecated: use z instead
--- Deprecation Warning: tests/warn/i19077.scala:11:13 ------------------------------------------------------------------
-11 |  f(w = 3, x = 1, y = 2) // warn // warn // warn
+-- Deprecation Warning: tests/warn/i19077.scala:15:6 -------------------------------------------------------------------
+15 |  f(x = 1, w = 3, y = 2) // warn // warn // warn
+   |    ^^^^^
+   |    naming parameter x is deprecated
+-- Deprecation Warning: tests/warn/i19077.scala:15:20 ------------------------------------------------------------------
+15 |  f(x = 1, w = 3, y = 2) // warn // warn // warn
+   |                  ^^^^^
+   |                  naming parameter y is deprecated
+-- Deprecation Warning: tests/warn/i19077.scala:15:13 ------------------------------------------------------------------
+15 |  f(x = 1, w = 3, y = 2) // warn // warn // warn
+   |           ^^^^^
+   |           the parameter name w is deprecated: use z instead
+-- Deprecation Warning: tests/warn/i19077.scala:16:13 ------------------------------------------------------------------
+16 |  f(w = 3, x = 1, y = 2) // warn // warn // warn
    |           ^^^^^
    |           naming parameter x is deprecated
--- Deprecation Warning: tests/warn/i19077.scala:11:20 ------------------------------------------------------------------
-11 |  f(w = 3, x = 1, y = 2) // warn // warn // warn
+-- Deprecation Warning: tests/warn/i19077.scala:16:20 ------------------------------------------------------------------
+16 |  f(w = 3, x = 1, y = 2) // warn // warn // warn
    |                  ^^^^^
    |                  naming parameter y is deprecated
--- Deprecation Warning: tests/warn/i19077.scala:11:6 -------------------------------------------------------------------
-11 |  f(w = 3, x = 1, y = 2) // warn // warn // warn
+-- Deprecation Warning: tests/warn/i19077.scala:16:6 -------------------------------------------------------------------
+16 |  f(w = 3, x = 1, y = 2) // warn // warn // warn
    |    ^^^^^
    |    the parameter name w is deprecated: use z instead
+-- Deprecation Warning: tests/warn/i19077.scala:20:8 -------------------------------------------------------------------
+20 |  X.f(v = 42) // warn
+   |      ^^^^^^
+   |      the parameter name v is deprecated (since 3.3): use x instead
diff --git a/tests/warn/i19077.scala b/tests/warn/i19077.scala
@@ -2,10 +2,25 @@
 
 def f(@deprecatedName("x") x: Int, @deprecatedName y: Int, @deprecatedName("w") z: Int) = x+y+z
 
+object X:
+  def f(@deprecatedName("v", since="3.3") x: Int) = x
+  def f(@deprecatedName("v") x: Int, y: Int) = x+y
+
 @main def Test =
   f(1, 2, 3) // nowarn
   f(1, 2, z = 3) // nowarn
   f(x = 1, 2, 3) // warn
   f(1, y = 2, 3) // warn
   f(1, 2, w = 3) // warn
+  f(x = 1, w = 3, y = 2) // warn // warn // warn
   f(w = 3, x = 1, y = 2) // warn // warn // warn
+
+  X.f(42)
+  X.f(x = 42)
+  X.f(v = 42) // warn
+  X.f(x = 42, y = 27)
+  X.f(y = 42, x = 27)
+
+object empty:
+  def f(@deprecatedName("y", since="") x: Int) = x
+  def g = f(y = 42) // warn but omit empty since
