Community
Participate
Working Groups
It would be very useful to have some more options for formatting code with annotations (see windows->preferences->CodeStyle->Formatter). Right now, it seems like the only formatting option for annotations is on the "New Lines" tab ("Annotations"->"Insert new line after annotations"). I think this setting is geared towards class and method level annotations. My problem is that if I annotate parameters of a method, things start looking really ugly. With the current "new line" option, I have the following two choices: [with newline = true, notice everything gets left justified.] @SomeAnnotation(arg1=foo, arg2=bar) public void myMethod( @ParamAnnotation(pArg=foo) Object a, @ParamAnnotation(pArg=bar) Object b ) [with newline = false, the params are tabbed over but the first line of the method is unreadable.] @SomeAnnotation(arg1=foo, arg2=bar) public void myMethod( ....@ParamAnnotation(pArg=foo) Object a, ....@ParamAnnotation(pArg=bar)Object b ) I'd like to see something like this: @SomeAnnotation(arg1=foo, arg2=bar) public void myMethod( ....@ParamAnnotation(pArg=foo) Object a, ....@ParamAnnotation(pArg=bar)Object b ) I might even be able to live with something like this: @SomeAnnotation(arg1=foo, arg2=bar) public void myMethod( ....@ParamAnnotation(pArg=foo) ....Object a, ....@ParamAnnotation(pArg=bar) ....Object b ) I think for now, I would be happy with something like "make a newline unless it is an annotation on a param" or perhaps adding something to the line wrapping section that said always put params on separate lines if they are annotated. As always, with code formatting, it's a very personal thing. I'm sure I haven't thought of all of the ways people would want to control the formatting of param annotations.
My suggestion/requirements are simply stated: The existing "new line" option *on* for class- and method-level annotations The existing "new line" option *off* for parameter annotations Would it be possible to just have two options? 1) newline on/off for class- and method-level annotations 2) newline on/off for parameter (inline?) annotations That would work for me. Then the rest of the formatter's options can hopefully cope with the other requirements mentioned by the original issue reporter - wrapping method params and so forth The results I am after are: @Override public void update( @SuppressWarnings( "unused" ) Context context ) ...
*** Bug 134625 has been marked as a duplicate of this bug. ***
*** Bug 156980 has been marked as a duplicate of this bug. ***
*** Bug 181547 has been marked as a duplicate of this bug. ***
Just another two cents: It would be nice if parameter annotations could be formatted depending on whether or not they're simple marker-style annotations (with no parens). For example: void meth(@Required Object param) { versus void meth( @Range(min = 0, max = 65535) int param) {
Created attachment 75636 [details] a quick patch for a seperate new line behavior
The attached patch is 3.4M4 compliant and works well. It's a huge improvement compared to default behavior that pretty much renders formating source code with parameter annotations useless.
Eric, please review.
Created attachment 87756 [details] [proposed patch + test case] on top v833 - all jdt.core tests OK Thanks for providing a patch. Please note that contributed patch should contain the necessary regression tests corresponding to the fix. Also be aware that such patch should, at a minimum, pass all jdt.core tests (one test case was failing with the provided patch), and for bugs (such as this one) that may impact other components (ui, text), we also run these components regression tests. This new patch contain an improved fix (in order to prevent the new preference to override the already existing one (Insert new line after annotation) and corresponding test cases. This new patch passes all jdt.core, ui and text tests. Olivier, may please have a look? - thanks.
Looks good to me. A new bug report against JDT/UI needs to be added for the UI side of this patch once the core side is released. So the patch should be split into the changes required in JDT/Core and JDT/Core tests + the changes required in JDT/UI.
(In reply to comment #10) thanks Olivier. Corresponding UI bug 216541.
Patch released for 3.4M5 in HEAD.
re-open to add a new patch
Created attachment 88022 [details] [patch] - proposed correction better fix to match expected behavior as specified in bug 216541.
Sorry, I have to reopen. Javadoc says that the new options default is INSERT, but it is not, the default is DO_NOT_INSERT (DefaultCodeFormatterOptions.setDefaultSettings()). I also think we need to update existing formatter profiles, there value for the new option must be equal to the value of FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION.
I just found the same problem with the default value. Also we should modify existing profiles only if they don't define the new constants. I don't like the fact that the constant value doesn't match the purpose of the constant. We either change the doc or we change the constant value.
To summarize, people want to insert a new line (or not) after an annotation on: 1. a class, a method, or a field 2. a method parameter However nobody talked about annotations on: 3. a local variable Right now the existing option is FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION and it implements 1 and 3 The original request is to add a separate option for 2. Tentative name is FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_PARAMETER. However the name of the existing option doesn't make sense any longer. We can deprecate it. And replace it with something like: FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_MEMBER. This name covers 1. But does it make sense to have FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_MEMBER for 3 as well? I would say no. Maybe we need a third option: FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_LOCAL_VARIABLE. What do people think? We need to clarify this issue before committing any change to CVS.
(In reply to comment #18) > But does it make sense to have > FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_MEMBER for 3 as well? I would say > no. Maybe we need a third option: > FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_LOCAL_VARIABLE. > > What do people think? We need to clarify this issue before committing any > change to CVS. I agree, it would be great to have a third option. The UI would then change from Current: [x] Insert new line after annotations to New: [x] Insert new line after annotations on members [x] Insert new line after annotations on parameters [x] Insert new line after annotations on local variables All 3 options enabled per default?
I would vote more for: FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_BODY_DECLARATION We have to consider that annotations on local variables are rare if we consider that there is no way to persist them in the class file for now. So should we really worry about them ? If yes, then the three options make sense. I won't be too picky on the name of the first one.
I use @SuppressWarnings("unchecked") on local variables so that I don't have to "dirty" the entire method with that annotation. In my case, I prefer to have local variable annotations on their own lines, same as method annotations, so I don't mind if those two locations use the same preference, but I could see that other people might want them to differ.
my vote goes to have the 3 locations as described in comment#18 by Jerome differentiated by 3 preferences: FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_MEMBER FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_PARAMETER FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_LOCAL_VARIABLE (All 3 preferences would be enabled by default) I believe that with 3 preferences, we'll be able to fulfill more user requirements than with only 2. Also, current preference FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION would have to be deprecated.
Nobody asked for a separate setting for local variables, so I don't think we have a justification for adding more clutter. Furthermore, I think we have a consensus that the default should be INSERT for FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION and DO_NOT_INSERT for FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_PARAMETER, and that the current behavior is considered a bug. I would just keep this simple: Leave the old option and add the ...ON_PARAMETER option with the right default. Add "@see #...ON_PARAMETER" to the old option. The UI could look like this: [ ] Insert new line after annotations on parameters [x] Insert new line after other annotations
(In reply to comment #23) Markus, I am not a big fan of "Insert new line after other annotations", which to me does not mean much. Though we currently have no requirement for the local variable differentiation, we'll be facing the exact same discussion as soon as we'll have a new similar requirement to add a new line annotations is such or such context. My opinion is that the 3 preferences really reflect the 3 main categories of possibilities where one would like to add a new line after an annotation. A possibility could be to: for jdt.core side: do as described in comment#22 (add the 3 preferences, deprecate the existing one) for jdt.ui pref page: only surface 2 preferences > [ ] Insert new line after annotations on parameters > [x] Insert new line after other annotations (still, I don't like "other annotations" ;-)
Created attachment 88283 [details] [proposed patch + test cases] on top v834 - all jdt.core tests OK as per my last comment, this new patch: - deprecates the existing preference - add 3 new formatting preferences regarding new lines and annotations: FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_MEMBER - default INSERT FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_PARAMETER - default DO_NOT_INSERT FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_LOCAL_VARIABLE - default INSERT It will be up to the UI pref page to surface 2 or 3 prefs. If surfacing only 2 options, when selecting /de-selecting option: [x] Insert new line after other annotations this will enable / disable 2 preferences: FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_MEMBER FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_LOCAL_VARIABLE If surfacing 3 options, then each check box will trigger corresponding preference.
Created attachment 88284 [details] sample file to test new line after annotation preference
(In reply to comment #25) > It will be up to the UI pref page to surface 2 or 3 prefs. If you provide all three options in Core then I will provide all three options in the UI: - It's harder to implement not providing this option - It's harder to test your options without a UI to set them - The local variable option makes sense to me: I would only enable FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_MEMBER in my formatter profile.
I agree with Benno. The UI should surface all JDT/Core options. Can we get annotations elsewhere ? If yes, we should cover this case as well. The default values of the three options should be: FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_MEMBER - default INSERT FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_PARAMETER - default DO_NOT_INSERT FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_LOCAL_VARIABLE - default DO_NOT_INSERT If the profile passed in to the formatter only contains the old options, the old option value should be used to set the value of the FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_MEMBER option. The two others should be set using the default values above. We cannot claim to be backward compatible so this should be consistent in the "new" way to format annotations. Any comment ?
(In reply to comment#27 and comment#28) >I would only enable > FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_MEMBER in my formatter profile. > Correct: to keep existing behavior that uses the to_be_deprecated existing option FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION we should, by default, enable both FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_MEMBER and FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_LOCAL_VARIABLE As underlined by Olivier, by having only FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_MEMBER enabled by default, we would introduce a new behavior. I'm happy with a "new" way to format annotations
To keep existing behavior for people upgrading, all three new preferences should take on the value of the old, deprecated preference iff it's available. In pseudocode: if isset(_AFTER_ANNOTATION) then _AFTER_ANNOTATION_ON_MEMBER := _AFTER_ANNOTATION _AFTER_ANNOTATION_ON_PARAMETER := _AFTER_ANNOTATION _AFTER_ANNOTATION_ON_LOCAL_VARIABLE := AFTER_ANNOTATION unset _AFTER_ANNOTATION else if not isset(_AFTER_ANNOTATION_ON_MEMBER) _AFTER_ANNOTATION_ON_MEMBER := INSERT if not isset(_AFTER_ANNOTATION_ON_PARAMETER) _AFTER_ANNOTATION_ON_PARAMETER := DO_NOT_INSERT if not isset(_AFTER_ANNOTATION_ON_LOCAL_VARIABLE) _AFTER_ANNOTATION_ON_LOCAL_VARIABLE := DO_NOT_INSERT endif
(In reply to comment #28) > If the profile passed in to the formatter only contains the old options, the > old option value should be used to set the value of the > FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_MEMBER option. The two others > should be set using the default values above. I don't agree, the old option should be used to set all three values: If I said I don't want new lines, I don't want any. And if I said I want new lines, I want it everywhere. Otherwise you will alter the behavior and if i.e. people have formatting on save enabled you will generate outgoing changes without the user having changed anything in the formatter options. The user then has to go and find the new option in the formatter preference dialog, which is not an easy task.
(In reply to comment #31) The question is: should the fix be backward compatible or do we accept a new way of formatting annotations? As Benno underlined (thanks for pointing this out), people using formatting on save will encounter unexpected changes, which is not acceptable I think. Which makes me believe we should be backward compatible and forget the new behavior. To achieve this, we should activate / deactivate the new preferences (2 or 3, does not make difference) the same way the existing (old) one is currently used. - all (new) preferences activated by default - if a formatting profile was using the existing (old) preference, then use its value to set the 3 (new) preferences. Any comment?
For new users, the situation is clear: The current behavior is considered a bug. Therefore, the default (Eclipse & Java) profiles must have FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION_ON_PARAMETER set to DO_NOT_INSERT. I still think we should also fix the bug for existing users by default, but I could accept if exisiting profiles were updated so that existing users stay bug-compatible.
Summing up the discussions I had with Eric and Benno, we came to this conclusion: 1. The default profiles from core should have: ON_MEMBER --> INSERT ON_PARAM --> DO_NOT_INSERT ON_LOCAL_VAR --> INSERT => correct defaults for new profiles 2. The formatter should: if one of the new options is available --> only use the new options, fall back to defaults from above if one is not available else --> use value from deprecated option for all new options => formatter prefers new options, but supports old option (no change for existing clients) 3. JDT/UI will implement the formatter profile updating such that existing profiles will be updated to use the new options (all options copied from deprecated option) => ensures non-breaking change for existing clients
(In reply to comment #34) > Summing up the discussions I had with Eric and Benno, we came to this > conclusion: yes, +1.
Created attachment 88538 [details] [proposed patch + test cases] on top v834 As per our last discussions: when setting formatter options, process the deprecated option first, then the new options afterwards if deprecated option was not used). Passes all jdt.core, ui and text tests
(In reply to comment #36) Patch released for 3.4M5 in HEAD.
*** Bug 207330 has been marked as a duplicate of this bug. ***
(In reply to comment #18) For the record, the following statement was inexact: > Right now the existing option is FORMATTER_INSERT_NEW_LINE_AFTER_ANNOTATION and > it implements 1 and 3 It also implemented 2.
*** Bug 213080 has been marked as a duplicate of this bug. ***
*** Bug 215190 has been marked as a duplicate of this bug. ***
Verified for 3.4M5 using I20080205-0010
Hello! Just a small comment on that improvement. I've seen it working fine in the Ganymede RC3 package. However, I would propose yet a fourth "object category" in addition to the three that we already have (MEMBER, PARAMETER, LOCAL_VARIABLE): RETURN_PARAMETER. This is the reason why: when annotating the return parameter of a method, I get the following: @MyAnnotation1 public @NonNull Set getReturnValue(@NonNull String myParam1) { return this.returnValue; } I would actually rather have the following: @MyAnnotation1 public @NonNull Set getReturnValue(@NonNull String myParam1) { return this.returnValue; } I hope I am not too bold in asking something here, this is just an improvement proposal. If you don't agree with it or find it just stupid, just tell me, I won't be offended at all (this is actually the first time I used the Eclipse Bugzilla to submit something). Thank you, Nicolas.
(In reply to comment #43) Nicolas, since this bug is already FIXED and VERIFIED, I would suggest to open a new enhancement request against JDT/Core.
(In reply to comment #43) Note that the Java language up to (including) 1.6 does not support annotations on the return type. All annotations that appear before the return type of a method belong to the whole method declaration. > @MyAnnotation1 > public @NonNull Set getReturnValue(@NonNull String myParam1) { From a language point of view, this is equivalent to @NonNull @MyAnnotation1 public Set getReturnValue(@NonNull String myParam1) { If you really think a new category is necessary, please open a new enhancement request. However, I think we shouldn't do this right now. We should only add a new category if Java 1.7 introduces annotations on type references.
