Disambiguate by owner score before considering further implicit arguments

This makes slick-migration-api-example work and makes the original scala-uri.scala fail.
See the comment in neg/scala-uri.scala for why this is so. It also needs a change in the
givens in dotty.tools.reporting.Formatting.

The motivation for honoring owner score over the others is that it is often used for explicit
prioritization, so we should take it into account more than other secondary criteria.

Author: odersky

Diff for: compiler/src/dotty/tools/dotc/printing/Formatting.scala

@@ -50,7 +50,11 @@ object Formatting {
     object ShowAny extends Show[Any]:
       def show(x: Any): Shown = x
 
-    class ShowImplicits3:
+    class ShowImplicits4:
+      given [X: Show]: Show[X | Null] with
+        def show(x: X | Null) = if x == null then "null" else CtxShow(toStr(x.nn))
+
+    class ShowImplicits3 extends ShowImplicits4:
       given Show[Product] = ShowAny
 
     class ShowImplicits2 extends ShowImplicits3:
@@ -80,9 +84,6 @@ object Formatting {
         def show(x: H *: T) =
           CtxShow(toStr(x.head) *: toShown(x.tail).asInstanceOf[Tuple])
 
-      given [X: Show]: Show[X | Null] with
-        def show(x: X | Null) = if x == null then "null" else CtxShow(toStr(x.nn))
-
       given Show[FlagSet] with
         def show(x: FlagSet) = x.flagsString
 

Diff for: compiler/src/dotty/tools/dotc/typer/Applications.scala

@@ -1865,7 +1865,7 @@ trait Applications extends Compatibility {
      *   - From Scala 3.6, `T <:p U` means `T <: U` or `T` convertible to `U`
      *     for overloading resolution (when `preferGeneral is false), and the opposite relation
      *     `U <: T` or `U convertible to `T` for implicit disambiguation between givens
-     *     (when `preferGeneral` is true). 
+     *     (when `preferGeneral` is true).
      *
      *   - In Scala 3.5 and Scala 3.6-migration, we issue a warning if the result under
      *     Scala 3.6 differ wrt to the old behavior up to 3.5.
@@ -1963,9 +1963,8 @@ trait Applications extends Compatibility {
       else if winsPrefix1 then 1
       else -1
 
-    val ownerScore = compareOwner(alt1.symbol.maybeOwner, alt2.symbol.maybeOwner)
-
     def compareWithTypes(tp1: Type, tp2: Type) =
+      val ownerScore = compareOwner(alt1.symbol.maybeOwner, alt2.symbol.maybeOwner)
       val winsType1 = isAsGood(alt1, tp1, alt2, tp2)
       val winsType2 = isAsGood(alt2, tp2, alt1, tp1)
 
@@ -1977,25 +1976,22 @@ trait Applications extends Compatibility {
         // (prefer the one that is not a method, but that's arbitrary).
         if alt1.widenExpr =:= alt2 then -1 else 1
       else
+        // For implicit resolution, take ownerscore as more significant than type resolution
+        // Reason: People use owner hierarchies to explicitly prioritize, we should not
+        // break that by changing implicit priority of types.
+        def drawOrOwner =
+          if preferGeneral && !ctx.mode.is(Mode.OldImplicitResolution)
+          then ownerScore
+          else 0
         ownerScore match
-          case  1 => if winsType1 || !winsType2 then  1 else 0
-          case -1 => if winsType2 || !winsType1 then -1 else 0
+          case  1 => if winsType1 || !winsType2 then  1 else drawOrOwner
+          case -1 => if winsType2 || !winsType1 then -1 else drawOrOwner
           case  0 =>
             if winsType1 != winsType2 then if winsType1 then 1 else -1
             else if alt1.symbol == alt2.symbol then comparePrefixes
             else 0
     end compareWithTypes
 
-    // For implicit resolution, take ownerscore as more significant than type resolution
-    // Reason: People use owner hierarchies to explicitly prioritize, we should not
-    // break that by changing implicit priority of types. On the other hand, we do
-    // want to exhaust all other possibilities before using owner score as a tie breaker.
-    // For instance, pos/scala-uri.scala depends on that.
-    def disambiguateWithOwner(result: Int) =
-      if result == 0 && preferGeneral && !ctx.mode.is(Mode.OldImplicitResolution)
-      then ownerScore
-      else result
-
     if alt1.symbol.is(ConstructorProxy) && !alt2.symbol.is(ConstructorProxy) then -1
     else if alt2.symbol.is(ConstructorProxy) && !alt1.symbol.is(ConstructorProxy) then 1
     else
@@ -2005,14 +2001,12 @@ trait Applications extends Compatibility {
       val strippedType2 = stripImplicit(fullType2)
 
       var result = compareWithTypes(strippedType1, strippedType2)
-      if result != 0 then return result
-      if strippedType1 eq fullType1 then
-        if strippedType2 eq fullType2
-        then disambiguateWithOwner(0)             // no implicits either side: its' a draw
+      if result != 0 then result
+      else if strippedType1 eq fullType1 then
+        if strippedType2 eq fullType2 then 0      // no implicits either side: its' a draw
         else 1                                    // prefer 1st alternative with no implicits
       else if strippedType2 eq fullType2 then -1  // prefer 2nd alternative with no implicits
-      else disambiguateWithOwner(
-        compareWithTypes(fullType1, fullType2))   // continue by comparing implicit parameters
+      else compareWithTypes(fullType1, fullType2) // continue by comparing implicit parameters
   }
   end compare
 

Diff for: tests/neg/scala-uri.check

@@ -0,0 +1,14 @@
+-- [E172] Type Error: tests/neg/scala-uri.scala:30:59 ------------------------------------------------------------------
+30 |@main def Test = summon[QueryKeyValue[(String, None.type)]]  // error
+   |                                                           ^
+   |No best given instance of type QueryKeyValue[(String, None.type)] was found for parameter x of method summon in object Predef.
+   |I found:
+   |
+   |    QueryKeyValue.tuple2QueryKeyValue[String, None.type](QueryKey.stringQueryKey,
+   |      QueryValue.optionQueryValue[A](
+   |        /* ambiguous: both value stringQueryValue in trait QueryValueInstances1 and value noneQueryValue in trait QueryValueInstances1 match type QueryValue[A] */
+   |          summon[QueryValue[A]]
+   |      )
+   |    )
+   |
+   |But both value stringQueryValue in trait QueryValueInstances1 and value noneQueryValue in trait QueryValueInstances1 match type QueryValue[A].

Diff for: tests/neg/scala-uri.scala

@@ -0,0 +1,30 @@
+import scala.language.implicitConversions
+
+trait QueryKey[A]
+object QueryKey extends QueryKeyInstances
+sealed trait QueryKeyInstances:
+  implicit val stringQueryKey: QueryKey[String] = ???
+
+trait QueryValue[-A]
+object QueryValue extends QueryValueInstances
+sealed trait QueryValueInstances1:
+  implicit final val stringQueryValue: QueryValue[String] = ???
+  implicit final val noneQueryValue: QueryValue[None.type] = ???
+    // The noneQueryValue makes no sense at this priority. Since QueryValue
+    // is contravariant, QueryValue[None.type] is always better than QueryValue[Option[A]]
+    // no matter whether it's old or new resolution. So taking both owner and type
+    // score into account, it's always a draw. With the new disambiguation, we prefer
+    // the optionQueryValue[A], which gives an ambiguity down the road, because we don't
+    // know what the  wrapped type A is. Previously, we preferred QueryValue[None.type]
+    // because it is unconditional. The solution is to put QueryValue[None.type] in the
+    // same trait as QueryValue[Option[A]], as is shown in pos/scala-uri.scala.
+
+sealed trait QueryValueInstances extends QueryValueInstances1:
+  implicit final def optionQueryValue[A: QueryValue]: QueryValue[Option[A]] = ???
+
+trait QueryKeyValue[A]
+object QueryKeyValue:
+  implicit def tuple2QueryKeyValue[K: QueryKey, V: QueryValue]: QueryKeyValue[(K, V)] = ???
+
+
+@main def Test = summon[QueryKeyValue[(String, None.type)]]  // error

Diff for: tests/pos/scala-uri.scala

@@ -9,21 +9,14 @@ trait QueryValue[-A]
 object QueryValue extends QueryValueInstances
 sealed trait QueryValueInstances1:
   implicit final val stringQueryValue: QueryValue[String] = ???
-  implicit final val noneQueryValue: QueryValue[None.type] = ???
 
 sealed trait QueryValueInstances extends QueryValueInstances1:
   implicit final def optionQueryValue[A: QueryValue]: QueryValue[Option[A]] = ???
+  implicit final val noneQueryValue: QueryValue[None.type] = ???
 
 trait QueryKeyValue[A]
 object QueryKeyValue:
   implicit def tuple2QueryKeyValue[K: QueryKey, V: QueryValue]: QueryKeyValue[(K, V)] = ???
 
 
 @main def Test = summon[QueryKeyValue[(String, None.type)]]
-
-/*(using
-  QueryKeyValue.tuple2QueryKeyValue[String, None.type](
-    QueryKey.stringQueryKey,
-    QueryValue.optionQueryValue[A]))*/
-
- // error

Diff for: tests/pos/slick-migration-api-example.scala

@@ -0,0 +1,23 @@
+trait Migration
+object Migration:
+  implicit class MigrationConcat[M <: Migration](m: M):
+    def &[N <: Migration, O](n: N)(implicit ccm: CanConcatMigrations[M, N, O]): O = ???
+
+trait ReversibleMigration extends Migration
+trait MigrationSeq extends Migration
+trait ReversibleMigrationSeq extends MigrationSeq with ReversibleMigration
+
+trait ToReversible[-A <: Migration]
+object ToReversible:
+  implicit val reversible: ToReversible[ReversibleMigration] = ???
+class CanConcatMigrations[-A, -B, +C]
+trait CanConcatMigrationsLow:
+  implicit def default[A <: Migration, B <: Migration]: CanConcatMigrations[A, B, MigrationSeq] = ???
+object CanConcatMigrations extends CanConcatMigrationsLow:
+  implicit def reversible[A <: Migration, B <: Migration](implicit reverseA: ToReversible[A],
+                                                          reverseB: ToReversible[B]): CanConcatMigrations[A, B, ReversibleMigrationSeq] = ???
+
+@main def Test =
+  val rm: ReversibleMigration = ???
+  val rms = rm & rm & rm
+  summon[rms.type <:< ReversibleMigrationSeq] // error Cannot prove that (rms : slick.migration.api.MigrationSeq) <:< slick.migration.api.ReversibleMigrationSeq.