GRECLIPSE

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

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: