Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.7.4, 1.8-beta-1
    • Fix Version/s: 1.7.4, 1.8-beta-1
    • Component/s: None
    • Labels:
      None

      Description

      Test.groovy
      class B extends A {
           static main(args){}
      }
      
      class A {}
      

      fails with

      Caught: groovy.lang.GroovyRuntimeException: This script or class could not be run.
      It should either: 
      - have a main method, 
      - be a JUnit test, TestNG test or extend GroovyTestCase, 
      - or implement the Runnable interface.
      
      1. 4264_v18x.patch
        4 kB
        Roshan Dawrani

        Issue Links

          Activity

          Hide
          Roshan Dawrani added a comment -

          ModuleNode#sortClasses() seems to be sorting classes in order of inheritance and that's why orders A ahead of B.

          In an example like the one given here, it looks wrong to me.

          In class sorting, should it take into account, which class is the executable one (as per the criteria used in GroovyShell#runScriptOrMainOrTestOrRunnable())?

          Show
          Roshan Dawrani added a comment - ModuleNode#sortClasses() seems to be sorting classes in order of inheritance and that's why orders A ahead of B. In an example like the one given here, it looks wrong to me. In class sorting, should it take into account, which class is the executable one (as per the criteria used in GroovyShell#runScriptOrMainOrTestOrRunnable())?
          Hide
          Jochen Theodorou added a comment -

          Class sorting cannot change, that would create other problems. If not, then please correct me. If the sorting doesn't give the desired output, then we have to define it by other means later.

          Show
          Jochen Theodorou added a comment - Class sorting cannot change, that would create other problems. If not, then please correct me. If the sorting doesn't give the desired output, then we have to define it by other means later.
          Hide
          Roshan Dawrani added a comment -

          Sorting is not giving what looks like the desired output - to me at least. But I have my silly hat on currently. You may want to close the issue.

          Show
          Roshan Dawrani added a comment - Sorting is not giving what looks like the desired output - to me at least. But I have my silly hat on currently. You may want to close the issue.
          Hide
          Paul King added a comment -

          Maybe there is nothing we can change. But it does lead to unexpected behavior, so we should document the behavior somewhere though. Perhaps we should put a comment under GROOVY-1328 related to the sorting/inheritance issue and close this issue.

          Show
          Paul King added a comment - Maybe there is nothing we can change. But it does lead to unexpected behavior, so we should document the behavior somewhere though. Perhaps we should put a comment under GROOVY-1328 related to the sorting/inheritance issue and close this issue.
          Hide
          Roshan Dawrani added a comment -

          Bug converted to a sub-task of GROOVY-1328. More chances of getting it fixed with it remaining open as a sub-task rather than getting lost in a comment.

          Show
          Roshan Dawrani added a comment - Bug converted to a sub-task of GROOVY-1328 . More chances of getting it fixed with it remaining open as a sub-task rather than getting lost in a comment.
          Hide
          Jochen Theodorou added a comment -

          I do not propose to close this bug. But I think "critical" is a too high setting for this issue. Then the sorting. The sorting is supposed to be inheritance based as some parts of the compiler need that and isnce I introduced that because of those parts. If we want to clean up this issue we have to come up with a solution that does not touch sorting

          Show
          Jochen Theodorou added a comment - I do not propose to close this bug. But I think "critical" is a too high setting for this issue. Then the sorting. The sorting is supposed to be inheritance based as some parts of the compiler need that and isnce I introduced that because of those parts. If we want to clean up this issue we have to come up with a solution that does not touch sorting
          Hide
          Roshan Dawrani added a comment -

          Reduced the priority.

          To users who don't care about internal sorting (which cannot change), it may look too small a thing to not be working.

          I am giving it 2 classes with 1 main method and just can't run them - in whatever order I place them. I wish I could place them horizontally at the same level and try but my editor wouldn't allow

          Show
          Roshan Dawrani added a comment - Reduced the priority. To users who don't care about internal sorting (which cannot change), it may look too small a thing to not be working. I am giving it 2 classes with 1 main method and just can't run them - in whatever order I place them. I wish I could place them horizontally at the same level and try but my editor wouldn't allow
          Hide
          Paul King added a comment -

          Perhaps a dual monitor would allow such code to run?

          Show
          Paul King added a comment - Perhaps a dual monitor would allow such code to run?
          Hide
          Roshan Dawrani added a comment -

          Yes and some more in a circle to run @Delegate

          Show
          Roshan Dawrani added a comment - Yes and some more in a circle to run @Delegate
          Hide
          Jochen Theodorou added a comment -

          well, all I am trying to say is that a solution must be parallel to sorting, since the sorting thing is internal stuff for the transforms and should not leak outside of that. This still is clearly a bug and needs to be fixed. But the solution has to be different than fixing sorting for this and breaking sorting for a lot of other things.

          Show
          Jochen Theodorou added a comment - well, all I am trying to say is that a solution must be parallel to sorting, since the sorting thing is internal stuff for the transforms and should not leak outside of that. This still is clearly a bug and needs to be fixed. But the solution has to be different than fixing sorting for this and breaking sorting for a lot of other things.
          Hide
          Roshan Dawrani added a comment -

          Correct, correct - that point was taken long back.

          Actually I never pointed to sorting as something that should change. That was just pointed as the origin of the current issue.

          Show
          Roshan Dawrani added a comment - Correct, correct - that point was taken long back. Actually I never pointed to sorting as something that should change. That was just pointed as the origin of the current issue.
          Hide
          Roshan Dawrani added a comment -

          Will it make sense to use an annotation to explicitly mark a class as the main executable class of a script file(module)?

          So, it can be like:

          1) If a module has some classes + a script - then script is the executable class, irrespective of the new annotation.

          2) If module has some classes + 1 of them having the new ann - then that gets executed by coming in front - the ann overrides the usual sorting here.

          3) If module has some classes + none of them having the new ann - the usual logic applies.

          Show
          Roshan Dawrani added a comment - Will it make sense to use an annotation to explicitly mark a class as the main executable class of a script file(module)? So, it can be like: 1) If a module has some classes + a script - then script is the executable class, irrespective of the new annotation. 2) If module has some classes + 1 of them having the new ann - then that gets executed by coming in front - the ann overrides the usual sorting here. 3) If module has some classes + none of them having the new ann - the usual logic applies.
          Hide
          Paul King added a comment -

          So you are talking about a user supplied annotation rather than an 'internal system' annotation, e.g. for the first example, the compiler could internally add a @GroovyMain onto class B since it was originally first in the file. Then it could find it later even if sorting changed the order. I guess an annotation is just one way to remember that fact. And I suspect this isn't what you meant but just thinking through all possibilities ...

          Show
          Paul King added a comment - So you are talking about a user supplied annotation rather than an 'internal system' annotation, e.g. for the first example, the compiler could internally add a @GroovyMain onto class B since it was originally first in the file. Then it could find it later even if sorting changed the order. I guess an annotation is just one way to remember that fact. And I suspect this isn't what you meant but just thinking through all possibilities ...
          Hide
          Roshan Dawrani added a comment - - edited

          A groovy defined annotation @GroovyMain that is not automatically added by groovy compiler, but explicitly by user. That way even if you write as below, you are fine

          class A {}
          @GroovyMain
          class B extends A {
             static void main(args){}
          }
          

          In the example here, @GroovyMain makes B as the class to be executed although the usual sorting says it should be A.

          Show
          Roshan Dawrani added a comment - - edited A groovy defined annotation @GroovyMain that is not automatically added by groovy compiler, but explicitly by user. That way even if you write as below, you are fine class A {} @GroovyMain class B extends A { static void main(args){} } In the example here, @GroovyMain makes B as the class to be executed although the usual sorting says it should be A.
          Hide
          Jochen Theodorou added a comment -

          I think we should simply save the first class (beware scripts! they are included in that) and save it as being first. Then we should add a method that allows anything using hte CompilationUnit to get this first class. This way we are independend of the sorting output and can say the output of the classes given by CompilationUnit is undefined, and this method should instead be used.

          Show
          Jochen Theodorou added a comment - I think we should simply save the first class (beware scripts! they are included in that) and save it as being first. Then we should add a method that allows anything using hte CompilationUnit to get this first class. This way we are independend of the sorting output and can say the output of the classes given by CompilationUnit is undefined, and this method should instead be used.
          Hide
          Roshan Dawrani added a comment -

          Attaching a patch for review. Fixes the issue by executing the first class in the file even in case of inheritance.

          Show
          Roshan Dawrani added a comment - Attaching a patch for review. Fixes the issue by executing the first class in the file even in case of inheritance.
          Hide
          Roshan Dawrani added a comment -

          Fixed.

          Show
          Roshan Dawrani added a comment - Fixed.

            People

            • Assignee:
              Roshan Dawrani
              Reporter:
              Roshan Dawrani
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development