groovy

@Immutable annotation does not allow untyped static fields

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.7.0
  • Fix Version/s: 1.7.2, 1.8-beta-1
  • Component/s: None
  • Labels:
    None
  • Testcase included:
    yes
  • Number of attachments :
    1

Description

I cannot compile a class with @Immutable if it has a static field declared with def. I don't think the transform should touch static fields.

~$ cat ImmutableTest.groovy
@Immutable                                 
class ImmutableTest {                      
  static foo = {}                          
}                                          
marcus@better:~$ groovyc ImmutableTest.groovy 
org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed:
General error during canonicalization: @Immutable processor doesn't know how to handle field 'foo' of type 'java.lang.Object or def' while compiling class ImmutableTest.                                                                         
@Immutable classes currently only support properties with known immutable types or types where special handling achieves immutable behavior, including:                                                                                           
- Strings, primitive types, wrapper types, BigInteger and BigDecimal                                                     
- enums, other @Immutable classes and known immutables (java.awt.Color)                                                  
- Cloneable classes, collections, maps and arrays, and other classes with special handling (java.util.Date)              
Other restrictions apply, please see the groovydoc for @Immutable for further details                                    

java.lang.RuntimeException: @Immutable processor doesn't know how to handle field 'foo' of type 'java.lang.Object or def' while compiling class ImmutableTest.                                                                                    
@Immutable classes currently only support properties with known immutable types or types where special handling achieves immutable behavior, including:                                                                                           
- Strings, primitive types, wrapper types, BigInteger and BigDecimal
- enums, other @Immutable classes and known immutables (java.awt.Color)
- Cloneable classes, collections, maps and arrays, and other classes with special handling (java.util.Date)
Other restrictions apply, please see the groovydoc for @Immutable for further details
        at org.codehaus.groovy.transform.ImmutableASTTransformation.createConstructorStatement(ImmutableASTTransformation.java:373)
        at org.codehaus.groovy.transform.ImmutableASTTransformation.createConstructorMap(ImmutableASTTransformation.java:324)
        at org.codehaus.groovy.transform.ImmutableASTTransformation.createConstructor(ImmutableASTTransformation.java:310)
        at org.codehaus.groovy.transform.ImmutableASTTransformation.visit(ImmutableASTTransformation.java:113)
        at org.codehaus.groovy.transform.ASTTransformationVisitor.visitClass(ASTTransformationVisitor.java:129)
        at org.codehaus.groovy.transform.ASTTransformationVisitor$2.call(ASTTransformationVisitor.java:173)
        at org.codehaus.groovy.control.CompilationUnit.applyToPrimaryClassNodes(CompilationUnit.java:957)
        at org.codehaus.groovy.control.CompilationUnit.doPhaseOperation(CompilationUnit.java:517)
        at org.codehaus.groovy.control.CompilationUnit.processPhaseOperations(CompilationUnit.java:495)
        at org.codehaus.groovy.control.CompilationUnit.compile(CompilationUnit.java:472)
        at org.codehaus.groovy.control.CompilationUnit.compile(CompilationUnit.java:456)
        at org.codehaus.groovy.tools.FileSystemCompiler.compile(FileSystemCompiler.java:57)
        at org.codehaus.groovy.tools.FileSystemCompiler.doCompilation(FileSystemCompiler.java:170)
        at org.codehaus.groovy.tools.FileSystemCompiler.commandLineCompile(FileSystemCompiler.java:138)
        at org.codehaus.groovy.tools.FileSystemCompiler.main(FileSystemCompiler.java:152)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:616)
        at org.codehaus.groovy.tools.GroovyStarter.rootLoader(GroovyStarter.java:108)
        at org.codehaus.groovy.tools.GroovyStarter.main(GroovyStarter.java:130)

1 error

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Roshan Dawrani added a comment -

It's not really related to field being static. Even untyped instance level fields are rejected:

@Immutable                                 
class ImmutableTest {                      
  def x = ""                          
}

@Immutable's documentation says "Fields that are enums or other @Immutable classes are allowed but for an otherwise possible mutable property type, an error is thrown.". So the behavior, to me, seems in line with the documentation.

Show
Roshan Dawrani added a comment - It's not really related to field being static. Even untyped instance level fields are rejected: 
@Immutable                                 
class ImmutableTest {                      
  def x = ""                          
}
@Immutable's documentation says "Fields that are enums or other @Immutable classes are allowed but for an otherwise possible mutable property type, an error is thrown.". So the behavior, to me, seems in line with the documentation.
Hide
Permalink
Paul King added a comment - - edited

