Uploaded image for project: 'Groovy'
  1. Groovy
  2. GROOVY-7773

Enum constructor with value throws "unexpected token" error

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4.6
    • Fix Version/s: 2.4.7
    • Component/s: Compiler
    • Labels:
      None
    • Environment:
      Windows 7 Professional
      JDK 1.8.0_45

      Description

      First time submitter to this JIRA so let me know if I've specified anything incorrectly. I just upgraded from 2.4.4 to 2.4.6 and now declaring an enum constructor accepting a value as follows:

      UsStates.groovy
      enum UsState {
        
        ID('Idaho'),
        IL('Illinois'),
        IN('Indiana'),
        
        UsState( String value ) { this.value = value }
      
        private final String value
        
        String toString() { return value }
      
      }
      
      println UsState.ID //Idaho
      

      throws the following compile error:

      org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed:
      C:\svn_qa\jenkins_trunk\scripts\UsStates.groovy: 7: unexpected token: this @ line 7, column 29.
           UsState( String value ) { this.value = value }
                                     ^
      
      1 error
      

      No such error occurred in 2.4.4, nor does it happen in 2.4.5. Here are a few contrasting things that do seem to work in 2.4.6:

      UsStatesBoring.groovy
      enum UsState {
        
        ID,
        IL,
        IN,
      
      }
      
      println UsState.ID //ID
      
      UsStatesNoEnum.groovy
      class UsState {
        
        UsState( String value ) { this.value = value }
      
        private final String value
        
        String toString() { return value }
      
      }
      
      final idaho = new UsState('Idaho')
      println idaho //Idaho
      

      Though this seems higher priority than Minor, I can work around it by simply downgrading to 2.4.5. I wish there was a Medium priority.

        Issue Links

          Activity

          Hide
          paulk Paul King added a comment -

          We did do a slight tweak to the grammar for enums (851fd58). Perhaps it is related to that. If you remove the last comma (i.e. after IN) then your example works. So, I think the priority is fine at least until we look into it more and decide which variations we want to support. Java would require a semicolon after such a dangling comma but we don't support that either. I suspect though that this edge case wasn't noted as a breaking change, I'll mark it as such once I confirm.

          Show
          paulk Paul King added a comment - We did do a slight tweak to the grammar for enums (851fd58). Perhaps it is related to that. If you remove the last comma (i.e. after IN) then your example works. So, I think the priority is fine at least until we look into it more and decide which variations we want to support. Java would require a semicolon after such a dangling comma but we don't support that either. I suspect though that this edge case wasn't noted as a breaking change, I'll mark it as such once I confirm.
          Hide
          brianeray Brian Ray added a comment -

          That change is a good suspect. Dangling commas are handy for adding new list items but deleting that comma is a much better workaround than downgrading. Thanks for looking into this.

          Show
          brianeray Brian Ray added a comment - That change is a good suspect. Dangling commas are handy for adding new list items but deleting that comma is a much better workaround than downgrading. Thanks for looking into this.
          Hide
          paulk Paul King added a comment -

          Agreed, we would like to support the dangling case as we did previously but the change I mentioned earlier was made for a reason, so we just need to check whether we can support both. It could just be that the previous fix just needs a little bit of finessing or it could end up being a limitation because of Groovy's support for removing semicolons. I imagine as a minimum we could (and should) support having the semicolon straight after the dangling comma.

          Show
          paulk Paul King added a comment - Agreed, we would like to support the dangling case as we did previously but the change I mentioned earlier was made for a reason, so we just need to check whether we can support both. It could just be that the previous fix just needs a little bit of finessing or it could end up being a limitation because of Groovy's support for removing semicolons. I imagine as a minimum we could (and should) support having the semicolon straight after the dangling comma.
          Hide
          paulk Paul King added a comment -

          I have cloned this issue. Firstly, I think we should at least support Java's syntax in this case (to be covered by this issue) by allowing a semi-colon to close off the constant declarations (with or without the dangling comma - but optional for the non-dangling case). Secondly, in the cloned issue, we can explore whether it is possible to tweak the grammar to remove that need, i.e. make it optional for the dangling case too if possible.

          Show
          paulk Paul King added a comment - I have cloned this issue. Firstly, I think we should at least support Java's syntax in this case (to be covered by this issue) by allowing a semi-colon to close off the constant declarations (with or without the dangling comma - but optional for the non-dangling case). Secondly, in the cloned issue, we can explore whether it is possible to tweak the grammar to remove that need, i.e. make it optional for the dangling case too if possible.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user paulk-asert opened a pull request:

          https://github.com/apache/groovy/pull/279

          GROOVY-7773: Enum constructor with value throws "unexpected token" er…

          …ror (closes #279)

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/paulk-asert/groovy groovy7773

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/groovy/pull/279.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #279


          commit 5b5a166c9e7cd89a7064bef369dbd96f599e5e7a
          Author: paulk <paulk@asert.com.au>
          Date: 2016-03-04T04:41:11Z

          GROOVY-7773: Enum constructor with value throws "unexpected token" error (closes #279)


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user paulk-asert opened a pull request: https://github.com/apache/groovy/pull/279 GROOVY-7773 : Enum constructor with value throws "unexpected token" er… …ror (closes #279) You can merge this pull request into a Git repository by running: $ git pull https://github.com/paulk-asert/groovy groovy7773 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/groovy/pull/279.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #279 commit 5b5a166c9e7cd89a7064bef369dbd96f599e5e7a Author: paulk <paulk@asert.com.au> Date: 2016-03-04T04:41:11Z GROOVY-7773 : Enum constructor with value throws "unexpected token" error (closes #279)
          Hide
          brianeray Brian Ray added a comment -

          Thank you.

          Show
          brianeray Brian Ray added a comment - Thank you.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/groovy/pull/279

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/groovy/pull/279
          Hide
          paulk Paul King added a comment -

          PR merged

          Show
          paulk Paul King added a comment - PR merged

            People

            • Assignee:
              paulk Paul King
              Reporter:
              brianeray Brian Ray
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development