Do not visit args recursively in issueErrors

Instead, fix the display of nested `AmbiguousImplicits` errors.

Author: mbovel

compiler/src/dotty/tools/dotc/reporting/messages.scala

@@ -2980,7 +2980,7 @@ class MissingImplicitArgument(
      *  def foo(implicit foo: Foo): Any = ???
      */
     arg.tpe match
-      case ambi: AmbiguousImplicits =>
+      case ambi: AmbiguousImplicits if !ambi.nested =>
         (ambi.alt1, ambi.alt2) match
           case (alt @ AmbiguousImplicitMsg(msg), _) =>
             userDefinedAmbiguousImplicitMsg(alt, msg)

compiler/src/dotty/tools/dotc/typer/Implicits.scala

@@ -537,7 +537,7 @@ object Implicits:
   end TooUnspecific
 
   /** An ambiguous implicits failure */
-  class AmbiguousImplicits(val alt1: SearchSuccess, val alt2: SearchSuccess, val expectedType: Type, val argument: Tree) extends SearchFailureType:
+  class AmbiguousImplicits(val alt1: SearchSuccess, val alt2: SearchSuccess, val expectedType: Type, val argument: Tree, val nested: Boolean = false) extends SearchFailureType:
 
     def msg(using Context): Message =
       var str1 = err.refStr(alt1.ref)

compiler/src/dotty/tools/dotc/typer/Typer.scala

@@ -3872,38 +3872,36 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
         end implicitArgs
 
         /** Reports errors for arguments of `appTree` that have a
-          * `SearchFailureType`, recursively traversing arguments that are
-          * themselves applications. `mt` must be the type of `appTree.fun`.
+          * `SearchFailureType`.
           */
-        def reportErrors(appTree: Apply, mt: MethodType): Unit =
-          val Apply(fun, args) = appTree
-          for (paramName, formal, arg) <- mt.paramNames.lazyZip(mt.paramInfos).lazyZip(args) do
+        def issueErrors(fun: Tree, args: List[Tree]): Tree =
+          def firstFailure = args.tpes.find(_.isInstanceOf[SearchFailureType]).getOrElse(NoType)
+          val errorType =
+            firstFailure match
+              case tp: AmbiguousImplicits =>
+                AmbiguousImplicits(tp.alt1, tp.alt2, tp.expectedType, tp.argument, nested = true)
+              case tp =>
+                tp
+          val res = untpd.Apply(fun, args).withType(errorType)
+
+          wtp.paramNames.lazyZip(wtp.paramInfos).lazyZip(args).foreach { (paramName, formal, arg) =>
             arg.tpe match
               case failure: SearchFailureType =>
-                arg match
-                  case childAppTree: Apply =>
-                    childAppTree.fun.tpe.widen match
-                      case childMt: MethodType => reportErrors(childAppTree, childMt)
-                      case _ => ()
-                  case _ => ()
-
                 val methodStr = err.refStr(methPart(fun).tpe)
                 val paramStr = implicitParamString(paramName, methodStr, fun)
-                val paramSymWithMethodCallTree =
-                  fun.symbol.paramSymss.flatten
-                    .find(_.name == paramName)
-                    .map((_, appTree))
-                val message = missingArgMsg(arg, formal, paramStr, paramSymWithMethodCallTree)
-                // Note: if the same error type appears on several trees, we
-                // might report it several times, but this is not a problem
-                // because only the first one will be displayed. We traverse in
-                // post-order, so that the most detailed message gets displayed.
-                report.error(message, fun.srcPos.endPos)
-              case _ => ()
+                val paramSym = fun.symbol.paramSymss.flatten.find(_.name == paramName)
+                val paramSymWithMethodCallTree = paramSym.map((_, res))
+                report.error(
+                   missingArgMsg(arg, formal, paramStr, paramSymWithMethodCallTree),
+                   tree.srcPos.endPos
+                 )
+              case _ =>
+          }
+
+          res
 
         val args = implicitArgs(wtp.paramInfos, 0, pt)
-        val firstFailure = args.tpes.find(_.isInstanceOf[SearchFailureType])
-        if (firstFailure.isDefined) {
+        if (args.tpes.exists(_.isInstanceOf[SearchFailureType])) {
           // If there are several arguments, some arguments might already
           // have influenced the context, binding variables, but later ones
           // might fail. In that case the constraint and instantiated variables
@@ -3934,14 +3932,9 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
             // node, we can report the errors for each argument nicely.
             // Otherwise, we don't report anything here.
             retyped match
-              case retyped: Apply if retyped.tpe.isError => reportErrors(retyped, wtp)
-              case _ => ()
-
-            retyped
-          else
-            val res = untpd.Apply(tree, args).withType(firstFailure.get)
-            reportErrors(res, wtp)
-            res
+              case Apply(tree, args) if retyped.tpe.isError => issueErrors(tree, args)
+              case _ => retyped
+          else issueErrors(tree, args)
         }
         else tree match {
           case tree: Block =>

tests/neg/19414-desugared.check

@@ -1,4 +1,14 @@
 -- [E172] Type Error: tests/neg/19414-desugared.scala:22:34 ------------------------------------------------------------
 22 |  summon[BodySerializer[JsObject]] // error: Ambiguous given instances
    |                                  ^
-   |Ambiguous given instances: both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[B] of parameter writer of given instance given_BodySerializer_B
+   |No given instance of type BodySerializer[JsObject] was found for parameter x of method summon in object Predef.
+   |I found:
+   |
+   |    given_BodySerializer_B[B](
+   |      writer =
+   |        /* ambiguous: both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[B] */
+   |          summon[Writer[B]]
+   |      ,
+   |    this.given_BodySerializer_B$default$2[B])
+   |
+   |But both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[B].

tests/neg/19414.check

@@ -1,4 +1,14 @@
 -- [E172] Type Error: tests/neg/19414.scala:15:34 ----------------------------------------------------------------------
 15 |  summon[BodySerializer[JsObject]] // error: Ambiguous given instances
    |                                  ^
-   |Ambiguous given instances: both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[B] of a context parameter of given instance given_BodySerializer_B
+   |No given instance of type BodySerializer[JsObject] was found for parameter x of method summon in object Predef.
+   |I found:
+   |
+   |    given_BodySerializer_B[B](
+   |      evidence$1 =
+   |        /* ambiguous: both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[B] */
+   |          summon[Writer[B]]
+   |      ,
+   |    this.given_BodySerializer_B$default$2[B])
+   |
+   |But both given instance given_Writer_JsValue and given instance given_Writer_JsObject match type Writer[B].

tests/neg/given-ambiguous-1.check

@@ -0,0 +1,9 @@
+-- [E172] Type Error: tests/neg/given-ambiguous-1.scala:12:23 ----------------------------------------------------------
+12 |def f: Unit = summon[B] // error: Ambiguous given instances
+   |                       ^
+   |                No given instance of type B was found for parameter x of method summon in object Predef.
+   |                I found:
+   |
+   |                    given_B(/* ambiguous: both given instance a1 and given instance a2 match type A */summon[A])
+   |
+   |                But both given instance a1 and given instance a2 match type A.

tests/neg/given-ambiguous-1.scala

@@ -0,0 +1,12 @@
+class A
+class B
+given a1: A = ???
+given a2: A = ???
+given (using a: A): B = ???
+
+// In this case, the ambiguous given instance is not directly the argument of
+// `summon`; it is the argument of `given_B` which is needed for the argument of
+// `summon`. This is a nested ambiguous implicit, thus we report an error in
+// the style "I found ... but". See `given-ambiguous-2` for a direct ambiguous
+// implicit error.
+def f: Unit = summon[B] // error: Ambiguous given instances

tests/neg/given-ambiguous-2.check

@@ -0,0 +1,4 @@
+-- [E172] Type Error: tests/neg/given-ambiguous-2.scala:10:15 ----------------------------------------------------------
+10 |def f: Unit = g // error: Ambiguous given instances
+   |               ^
+   | Ambiguous given instances: both given instance a1 and given instance a2 match type A of parameter a of method g

tests/neg/given-ambiguous-2.scala

@@ -0,0 +1,10 @@
+class A
+class B
+given a1: A = ???
+given a2: A = ???
+def g(using a: A): B = ???
+
+// In this case, the ambiguous given instance is directly the argument of
+// `summon`. This is a direct ambiguous implicit, thus we report the error
+// directly. See `given-ambiguous-1` for a nested ambiguous implicit error.
+def f: Unit = g // error: Ambiguous given instances

tests/neg/given-ambiguous-default-1.check

@@ -1,4 +1,9 @@
 -- [E172] Type Error: tests/neg/given-ambiguous-default-1.scala:18:23 --------------------------------------------------
 18 |def f: Unit = summon[B] // error: Ambiguous given instances
    |                       ^
-   |Ambiguous given instances: both given instance a1 and given instance a2 match type A of parameter a of given instance given_B
+   |            No given instance of type B was found for parameter x of method summon in object Predef.
+   |            I found:
+   |
+   |                given_B(a = /* ambiguous: both given instance a1 and given instance a2 match type A */summon[A])
+   |
+   |            But both given instance a1 and given instance a2 match type A.