Statics are usually a bad idea for Immutable objects, e.g. consider this example:

@Immutable class Person {
   String first, last
   static species = 'Human'
   String getFullname() {
     "$first $last"
   }
   String getDescription() {
     "$fullname is a $species"
   }
}

def spock = new Person('Leonard', 'Nimoy')
assert spock.species == 'Human'
assert spock.fullname == 'Leonard Nimoy'
assert spock.description == 'Leonard Nimoy is a Human'

spock.species = 'Romulan'
assert spock.species == 'Romulan'

Person.species = 'Vulcan'
assert spock.species == 'Vulcan'
assert spock.fullname == 'Leonard Nimoy'
assert spock.description == 'Leonard Nimoy is a Vulcan'

Normally for an immutable object you typically expect method calls to be idempotent. In this context, by that I mean that I can call any of the methods of a Person instance multiple times and I should get the same result. E.g. no matter how many times I call getFullname() it always yields the same value. Code which is dealing with such objects can get away with caching results (if it wanted to) assuming this characteristic. As can be seen above, calls to getDescription() yields changing results because of the static property. Code assuming idempotency could yield subtle bugs.

Having said that, in the spirit of the design of @Immutable, it isn't fully locking the objects down to be immutable, just providing a framework to make it easy for you to do the right thing. So, it makes sense for the annotation to allow but not touch (as you point out) static properties.

Just for the record, I do deem the above to be bad design. Decide whether 'species' is part of your domain object or not. If it is, add it in as a normal property. If not, remove it. Place it in some kind of system Species class.

Show
Paul King added a comment - - edited Statics are usually a bad idea for Immutable objects, e.g. consider this example: 
@Immutable class Person {
   String first, last
   static species = 'Human'
   String getFullname() {
     "$first $last"
   }
   String getDescription() {
     "$fullname is a $species"
   }
}

def spock = new Person('Leonard', 'Nimoy')
assert spock.species == 'Human'
assert spock.fullname == 'Leonard Nimoy'
assert spock.description == 'Leonard Nimoy is a Human'

spock.species = 'Romulan'
assert spock.species == 'Romulan'

Person.species = 'Vulcan'
assert spock.species == 'Vulcan'
assert spock.fullname == 'Leonard Nimoy'
assert spock.description == 'Leonard Nimoy is a Vulcan'
Normally for an immutable object you typically expect method calls to be idempotent. In this context, by that I mean that I can call any of the methods of a Person instance multiple times and I should get the same result. E.g. no matter how many times I call getFullname() it always yields the same value. Code which is dealing with such objects can get away with caching results (if it wanted to) assuming this characteristic. As can be seen above, calls to getDescription() yields changing results because of the static property. Code assuming idempotency could yield subtle bugs. Having said that, in the spirit of the design of @Immutable, it isn't fully locking the objects down to be immutable, just providing a framework to make it easy for you to do the right thing. So, it makes sense for the annotation to allow but not touch (as you point out) static properties. Just for the record, I do deem the above to be bad design. Decide whether 'species' is part of your domain object or not. If it is, add it in as a normal property. If not, remove it. Place it in some kind of system Species class.
Hide
Permalink
Paul King added a comment -

The alternative would be to ban statics altogether (perhaps allowing just final initialized values) which is somewhat attractive but I suspect too restrictive. We could provide an optional flag to turn on a strict mode in the future which might turn this on.

Show
Paul King added a comment - The alternative would be to ban statics altogether (perhaps allowing just final initialized values) which is somewhat attractive but I suspect too restrictive. We could provide an optional flag to turn on a strict mode in the future which might turn this on.
Hide
Permalink
Paul King added a comment -

Added. Design and javadoc changed to not consider static properties.

Show
Paul King added a comment - Added. Design and javadoc changed to not consider static properties.
Hide
Permalink
Marcus Better added a comment -

FYI my use-case was a domain class in Grails, which has static fields like "constraints".

Show
Marcus Better added a comment - FYI my use-case was a domain class in Grails, which has static fields like "constraints".
Hide
Permalink
Paul King added a comment -

Hi Marcus, that will probably be fine. I added the example in the issue not specifically in response to your example but for future searches. People will see the feature has been added but might not realize the downsides.

Show
Paul King added a comment - Hi Marcus, that will probably be fine. I added the example in the issue not specifically in response to your example but for future searches. People will see the feature has been added but might not realize the downsides.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Codehaus. Try JIRA - bug tracking software for your team.