From c53140a75a793e560dfad01bf8b5e49c3ce242e2 Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Fri, 16 Oct 2020 18:11:52 +0200 Subject: [PATCH 1/2] fix #9324: forbid no-arg java.lang.Enum unless enum --- .../tools/dotc/reporting/ErrorMessageID.scala | 3 ++- .../dotty/tools/dotc/reporting/messages.scala | 6 +++++ .../dotty/tools/dotc/typer/RefChecks.scala | 25 ++++++++++++++----- tests/neg/extend-java-enum-migration.check | 12 +++++++++ tests/neg/extend-java-enum-migration.scala | 14 +++++++++++ 5 files changed, 53 insertions(+), 7 deletions(-) create mode 100644 tests/neg/extend-java-enum-migration.check create mode 100644 tests/neg/extend-java-enum-migration.scala diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala index 37a675f6e35b..c90128b0fae0 100644 --- a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala +++ b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala @@ -167,7 +167,8 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] { ModifierNotAllowedForDefinitionID, CannotExtendJavaEnumID, InvalidReferenceInImplicitNotFoundAnnotationID, - TraitMayNotDefineNativeMethodID + TraitMayNotDefineNativeMethodID, + JavaEnumParentArgsID def errorNumber = ordinal - 2 } diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index bd6e01c85e14..2a1b536e76fb 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -1528,6 +1528,12 @@ import transform.SymUtils._ def explain = "" } + class JavaEnumParentArgs(parent: Type)(using Context) + extends TypeMsg(JavaEnumParentArgsID) { + def msg = em"""not enough arguments for constructor Enum: ${hl("(name: String, ordinal: Int)")}: ${hl(parent.show)}""" + def explain = "" + } + class CannotHaveSameNameAs(sym: Symbol, cls: Symbol, reason: CannotHaveSameNameAs.Reason)(using Context) extends SyntaxMsg(CannotHaveSameNameAsID) { import CannotHaveSameNameAs._ diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index 1a6fca704e7d..86e18b9f9fc6 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -95,7 +95,7 @@ object RefChecks { * and required classes. Also check that only `enum` constructs extend * `java.lang.Enum`. */ - private def checkParents(cls: Symbol)(using Context): Unit = cls.info match { + private def checkParents(cls: Symbol, parentTrees: List[Tree])(using Context): Unit = cls.info match { case cinfo: ClassInfo => def checkSelfConforms(other: ClassSymbol, category: String, relation: String) = { val otherSelf = other.declaredSelfTypeAsSeenFrom(cls.thisType) @@ -109,12 +109,25 @@ object RefChecks { for (reqd <- cinfo.cls.givenSelfType.classSymbols) checkSelfConforms(reqd, "missing requirement", "required") + def illegalEnumFlags = !cls.isOneOf(Enum | Trait) + def isJavaEnum = parents.exists(_.classSymbol == defn.JavaEnumClass) + // Prevent wrong `extends` of java.lang.Enum - if !migrateTo3 && - !cls.isOneOf(Enum | Trait) && - parents.exists(_.classSymbol == defn.JavaEnumClass) - then + if !migrateTo3 && illegalEnumFlags && isJavaEnum then report.error(CannotExtendJavaEnum(cls), cls.sourcePos) + else if illegalEnumFlags && isJavaEnum then + val javaEnumCtor = defn.JavaEnumClass.primaryConstructor + parentTrees.exists(parent => + parent.tpe.typeSymbol == defn.JavaEnumClass + && ( + parent match + case tpd.Apply(tpd.TypeApply(fn, _), _) if fn.tpe.termSymbol eq javaEnumCtor => + // here we are simulating the error for missing arguments to a constructor. + report.error(JavaEnumParentArgs(parent.tpe), cls.sourcePos) + true + case _ => + false + )) case _ => } @@ -1089,7 +1102,7 @@ class RefChecks extends MiniPhase { thisPhase => override def transformTemplate(tree: Template)(using Context): Tree = try { val cls = ctx.owner.asClass checkOverloadedRestrictions(cls) - checkParents(cls) + checkParents(cls, tree.parents) if (cls.is(Trait)) tree.parents.foreach(checkParentPrefix(cls, _)) checkCompanionNameClashes(cls) checkAllOverrides(cls) diff --git a/tests/neg/extend-java-enum-migration.check b/tests/neg/extend-java-enum-migration.check new file mode 100644 index 000000000000..568faf8f90cd --- /dev/null +++ b/tests/neg/extend-java-enum-migration.check @@ -0,0 +1,12 @@ +-- [E160] Type Error: tests/neg/extend-java-enum-migration.scala:9:12 -------------------------------------------------- +9 |final class C extends jl.Enum[C] // error + | ^ + | not enough arguments for constructor Enum: (name: String, ordinal: Int): Enum[C] +-- [E160] Type Error: tests/neg/extend-java-enum-migration.scala:11:7 -------------------------------------------------- +11 |object O extends jl.Enum[O.type] // error + | ^ + | not enough arguments for constructor Enum: (name: String, ordinal: Int): Enum[O.type] +-- [E160] Type Error: tests/neg/extend-java-enum-migration.scala:14:6 -------------------------------------------------- +14 |class Sub extends T // error + | ^ + | not enough arguments for constructor Enum: (name: String, ordinal: Int): Enum[T] diff --git a/tests/neg/extend-java-enum-migration.scala b/tests/neg/extend-java-enum-migration.scala new file mode 100644 index 000000000000..5a02fcbd1cc2 --- /dev/null +++ b/tests/neg/extend-java-enum-migration.scala @@ -0,0 +1,14 @@ +import java.{lang => jl} + +import language.`3.0-migration` + +// This file is different from `tests/neg/extend-java-enum.scala` as we +// are testing that it is illegal to *not* pass arguments to jl.Enum +// in 3.0-migration + +final class C extends jl.Enum[C] // error + +object O extends jl.Enum[O.type] // error + +trait T extends jl.Enum[T] // ok +class Sub extends T // error From ba700249748eee108bc340073d90e424c6e11e8c Mon Sep 17 00:00:00 2001 From: bishabosha Date: Mon, 19 Oct 2020 12:28:13 +0200 Subject: [PATCH 2/2] refactor condition --- .../dotty/tools/dotc/typer/RefChecks.scala | 35 ++++++++++--------- tests/neg/extend-java-enum-migration.check | 4 +++ tests/neg/extend-java-enum-migration.scala | 3 ++ 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index 86e18b9f9fc6..60e96102c287 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -109,25 +109,26 @@ object RefChecks { for (reqd <- cinfo.cls.givenSelfType.classSymbols) checkSelfConforms(reqd, "missing requirement", "required") - def illegalEnumFlags = !cls.isOneOf(Enum | Trait) - def isJavaEnum = parents.exists(_.classSymbol == defn.JavaEnumClass) + def isClassExtendingJavaEnum = + !cls.isOneOf(Enum | Trait) && parents.exists(_.classSymbol == defn.JavaEnumClass) // Prevent wrong `extends` of java.lang.Enum - if !migrateTo3 && illegalEnumFlags && isJavaEnum then - report.error(CannotExtendJavaEnum(cls), cls.sourcePos) - else if illegalEnumFlags && isJavaEnum then - val javaEnumCtor = defn.JavaEnumClass.primaryConstructor - parentTrees.exists(parent => - parent.tpe.typeSymbol == defn.JavaEnumClass - && ( - parent match - case tpd.Apply(tpd.TypeApply(fn, _), _) if fn.tpe.termSymbol eq javaEnumCtor => - // here we are simulating the error for missing arguments to a constructor. - report.error(JavaEnumParentArgs(parent.tpe), cls.sourcePos) - true - case _ => - false - )) + if isClassExtendingJavaEnum then + if !migrateTo3 then // always error, only traits or enum-syntax is possible under scala 3.x + report.error(CannotExtendJavaEnum(cls), cls.sourcePos) + else + // conditionally error, we allow classes to extend java.lang.Enum in scala 2 migration mode, + // however the no-arg constructor is forbidden, we must look at the parent trees to see + // which overload is called. + val javaEnumCtor = defn.JavaEnumClass.primaryConstructor + parentTrees.exists { + case parent @ tpd.Apply(tpd.TypeApply(fn, _), _) if fn.tpe.termSymbol eq javaEnumCtor => + // here we are simulating the error for missing arguments to a constructor. + report.error(JavaEnumParentArgs(parent.tpe), cls.sourcePos) + true + case _ => + false + } case _ => } diff --git a/tests/neg/extend-java-enum-migration.check b/tests/neg/extend-java-enum-migration.check index 568faf8f90cd..53e893634dd3 100644 --- a/tests/neg/extend-java-enum-migration.check +++ b/tests/neg/extend-java-enum-migration.check @@ -10,3 +10,7 @@ 14 |class Sub extends T // error | ^ | not enough arguments for constructor Enum: (name: String, ordinal: Int): Enum[T] +-- [E160] Type Error: tests/neg/extend-java-enum-migration.scala:17:10 ------------------------------------------------- +17 |val foo = new java.lang.Enum[Color] {} // error + | ^ + | not enough arguments for constructor Enum: (name: String, ordinal: Int): Enum[Color] diff --git a/tests/neg/extend-java-enum-migration.scala b/tests/neg/extend-java-enum-migration.scala index 5a02fcbd1cc2..ce781daf8668 100644 --- a/tests/neg/extend-java-enum-migration.scala +++ b/tests/neg/extend-java-enum-migration.scala @@ -12,3 +12,6 @@ object O extends jl.Enum[O.type] // error trait T extends jl.Enum[T] // ok class Sub extends T // error + +abstract class Color(name: String, ordinal: Int) extends java.lang.Enum[Color](name, ordinal) // ok +val foo = new java.lang.Enum[Color] {} // error