Change Ambiguous Import error in Typer to case class (#2362)

* Change Ambiguous Import error in Typer to case class

* Remove stripMargin

Author: ennru

compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java

@@ -56,6 +56,7 @@ public enum ErrorMessageID {
     CyclicReferenceInvolvingID,
     CyclicReferenceInvolvingImplicitID,
     SuperQualMustBeParentID,
+    AmbiguousImportID,
     ;
 
     public int errorNumber() {

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

@@ -1218,4 +1218,39 @@ object messages {
            |Attempting to define a field in a method signature after a varargs field is an error.
            |""".stripMargin
   }
+
+  case class AmbiguousImport(name: Names.Name, newPrec: Int, prevPrec: Int, prevCtx: Context)(implicit ctx: Context)
+    extends Message(AmbiguousImportID) {
+
+    import typer.Typer.BindingPrec._
+
+    /** A string which explains how something was bound; Depending on `prec` this is either
+      *      imported by <tree>
+      *  or  defined in <symbol>
+      */
+    private def bindingString(prec: Int, whereFound: Context, qualifier: String = "") =
+      if (isImportPrec(prec)) {
+        ex"""imported$qualifier by ${hl"${whereFound.importInfo}"}"""
+      } else
+        ex"""defined$qualifier in ${hl"${whereFound.owner.toString}"}"""
+
+
+    val msg =
+      i"""|reference to `${hl"$name"}` is ambiguous
+          |it is both ${bindingString(newPrec, ctx)}
+          |and ${bindingString(prevPrec, prevCtx, " subsequently")}"""
+
+    val kind = "Reference"
+
+    val explanation =
+      hl"""|The compiler can't decide which of the possible choices you
+           |are referencing with $name.
+           |Note:
+           |- Definitions take precedence over imports
+           |- Named imports take precedence over wildcard imports
+           |- You may replace a name when imported using
+           |  ${"import"} scala.{ $name => ${name.show + "Tick"} }
+           |"""
+  }
+
 }

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

@@ -140,16 +140,6 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
     def findRef(previous: Type, prevPrec: Int, prevCtx: Context)(implicit ctx: Context): Type = {
       import BindingPrec._
 
-      /** A string which explains how something was bound; Depending on `prec` this is either
-       *      imported by <tree>
-       *  or  defined in <symbol>
-       */
-      def bindingString(prec: Int, whereFound: Context, qualifier: String = "") =
-        if (prec == wildImport || prec == namedImport) {
-          ex"""imported$qualifier by ${hl"${whereFound.importInfo}"}"""
-        } else
-          ex"""defined$qualifier in ${hl"${whereFound.owner.toString}"}"""
-
       /** Check that any previously found result from an inner context
        *  does properly shadow the new one from an outer context.
        *  @param found     The newly found result
@@ -170,11 +160,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
         }
         else {
           if (!scala2pkg && !previous.isError && !found.isError) {
-            error(
-              ex"""|reference to `$name` is ambiguous
-                   |it is both ${bindingString(newPrec, ctx, "")}
-                   |and ${bindingString(prevPrec, prevCtx, " subsequently")}""",
-              tree.pos)
+            error(AmbiguousImport(name, newPrec, prevPrec, prevCtx), tree.pos)
           }
           previous
         }

compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala

@@ -328,4 +328,34 @@ class ErrorMessagesTests extends ErrorMessagesTest {
         assertEquals("B", qual.show)
         assertEquals("class C", cls.show)
       }
+
+  @Test def ambiguousImport =
+    checkMessagesAfter("frontend") {
+      """
+        |object A {
+        |  class ToBeImported
+        |}
+        |object B {
+        |  class ToBeImported
+        |}
+        |class C {
+        |  import A.ToBeImported
+        |  import B.ToBeImported
+        |
+        |  val value: ToBeImported = ???
+        |}
+      """.stripMargin
+    }
+      .expect { (ictx, messages) =>
+        implicit val ctx: Context = ictx
+        val defn = ictx.definitions
+
+        import typer.Typer.BindingPrec._
+
+        assertMessageCount(1, messages)
+        val AmbiguousImport(name, newPrec, prevPrec, prevCtx) :: Nil = messages
+        assertEquals("ToBeImported", name.show)
+        assertEquals(namedImport, newPrec)
+        assertEquals(namedImport, prevPrec)
+      }
 }