groovy

static import does not work in Groovy 1.7.4

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.7.4
  • Fix Version/s: 1.7.6, 1.8-beta-3
  • Component/s: None
  • Labels:
    None
  • Environment:
    Windows, Groovy Version: 1.7.4 JVM: 1.6.0_21
  • Number of attachments :
    2

Description

Static import does not work from Groovy scripts.

Example:

foo/Constants.groovy:

package foo

class Constants {
    static final PI = 3.14
}

foo/Test.groovy

package foo
import static foo.Constants.PI

class Test {
    static main(args) {
        println("pi=" + PI)
    }
}

When you try to run this (without compling with groovyc first), you get:

groovy -cp . foo\Test.groovy
Caught: groovy.lang.MissingPropertyException: No such property: PI for class: foo.Test
        at foo.Test.main(Test.groovy:6)

If you first run "groovyc foo\Constants.groovy" the above command succeeds.

  1. Text File
    p1.patch
    13/Dec/10 12:08 PM
    1 kB
    blackdrag blackdrag
  2. Text File
    p2.patch
    13/Dec/10 12:09 PM
    1 kB
    blackdrag blackdrag

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Frode Stokke added a comment -

Same problem with Groovy 1.7.5

Show
Frode Stokke added a comment - Same problem with Groovy 1.7.5
Hide
Permalink
Grégory Joseph added a comment -

Any feedback on this ?

Show
Grégory Joseph added a comment - Any feedback on this ?
Hide
Permalink
blackdrag blackdrag added a comment -

I am assigning this to Paul as static imports are his baby...

Show
blackdrag blackdrag added a comment - I am assigning this to Paul as static imports are his baby...
Hide
Permalink
blackdrag blackdrag added a comment -

unless Roshan wants to take care of this... I am sure Paul is not intending on keeping this one

Show
blackdrag blackdrag added a comment - unless Roshan wants to take care of this... I am sure Paul is not intending on keeping this one
Hide
Permalink
Alexey Sergeev added a comment -

This is very serious bug and the main reason why I can't migrate my applications to latest Groovy & Grails. Could anybody from Groovy team cope with the this?

Show
Alexey Sergeev added a comment - This is very serious bug and the main reason why I can't migrate my applications to latest Groovy & Grails. Could anybody from Groovy team cope with the this?
Hide
Permalink
Hamlet D'Arcy added a comment - - edited

Is this really a bug? PI is defined in a class named Constants. if you run "groovy Test.groovy" without first compiling COnstants.groovy into Constants.class then the class is not available and it is not found. If you compile Constants.groovy then Constants.class exists on the classpath and it is found. This used to work in a past version? That surprises me.

Show
Hamlet D'Arcy added a comment - - edited Is this really a bug? PI is defined in a class named Constants. if you run "groovy Test.groovy" without first compiling COnstants.groovy into Constants.class then the class is not available and it is not found. If you compile Constants.groovy then Constants.class exists on the classpath and it is found. This used to work in a past version? That surprises me.
Hide
Permalink
Alexander Chernyakevich added a comment - - edited

This is serious backward compatibility issue. Because, for example, for me the same as for Mr. Sergeev this completely prevent from migration to newer version of Grails 1.3.5 that use Groovy 1.7.5 because huge number of customer specific scripts will need to be manually updated to avoid this issue as soon as in older version of Groovy 1.7.2 that Grails 1.3.1 is based on there was not this bug.

And there was only minor version number changes but so radical influence.

Show
Alexander Chernyakevich added a comment - - edited This is serious backward compatibility issue. Because, for example, for me the same as for Mr. Sergeev this completely prevent from migration to newer version of Grails 1.3.5 that use Groovy 1.7.5 because huge number of customer specific scripts will need to be manually updated to avoid this issue as soon as in older version of Groovy 1.7.2 that Grails 1.3.1 is based on there was not this bug. And there was only minor version number changes but so radical influence.
Hide
Permalink
blackdrag blackdrag added a comment -

when compiling Test.groovy, the compiler should recognize, that there is a class foo.Constants and enqueue it for compiling. But in the end that is not even important. The compiler should recognize the static import and see that PI is imported that way. In theory the compiler could, without compiling Constants, issue the needed bytecode for this. The Groovy compiler does normally not inline, so that would be a PropertyExpression with foo.Constants as ClassExpression in the object part and PI as property string. There is one problem though with this. from the static import alone and without looking at the class the compiler cannot easily know if we import a method or an constant/static property. Also the problem is on what to do should the name PI reappear in the class... which is not the case here, but it is a potential problem.

Maybe we should make our lifes here more easy than we did in the past. Maybe we should simply say that if you static import, that you cannot declare a property or method of that name and that if you use the name as property or method, that then we will do the redirection.

@Alexander, that radical influence was surely not intended and was by accident. Actually it is not easy to write test cases for this kind of thing.

Show
blackdrag blackdrag added a comment - when compiling Test.groovy, the compiler should recognize, that there is a class foo.Constants and enqueue it for compiling. But in the end that is not even important. The compiler should recognize the static import and see that PI is imported that way. In theory the compiler could, without compiling Constants, issue the needed bytecode for this. The Groovy compiler does normally not inline, so that would be a PropertyExpression with foo.Constants as ClassExpression in the object part and PI as property string. There is one problem though with this. from the static import alone and without looking at the class the compiler cannot easily know if we import a method or an constant/static property. Also the problem is on what to do should the name PI reappear in the class... which is not the case here, but it is a potential problem. Maybe we should make our lifes here more easy than we did in the past. Maybe we should simply say that if you static import, that you cannot declare a property or method of that name and that if you use the name as property or method, that then we will do the redirection. @Alexander, that radical influence was surely not intended and was by accident. Actually it is not easy to write test cases for this kind of thing.
Hide
Permalink
Paul King added a comment -

Just confirming that it is a regression between 1.7.3 and 1.7.4.

Show
Paul King added a comment - Just confirming that it is a regression between 1.7.3 and 1.7.4.
Hide
Permalink
blackdrag blackdrag added a comment -

I attached two possible patches. Both try somehow to give ResolveVisitor its own phase, without disturbing too muhc of the overall infrastructure. Both are quite a hack. While p1 makes a more fundamental change in that every SourceUnitOperation might be run again, p2 is very specific to ResolveVisitor. I am not sure yet they really fix the issue at hand though, would be nice if that could be confirmed. If p2 fixes the problem I would vote for having p2 in 1.8 and 1.7. p1 is maybe good only for 1.8

Show
blackdrag blackdrag added a comment - I attached two possible patches. Both try somehow to give ResolveVisitor its own phase, without disturbing too muhc of the overall infrastructure. Both are quite a hack. While p1 makes a more fundamental change in that every SourceUnitOperation might be run again, p2 is very specific to ResolveVisitor. I am not sure yet they really fix the issue at hand though, would be nice if that could be confirmed. If p2 fixes the problem I would vote for having p2 in 1.8 and 1.7. p1 is maybe good only for 1.8
Hide
Permalink
Paul King added a comment -

Applied Jochen's patch (p2).

Show
Paul King added a comment - Applied Jochen's patch (p2).

People

  • Assignee:
    Paul King
    Reporter:
    Frode Stokke
Vote (15)
Watch (6)

Dates

  • Created:
    Updated:
    Resolved: