Remove synthetic record constructor if the user has written one manually

Co-authored-by: Guillaume Martres <smarter@users.noreply.github.com>
scala · May 21, 2023 · da4996a · da4996a
1 parent d1df3ee
commit da4996a
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 17 deletions.
diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala
@@ -688,6 +688,7 @@ class Definitions {
   @tu lazy val JavaCalendarClass: ClassSymbol = requiredClass("java.util.Calendar")
   @tu lazy val JavaDateClass: ClassSymbol = requiredClass("java.util.Date")
   @tu lazy val JavaFormattableClass: ClassSymbol = requiredClass("java.util.Formattable")
+  @tu lazy val JavaRecordClass: Symbol = getClassIfDefined("java.lang.Record")
 
   @tu lazy val JavaEnumClass: ClassSymbol = {
     val cls = requiredClass("java.lang.Enum")

diff --git a/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala b/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala
@@ -21,8 +21,7 @@ import reporting._
 import dotty.tools.dotc.util.SourceFile
 import util.Spans._
 
-import scala.collection.mutable.ListBuffer
-import scala.collection.immutable.ListMap
+import scala.collection.mutable.{ListBuffer, LinkedHashMap}
 
 object JavaParsers {
 
@@ -839,7 +838,7 @@ object JavaParsers {
 
       // We need to generate accessors for every param, if no method with the same name is already defined
 
-      var fieldsByName = header.map(v => (v.name, (v.tpt, v.mods.annotations))).to(ListMap)
+      var fieldsByName = header.map(v => (v.name, (v.tpt, v.mods.annotations))).to(LinkedHashMap)
 
       for case DefDef(name, paramss, _, _) <- body
       if paramss.isEmpty && fieldsByName.contains(name)
@@ -855,7 +854,8 @@ object JavaParsers {
 
       // generate the canonical constructor
       val canonicalConstructor =
-        DefDef(nme.CONSTRUCTOR, joinParams(tparams, List(header)), TypeTree(), EmptyTree).withMods(Modifiers(Flags.JavaDefined, mods.privateWithin))
+        DefDef(nme.CONSTRUCTOR, joinParams(tparams, List(header)), TypeTree(), EmptyTree)
+          .withMods(Modifiers(Flags.JavaDefined | Flags.Synthetic, mods.privateWithin))
 
       // return the trees
       val recordTypeDef = atSpan(start, nameOffset) {
@@ -866,7 +866,7 @@ object JavaParsers {
             tparams = tparams,
             true
           )
-        )
+        ).withMods(mods)
       }
       addCompanionObject(statics, recordTypeDef)
     end recordDecl

diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala
@@ -862,7 +862,6 @@ class Namer { typer: Typer =>
      *  with a user-defined method in the same scope with a matching type.
      */
     private def invalidateIfClashingSynthetic(denot: SymDenotation): Unit =
-
       def isCaseClassOrCompanion(owner: Symbol) =
         owner.isClass && {
           if (owner.is(Module)) owner.linkedClass.is(CaseClass)
@@ -879,10 +878,19 @@ class Namer { typer: Typer =>
             !sd.symbol.is(Deferred) && sd.matches(denot)))
 
       val isClashingSynthetic =
-        denot.is(Synthetic, butNot = ConstructorProxy)
-        && desugar.isRetractableCaseClassMethodName(denot.name)
-        && isCaseClassOrCompanion(denot.owner)
-        && (definesMember || inheritsConcreteMember)
+        denot.is(Synthetic, butNot = ConstructorProxy) &&
+        (
+          (desugar.isRetractableCaseClassMethodName(denot.name)
+            && isCaseClassOrCompanion(denot.owner)
+            && (definesMember || inheritsConcreteMember)
+          )
+          ||
+          // remove synthetic constructor of a java Record if it clashes with a non-synthetic constructor
+          (denot.isConstructor
+            && denot.owner.is(JavaDefined) && denot.owner.derivesFrom(defn.JavaRecordClass)
+            && denot.owner.unforcedDecls.lookupAll(denot.name).exists(c => c != denot.symbol && c.info.matches(denot.info))
+          )
+        )
 
       if isClashingSynthetic then
         typr.println(i"invalidating clashing $denot in ${denot.owner}")

diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala
@@ -2433,11 +2433,17 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
   }
 
   def typedDefDef(ddef: untpd.DefDef, sym: Symbol)(using Context): Tree = {
-    if (!sym.info.exists) { // it's a discarded synthetic case class method, drop it
-      assert(sym.is(Synthetic) && desugar.isRetractableCaseClassMethodName(sym.name))
+    def canBeInvalidated(sym: Symbol): Boolean =
+      sym.is(Synthetic)
+      && (desugar.isRetractableCaseClassMethodName(sym.name) ||
+        (sym.isConstructor && sym.owner.derivesFrom(defn.JavaRecordClass)))
+
+    if !sym.info.exists then
+      // it's a discarded method (synthetic case class method or synthetic java record constructor), drop it
+      assert(canBeInvalidated(sym))
       sym.owner.info.decls.openForMutations.unlink(sym)
       return EmptyTree
-    }
+
     // TODO: - Remove this when `scala.language.experimental.erasedDefinitions` is no longer experimental.
     //       - Modify signature to `erased def erasedValue[T]: T`
     if sym.eq(defn.Compiletime_erasedValue) then

diff --git a/tests/pos-java16+/java-records/R2.java b/tests/pos-java16+/java-records/R2.java
@@ -5,9 +5,9 @@ public int getInt() {
         }
 
         // Canonical constructor
-        // public R(int i, java.lang.String s) {
-        //     this.i = i;
-        //     this.s = s.intern();
-        // }
+        public R(int i, java.lang.String s) {
+            this.i = i;
+            this.s = s.intern();
+        }
     }
 }