Merge pull request #12008 from dotty-staging/fix-11963

Streamline flag checking

Author: odersky

compiler/src/dotty/tools/dotc/ast/Desugar.scala

@@ -7,7 +7,7 @@ import util.Spans._, Types._, Contexts._, Constants._, Names._, NameOps._, Flags
 import Symbols._, StdNames._, Trees._, Phases._, ContextOps._
 import Decorators._, transform.SymUtils._
 import NameKinds.{UniqueName, EvidenceParamName, DefaultGetterName}
-import typer.{FrontEnd, Namer}
+import typer.{FrontEnd, Namer, Checking}
 import util.{Property, SourceFile, SourcePosition}
 import config.Feature.{sourceVersion, migrateTo3, enabled}
 import config.SourceVersion._
@@ -859,16 +859,7 @@ object desugar {
     val mods = mdef.mods
     val moduleName = normalizeName(mdef, impl).asTermName
     def isEnumCase = mods.isEnumCase
-
-    def flagSourcePos(flag: FlagSet) = mods.mods.find(_.flags == flag).fold(mdef.sourcePos)(_.sourcePos)
-
-    if (mods.is(Abstract))
-      report.error(AbstractCannotBeUsedForObjects(mdef), flagSourcePos(Abstract))
-    if (mods.is(Sealed))
-      report.error(ModifierRedundantForObjects(mdef, "sealed"), flagSourcePos(Sealed))
-    // Maybe this should be an error; see https://github.com/scala/bug/issues/11094.
-    if (mods.is(Final) && !mods.is(Synthetic))
-      report.warning(ModifierRedundantForObjects(mdef, "final"), flagSourcePos(Final))
+    Checking.checkWellFormedModule(mdef)
 
     if (mods.is(Package))
       packageModuleDef(mdef)

compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

@@ -154,8 +154,8 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] {
     ErasedTypesCanOnlyBeFunctionTypesID,
     CaseClassMissingNonImplicitParamListID,
     EnumerationsShouldNotBeEmptyID,
-    AbstractCannotBeUsedForObjectsID,
-    ModifierRedundantForObjectsID,
+    UNUSED_1,
+    RedundantModifierID,
     TypedCaseDoesNotExplicitlyExtendTypedEnumID,
     IllegalRedefinitionOfStandardKindID,
     NoExtensionMethodAllowedID,

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

@@ -2364,32 +2364,6 @@ import transform.SymUtils._
            |""".stripMargin
   }
 
-  class AbstractCannotBeUsedForObjects(mdef: untpd.ModuleDef)(using Context)
-    extends SyntaxMsg(AbstractCannotBeUsedForObjectsID) {
-    def msg = em"${hl("abstract")} modifier cannot be used for objects"
-
-    def explain =
-      em"""|Objects are final and cannot be extended, thus cannot have the ${hl("abstract")} modifier
-           |
-           |You may want to define an abstract class:
-           | ${hl("abstract")} ${hl("class")} Abstract${mdef.name} { }
-           |
-           |And extend it in an object:
-           | ${hl("object")} ${mdef.name} ${hl("extends")} Abstract${mdef.name} { }
-           |""".stripMargin
-  }
-
-  class ModifierRedundantForObjects(mdef: untpd.ModuleDef, modifier: String)(using Context)
-    extends SyntaxMsg(ModifierRedundantForObjectsID) {
-    def msg = em"${hl(modifier)} modifier is redundant for objects"
-
-    def explain =
-      em"""|Objects cannot be extended making the ${hl(modifier)} modifier redundant.
-           |You may want to define the object without it:
-           | ${hl("object")} ${mdef.name} { }
-           |""".stripMargin
-  }
-
   class TypedCaseDoesNotExplicitlyExtendTypedEnum(enumDef: Symbol, caseDef: untpd.TypeDef)(using Context)
     extends SyntaxMsg(TypedCaseDoesNotExplicitlyExtendTypedEnumID) {
     def msg = i"explicit extends clause needed because both enum case and enum class have type parameters"
@@ -2481,9 +2455,15 @@ import transform.SymUtils._
            |""".stripMargin
   }
 
-  class ModifierNotAllowedForDefinition(flag: FlagSet)(using Context)
+  class ModifierNotAllowedForDefinition(flag: Flag)(using Context)
     extends SyntaxMsg(ModifierNotAllowedForDefinitionID) {
-    def msg = s"Modifier `${flag.flagsString}` is not allowed for this definition"
+    def msg = em"Modifier ${hl(flag.flagsString)} is not allowed for this definition"
+    def explain = ""
+  }
+
+  class RedundantModifier(flag: Flag)(using Context)
+    extends SyntaxMsg(RedundantModifierID) {
+    def msg = em"Modifier ${hl(flag.flagsString)} is redundant for this definition"
     def explain = ""
   }
 

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

