Support extension methods imported from different objects (#17050)

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/config/Feature.scala

@@ -29,6 +29,7 @@ object Feature:
   val fewerBraces = experimental("fewerBraces")
   val saferExceptions = experimental("saferExceptions")
   val clauseInterleaving = experimental("clauseInterleaving")
+  val relaxedExtensionImports = experimental("relaxedExtensionImports")
   val pureFunctions = experimental("pureFunctions")
   val captureChecking = experimental("captureChecking")
   val into = experimental("into")

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

@@ -193,6 +193,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case ConstrProxyShadowsID // errorNumber 177
   case MissingArgumentListID // errorNumber: 178
   case MatchTypeScrutineeCannotBeHigherKindedID // errorNumber: 179
+  case AmbiguousExtensionMethodID // errorNumber 180
 
   def errorNumber = ordinal - 1
 

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

@@ -1434,6 +1434,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

@@ -159,8 +159,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)
@@ -231,15 +236,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 Feature.enabled(Feature.relaxedExtensionImports) && 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
@@ -329,7 +371,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?
@@ -450,11 +491,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)
@@ -3412,11 +3453,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,18 @@ 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.
+
+    **Note**: This relaxation is currently enabled only under the `experimental.relaxedExtensionImports` language import.
+
  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
 

library/src/scala/runtime/stdLibPatches/language.scala

@@ -69,6 +69,14 @@ object language:
     @compileTimeOnly("`clauseInterleaving` can only be used at compile time in import statements")
     object clauseInterleaving
 
+    /** Adds support for relaxed imports of extension methods.
+      * Extension methods with the same name can be imported from several places.
+      *
+      * @see [[http://dotty.epfl.ch/docs/reference/contextual/extension-methods]]
+      */
+    @compileTimeOnly("`relaxedExtensionImports` can only be used at compile time in import statements")
+    object relaxedExtensionImports
+
     /** Experimental support for pure function type syntax
      *
      *  @see [[https://dotty.epfl.ch/docs/reference/experimental/purefuns]]

project/MiMaFilters.scala

@@ -26,6 +26,8 @@ object MiMaFilters {
     ProblemFilters.exclude[MissingClassProblem]("scala.runtime.stdLibPatches.language$experimental$clauseInterleaving$"),
     ProblemFilters.exclude[MissingFieldProblem]("scala.runtime.stdLibPatches.language#experimental.into"),
     ProblemFilters.exclude[MissingClassProblem]("scala.runtime.stdLibPatches.language$experimental$into$"),
+    ProblemFilters.exclude[MissingFieldProblem]("scala.runtime.stdLibPatches.language#experimental.relaxedExtensionImports"),
+    ProblemFilters.exclude[MissingClassProblem]("scala.runtime.stdLibPatches.language$experimental$relaxedExtensionImports$"),
     // end of New experimental features in 3.3.X
 
     // Added java.io.Serializable as LazyValControlState supertype

tests/neg/i13558.check

@@ -0,0 +1,88 @@
+-- [E008] Not Found Error: tests/neg/i16920.scala:20:11 ----------------------------------------------------------------
+20 |    "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:28:6 -----------------------------------------------------------------
+28 |    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:29:11 ----------------------------------------------------------------
+29 |    "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:36:6 -----------------------------------------------------------------
+36 |    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:44:11 ----------------------------------------------------------------
+44 |    "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:51:11 ----------------------------------------------------------------
+51 |    "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:58:6 -----------------------------------------------------------------
+58 |    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,59 @@
+import language.experimental.relaxedExtensionImports
+
+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