groovy
  1. groovy
  2. GROOVY-4267

Unable to import static for static inner classes

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.7.3
    • Fix Version/s: 1.7.4, 1.8-beta-1
    • Component/s: None
    • Labels:
      None
    • Number of attachments :
      0

      Description

      This issue tracks partial implementation of this functionality. See note at end of description.


      Given this Java class (note the static inner class):

      package pkg;
      
      public class Outer {
          public static class Inner {}
      }
      

      The inner class can be printed from Java using this:

      import pkg.Outer.Inner;
      //import static pkg.Outer.Inner;
      
      public class Main {
          public static void main(String[] args) {
              System.out.println(Inner.class.getName());
          }
      }
      

      Note that both the non-static or static version (shown commented out) will work.

      For Groovy, the non-static version works:

      import pkg.Outer.Inner
      
      println Inner.class.name
      

      but the static version doesn't:

      import static pkg.Outer.Inner
      
      println Inner.class.name
      

      instead, it fails with:

      Caught: groovy.lang.MissingPropertyException: No such property: Inner for class: TestStaticImport
      

      NOTE: The initial implementation of functionality for this issue works for external jars/classes on the classpath and for classes included in the same file but doesn't work for the critical case of external groovy files. As that case also doesn't work for attempts to import using a non-static variation of import, I think it is worthy of its own issue and so GROOVY-4287 has been created to track remaining work.

        Activity

        Hide
        Paul King added a comment -

        Java also supports star import variation:

        import static pkg.Outer.*
        

        the star can be an inner class not just a field or method.

        Show
        Paul King added a comment - Java also supports star import variation: import static pkg.Outer.* the star can be an inner class not just a field or method.
        Hide
        Roshan Dawrani added a comment -

        The static import of the inner class does not work in the following scenario. Run Test.groovy to reproduce with both files in the same directory.

        Test.groovy
        import static Outer.*
        
        assert PI == 3.14
        assert Inner.class.name != null
        
        Outer.groovy
        class Outer {
            static PI = 3.14
            static class Inner {}
        }
        

        Output:

        Caught: groovy.lang.MissingPropertyException: No such property: Inner for class: Test
                at Test.run(Test.groovy:4)
        
        Show
        Roshan Dawrani added a comment - The static import of the inner class does not work in the following scenario. Run Test.groovy to reproduce with both files in the same directory. Test.groovy import static Outer.* assert PI == 3.14 assert Inner.class.name != null Outer.groovy class Outer { static PI = 3.14 static class Inner {} } Output: Caught: groovy.lang.MissingPropertyException: No such property: Inner for class: Test at Test.run(Test.groovy:4)
        Hide
        Paul King added a comment -

        If you change the import in Test.groovy to either:

        import static Outer.PI
        import Outer.*
        

        or

        import static Outer.PI
        import Outer.Inner
        

        it still won't run.

        I suspect we need to go through a bunch of edge cases and clarify behaviour.

        Show
        Paul King added a comment - If you change the import in Test.groovy to either: import static Outer.PI import Outer.* or import static Outer.PI import Outer.Inner it still won't run. I suspect we need to go through a bunch of edge cases and clarify behaviour.
        Hide
        Roshan Dawrani added a comment -

        Import of static inner classes that only works if the importer and importee are in the same file looks a half-baked feature to me. It is not an edge case to have your code in more than one file.

        Useless to say that groovy now supports imports of static inner classes alongwith fields and methods but which breaks as soon as we spread code outside 1 file.

        You point to non-static import here, which does not work - but I see it as 2 incomplete features both of which we should try to fix.

        Show
        Roshan Dawrani added a comment - Import of static inner classes that only works if the importer and importee are in the same file looks a half-baked feature to me. It is not an edge case to have your code in more than one file. Useless to say that groovy now supports imports of static inner classes alongwith fields and methods but which breaks as soon as we spread code outside 1 file. You point to non-static import here, which does not work - but I see it as 2 incomplete features both of which we should try to fix.
        Hide
        Paul King added a comment -

        I agree with your points and think all cases need fixing. I don't think it is to do with being in the same file but pre-compiled vs not.

        Show
        Paul King added a comment - I agree with your points and think all cases need fixing. I don't think it is to do with being in the same file but pre-compiled vs not.
        Hide
        Roshan Dawrani added a comment -

        Don't know about the internals. I haven't looked at it yet. So you may be right that the cause is pre-compiled or not. I meant that at the usage level it works only if in one file and not otherwise.

        Even when you have importer and importee in one file, there are still bugs in the implementation:

        1) The following example works only when the module has a package. If you uncomment "package test", it works, but if the code is in default package, it does not work.

        //package test
        
        import static Outer.Inner
        
        assert Inner.class != null
        
        class Outer {
            static class Inner {}
        }
        

        2) Another issue is that the code changes made wrongly enable "resolve" of non-static inner classes through "import static" as well

        The following code works, but should not

        import static Outer.* /* this is the line that enables non-static Outer$Inner's resolution */
        
        // import static Outer.Inner - if instead this form is used, the behavior is correct
        
        assert Inner.class != null
        
        class Outer {
            class Inner {} /* this is a non-static inner class */
        }
        
        Show
        Roshan Dawrani added a comment - Don't know about the internals. I haven't looked at it yet. So you may be right that the cause is pre-compiled or not. I meant that at the usage level it works only if in one file and not otherwise. Even when you have importer and importee in one file, there are still bugs in the implementation: 1) The following example works only when the module has a package. If you uncomment "package test", it works, but if the code is in default package, it does not work. // package test import static Outer.Inner assert Inner.class != null class Outer { static class Inner {} } 2) Another issue is that the code changes made wrongly enable "resolve" of non-static inner classes through "import static" as well The following code works, but should not import static Outer.* /* this is the line that enables non- static Outer$Inner's resolution */ // import static Outer.Inner - if instead this form is used, the behavior is correct assert Inner.class != null class Outer { class Inner {} /* this is a non- static inner class */ }
        Hide
        Paul King added a comment -

        Above examples now work but still more testing would be useful

        Show
        Paul King added a comment - Above examples now work but still more testing would be useful
        Hide
        Roshan Dawrani added a comment -

        Looks ok, except the first issue reported. I understand from Paul that it is intentionally commented out for now, but just for completeness of the picture here, the following example still doesn't work, i.e., if you have a script in a separate file, you cannot import its static inner classes.

        Outer.groovy
        class Outer {
            static class Inner {}
        }
        
        Test.groovy
        import static Outer.*
        
        println Inner.class
        

        fails with

        Caught: groovy.lang.MissingPropertyException: No such property: Inner for class: Test
                at Test.run(Test.groovy:4)
        

        I will update if I find any further issues.

        Show
        Roshan Dawrani added a comment - Looks ok, except the first issue reported. I understand from Paul that it is intentionally commented out for now, but just for completeness of the picture here, the following example still doesn't work, i.e., if you have a script in a separate file, you cannot import its static inner classes. Outer.groovy class Outer { static class Inner {} } Test.groovy import static Outer.* println Inner.class fails with Caught: groovy.lang.MissingPropertyException: No such property: Inner for class: Test at Test.run(Test.groovy:4) I will update if I find any further issues.
        Hide
        Paul King added a comment -

        But ok if there is a package?

        Show
        Paul King added a comment - But ok if there is a package?
        Hide
        Roshan Dawrani added a comment -

        Import of static inner classes from other files is failing with the package or without package.

        Show
        Roshan Dawrani added a comment - Import of static inner classes from other files is failing with the package or without package.
        Hide
        Paul King added a comment -

        Strange. This works fine for me:

        pkg/Outer.groovy
        package pkg
        class Outer {
            static class Inner {}
        }
        
        Test.groovy
        import static pkg.Outer.Inner
        // import static pkg.Outer.* // or this
        
        println Inner.class.name
        

        succeeds with

        pkg.Outer$Inner
        
        Show
        Paul King added a comment - Strange. This works fine for me: pkg/Outer.groovy package pkg class Outer { static class Inner {} } Test.groovy import static pkg.Outer.Inner // import static pkg.Outer.* // or this println Inner.class.name succeeds with pkg.Outer$Inner
        Hide
        Roshan Dawrani added a comment -

        I had the Test.groovy also in the same package and then it did not work. Can u check if that works for you?

        Show
        Roshan Dawrani added a comment - I had the Test.groovy also in the same package and then it did not work. Can u check if that works for you?
        Hide
        Roshan Dawrani added a comment -

        Even the pkg/Outer.groovy + Test.groovy combination does not work for me. Maybe someone else can give it a try.

        How about no-package scenario? Does that work for you as well?

        Show
        Roshan Dawrani added a comment - Even the pkg/Outer.groovy + Test.groovy combination does not work for me. Maybe someone else can give it a try. How about no-package scenario? Does that work for you as well?
        Hide
        Paul King added a comment -

        I had a precompiled pkg.Outer hiding there from an earlier test. That is gone now and I think I have the same results as you now. Of course if I just use a PI field or property I get the same result so nothing to do with static Inner classes per se.

        Show
        Paul King added a comment - I had a precompiled pkg.Outer hiding there from an earlier test. That is gone now and I think I have the same results as you now. Of course if I just use a PI field or property I get the same result so nothing to do with static Inner classes per se.
        Hide
        Roshan Dawrani added a comment -

        I am not sure I understand your comparison with PI.
        If I have the following code

        Test.groovy
        import static Outer.*
        
        println PI
        
        println Inner.class
        

        and

        Outer.groovy
        class Outer {
            static PI = 3.14
            static class Inner {}
        }
        

        then I get the output as

        3.14
        Caught: groovy.lang.MissingPropertyException: No such property: Inner for class: Test
                at Test.run(Test.groovy:5)
        

        Doesn't that mean that PI is working as expected but static inner is not?

        Show
        Roshan Dawrani added a comment - I am not sure I understand your comparison with PI. If I have the following code Test.groovy import static Outer.* println PI println Inner.class and Outer.groovy class Outer { static PI = 3.14 static class Inner {} } then I get the output as 3.14 Caught: groovy.lang.MissingPropertyException: No such property: Inner for class: Test at Test.run(Test.groovy:5) Doesn't that mean that PI is working as expected but static inner is not?
        Hide
        Paul King added a comment -

        Sorry, ignore the PI reference - also due to the temporary test case I deleted. Can you try:

        import static Outer.PI
        import Outer.Inner
        
        Show
        Paul King added a comment - Sorry, ignore the PI reference - also due to the temporary test case I deleted. Can you try: import static Outer.PI import Outer.Inner
        Hide
        Roshan Dawrani added a comment - - edited

        Paul, I think you are again going back to what we started with.

        PreviouslyRaised.Roshan
        Import of static inner classes that only works if the importer and importee 
        are in the same file looks a half-baked feature to me. It is not an 
        edge case to have your code in more than one file. 
        
        ...
        
        You point to non-static import here, which does not work - but I see it 
        as 2 incomplete features both of which we should try to fix.
        
        Replied.Paul
        I agree with your points and think all cases need fixing.
        

        Anyway, here is the output you asked for:

        import static Outer.PI // works - I think we can ignore this now, as it always works
        import Outer.Inner // does not work 
        import static Outer.Inner // does not work 
        import static Outer.* // does not work 
        
        Show
        Roshan Dawrani added a comment - - edited Paul, I think you are again going back to what we started with. PreviouslyRaised.Roshan Import of static inner classes that only works if the importer and importee are in the same file looks a half-baked feature to me. It is not an edge case to have your code in more than one file. ... You point to non- static import here, which does not work - but I see it as 2 incomplete features both of which we should try to fix. Replied.Paul I agree with your points and think all cases need fixing. Anyway, here is the output you asked for: import static Outer.PI // works - I think we can ignore this now, as it always works import Outer.Inner // does not work import static Outer.Inner // does not work import static Outer.* // does not work
        Hide
        Paul King added a comment -

        Yes, just trying to draw a line in the sand now with current codebase - which I think we have well and truly done!

        Show
        Paul King added a comment - Yes, just trying to draw a line in the sand now with current codebase - which I think we have well and truly done!
        Hide
        Paul King added a comment -

        I thought the svn changes and comments were already getting long and tracking further changes including the non-static stuff all under the one issue might have been messy, so I split out remaining work into GROOVY-4287.

        Show
        Paul King added a comment - I thought the svn changes and comments were already getting long and tracking further changes including the non-static stuff all under the one issue might have been messy, so I split out remaining work into GROOVY-4287 .

          People

          • Assignee:
            Paul King
            Reporter:
            Paul King
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: