Bug 133440 - [1.5][compiler] JDT allows annotation to have a null default
Summary: [1.5][compiler] JDT allows annotation to have a null default
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.1.2   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.2 M6   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-03-27 18:01 EST by Daniel R Somerfield CLA
Modified: 2006-03-29 14:09 EST (History)
2 users (show)

See Also:

Attachments
Proposed fix (943 bytes, patch)
2006-03-27 23:18 EST, Olivier Thomann CLA 		no flags Details | Diff
Corresponding regression test (1.58 KB, patch)
2006-03-27 23:20 EST, Olivier Thomann CLA 		no flags Details | Diff
Fix for IllegalStateException (842 bytes, patch)
2006-03-29 14:01 EST, Jess Garms CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel R Somerfield CLA 2006-03-27 18:01:50 EST 
The following invalid annotation is accepted as legitimate:

public @interface MyAnnotation {
   
    enum MyEnum
    {
        VAL_1, VAL_2
    }
   
    public MyEnum theValue() default null; //<-- this should not be allowed
}

To compound the problem, if you call MethodBinding.getDefaultValue() on that method, you get an IllegalStateException.

java.lang.IllegalStateException: java.lang.Object@b3a420
at org.eclipse.jdt.core.dom.ResolvedMemberValuePair.buildDOMValue(ResolvedMemberValuePair.java:91)
at org.eclipse.jdt.core.dom.MethodBinding.getDefaultValue(MethodBinding.java:451)
Comment 1 Olivier Thomann CLA 2006-03-27 23:17:18 EST 
The null case is not checked in the enum case.
Comment 2 Olivier Thomann CLA 2006-03-27 23:18:03 EST 
Created attachment 37042 [details]
Proposed fix
Comment 3 Olivier Thomann CLA 2006-03-27 23:20:51 EST 
Created attachment 37043 [details]
Corresponding regression test
Comment 4 Philipe Mulet CLA 2006-03-28 05:46:58 EST 
Fix looks good
Comment 5 Olivier Thomann CLA 2006-03-28 09:46:26 EST 
Fixed and released in HEAD.
Regression test in org.eclipse.jdt.core.tests.compiler.regression.AnnotationTest.test196
Comment 6 Daniel R Somerfield CLA 2006-03-28 14:43:02 EST 
After this fix, will a call to getDefaultValue() on a method with an invalid null default still throw a IllegalStateException?
Comment 7 Olivier Thomann CLA 2006-03-28 14:45:58 EST 
Yes, since null should not happen.
Comment 8 Olivier Thomann CLA 2006-03-28 15:02:28 EST 
I could not get the IllegalStateException with or without the fix on a 3.2 build.
null is returned instead.
Comment 9 Daniel R Somerfield CLA 2006-03-28 15:55:57 EST 
We are still seeing the exception. I should have made this clear: I cut off the stack trace so it is:

java.lang.IllegalStateException: java.lang.Object@b3a420
at
org.eclipse.jdt.core.dom.ResolvedMemberValuePair.buildDOMValue(ResolvedMemberValuePair.java:91)
at
org.eclipse.jdt.core.dom.MethodBinding.getDefaultValue(MethodBinding.java:451)
at ...

where "..." is external (non-JDT) code calling in. I think any time code calls getDefaultValue() on an invalid null default value. This is a problem, because even though the code is not valid, it can still show up in source.

Are you seeing something different?
Comment 10 Olivier Thomann CLA 2006-03-28 21:17:59 EST 
What version are you using? I don't have the class org.eclipse.jdt.core.dom.ResolvedMemberValuePair anymore. It is now called org.eclipse.jdt.core.dom.MemberValuePairBinding.
Comment 11 Frederic Fusier CLA 2006-03-29 05:42:03 EST 
Verified for 3.2 M6 using build I20060329-0010.
Comment 12 Jess Garms CLA 2006-03-29 14:00:51 EST 
Dan was using the APT branch of jdt.core (3.1.2). Looking at the latest in HEAD, the same problem still exists: an IllegalStateException could be thrown if the user's source code is invalid.

I'll attach a proposed fix for the IllegalStateException: return null instead.
Comment 13 Jess Garms CLA 2006-03-29 14:01:18 EST 
Created attachment 37224 [details]
Fix for IllegalStateException
Comment 14 Olivier Thomann CLA 2006-03-29 14:09:27 EST 
Fixed and released in HEAD.
Comment 15 Olivier Thomann CLA 2006-03-29 14:09:51 EST 
Verified. There is nothing more to verify that what was done before.