@@ -433,9 +433,10 @@ object Checking {
       if (sym.isAllOf(flag1 | flag2)) fail(msg)
     def checkCombination(flag1: FlagSet, flag2: FlagSet) =
       if sym.isAllOf(flag1 | flag2) then fail(i"illegal combination of modifiers: `${flag1.flagsString}` and `${flag2.flagsString}` for: $sym")
-    def checkApplicable(flag: FlagSet, ok: Boolean) =
-      if (!ok && !sym.is(Synthetic))
+    def checkApplicable(flag: Flag, ok: Boolean) =
+      if sym.is(flag, butNot = Synthetic) && !ok then
         fail(ModifierNotAllowedForDefinition(flag))
+        sym.resetFlag(flag)
 
     if (sym.is(Inline) &&
           (  sym.is(ParamAccessor) && sym.owner.isClass
@@ -485,6 +486,15 @@ object Checking {
     if (sym.isConstructor && !sym.isPrimaryConstructor && sym.owner.is(Trait, butNot = JavaDefined))
       val addendum = if ctx.settings.Ydebug.value then s" ${sym.owner.flagsString}" else ""
       fail("Traits cannot have secondary constructors" + addendum)
+    checkApplicable(Inline, sym.isTerm && !sym.isOneOf(Mutable | Module))
+    checkApplicable(Lazy, !sym.isOneOf(Method | Mutable))
+    if (sym.isType && !sym.is(Deferred))
+      for (cls <- sym.allOverriddenSymbols.filter(_.isClass)) {
+        fail(CannotHaveSameNameAs(sym, cls, CannotHaveSameNameAs.CannotBeOverridden))
+        sym.setFlag(Private) // break the overriding relationship by making sym Private
+      }
+    checkApplicable(Erased,
+      !sym.isOneOf(MutableOrLazy, butNot = Given) && !sym.isType || sym.isClass)
     checkCombination(Final, Open)
     checkCombination(Sealed, Open)
     checkCombination(Final, Sealed)
@@ -493,18 +503,22 @@ object Checking {
     checkCombination(Private, Override)
     checkCombination(Lazy, Inline)
     checkNoConflict(Lazy, ParamAccessor, s"parameter may not be `lazy`")
-    if (sym.is(Inline)) checkApplicable(Inline, sym.isTerm && !sym.isOneOf(Mutable | Module))
-    if (sym.is(Lazy)) checkApplicable(Lazy, !sym.isOneOf(Method | Mutable))
-    if (sym.isType && !sym.is(Deferred))
-      for (cls <- sym.allOverriddenSymbols.filter(_.isClass)) {
-        fail(CannotHaveSameNameAs(sym, cls, CannotHaveSameNameAs.CannotBeOverridden))
-        sym.setFlag(Private) // break the overriding relationship by making sym Private
-      }
-    if (sym.is(Erased))
-      checkApplicable(Erased,
-        !sym.isOneOf(MutableOrLazy, butNot = Given) && !sym.isType || sym.isClass)
   }
 
+  /** Check for illegal or redundant modifiers on modules. This is done separately
+   *  from checkWellformed, since the original module modifiers don't surivive desugaring
+   */
+  def checkWellFormedModule(mdef: untpd.ModuleDef)(using Context) =
+    val mods = mdef.mods
+    def flagSourcePos(flag: FlagSet) =
+      mods.mods.find(_.flags == flag).getOrElse(mdef).srcPos
+    if mods.is(Abstract) then
+      report.error(ModifierNotAllowedForDefinition(Abstract), flagSourcePos(Abstract))
+    if mods.is(Sealed) then
+      report.error(ModifierNotAllowedForDefinition(Sealed), flagSourcePos(Sealed))
+    if mods.is(Final, butNot = Synthetic) then
+      report.warning(RedundantModifier(Final), flagSourcePos(Final))
+
   /** Check the type signature of the symbol `M` defined by `tree` does not refer
    *  to a private type or value which is invisible at a point where `M` is still
    *  visible.

tests/neg/AbstractObject.check

@@ -1,4 +1,12 @@
 -- [E156] Syntax Error: tests/neg/modifier-not-allowed.scala:2:13 ------------------------------------------------------
 2 |  opaque def o: Int = 3 // error
   |  ^^^^^^^^^^^^^^^^^^^^^
-  |  Modifier `opaque` is not allowed for this definition
+  |  Modifier opaque is not allowed for this definition
+-- [E156] Syntax Error: tests/neg/modifier-not-allowed.scala:3:2 -------------------------------------------------------
+3 |  abstract object A {} // error
+  |  ^^^^^^^^
+  |  Modifier abstract is not allowed for this definition
+-- [E156] Syntax Error: tests/neg/modifier-not-allowed.scala:4:2 -------------------------------------------------------
+4 |  sealed object A {} // error
+  |  ^^^^^^
+  |  Modifier sealed is not allowed for this definition

tests/neg/AbstractObject.scala

@@ -1,3 +1,5 @@
 object Test {
   opaque def o: Int = 3 // error
+  abstract object A {} // error
+  sealed object A {} // error
 }