GRECLIPSE
  1. GRECLIPSE
  2. GRECLIPSE-929

Organize imports removes package and static imports

    Details

    • Type: Bug Bug
    • Status: Resolved Resolved
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.1.1RC1
    • Fix Version/s: 2.1.1Release
    • Component/s: Editor
    • Labels:
      None
    • Environment:
      Groovy-Eclipse plugin
      Version: 2.1.1.xx-20101209-2200-e36
      Groovy version: org.codehaus.groovy
    • Number of attachments :
      0

      Description

      When I press CTRL+SHIFT+O the 1st time, line 1 ("package some.pckg.structure") is removed.
      When I press CTRL+SHIFT+O the 2nd time, line 3 ("import static java.lang.String.*") is removed.

      This problem also occurs if the println statement is used in a script without any class.

      package some.pckg.structure
      
      import static java.lang.String.*
      
      class OrganizeImports {
      
          public static void main(String[] args) {
              println format('!!! %s !!!', 'bug :-(')
          }
      }
      

        Activity

        Hide
        Andrew Eisenberg added a comment -

        Must be related to some of the new organize import support.

        Show
        Andrew Eisenberg added a comment - Must be related to some of the new organize import support.
        Andrew Eisenberg made changes -
        Field Original Value New Value
        Assignee Andrew Eisenberg [ werdna ]
        Fix Version/s 2.1.1Release [ 16772 ]
        Priority Major [ 3 ] Critical [ 2 ]
        Andrew Eisenberg made changes -
        Affects Version/s 2.1.1RC1 [ 17002 ]
        Affects Version/s 2.1.1Release [ 16772 ]
        Hide
        Andrew Eisenberg added a comment -

        The new work in this area has uncovered some problems with source locations of import statements. Essentially, static on demand imports do not have a source location provided by the antlr parser (or more precisely, the source location is applied to the import name, rather than the whole import statement.

        Show
        Andrew Eisenberg added a comment - The new work in this area has uncovered some problems with source locations of import statements. Essentially, static on demand imports do not have a source location provided by the antlr parser (or more precisely, the source location is applied to the import name, rather than the whole import statement.
        Hide
        Andrew Eisenberg added a comment -

        OK. I have fixed the first problem where the sloc of the static on demand import is invalid. It is now correctly being calculated by the antlr parser.

        The next problem is that we are not properly recording the imports in the ImportRewrite and so the object thinks that they should be deleted (since we haven't recorded them).

        In the long term, we should properly walk the AST and look for references to the import (and remove if there really are no more references), but for now, we can just ignore them and always keep them around if they exist.

        Show
        Andrew Eisenberg added a comment - OK. I have fixed the first problem where the sloc of the static on demand import is invalid. It is now correctly being calculated by the antlr parser. The next problem is that we are not properly recording the imports in the ImportRewrite and so the object thinks that they should be deleted (since we haven't recorded them). In the long term, we should properly walk the AST and look for references to the import (and remove if there really are no more references), but for now, we can just ignore them and always keep them around if they exist.
        Hide
        Andrew Eisenberg added a comment -

        Getting there...static imports and static on-demand imports are now never removed from the import container. I'll raise a bug for this because this behavior is not ideal, but it is reasonable for now.

        Show
        Andrew Eisenberg added a comment - Getting there...static imports and static on-demand imports are now never removed from the import container. I'll raise a bug for this because this behavior is not ideal, but it is reasonable for now.
        Hide
        Andrew Eisenberg added a comment -

        I'm considering also ensuring that non-static on demand imports are never removed either. Sometimes it seems like they are incorrectly removed and nothing is added in its place. The proper behavior would be to remove them and add the appropriate regular imports. The ImportRewriter will automatically convert them to on-demand if the threshold has been reached.

        Show
        Andrew Eisenberg added a comment - I'm considering also ensuring that non-static on demand imports are never removed either. Sometimes it seems like they are incorrectly removed and nothing is added in its place. The proper behavior would be to remove them and add the appropriate regular imports. The ImportRewriter will automatically convert them to on-demand if the threshold has been reached.
        Hide
        Andrew Eisenberg added a comment -

        OK. fixed now with regression tests. I'll probably make an RC2 for this bug.

        Show
        Andrew Eisenberg added a comment - OK. fixed now with regression tests. I'll probably make an RC2 for this bug.
        Andrew Eisenberg made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Andrew Eisenberg added a comment -

        See GRECLIPSE-930 for the remaining work on this.

        Show
        Andrew Eisenberg added a comment - See GRECLIPSE-930 for the remaining work on this.

          People

          • Assignee:
            Andrew Eisenberg
            Reporter:
            Rene Scheibe
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: