Bug 190622

Summary: type binding marked as recovered but all is compiling
Product: [Eclipse Project] JDT Reporter: Matt Whitlock <eclipse>
Component: CoreAssignee: Olivier Thomann <Olivier_Thomann>
Status: VERIFIED FIXED QA Contact:
Severity: trivial    
Priority: P3 CC: jerome_lanneluc, martinae, Olivier_Thomann
Version: 3.3   
Target Milestone: 3.3.1   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
Proposed fix
none
Regression test none

Description Matt Whitlock CLA 2007-06-01 23:28:22 EDT
Build ID: I20070525-1350

Steps To Reproduce:

--- pkg1/Foo.java ---
package pkg1;
public class Foo {
    public @interface Moo {
        Class<?> value();
    }
    @Moo(Bar.Baz.class)
    public static class Bar {
        public static class Baz {
        }
    }
}
------

--- pkg2/Bar.java ---
package pkg2;
public class Bar {
}
------

Create a new Java Project and add those two source files to it.  Perform Organize Imports on Foo.  The "Bar" in the annotation will be highlighted and a disambiguation dialog will appear containing "pkg1.Foo.Bar" and "pkg2.Bar".  Choosing "pkg1.Foo.Bar" produces no change to the file.  Performing Organize Imports again repeats the pattern.
Comment 1 Jerome Lanneluc CLA 2007-06-02 05:38:32 EDT
Moving to JDT/UI
Comment 2 Martin Aeschlimann CLA 2007-08-22 09:30:48 EDT
The reason why organize prompts for a type is that the type binding for 'Bar' is marked as 'recovered'.
As this CU is compiling, this seems to be wrong.

When looking at the CU in the ASTView, another problem shows up: The Java element returned by the binding (binding.getJavaElement) returns a CU with 'null' as parent.
Should be a IType (or null). CU's should never return 'null' as parent, but always a package.

I marked this bug as 3.3.1, but jdt.core can decide if they prefer 3.4.
Comment 3 Olivier Thomann CLA 2007-08-24 11:18:12 EDT
Reproduced.
I am investigating.
Comment 4 Olivier Thomann CLA 2007-08-24 12:15:40 EDT
(In reply to comment #2)
> When looking at the CU in the ASTView, another problem shows up: The Java
> element returned by the binding (binding.getJavaElement) returns a CU with
> 'null' as parent.
> Should be a IType (or null). CU's should never return 'null' as parent, but
> always a package.
I believe that the java element for a recovered type binding should always be null. Returning a fake compilation unit looks boggus.

I fixed the initial problem which also fix the java element.
Comment 5 Olivier Thomann CLA 2007-08-24 12:26:48 EDT
If we don't set null for the compilation unit parent, we need to provide a package fragment and therefore we need to provide a package fragment root.
I opened bug 201104 to track this issue.
Comment 6 Olivier Thomann CLA 2007-08-24 13:16:59 EDT
Jérôme,

Candidate for 3.3.1?
Potentially all qualified name inside annotation on a type declaration cannot properly get a binding.

Released for 3.4M2.
Regression test added org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0282
Comment 7 Martin Aeschlimann CLA 2007-08-24 13:19:35 EDT
I agree that null is much better answer here. getJavaElement() for a ITypeBinding should always be an IType or null.

In our code we assume that getParent() of a ICompilationUnit is never null.
This has been true since 1.0 I believe although it hasn't been explicitly stated.
Only IJavaModel.getParent returns null.

Comment 8 Jerome Lanneluc CLA 2007-08-27 05:02:58 EDT
Olivier, please attach the corresponding patch.
Comment 9 Olivier Thomann CLA 2007-08-27 09:32:50 EDT
Created attachment 77024 [details]
Proposed fix

Sorry, I forgot the patch.
Comment 10 Olivier Thomann CLA 2007-08-27 09:33:05 EDT
Created attachment 77025 [details]
Regression test
Comment 11 Jerome Lanneluc CLA 2007-08-27 12:35:43 EDT
+1 for 3.3.1 since this is a regression comparing to 3.2 and the fix is low risk.
Comment 12 Olivier Thomann CLA 2007-08-27 15:14:56 EDT
reopen for 3.3.1
Comment 13 Olivier Thomann CLA 2007-08-27 15:20:36 EDT
Released for 3.3.1.
Regression test added
org.eclipse.jdt.core.tests.dom.ASTConverter15Test#test0282
Comment 14 Eric Jodet CLA 2007-09-03 09:23:18 EDT
Verified for 3.3.1 using build M20070831-2000