Scala Programming Language
  1. Scala Programming Language
  2. SI-5147

Enumerations Sometimes Fail To Correctly Populate The Name of Values

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: Scala 2.9.1
    • Fix Version/s: None
    • Component/s: Enumeration
    • Labels:
    • Environment:

      OS X 10.7.2

      Description

      If you have a class that extends Enumeration that defines the actual Enumeration and then have a companion object extends from that class, the string values of the enumeration values is improperly handled. For example:

      class Foo extends Enumeration {
        val A = Value
      }
      object Foo extends Foo
      

      ...and then you try to print Foo.A, it'll print "<Invalid enum: no field for #0>" rather than "A".

        Issue Links

          Activity

          Hide
          Simon Ochsenreither added a comment -

          Can't we just get rid of the whole class and think about a better way to provide this functionality?

          This class has been bug-ridden since I started to work with Scala and has a highly disproportional ratio of LOC/bugs. It is not even used in Scala at all except one occurrence in Scala Swing afaik.

          Most code in the wild doesn't even bother using it and takes the expensive route of

          abstract class Foo
          object Foo1 
          object Foo2
          ... 
          

          instead.

          I don't have any hopes left that this functionality doesn't keep breaking like it did in the past.

          Maybe it would be better to tell people to define enums in Java instead (as it is already done for runtime-visible annotations) and deprecate/remove this class?

          Show
          Simon Ochsenreither added a comment - Can't we just get rid of the whole class and think about a better way to provide this functionality? This class has been bug-ridden since I started to work with Scala and has a highly disproportional ratio of LOC/bugs. It is not even used in Scala at all except one occurrence in Scala Swing afaik. Most code in the wild doesn't even bother using it and takes the expensive route of abstract class Foo object Foo1 object Foo2 ... instead. I don't have any hopes left that this functionality doesn't keep breaking like it did in the past. Maybe it would be better to tell people to define enums in Java instead (as it is already done for runtime-visible annotations) and deprecate/remove this class?
          Hide
          Paul Phillips added a comment -

          "It is not even used in Scala at all except one occurrence in Scala Swing afaik."

          Without necessarily disagreeing with the idea that Enumeration should be marked for death:

            object State extends Enumeration {
          object Contexts extends Enumeration {
            object InfoLevel extends Enumeration {
              object InfixMode extends Enumeration {
              object NonPublicRefs extends Enumeration {
            // object AccessMode extends Enumeration("AccessMode") {
            object severity extends Enumeration
            object Level extends Enumeration {
           *    object WeekDay extends Enumeration {
            object RoundingMode extends Enumeration(java.math.RoundingMode.values map (_.toString) : _*) with Serializable {
          object Modifier extends Enumeration {
            object Category extends Enumeration {
          object Alignment extends Enumeration {
            object Position extends Enumeration {
          object Key extends Enumeration {
            object Location extends Enumeration {
            object Result extends Enumeration {
            object SelectionMode extends Enumeration {
            object Alignment extends Enumeration {
            object FocusLostBehavior extends Enumeration {
            object Fill extends Enumeration {
            object Anchor extends Enumeration {
            object IntervalMode extends Enumeration {
          object Orientation extends Enumeration {
            object Message extends Enumeration {
            object Options extends Enumeration {
            object Result extends Enumeration {
            object BarPolicy extends Enumeration {
            object Layout extends Enumeration {
            object AutoResizeMode extends Enumeration {
            object IntervalMode extends Enumeration {
            object ElementMode extends Enumeration {
            object FocusLostBehavior extends Enumeration {
          
          Show
          Paul Phillips added a comment - "It is not even used in Scala at all except one occurrence in Scala Swing afaik." Without necessarily disagreeing with the idea that Enumeration should be marked for death: object State extends Enumeration { object Contexts extends Enumeration { object InfoLevel extends Enumeration { object InfixMode extends Enumeration { object NonPublicRefs extends Enumeration { // object AccessMode extends Enumeration("AccessMode") { object severity extends Enumeration object Level extends Enumeration { * object WeekDay extends Enumeration { object RoundingMode extends Enumeration(java.math.RoundingMode.values map (_.toString) : _*) with Serializable { object Modifier extends Enumeration { object Category extends Enumeration { object Alignment extends Enumeration { object Position extends Enumeration { object Key extends Enumeration { object Location extends Enumeration { object Result extends Enumeration { object SelectionMode extends Enumeration { object Alignment extends Enumeration { object FocusLostBehavior extends Enumeration { object Fill extends Enumeration { object Anchor extends Enumeration { object IntervalMode extends Enumeration { object Orientation extends Enumeration { object Message extends Enumeration { object Options extends Enumeration { object Result extends Enumeration { object BarPolicy extends Enumeration { object Layout extends Enumeration { object AutoResizeMode extends Enumeration { object IntervalMode extends Enumeration { object ElementMode extends Enumeration { object FocusLostBehavior extends Enumeration {
          Hide
          Turing Eret added a comment -

          So, it looks like the culprit was a fix for SI-4570 (specifically commit 679d803402c44c9fcb47eba732d997280345667f). In that fix, Paul made these changes to src/library/scala/Enumeration.scala:

          @@ -157,10 +157,14 @@ abstract class Enumeration(initial: Int, names: String*) extends Serializable {
             protected final def Value(i: Int, name: String): Value = new Val(i, name)
           
             private def populateNameMap() {
          +    val fields = getClass.getDeclaredFields
          +    def isValDef(m: JMethod) = fields exists (fd => fd.getName == m.getName && fd.getType == m.getReturnType)
          +
               // The list of possible Value methods: 0-args which return a conforming type
               val methods = getClass.getMethods filter (m => m.getParameterTypes.isEmpty &&
                                                              classOf[Value].isAssignableFrom(m.getReturnType) &&
          -                                                   m.getDeclaringClass != classOf[Enumeration])
          +                                                   m.getDeclaringClass != classOf[Enumeration] &&
          +                                                   isValDef(m))
          

          The problem is that isValDef only checks to see if it is a val defined in the current class. If you extends from a class where the actual enumeration value is defined, then it won't find it.

          The following diff seems to fix this issue:

          diff --git a/src/library/scala/Enumeration.scala b/src/library/scala/Enumeration.scala
          index 45cb3a6..30bb274 100644
          --- a/src/library/scala/Enumeration.scala
          +++ b/src/library/scala/Enumeration.scala
          @@ -161,7 +161,10 @@ abstract class Enumeration(initial: Int, names: String*) extends Serializable {
             protected final def Value(i: Int, name: String): Value = new Val(i, name)
          
             private def populateNameMap() {
          -    val fields = getClass.getDeclaredFields
          +    def getFields(clazz: Class[_]) : Array[JField] = {
          +      clazz.getDeclaredFields ++ (if(clazz.getSuperclass != null) getFields(clazz.getSuperclass) else Nil)
          +    }
          +    val fields = getFields(getClass)
               def isValDef(m: JMethod) = fields exists (fd => fd.getName == m.getName && fd.getType == m.getReturnType)
          
               // The list of possible Value methods: 0-args which return a conforming type
          diff --git a/test/files/run/t5147.check b/test/files/run/t5147.check
          new file mode 100644
          index 0000000..f70f10e
          --- /dev/null
          +++ b/test/files/run/t5147.check
          @@ -0,0 +1 @@
          +A
          diff --git a/test/files/run/t5147.scala b/test/files/run/t5147.scala
          new file mode 100644
          index 0000000..6261336
          --- /dev/null
          +++ b/test/files/run/t5147.scala
          @@ -0,0 +1,9 @@
          +class Test extends Enumeration {
          +  val A = Value
          +}
          +object Test extends Test
          +object Test5147 {
          +  def main(args: Array[String]): Unit = {
          +    println(Test.A)
          +  }
          +}
          
          Show
          Turing Eret added a comment - So, it looks like the culprit was a fix for SI-4570 (specifically commit 679d803402c44c9fcb47eba732d997280345667f). In that fix, Paul made these changes to src/library/scala/Enumeration.scala: @@ -157,10 +157,14 @@ abstract class Enumeration(initial: Int, names: String*) extends Serializable { protected final def Value(i: Int, name: String): Value = new Val(i, name) private def populateNameMap() { + val fields = getClass.getDeclaredFields + def isValDef(m: JMethod) = fields exists (fd => fd.getName == m.getName && fd.getType == m.getReturnType) + // The list of possible Value methods: 0-args which return a conforming type val methods = getClass.getMethods filter (m => m.getParameterTypes.isEmpty && classOf[Value].isAssignableFrom(m.getReturnType) && - m.getDeclaringClass != classOf[Enumeration]) + m.getDeclaringClass != classOf[Enumeration] && + isValDef(m)) The problem is that isValDef only checks to see if it is a val defined in the current class. If you extends from a class where the actual enumeration value is defined, then it won't find it. The following diff seems to fix this issue: diff --git a/src/library/scala/Enumeration.scala b/src/library/scala/Enumeration.scala index 45cb3a6..30bb274 100644 --- a/src/library/scala/Enumeration.scala +++ b/src/library/scala/Enumeration.scala @@ -161,7 +161,10 @@ abstract class Enumeration(initial: Int, names: String*) extends Serializable { protected final def Value(i: Int, name: String): Value = new Val(i, name) private def populateNameMap() { - val fields = getClass.getDeclaredFields + def getFields(clazz: Class[_]) : Array[JField] = { + clazz.getDeclaredFields ++ (if(clazz.getSuperclass != null) getFields(clazz.getSuperclass) else Nil) + } + val fields = getFields(getClass) def isValDef(m: JMethod) = fields exists (fd => fd.getName == m.getName && fd.getType == m.getReturnType) // The list of possible Value methods: 0-args which return a conforming type diff --git a/test/files/run/t5147.check b/test/files/run/t5147.check new file mode 100644 index 0000000..f70f10e --- /dev/null +++ b/test/files/run/t5147.check @@ -0,0 +1 @@ +A diff --git a/test/files/run/t5147.scala b/test/files/run/t5147.scala new file mode 100644 index 0000000..6261336 --- /dev/null +++ b/test/files/run/t5147.scala @@ -0,0 +1,9 @@ +class Test extends Enumeration { + val A = Value +} +object Test extends Test +object Test5147 { + def main(args: Array[String]): Unit = { + println(Test.A) + } +}
          Hide
          Commit Message Bot added a comment -

          (extempore in r25943) Fix for Enumeration.

          Closes SI-5147.

          Show
          Commit Message Bot added a comment - (extempore in r25943 ) Fix for Enumeration. Closes SI-5147 .
          Hide
          Paul Phillips added a comment -

          Oops, I just checked in some code accidentally. Please ignore the man behind the curtain.

          Show
          Paul Phillips added a comment - Oops, I just checked in some code accidentally. Please ignore the man behind the curtain.
          Hide
          Commit Message Bot added a comment -

          (extempore in r25946) Revert "Fix for Enumeration."

          Oops, didn't mean to commit that one. Opens SI-5147. No review.

          Show
          Commit Message Bot added a comment - (extempore in r25946 ) Revert "Fix for Enumeration." Oops, didn't mean to commit that one. Opens SI-5147 . No review.
          Hide
          Turing Eret added a comment -

          Did you mean to reopen this, Paul?

          Show
          Turing Eret added a comment - Did you mean to reopen this, Paul?
          Hide
          Paul Phillips added a comment -

          Yeah, I had no idea whether the commit bot would act on "opens" but I figured I'd give it a shot.

          Show
          Paul Phillips added a comment - Yeah, I had no idea whether the commit bot would act on "opens" but I figured I'd give it a shot.
          Hide
          Trond Olsen added a comment -

          The Closed Collector: Bring out yer closed.
          [a man puts SI-4805 on the heap with SI-3615 SI-3815 SI-3186 SI-2214 SI-4334 SI-1505 SI-3134 SI-3687 SI-4570 SI-3616 SI-3817 SI-2827 SI-2111 SI-3719 SI-2211 SI-4333 SI-3314 SI-4045 SI-1496 SI-4571 SI-3174 SI-3950 SI-2527 SI-5117 SI-1849 SI-4292 SI-1089 SI-4236 SI-2933 SI-1550]
          Large Man with Closed SI-5147: Here's one.
          The Closed Collector: That'll be ninepence.
          The Closed SI-5147 That Claims It Isn't: I'm not closed.
          The Closed Collector: What?
          Large Man with Closed SI-5147: Nothing. There's your ninepence.

          Show
          Trond Olsen added a comment - The Closed Collector: Bring out yer closed. [a man puts SI-4805 on the heap with SI-3615 SI-3815 SI-3186 SI-2214 SI-4334 SI-1505 SI-3134 SI-3687 SI-4570 SI-3616 SI-3817 SI-2827 SI-2111 SI-3719 SI-2211 SI-4333 SI-3314 SI-4045 SI-1496 SI-4571 SI-3174 SI-3950 SI-2527 SI-5117 SI-1849 SI-4292 SI-1089 SI-4236 SI-2933 SI-1550 ] Large Man with Closed SI-5147 : Here's one. The Closed Collector: That'll be ninepence. The Closed SI-5147 That Claims It Isn't: I'm not closed. The Closed Collector: What? Large Man with Closed SI-5147 : Nothing. There's your ninepence.

            People

            • Assignee:
              Unassigned
              Reporter:
              Turing Eret
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:

                Development