Organize imports: moves package statement after some imports

Can you attach a sample? There's not much I can do without this.

Hi, this is presumably a very related test-case...

1. Create the groovy class foo.Greclipse1107 (as shown below)
2. Create a simple groovy class foo.bar.Blah
3. In the file Greclipse1107.groovy, place the cursor after Blah
4. Do cntrl-space to automatically add the import

The line "import foo.bar.Blah;" is added at the top of the file, before the package statement.

Also note that in this test case doing "organize imports" doesn't add an import for Blah at all.

package foo

class Greclipse1107 {

    public void foo() {
        Blah blah = this
        do {
        } while (blah != null)
       
        return null
    }
}

Thanks for the test case. The reason why you are seeing this is because the do...while construct is not a valid construct in Groovy. When this construct exists, it breaks the parser. So, when you do content assist, the content assistant can determine that foo.bar.Blah is a missing type, but it doesn't know where to put it, so it gets placed at the top.

If you fix the syntax errors in the code, then content assist works properly.

That's not to say that this isn't a bug in Groovy-Eclipse (it is). Groovy-Eclipse needs to either:

1. Avoid proposing anything if there is a syntax error like this.
2. Or, figure out that the import proposal should be placed after the package statement.

Thanks Andrew,

I'd definitely prefer option 2, putting the import after the package statement (and always ensuring the import is only added if it does not already exist).

Certainly when working with Java code in the IDE, I organize imports frequently. If I have errors when making changes to a class, I almost always hit ctrl-shift-o to get rid of any errors caused by missing imports, and then I look at fixing other errors such as incorrect constructs. I presume a lot of other developers are trigger-happy with hitting ctrl-shift-o a lot as well.

Digging a little deeper into the problem it looks like in this case the syntax error is causing the package statement and any existing imports to be removed from the Antlr AST. This means that the root cause is coming from the error recovery in the grammar.

This is not a piece that I have built myself, so I am assigning this to Andy.

added more recovery, will now cope with code busted like this and not discard the earlier stuff that was successfully parsed. Not fixed in 1.7, only in 1.8.