Improve double definition error reporting

- Rename the error from DoubleDeclaration to DoubleDefinition so that
  we can also use it in post-erasure checks.
- Display line numbers only when they make sense
- Display the type signatures as written by the user

Author: smarter

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

@@ -128,7 +128,7 @@ public enum ErrorMessageID {
     PolymorphicMethodMissingTypeInParentID,
     ParamsNoInlineID,
     JavaSymbolIsNotAValueID,
-    DoubleDeclarationID,
+    DoubleDefinitionID,
     MatchCaseOnlyNullWarningID,
     ImportRenamedTwiceID,
     TypeTestAlwaysSucceedsID,

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

@@ -2137,18 +2137,30 @@ object messages {
     val explanation: String = ""
   }
 
-  case class DoubleDeclaration(decl: Symbol, previousDecl: Symbol)(implicit ctx: Context) extends Message(DoubleDeclarationID) {
+  case class DoubleDefinition(decl: Symbol, previousDecl: Symbol)(implicit ctx: Context) extends Message(DoubleDefinitionID) {
     val kind: String = "Duplicate Symbol"
     val msg: String = {
       val details = if (decl.isRealMethod && previousDecl.isRealMethod) {
         // compare the signatures when both symbols represent methods
         decl.signature.matchDegree(previousDecl.signature) match {
-          /* case Signature.NoMatch => // can't happen because decl.matches(previousDecl) is checked before reporting this error */
-          case Signature.ParamMatch => "\nOverloads with matching parameter types are not allowed."
-          case _ /* Signature.FullMatch */ => "\nThe definitions have matching type signatures after erasure."
+          case Signature.NoMatch =>
+            "" // shouldn't be reachable
+          case Signature.ParamMatch =>
+            "have matching parameter types."
+          case Signature.FullMatch =>
+            "have the same type after erasure."
         }
       } else ""
-      hl"${decl.showLocated} is already defined as ${previousDecl.showDcl} ${if (previousDecl.span.exists) s"at line ${previousDecl.sourcePos.line + 1}" else ""}." + details
+      def symLocation(sym: Symbol) = {
+        val lineDesc =
+          if (sym.span.exists && sym.span != sym.owner.span)
+            s" at line ${sym.sourcePos.line + 1}" else ""
+        i"in ${sym.owner}${lineDesc}"
+      }
+      em"""Double definition:
+          |${previousDecl.initial.showDcl} ${symLocation(previousDecl)} and
+          |${decl.initial.showDcl} ${symLocation(decl)}
+          |""" + details
     }
     val explanation: String = ""
   }

compiler/src/dotty/tools/dotc/transform/ElimErasedValueType.scala

@@ -7,6 +7,7 @@ import MegaPhase._
 import Types._, Contexts._, Flags._, DenotTransformers._
 import Symbols._, StdNames._, Trees._
 import TypeErasure.ErasedValueType, ValueClasses._
+import reporting.diagnostic.messages.DoubleDefinition
 
 object ElimErasedValueType {
   val name: String = "elimErasedValueType"
@@ -103,12 +104,7 @@ class ElimErasedValueType extends MiniPhase with InfoTransformer {
         //
         // The problem is that `map` was forwarded twice, with different instantiated types.
         // Maybe we should move mixin forwarding after erasure to avoid redundant forwarders like these.
-        ctx.error(
-            em"""double definition:
-                |$sym1: $info1 in ${sym1.owner} and
-                |$sym2: $info2 in ${sym2.owner}
-                |have same type after erasure: $info""",
-            root.sourcePos)
+        ctx.error(DoubleDefinition(sym1, sym2), root.sourcePos)
     }
     val earlyCtx = ctx.withPhase(ctx.elimRepeatedPhase.next)
     while (opc.hasNext) {

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

@@ -755,7 +755,7 @@ trait Checking {
         if (decl.matches(other) && !javaFieldMethodPair) {
           def doubleDefError(decl: Symbol, other: Symbol): Unit =
             if (!decl.info.isErroneous && !other.info.isErroneous)
-              ctx.error(DoubleDeclaration(decl, other), decl.sourcePos)
+              ctx.error(DoubleDefinition(decl, other), decl.sourcePos)
           if (decl is Synthetic) doubleDefError(other, decl)
           else doubleDefError(decl, other)
         }

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

@@ -1514,7 +1514,7 @@ class ErrorMessagesTests extends ErrorMessagesTest {
     }.expect { (ictx, messages) =>
       implicit val ctx: Context = ictx
       assertMessageCount(1, messages)
-      val DoubleDeclaration(symbol, previousSymbol) :: Nil = messages
+      val DoubleDefinition(symbol, previousSymbol) :: Nil = messages
       assertEquals(symbol.name.mangledString, "a")
   }
 

tests/neg/doubleDefinition.check

@@ -0,0 +1,84 @@
+<2016..2016> in doubleDefinition.scala
+Double definition:
+var foo: Int in class Test16 at line 115 and
+def foo: => Int in class Test16 at line 116
+<1949..1949> in doubleDefinition.scala
+Double definition:
+var foo: Int in class Test15 at line 111 and
+def foo: => String in class Test15 at line 112
+<1885..1885> in doubleDefinition.scala
+Double definition:
+var foo: Int in class Test14 at line 107 and
+def foo: => Int in class Test14 at line 108
+<1827..1827> in doubleDefinition.scala
+Double definition:
+var foo: Int in class Test13 at line 103 and
+def foo: => String in class Test13 at line 104
+<1772..1772> in doubleDefinition.scala
+Double definition:
+val foo: Int in class Test12 at line 99 and
+def foo: => Int in class Test12 at line 100
+<1705..1705> in doubleDefinition.scala
+Double definition:
+val foo: Int in class Test11 at line 95 and
+def foo: => String in class Test11 at line 96
+<1641..1641> in doubleDefinition.scala
+Double definition:
+val foo: Int in class Test10 at line 91 and
+def foo: => Int in class Test10 at line 92
+<1583..1583> in doubleDefinition.scala
+Double definition:
+val foo: Int in class Test9 at line 87 and
+def foo: => String in class Test9 at line 88
+<1481..1481> in doubleDefinition.scala
+Double definition:
+var foo: Int in class Test8d at line 81 and
+def foo: => Int in class Test8d at line 82
+<1428..1428> in doubleDefinition.scala
+Double definition:
+def foo: => Int in class Test8c at line 76 and
+var foo: Int in class Test8c at line 77
+<1375..1375> in doubleDefinition.scala
+Double definition:
+def foo: => Int in class Test8b at line 71 and
+val foo: Int in class Test8b at line 72
+<1322..1322> in doubleDefinition.scala
+Double definition:
+val foo: Int in class Test8 at line 66 and
+def foo: => Int in class Test8 at line 67
+<1236..1236> in doubleDefinition.scala
+Double definition:
+def foo(x: List[A]): A => A in trait Test7 at line 61 and
+def foo(x: List[A]): (B, B) => B in trait Test7 at line 62
+have matching parameter types.
+<984..984> in doubleDefinition.scala
+Double definition:
+def foo(x: List[A]): A => A in trait Test6 at line 54 and
+def foo(x: List[B]): B => B in trait Test6 at line 55
+have the same type after erasure.
+<739..739> in doubleDefinition.scala
+Double definition:
+var foo: Int in class Test4d at line 40 and
+def foo: => Int in class Test4d at line 41
+<686..686> in doubleDefinition.scala
+Double definition:
+def foo: => Int in class Test4c at line 35 and
+var foo: Int in class Test4c at line 36
+<633..633> in doubleDefinition.scala
+Double definition:
+def foo: => Int in class Test4b at line 30 and
+val foo: Int in class Test4b at line 31
+<580..580> in doubleDefinition.scala
+Double definition:
+val foo: Int in class Test4 at line 25 and
+def foo: => Int in class Test4 at line 26
+<494..494> in doubleDefinition.scala
+Double definition:
+def foo(x: List[A]): A => A in class Test3 at line 20 and
+def foo(x: List[A]): (B, B) => B in class Test3 at line 21
+have matching parameter types.
+<242..242> in doubleDefinition.scala
+Double definition:
+def foo(x: List[A]): A => A in class Test2 at line 13 and
+def foo(x: List[B]): B => B in class Test2 at line 14
+have the same type after erasure.