From 639d8e547b8a9841226c0e50614882ed9bf9087f Mon Sep 17 00:00:00 2001 From: Jamie Thompson Date: Fri, 24 Nov 2023 14:54:58 +0100 Subject: [PATCH] fix sealedStrictDescendants for Java enum - Rename JavaEnumTrait flags to JavaEnum (the actual flags set) - test java enum in SealedDescendantsTest --- .../tools/backend/jvm/BCodeHelpers.scala | 2 +- .../tools/backend/jvm/BTypesFromSymbols.scala | 2 +- .../src/dotty/tools/dotc/core/Flags.scala | 2 +- .../tools/dotc/core/SymDenotations.scala | 2 +- .../tools/dotc/parsing/JavaParsers.scala | 4 +-- .../tools/dotc/transform/patmat/Space.scala | 4 +-- .../dotc/core/SealedDescendantsTest.scala | 33 ++++++++++++++++--- .../pc/completions/MatchCaseCompletions.scala | 10 ++---- .../backend/jvm/BCodeHelpers.scala | 2 +- .../backend/jvm/BTypesFromSymbols.scala | 2 +- 10 files changed, 41 insertions(+), 22 deletions(-) diff --git a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala b/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala index 3779f59d33b0..6331913049c2 100644 --- a/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala +++ b/compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala @@ -292,7 +292,7 @@ trait BCodeHelpers extends BCodeIdiomatic { } case Ident(nme.WILDCARD) => // An underscore argument indicates that we want to use the default value for this parameter, so do not emit anything - case t: tpd.RefTree if t.symbol.owner.linkedClass.isAllOf(JavaEnumTrait) => + case t: tpd.RefTree if t.symbol.owner.linkedClass.isAllOf(JavaEnum) => val edesc = innerClasesStore.typeDescriptor(t.tpe) // the class descriptor of the enumeration class. val evalue = t.symbol.javaSimpleName // value the actual enumeration value. av.visitEnum(name, edesc, evalue) diff --git a/compiler/src/dotty/tools/backend/jvm/BTypesFromSymbols.scala b/compiler/src/dotty/tools/backend/jvm/BTypesFromSymbols.scala index 55619f31ec32..b8d7ee04c870 100644 --- a/compiler/src/dotty/tools/backend/jvm/BTypesFromSymbols.scala +++ b/compiler/src/dotty/tools/backend/jvm/BTypesFromSymbols.scala @@ -304,7 +304,7 @@ class BTypesFromSymbols[I <: DottyBackendInterface](val int: I, val frontendAcce .addFlagIf(sym.is(Bridge), ACC_BRIDGE | ACC_SYNTHETIC) .addFlagIf(sym.is(Artifact), ACC_SYNTHETIC) .addFlagIf(sym.isClass && !sym.isInterface, ACC_SUPER) - .addFlagIf(sym.isAllOf(JavaEnumTrait), ACC_ENUM) + .addFlagIf(sym.isAllOf(JavaEnum), ACC_ENUM) .addFlagIf(sym.is(JavaVarargs), ACC_VARARGS) .addFlagIf(sym.is(Synchronized), ACC_SYNCHRONIZED) .addFlagIf(sym.isDeprecated, ACC_DEPRECATED) diff --git a/compiler/src/dotty/tools/dotc/core/Flags.scala b/compiler/src/dotty/tools/dotc/core/Flags.scala index 6ae9541a327f..8c1b715e3e30 100644 --- a/compiler/src/dotty/tools/dotc/core/Flags.scala +++ b/compiler/src/dotty/tools/dotc/core/Flags.scala @@ -576,7 +576,7 @@ object Flags { val InlineMethod: FlagSet = Inline | Method val InlineParam: FlagSet = Inline | Param val InlineByNameProxy: FlagSet = InlineProxy | Method - val JavaEnumTrait: FlagSet = JavaDefined | Enum // A Java enum trait + val JavaEnum: FlagSet = JavaDefined | Enum // A Java enum trait val JavaEnumValue: FlagSet = JavaDefined | EnumValue // A Java enum value val StaticProtected: FlagSet = JavaDefined | JavaStatic | Protected // Java symbol which is `protected` and `static` val JavaModule: FlagSet = JavaDefined | Module // A Java companion object diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 5fa9afbcd171..83362897e5e3 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1695,7 +1695,7 @@ object SymDenotations { c.ensureCompleted() end completeChildrenIn - if is(Sealed) || isAllOf(JavaEnumTrait) then + if is(Sealed) || isAllOf(JavaEnum) && isClass then if !is(ChildrenQueried) then // Make sure all visible children are completed, so that // they show up in Child annotations. A possible child is visible if it diff --git a/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala b/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala index 8e075acdf5e3..1c1c47ad68ce 100644 --- a/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/JavaParsers.scala @@ -992,7 +992,7 @@ object JavaParsers { Select(New(javaLangDot(tpnme.Enum)), nme.CONSTRUCTOR), List(enumType)), Nil) val enumclazz = atSpan(start, nameOffset) { TypeDef(name, - makeTemplate(superclazz :: interfaces, body, List(), true)).withMods(mods | Flags.JavaEnumTrait) + makeTemplate(superclazz :: interfaces, body, List(), true)).withMods(mods | Flags.JavaEnum) } addCompanionObject(consts ::: statics ::: predefs, enumclazz) } @@ -1011,7 +1011,7 @@ object JavaParsers { skipAhead() accept(RBRACE) } - ValDef(name.toTermName, enumType, unimplementedExpr).withMods(Modifiers(Flags.JavaEnumTrait | Flags.StableRealizable | Flags.JavaDefined | Flags.JavaStatic)) + ValDef(name.toTermName, enumType, unimplementedExpr).withMods(Modifiers(Flags.JavaEnumValue | Flags.JavaStatic)) } } diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index f3a883df9e4f..beb4119775d0 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -626,7 +626,7 @@ object SpaceEngine { case tp if tp.isRef(defn.UnitClass) => ConstantType(Constant(())) :: Nil case tp @ NamedType(Parts(parts), _) => parts.map(tp.derivedSelect) case _: SingletonType => ListOfNoType - case tp if tp.classSymbol.isAllOf(JavaEnumTrait) => tp.classSymbol.children.map(_.termRef) + case tp if tp.classSymbol.isAllOf(JavaEnum) => tp.classSymbol.children.map(_.termRef) // the class of a java enum value is the enum class, so this must follow SingletonType to not loop infinitely case tp @ AppliedType(Parts(parts), targs) if tp.classSymbol.children.isEmpty => @@ -843,7 +843,7 @@ object SpaceEngine { isCheckable(and.tp1) || isCheckable(and.tp2) }) || tpw.isRef(defn.BooleanClass) || - classSym.isAllOf(JavaEnumTrait) || + classSym.isAllOf(JavaEnum) || classSym.is(Case) && { if seen.add(tpw) then productSelectorTypes(tpw, sel.srcPos).exists(isCheckable(_)) else true // recursive case class: return true and other members can still fail the check diff --git a/compiler/test/dotty/tools/dotc/core/SealedDescendantsTest.scala b/compiler/test/dotty/tools/dotc/core/SealedDescendantsTest.scala index 0ae9069c03d1..4726596c0428 100644 --- a/compiler/test/dotty/tools/dotc/core/SealedDescendantsTest.scala +++ b/compiler/test/dotty/tools/dotc/core/SealedDescendantsTest.scala @@ -47,6 +47,19 @@ class SealedDescendantsTest extends DottyTest { ) end enumOpt + @Test + def javaEnum: Unit = + expectedDescendents("java.util.concurrent.TimeUnit", + "TimeUnit" :: + "NANOSECONDS.type" :: + "MICROSECONDS.type" :: + "MILLISECONDS.type" :: + "SECONDS.type" :: + "MINUTES.type" :: + "HOURS.type" :: + "DAYS.type" :: Nil + ) + @Test def hierarchicalSharedChildren: Unit = // Q is a child of both Z and A and should appear once @@ -91,10 +104,22 @@ class SealedDescendantsTest extends DottyTest { ) end hierarchicalSharedChildrenB - def expectedDescendents(source: String, root: String, expected: List[String]) = - exploreRoot(source, root) { rootCls => - val descendents = rootCls.sealedDescendants.map(sym => s"${sym.name}${if (sym.isTerm) ".type" else ""}") - assertEquals(expected.toString, descendents.toString) + def assertMatchingDescenants(rootCls: Symbol, expected: List[String])(using Context): Unit = + val descendents = rootCls.sealedDescendants.map(sym => s"${sym.name}${if (sym.isTerm) ".type" else ""}") + assertEquals(expected.toString, descendents.toString) + + def expectedDescendents(root: String, expected: List[String]): Unit = + exploreRootNoSource(root)(assertMatchingDescenants(_, expected)) + + def expectedDescendents(source: String, root: String, expected: List[String]): Unit = + exploreRoot(source, root)(assertMatchingDescenants(_, expected)) + + def exploreRootNoSource(root: String)(op: Context ?=> ClassSymbol => Unit) = + val source1 = s"""package testsealeddescendants + |object Foo { def foo: $root = ??? }""".stripMargin + checkCompile("typer", source1) { (_, context) => + given Context = context + op(requiredClass(root)) } def exploreRoot(source: String, root: String)(op: Context ?=> ClassSymbol => Unit) = diff --git a/presentation-compiler/src/main/dotty/tools/pc/completions/MatchCaseCompletions.scala b/presentation-compiler/src/main/dotty/tools/pc/completions/MatchCaseCompletions.scala index fe9a73655835..a3d5d8814c48 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/completions/MatchCaseCompletions.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/completions/MatchCaseCompletions.scala @@ -349,11 +349,7 @@ object CaseKeywordCompletion: symTpe <:< tpe val parents = getParentTypes(tpe, List.empty) - parents.toList.map { parent => - // There is an issue in Dotty, `sealedStrictDescendants` ends in an exception for java enums. https://github.com/lampepfl/dotty/issues/15908 - if parent.isAllOf(JavaEnumTrait) then parent.children - else sealedStrictDescendants(parent) - } match + parents.toList.map(sealedStrictDescendants) match case Nil => Nil case subcls :: Nil => subcls case subcls => @@ -409,9 +405,7 @@ class CompletionValueGenerator( Context ): Option[String] = val isModuleLike = - sym.is(Flags.Module) || sym.isOneOf(JavaEnumTrait) || sym.isOneOf( - JavaEnumValue - ) || sym.isAllOf(EnumCase) + sym.is(Flags.Module) || sym.isOneOf(JavaEnum) || sym.isOneOf(JavaEnumValue) || sym.isAllOf(EnumCase) if isModuleLike && hasBind then None else val pattern = diff --git a/tests/pos-with-compiler-cc/backend/jvm/BCodeHelpers.scala b/tests/pos-with-compiler-cc/backend/jvm/BCodeHelpers.scala index 3ab75bda787e..2454bca9d653 100644 --- a/tests/pos-with-compiler-cc/backend/jvm/BCodeHelpers.scala +++ b/tests/pos-with-compiler-cc/backend/jvm/BCodeHelpers.scala @@ -374,7 +374,7 @@ trait BCodeHelpers extends BCodeIdiomatic with BytecodeWriters { } case Ident(nme.WILDCARD) => // An underscore argument indicates that we want to use the default value for this parameter, so do not emit anything - case t: tpd.RefTree if t.symbol.owner.linkedClass.isAllOf(JavaEnumTrait) => + case t: tpd.RefTree if t.symbol.owner.linkedClass.isAllOf(JavaEnum) => val edesc = innerClasesStore.typeDescriptor(t.tpe) // the class descriptor of the enumeration class. val evalue = t.symbol.javaSimpleName // value the actual enumeration value. av.visitEnum(name, edesc, evalue) diff --git a/tests/pos-with-compiler-cc/backend/jvm/BTypesFromSymbols.scala b/tests/pos-with-compiler-cc/backend/jvm/BTypesFromSymbols.scala index 54dafe6f0032..d78008d65cc6 100644 --- a/tests/pos-with-compiler-cc/backend/jvm/BTypesFromSymbols.scala +++ b/tests/pos-with-compiler-cc/backend/jvm/BTypesFromSymbols.scala @@ -330,7 +330,7 @@ class BTypesFromSymbols[I <: DottyBackendInterface](val int: I) extends BTypes { .addFlagIf(sym.is(Bridge), ACC_BRIDGE | ACC_SYNTHETIC) .addFlagIf(sym.is(Artifact), ACC_SYNTHETIC) .addFlagIf(sym.isClass && !sym.isInterface, ACC_SUPER) - .addFlagIf(sym.isAllOf(JavaEnumTrait), ACC_ENUM) + .addFlagIf(sym.isAllOf(JavaEnum), ACC_ENUM) .addFlagIf(sym.is(JavaVarargs), ACC_VARARGS) .addFlagIf(sym.is(Synchronized), ACC_SYNCHRONIZED) .addFlagIf(sym.isDeprecated, ACC_DEPRECATED)