groovy
  1. groovy
  2. GROOVY-4386

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. p1.patch
        1 kB
        blackdrag blackdrag
      2. p2.patch
        1 kB
        blackdrag blackdrag

        Activity

        Hide
        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
        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
        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
        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
        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
          • Votes:
            15 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: