Support extension methods imported from different objects

Add a special case to name resolution so that when expanding an
extension method from `e.m` to `m(e)` and `m` is imported by several
imports on the same level, we try to typecheck under every such import
and pick the successful alternative if it exists and is unambiguous.

Fixes #16920

Author: odersky

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

@@ -190,6 +190,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case InlineGivenShouldNotBeFunctionID // errorNumber 174
   case ValueDiscardingID // errorNumber 175
   case UnusedNonUnitValueID // errorNumber 176
+  case AmbiguousExtensionMethodID // errorNumber 177
 
   def errorNumber = ordinal - 1
 

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

@@ -1403,6 +1403,15 @@ extends ReferenceMsg(AmbiguousOverloadID), NoDisambiguation {
         |"""
 }
 
+class AmbiguousExtensionMethod(tree: untpd.Tree, expansion1: tpd.Tree, expansion2: tpd.Tree)(using Context)
+  extends ReferenceMsg(AmbiguousExtensionMethodID), NoDisambiguation:
+  def msg(using Context) =
+    i"""Ambiguous extension methods:
+       |both $expansion1
+       |and  $expansion2
+       |are possible expansions of $tree"""
+  def explain(using Context) = ""
+
 class ReassignmentToVal(name: Name)(using Context)
   extends TypeMsg(ReassignmentToValID) {
   def msg(using Context) = i"""Reassignment to val $name"""

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

@@ -158,8 +158,13 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
    *   @param required   flags the result's symbol must have
    *   @param excluded   flags the result's symbol must not have
    *   @param pos        indicates position to use for error reporting
+   *   @param altImports a ListBuffer in which alternative imported references are
+   *                     collected in case `findRef` is called from an expansion of
+   *                     an extension method, i.e. when `e.m` is expanded to `m(e)` and
+   *                     a reference for `m` is searched. `null` in all other situations.
    */
-  def findRef(name: Name, pt: Type, required: FlagSet, excluded: FlagSet, pos: SrcPos)(using Context): Type = {
+  def findRef(name: Name, pt: Type, required: FlagSet, excluded: FlagSet, pos: SrcPos,
+      altImports: mutable.ListBuffer[TermRef] | Null = null)(using Context): Type = {
     val refctx = ctx
     val noImports = ctx.mode.is(Mode.InPackageClauseName)
     def suppressErrors = excluded.is(ConstructorProxy)
@@ -230,15 +235,52 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
             fail(AmbiguousReference(name, newPrec, prevPrec, prevCtx))
           previous
 
-      /** Recurse in outer context. If final result is same as `previous`, check that it
-       *  is new or shadowed. This order of checking is necessary since an
-       *  outer package-level definition might trump two conflicting inner
-       *  imports, so no error should be issued in that case. See i7876.scala.
+      /** Assemble and check alternatives to an imported reference. This implies:
+       *   - If we expand an extension method (i.e. altImports != null),
+       *     search imports on the same level for other possible resolutions of `name`.
+       *     The result and altImports together then contain all possible imported
+       *     references of the highest possible precedence, where `NamedImport` beats
+       *     `WildImport`.
+       *   - Find a posssibly shadowing reference in an outer context.
+       *     If the result is the same as `previous`, check that it is new or
+       *     shadowed. This order of checking is necessary since an outer package-level
+       *     definition might trump two conflicting inner imports, so no error should be
+       *     issued in that case. See i7876.scala.
+       *  @param previous   the previously found reference (which is an import)
+       *  @param prevPrec   the precedence of the reference (either NamedImport or WildImport)
+       *  @param prevCtx    the context in which the reference was found
+       *  @param using_Context the outer context of `precCtx`
        */
-      def recurAndCheckNewOrShadowed(previous: Type, prevPrec: BindingPrec, prevCtx: Context)(using Context): Type =
-        val found = findRefRecur(previous, prevPrec, prevCtx)
-        if found eq previous then checkNewOrShadowed(found, prevPrec)(using prevCtx)
-        else found
+      def checkImportAlternatives(previous: Type, prevPrec: BindingPrec, prevCtx: Context)(using Context): Type =
+
+        def addAltImport(altImp: TermRef) =
+          if !TypeComparer.isSameRef(previous, altImp)
+              && !altImports.uncheckedNN.exists(TypeComparer.isSameRef(_, altImp))
+          then
+            altImports.uncheckedNN += altImp
+
+        if altImports != null && ctx.isImportContext then
+          val curImport = ctx.importInfo.uncheckedNN
+          namedImportRef(curImport) match
+            case altImp: TermRef =>
+              if prevPrec == WildImport then
+                // Discard all previously found references and continue with `altImp`
+                altImports.clear()
+                checkImportAlternatives(altImp, NamedImport, ctx)(using ctx.outer)
+              else
+                addAltImport(altImp)
+                checkImportAlternatives(previous, prevPrec, prevCtx)(using ctx.outer)
+            case _ =>
+              if prevPrec == WildImport then
+                wildImportRef(curImport) match
+                  case altImp: TermRef => addAltImport(altImp)
+                  case _ =>
+              checkImportAlternatives(previous, prevPrec, prevCtx)(using ctx.outer)
+        else
+          val found = findRefRecur(previous, prevPrec, prevCtx)
+          if found eq previous then checkNewOrShadowed(found, prevPrec)(using prevCtx)
+          else found
+      end checkImportAlternatives
 
       def selection(imp: ImportInfo, name: Name, checkBounds: Boolean): Type =
         imp.importSym.info match
@@ -328,7 +370,6 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
         if (ctx.scope eq EmptyScope) previous
         else {
           var result: Type = NoType
-
           val curOwner = ctx.owner
 
           /** Is curOwner a package object that should be skipped?
@@ -449,11 +490,11 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
             else if (isPossibleImport(NamedImport) && (curImport nen outer.importInfo)) {
               val namedImp = namedImportRef(curImport.uncheckedNN)
               if (namedImp.exists)
-                recurAndCheckNewOrShadowed(namedImp, NamedImport, ctx)(using outer)
+                checkImportAlternatives(namedImp, NamedImport, ctx)(using outer)
               else if (isPossibleImport(WildImport) && !curImport.nn.importSym.isCompleting) {
                 val wildImp = wildImportRef(curImport.uncheckedNN)
                 if (wildImp.exists)
-                  recurAndCheckNewOrShadowed(wildImp, WildImport, ctx)(using outer)
+                  checkImportAlternatives(wildImp, WildImport, ctx)(using outer)
                 else {
                   updateUnimported()
                   loop(ctx)(using outer)
@@ -3359,11 +3400,37 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
     def selectionProto = SelectionProto(tree.name, mbrProto, compat, privateOK = inSelect)
 
     def tryExtension(using Context): Tree =
-      findRef(tree.name, WildcardType, ExtensionMethod, EmptyFlags, qual.srcPos) match
+      val altImports = new mutable.ListBuffer[TermRef]()
+      findRef(tree.name, WildcardType, ExtensionMethod, EmptyFlags, qual.srcPos, altImports) match
         case ref: TermRef =>
-          extMethodApply(untpd.TypedSplice(tpd.ref(ref).withSpan(tree.nameSpan)), qual, pt)
+          def tryExtMethod(ref: TermRef)(using Context) =
+            extMethodApply(untpd.TypedSplice(tpd.ref(ref).withSpan(tree.nameSpan)), qual, pt)
+          if altImports.isEmpty then
+            tryExtMethod(ref)
+          else
+            // Try all possible imports and collect successes and failures
+            val successes, failures = new mutable.ListBuffer[(Tree, TyperState)]
+            for alt <- ref :: altImports.toList do
+              val nestedCtx = ctx.fresh.setNewTyperState()
+              val app = tryExtMethod(alt)(using nestedCtx)
+              (if nestedCtx.reporter.hasErrors then failures else successes)
+                += ((app, nestedCtx.typerState))
+            typr.println(i"multiple extensioin methods, success: ${successes.toList}, failure: ${failures.toList}")
+
+            def pick(alt: (Tree, TyperState)): Tree =
+              val (app, ts) = alt
+              ts.commit()
+              app
+
+            successes.toList match
+              case Nil => pick(failures.head)
+              case success :: Nil => pick(success)
+              case (expansion1, _) :: (expansion2, _) :: _ =>
+                report.error(AmbiguousExtensionMethod(tree, expansion1, expansion2), tree.srcPos)
+                expansion1
         case _ =>
           EmptyTree
+    end tryExtension
 
     def nestedFailure(ex: TypeError) =
       rememberSearchFailure(qual,

docs/_docs/reference/contextual/extension-methods.md

@@ -244,7 +244,16 @@ The precise rules for resolving a selection to an extension method are as follow
 Assume a selection `e.m[Ts]` where `m` is not a member of `e`, where the type arguments `[Ts]` are optional, and where `T` is the expected type.
 The following two rewritings are tried in order:
 
- 1. The selection is rewritten to `m[Ts](e)`.
+ 1. The selection is rewritten to `m[Ts](e)` and typechecked, using the following
+    slight modification of the name resolution rules:
+
+  - If `m` is imported by several imports which are all on the nesting level,
+    try each import as an extension method instead of failing with an ambiguity.
+    If only one import leads to an expansion that typechecks without errors, pick
+    that expansion. If there are several such imports, but only one import which is
+    not a wildcard import, pick the expansion from that import. Otherwise, report
+    an ambiguous reference error.    
+
  2. If the first rewriting does not typecheck with expected type `T`,
     and there is an extension method `m` in some eligible object `o`, the selection is rewritten to `o.m[Ts](e)`. An object `o` is _eligible_ if
 

tests/neg/i13558.check

@@ -0,0 +1,88 @@
+-- [E008] Not Found Error: tests/neg/i16920.scala:18:11 ----------------------------------------------------------------
+18 |    "five".wow // error
+   |    ^^^^^^^^^^
+   |    value wow is not a member of String.
+   |    An extension method was tried, but could not be fully constructed:
+   |
+   |        Two.wow("five")
+   |
+   |        failed with:
+   |
+   |            Found:    ("five" : String)
+   |            Required: Int
+-- [E008] Not Found Error: tests/neg/i16920.scala:26:6 -----------------------------------------------------------------
+26 |    5.wow // error
+   |    ^^^^^
+   |    value wow is not a member of Int.
+   |    An extension method was tried, but could not be fully constructed:
+   |
+   |        AlsoFails.wow(5)
+   |
+   |        failed with:
+   |
+   |            Found:    (5 : Int)
+   |            Required: Boolean
+-- [E008] Not Found Error: tests/neg/i16920.scala:27:11 ----------------------------------------------------------------
+27 |    "five".wow // error
+   |    ^^^^^^^^^^
+   |    value wow is not a member of String.
+   |    An extension method was tried, but could not be fully constructed:
+   |
+   |        AlsoFails.wow("five")
+   |
+   |        failed with:
+   |
+   |            Found:    ("five" : String)
+   |            Required: Boolean
+-- [E008] Not Found Error: tests/neg/i16920.scala:34:6 -----------------------------------------------------------------
+34 |    5.wow // error
+   |    ^^^^^
+   |    value wow is not a member of Int.
+   |    An extension method was tried, but could not be fully constructed:
+   |
+   |        Three.wow(5)
+   |
+   |        failed with:
+   |
+   |            Ambiguous extension methods:
+   |            both Three.wow(5)
+   |            and  Two.wow(5)
+   |            are possible expansions of 5.wow
+-- [E008] Not Found Error: tests/neg/i16920.scala:42:11 ----------------------------------------------------------------
+42 |    "five".wow // error
+   |    ^^^^^^^^^^
+   |    value wow is not a member of String.
+   |    An extension method was tried, but could not be fully constructed:
+   |
+   |        Two.wow("five")
+   |
+   |        failed with:
+   |
+   |            Found:    ("five" : String)
+   |            Required: Int
+-- [E008] Not Found Error: tests/neg/i16920.scala:49:11 ----------------------------------------------------------------
+49 |    "five".wow // error
+   |    ^^^^^^^^^^
+   |    value wow is not a member of String.
+   |    An extension method was tried, but could not be fully constructed:
+   |
+   |        Two.wow("five")
+   |
+   |        failed with:
+   |
+   |            Found:    ("five" : String)
+   |            Required: Int
+-- [E008] Not Found Error: tests/neg/i16920.scala:56:6 -----------------------------------------------------------------
+56 |    5.wow // error
+   |    ^^^^^
+   |    value wow is not a member of Int.
+   |    An extension method was tried, but could not be fully constructed:
+   |
+   |        Three.wow(5)
+   |
+   |        failed with:
+   |
+   |            Ambiguous extension methods:
+   |            both Three.wow(5)
+   |            and  Two.wow(5)
+   |            are possible expansions of 5.wow

tests/neg/i16920.check

@@ -0,0 +1,57 @@
+object One:
+  extension (s: String)
+    def wow: Unit = println(s)
+
+object Two:
+  extension (i: Int)
+    def wow: Unit = println(i)
+
+object Three:
+  extension (i: Int)
+    def wow: Unit = println(i)
+
+object Fails:
+  import One._
+  def test: Unit =
+    import Two._
+    5.wow
+    "five".wow // error
+
+object AlsoFails:
+  extension (s: Boolean)
+    def wow = println(s)
+  import One._
+  import Two._
+  def test: Unit =
+    5.wow // error
+    "five".wow // error
+
+object Fails2:
+  import One._
+  import Two._
+  import Three._
+  def test: Unit =
+    5.wow // error
+    "five".wow // ok
+
+object Fails3:
+  import One._
+  import Two.wow
+  def test: Unit =
+    5.wow // ok
+    "five".wow // error
+
+object Fails4:
+  import Two.wow
+  import One._
+  def test: Unit =
+    5.wow // ok
+    "five".wow // error
+
+object Fails5:
+  import One.wow
+  import Two.wow
+  import Three.wow
+  def test: Unit =
+    5.wow // error
+    "five".wow // ok

tests/neg/i16920.scala

@@ -1,2 +1,9 @@
-object Test:
-  def test = 0
+
+def Block(f: => Int): Int = f
+
+def test(): String = {
+  Block:
+      2 + 2
+    .toString
+}
+val x = test()

tests/new/test.scala

@@ -20,12 +20,12 @@ object Main {
     import ExtensionB._
     import ExtensionA._
     val a = A()
-    println(a.id) // error
+    println(a.id) // now ok
   }
   def main2(args: Array[String]): Unit = {
     import ExtensionA._
     import ExtensionB._
     val a = A()
-    println(a.id) // error
+    println(a.id) // now ok
   }
 }