Merge pull request #4064 from jendrikw/error-double-definition

Add error message about double definition (#1589)

Author: allanrenucci

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

@@ -127,6 +127,7 @@ public enum ErrorMessageID {
     PolymorphicMethodMissingTypeInParentID,
     ParamsNoInlineID,
     JavaSymbolIsNotAValueID,
+    DoubleDeclarationID,
     ;
 
     public int errorNumber() {

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

@@ -2079,4 +2079,20 @@ object messages {
     }
     val explanation = ""
   }
+
+  case class DoubleDeclaration(decl: Symbol, previousDecl: Symbol)(implicit ctx: Context) extends Message(DoubleDeclarationID) {
+    val kind = "Duplicate Symbol"
+    val msg = {
+      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."
+        }
+      } else ""
+      hl"${decl.showLocated} is already defined as ${previousDecl.showDcl} at line ${previousDecl.pos.line + 1}." + details
+    }
+    val explanation = ""
+  }
 }

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

@@ -613,23 +613,19 @@ trait Checking {
       }
     }
 
-  /** Check that class does not define same symbol twice */
-  def checkNoDoubleDefs(cls: Symbol)(implicit ctx: Context): Unit = {
+  /** Check that class does not declare same symbol twice */
+  def checkNoDoubleDeclaration(cls: Symbol)(implicit ctx: Context): Unit = {
     val seen = new mutable.HashMap[Name, List[Symbol]] {
       override def default(key: Name) = Nil
     }
-    typr.println(i"check no double defs $cls")
+    typr.println(i"check no double declarations $cls")
 
     def checkDecl(decl: Symbol): Unit = {
       for (other <- seen(decl.name)) {
         typr.println(i"conflict? $decl $other")
         if (decl.matches(other)) {
           def doubleDefError(decl: Symbol, other: Symbol): Unit = {
-            def ofType = if (decl.isType) "" else em": ${other.info}"
-            def explanation =
-              if (!decl.isRealMethod) ""
-              else "\n(the definitions have matching type signatures)"
-            ctx.error(em"$decl is already defined as $other$ofType$explanation", decl.pos)
+            ctx.error(DoubleDeclaration(decl, other), decl.pos)
           }
           if (decl is Synthetic) doubleDefError(other, decl)
           else doubleDefError(decl, other)
@@ -872,7 +868,7 @@ trait NoChecking extends ReChecking {
   override def checkImplicitParamsNotSingletons(vparamss: List[List[ValDef]])(implicit ctx: Context): Unit = ()
   override def checkFeasibleParent(tp: Type, pos: Position, where: => String = "")(implicit ctx: Context): Type = tp
   override def checkInlineConformant(tree: Tree, what: => String)(implicit ctx: Context) = ()
-  override def checkNoDoubleDefs(cls: Symbol)(implicit ctx: Context): Unit = ()
+  override def checkNoDoubleDeclaration(cls: Symbol)(implicit ctx: Context): Unit = ()
   override def checkParentCall(call: Tree, caller: ClassSymbol)(implicit ctx: Context) = ()
   override def checkSimpleKinded(tpt: Tree)(implicit ctx: Context): Tree = tpt
   override def checkNotSingleton(tpt: Tree, where: String)(implicit ctx: Context): Tree = tpt

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

@@ -1512,7 +1512,7 @@ class Typer extends Namer
       // Expand comments and type usecases
       cookComments(body1.map(_.symbol), self1.symbol)(ctx.localContext(cdef, cls).setNewScope)
 
-      checkNoDoubleDefs(cls)
+      checkNoDoubleDeclaration(cls)
       val impl1 = cpy.Template(impl)(constr1, parents1, self1, body1)
         .withType(dummy.termRef)
       checkVariance(impl1)

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

@@ -2,12 +2,10 @@ package dotty.tools
 package dotc
 package reporting
 
-import core.Contexts.Context
-import diagnostic.messages._
-import dotty.tools.dotc.core.Flags
-import dotty.tools.dotc.core.Flags.FlagSet
+import dotty.tools.dotc.core.Contexts.Context
 import dotty.tools.dotc.core.Types.WildcardType
 import dotty.tools.dotc.parsing.Tokens
+import dotty.tools.dotc.reporting.diagnostic.messages._
 import org.junit.Assert._
 import org.junit.Test
 
@@ -1293,4 +1291,19 @@ class ErrorMessagesTests extends ErrorMessagesTest {
 
       assert(ctx.reporter.hasErrors)
     }
+
+  @Test def typeDoubleDeclaration =
+    checkMessagesAfter("frontend") {
+      """
+        |class Foo {
+        |  val a = 1
+        |  val a = 2
+        |}
+      """.stripMargin
+    }.expect { (ictx, messages) =>
+      implicit val ctx: Context = ictx
+      assertMessageCount(1, messages)
+      val DoubleDeclaration(symbol, previousSymbol) :: Nil = messages
+      assertEquals(symbol.name.mangledString, "a")
+  }
 }

tests/neg/doubleDefinition.scala

@@ -0,0 +1,133 @@
+trait A
+trait B
+
+// test with classes
+
+class Test1 {
+	def foo(x: List[A]): Function1[A, A] = ???
+	def foo(x: List[B]): Function2[B, B, B] = ???
+	// ok, different jvm signature
+}
+
+class Test2 {
+	def foo(x: List[A]): Function1[A, A] = ???
+	def foo(x: List[B]): Function1[B, B] = ??? // error: same jvm signature
+	// scalac calls this "have same type after erasure"
+}
+
+class Test3 {
+	// overload with same argument type, but different return types
+	def foo(x: List[A]): Function1[A, A] = ???
+	def foo(x: List[A]): Function2[B, B, B] = ??? // error
+}
+
+class Test4 {
+	val foo = 1
+	def foo = 2 // error
+}
+
+class Test4b {
+	def foo = 2
+	val foo = 1 // error
+}
+
+class Test4c {
+	def foo = 2
+	var foo = 1 // error
+}
+
+class Test4d {
+	var foo = 1
+	def foo = 2 // error
+}
+
+
+// test with traits
+
+trait Test5 {
+	def foo(x: List[A]): Function1[A, A] = ???
+	def foo(x: List[B]): Function2[B, B, B] = ???
+	// ok, different jvm signature
+}
+
+trait Test6 {
+	def foo(x: List[A]): Function1[A, A] = ???
+	def foo(x: List[B]): Function1[B, B] = ??? // error: same jvm signature
+	// scalac calls this "have same type after erasure"
+}
+
+trait Test7 {
+	// overload with same argument type, but different return types
+	def foo(x: List[A]): Function1[A, A] = ???
+	def foo(x: List[A]): Function2[B, B, B] = ??? // error
+}
+
+class Test8 {
+	val foo = 1
+	def foo = 2 // error
+}
+
+class Test8b {
+	def foo = 2
+	val foo = 1 // error
+}
+
+class Test8c {
+	def foo = 2
+	var foo = 1 // error
+}
+
+class Test8d {
+	var foo = 1
+	def foo = 2 // error
+}
+
+// test method and contructor argument clashing
+
+class Test9(val foo: Int) {
+	def foo: String // error
+}
+
+class Test10(val foo: Int) {
+	def foo: Int // error
+}
+
+abstract class Test11(val foo: Int) {
+	def foo: String // error
+}
+
+abstract class Test12(val foo: Int) {
+	def foo: Int // error
+}
+
+class Test13(var foo: Int) {
+	def foo: String // error
+}
+
+class Test14(var foo: Int) {
+	def foo: Int // error
+}
+
+abstract class Test15(var foo: Int) {
+	def foo: String // error
+}
+
+abstract class Test16(var foo: Int) {
+	def foo: Int // error
+}
+
+// don't error when shadowing
+
+class Test17 {
+	val foo = 1
+	def bar() = {
+		val foo = ""
+	}
+}
+
+// no error when overloading
+
+class Test18 {
+	def foo(a: A) = 1
+	def foo(b: B) = 1
+}

tests/neg/singletonOrs.scala

@@ -1,6 +1,6 @@
 object Test {
-   def foo: 1 | 2 = 1 // error // error
-   def bar: 3 | 4 = foo // error // error
-   def foo: 1 | 2 = 1 // error // error
-   def bar: 1 = foo
+   def a: 1 | 2 = 1 // error // error
+   def b: 3 | 4 = a // error // error
+   def c: 1 | 2 = 1 // error // error
+   def d: 1 = a
 }