Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 328400

Summary: TextEdit computed incorrectly for inserting annotation before package declaration
Product: [Eclipse Project] JDT Reporter: Paul Fullbright <paul.fullbright>
Component: CoreAssignee: Olivier Thomann <Olivier_Thomann>
Status: VERIFIED FIXED QA Contact:
Severity: major    
Priority: P3 CC: brian.vosburgh, daniel_megert, jarthana, karenfbutzke, neil.hauge, Olivier_Thomann, satyam.kandula
Version: 3.6.1Flags: daniel_megert: pmc_approved+
Target Milestone: 3.6.2   
Hardware: PC   
OS: Windows Vista   
Whiteboard:
Attachments:
Description Flags
Proposed fix + regression tests none

Description Paul Fullbright CLA 2010-10-21 15:36:29 EDT 
- start with CompilationUnit (  ... package foo; ...")
- create a new MarkerAnnotation using ast.newMarkerAnnotation() 
- set the type name( "javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter" in this case )
- add the annotation using PackageDeclaration.getAnnotations().add( <the marker annotation> )
- then generate TextEdits using astRoot.rewrite(doc, compilationUnit.getJavaProject().getOptions(true));

When applying the TextEdits using edits.apply(doc, TextEdit.UPDATE_REGIONS) the resulting document is written ( ... @javax.xml.bind.annotation.adapters.XmlJavaTypeAdapterpackage foo; ...)
That is, there is no space inserted between the annotation and the package declaration in the way that it is for classes, fields, or methods.
Comment 1 Paul Fullbright CLA 2010-10-22 11:53:08 EDT 
I'm bumping the severity of this.  It prevents our ability to add annotations programmaticaly to package declarations, as the package declaration is unparseable afterward.
Comment 2 Olivier Thomann CLA 2010-10-22 13:43:46 EDT 
Could you please provide a small test case that shows the problem so I can see how you are using the ASTRewrite?
Thanks.
Comment 3 Olivier Thomann CLA 2010-10-22 13:50:29 EDT 
Ok, I reproduced the issue.
Investigating.
Comment 4 Olivier Thomann CLA 2010-10-22 14:30:32 EDT 
Created attachment 181528 [details]
Proposed fix + regression tests
Comment 5 Olivier Thomann CLA 2010-10-22 16:12:21 EDT 
Released in HEAD.
Added regression tests in:
org.eclipse.jdt.core.tests.rewrite.modifying.ASTRewritingModifyingInsertTest#test0016
org.eclipse.jdt.core.tests.rewrite.describing.ASTRewritingPackageDeclTest
Comment 6 Olivier Thomann CLA 2010-10-22 16:12:52 EDT 
Daniel,

I think this should be backported to 3.6.2.
Comment 7 Paul Fullbright CLA 2010-10-22 16:24:08 EDT 
Fantastic!  Thanks for the quick turnaround!
Comment 8 Olivier Thomann CLA 2010-10-22 17:51:03 EDT 
Could you please check that the fix is working for you ?
Comment 9 Dani Megert CLA 2010-10-25 06:04:23 EDT 
Paul, please confirm that the fix works for you.

Olivier, shouldn't we add an else branch where we call rewriteModifiers(...)? At least the other *Declaration visit methods do so.
Comment 10 Olivier Thomann CLA 2010-10-25 08:49:14 EDT 
(In reply to comment #9)
> Olivier, shouldn't we add an else branch where we call rewriteModifiers(...)?
> At least the other *Declaration visit methods do so.
I don't think so as the other calls also needs to handle the old getModifiers() call.
Package declaration's annotations are only available in JLS3 so there is no need to try to handle the call if the JLS level < JLS3.
Comment 11 Paul Fullbright CLA 2010-10-25 10:48:00 EDT 
(In reply to comment #8)
> Could you please check that the fix is working for you ?

Yes it is.  In fact, there is also a return/new line after the new annotation, which is a very nice plus.  Thanks.
Comment 12 Dani Megert CLA 2010-10-25 11:05:02 EDT 
> Package declaration's annotations are only available in JLS3 so there is no
> need to try to handle the call if the JLS level < JLS3.
True.

+1 for 3.6.2.
Comment 13 Satyam Kandula CLA 2010-10-26 08:14:03 EDT 
Verified for 3.7M3 using build I20101025-0901
Not marking it verified as this has to be fixed for 3.6.2
Comment 14 Olivier Thomann CLA 2010-10-26 14:24:56 EDT 
Reopening to target 3.6.2.
Comment 15 Olivier Thomann CLA 2010-10-26 14:25:31 EDT 
(In reply to comment #11)
> Yes it is.  In fact, there is also a return/new line after the new annotation,
> which is a very nice plus.  Thanks.
It should respect the formatter settings which is to add a new line by default.
Comment 16 Olivier Thomann CLA 2010-10-26 21:00:12 EDT 
Released for 3.6.2.
Same patch was applied on 3.6 maintenance stream.
Comment 17 Jay Arthanareeswaran CLA 2011-01-20 05:17:52 EST 
Verified for 3.6.2 using build M20110119-0834