GRECLIPSE
  1. GRECLIPSE
  2. GRECLIPSE-1219

Annotations on import statements are removed after organize imports is called

    Details

    • Type: Bug Bug
    • Status: Resolved Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.5.2.Release
    • Fix Version/s: 2.6.1.Release
    • Component/s: Editor, Formatting
    • Labels:
    • Number of attachments :
      0

      Description

      I use a workspace config that has "Organize imports" set as a save action for Java. This seems to immediately remove any @Grab lines from the begining of the groovy scripts as soon as I save them. If i put the line after my imports, I get an error instead:

      !ENTRY org.eclipse.jdt.ui 4 10006 2011-10-13 10:10:34.836
      !MESSAGE The save participant 'org.eclipse.jdt.ui.postsavelistener.cleanup' caused an exception: java.lang.NullPointerException
      !STACK 0
      java.lang.NullPointerException
      at org.codehaus.groovy.eclipse.refactoring.actions.GroovyImportsCleanUp.createFix(GroovyImportsCleanUp.java:61)
      at org.eclipse.jdt.internal.corext.fix.CleanUpRefactoring.calculateChange(CleanUpRefactoring.java:803)
      at org.codehaus.groovy.eclipse.refactoring.actions.CleanUpPostSaveListener.saved(CleanUpPostSaveListener.java:350)
      at org.codehaus.groovy.eclipse.refactoring.actions.DelegatingCleanUpPostSaveListener.saved(DelegatingCleanUpPostSaveListener.java:120)
      at org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitDocumentProvider$5.run(CompilationUnitDocumentProvider.java:1590)
      ...

        Issue Links

          Activity

          Hide
          John Bäckstrand added a comment - - edited

          Actually, the error might not be so important since I put the annotation in an invalid position. If I put it on a class after my imports everything works properly!

          This works:

          import static groovyx.net.http.ContentType.*
          import static groovyx.net.http.Method.*
          
          import java.security.*
          import java.security.interfaces.*
          
          import javax.crypto.Cipher
          import javax.xml.bind.DatatypeConverter
          
          import groovyx.net.http.*
          @Grab(group='org.codehaus.groovy.modules.http-builder', module='http-builder', version='0.5.1' )
          public class sh{}
          
          println "doh"

          Whereas this doesn't:

          @Grab(group='org.codehaus.groovy.modules.http-builder', module='http-builder', version='0.5.1' )
          import static groovyx.net.http.ContentType.*
          import static groovyx.net.http.Method.*
          
          import java.security.*
          import java.security.interfaces.*
          
          import javax.crypto.Cipher
          import javax.xml.bind.DatatypeConverter
          
          import groovyx.net.http.*
          public class sh{}
          
          println "doh"
          Show
          John Bäckstrand added a comment - - edited Actually, the error might not be so important since I put the annotation in an invalid position. If I put it on a class after my imports everything works properly! This works: import static groovyx.net.http.ContentType.* import static groovyx.net.http.Method.* import java.security.* import java.security.interfaces.* import javax.crypto.Cipher import javax.xml.bind.DatatypeConverter import groovyx.net.http.* @Grab(group='org.codehaus.groovy.modules.http-builder', module='http-builder', version='0.5.1' ) public class sh{} println "doh" Whereas this doesn't: @Grab(group='org.codehaus.groovy.modules.http-builder', module='http-builder', version='0.5.1' ) import static groovyx.net.http.ContentType.* import static groovyx.net.http.Method.* import java.security.* import java.security.interfaces.* import javax.crypto.Cipher import javax.xml.bind.DatatypeConverter import groovyx.net.http.* public class sh{} println "doh"
          Hide
          Andrew Eisenberg added a comment -

          The NPE that you see has been fixed with GRECLIPSE-1217. But, there is a new issue that you have uncovered. It looks like annotations are removed from import statements after an organize imports. I have changed the title to reflect the new problem.

          Just to be sure...after placing the annotation on the class declaration, you are no longer having any problems?

          Show
          Andrew Eisenberg added a comment - The NPE that you see has been fixed with GRECLIPSE-1217 . But, there is a new issue that you have uncovered. It looks like annotations are removed from import statements after an organize imports. I have changed the title to reflect the new problem. Just to be sure...after placing the annotation on the class declaration, you are no longer having any problems?
          Hide
          John Bäckstrand added a comment -

          Yes, that is what I am seeing.

          Show
          John Bäckstrand added a comment - Yes, that is what I am seeing.
          Hide
          Andrew Eisenberg added a comment -

          Fixed two related problems here:

          • No hovers or navigation for annotations on imports
          • No find occurrences either

          Both were easy fixes. Unfortunately, fixing organize imports is not simple. We have inherited most of the organize imports code from JDT. The JDT behavior is to strip everything except for the type name from imports after organizing.

          Show
          Andrew Eisenberg added a comment - Fixed two related problems here: No hovers or navigation for annotations on imports No find occurrences either Both were easy fixes. Unfortunately, fixing organize imports is not simple. We have inherited most of the organize imports code from JDT. The JDT behavior is to strip everything except for the type name from imports after organizing.
          Hide
          Andrew Eisenberg added a comment -

          Actually...ha! This did turn out to be easy. It turns out that there are different ways of configuring the ImportRewriter. One way keeps old imports and another way removes them. For organize imports, JDT configures it the second way as does Groovy-Eclipse. But, if Groovy-Eclipse changes to the first way, then nothing appears to break and comments are kept as expected.

          All tests are still passing and I created a few regression tests for the new functionality (including the other fixes mentioned in my previous comment).

          Show
          Andrew Eisenberg added a comment - Actually...ha! This did turn out to be easy. It turns out that there are different ways of configuring the ImportRewriter. One way keeps old imports and another way removes them. For organize imports, JDT configures it the second way as does Groovy-Eclipse. But, if Groovy-Eclipse changes to the first way, then nothing appears to break and comments are kept as expected. All tests are still passing and I created a few regression tests for the new functionality (including the other fixes mentioned in my previous comment).
          Hide
          Andrew Eisenberg added a comment -

          committed the fix.

          Show
          Andrew Eisenberg added a comment - committed the fix.

            People

            • Assignee:
              Andrew Eisenberg
              Reporter:
              John Bäckstrand
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: