Comment 1 ANIRBAN CHAKRABORTY 2013-04-19 02:53:42 EDT

Hello,
I'll be working on this.
Thanks
Anirban

Comment 2 ANIRBAN CHAKRABORTY 2013-06-09 11:40:56 EDT

Created attachment 232144 [details]
Patch for the fix

Patch for the fix

Comment 3 Srikanth Sankaran 2013-06-18 02:15:38 EDT

Here are the review comments:

1. I would remove the test testBug405250a and the associated code in
RecoveredUnit. That test is testing a totally broken scenario and I am
comfortable if we suggest some proposals that are out of line in that
scenario. Adding code to prevent illegitimate proposals in broken
scenarios is a project that would not scale well.

2. Please follow project indentation guidelines/prior usage. Space after
while and if is needed for example.

3. Is K_IMPORT_DELIMITER better named K_INSIDE_IMPORT_STATEMENT ? (like
K_INSIDE_THROW_STATEMENT ? 

4. isInImportSpecification - is it better named isInImportStatement ?

5. I don't think consumeSingleTypeImportDeclarationName is the best place
to pop K_IMPORT_DELIMITER/K_INSIDE_IMPORT_STATEMENT - there are 4 types of
import statement and only one is covered by consumeSingleTypeImportDeclarationName.

Comment 4 ANIRBAN CHAKRABORTY 2013-06-20 16:41:37 EDT

Created attachment 232622 [details]
Patch after accommodating the review comments

Patch after accommodating the review comments

Comment 5 Srikanth Sankaran 2013-06-24 04:52:40 EDT

Patch looks good. Released here: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?h=BETA_JAVA8&id=2385c0aeda6d8b742ca24edae81655117601babd.

Thanks Anirban.