Community
Participate
Working Groups
Created attachment 224287 [details] Proposed Patch Modified Patch to suit the changes made while reworking Bug 391898
Created attachment 226900 [details] Proposed Patch Patch updated to the latest source and additional tests added.
ASTRewriteAnalyzer: - rewriteVarargsAnnotations(..) doesn't need the API level check. - Don't create a third copy of rewriteModifiers2(..). Merge rewriteTypeAnnotations(..) and rewriteVarargsAnnotations(..) functionality into rewriteModifiers2(..). Moving lastChild up like in rewriteTypeAnnotations(..) makes sense. - visit(SingleVariableDeclaration): - computes ellipsisEnd twice - node.varargsAnnotations().toString().length() is not correct ASTRewriteFormatter: - VARARGS_ANNOTATION_SEPARATION doesn't look correct nor necessary. PARAM_ANNOTATION_SEPARATION is already broken, since the first string contains a syntax error (missing parameter type). I'll fix that in BETA_JAVA8. - If we need a new FormattingPrefix, then it would be one that measures the space between type/annotation and "...". But note that the formatter currently doesn't work for Java 8, see bug 400830. The default formatting option for whitespace in front of "..." is "disabled", so there shouldn't be any space there in the tests.
Created attachment 227309 [details] Proposed Patch Comments addressed with the notes below: - rewriteTypeAnnotations() and rewriteVarargsAnnotations() have been made as wrapper functions for rewriteModifiers2() and call sites have not been changed to enhance readability. Can modify the call sites as well if deemed better. - Formatter changes : using the existing formatter without defining a new formatter. need to be reviewed closely.
The merging of ASTRewriteAnalyzer#rewriteModifiers2(..) looked good. The tests had a nice selection of rewrites, but showed a few remaining problems: - foo12 was not used. Added another variant for that one. - Handling of spaces was not addressed. I know this is a tricky business, and it also took me a while to understand how things work. When I was done investigating, I already had the fix together, so I just released everything at once. As you can see, I also added a variant in testVarArgs_since_3 to test the "space before ellipsis" formatter option. This was already missing in 3.8, but it became more apparent now. Fixed with http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=6cb1a46319677820636b0fbe76d096b92fe1835a Manoj, please have a look at my changes and speak up if you think something's weird.
