Community
Participate
Working Groups
As bug 186342 is converging towards release, now would be a good time to make the new configuration options available in the UI. Please see the new preference constants in JavaCore inside attachment 207270 [details] Basically we'd need: [x] Enable annotation based null analysis FQN of the annotation type to be used for marking nullable types _____ FQN of the annotation type to be used for marking non-null types _____ FQN of the annotation type to be used for making non-null the default ____ [x] Globally make non-null the default Reporting: Violation of null specification [Error/Warning/Ignore] Violation of null specifications with potential null value [Error/Warning/Ignore] Insufficient information for analysing adherence to null specifications [Error/Warning/Ignore] Redundant null annotations [Error/Warning/Ignore] Obviously all these preferences depend on the first ("Enable ..."). Please consider the wording just as initial drafts. Let me know if anything is unclear.
Oops, I just re-discovered my own bug 334455. Feel free to choose which one to close as dup of the other.
*** Bug 334455 has been marked as a duplicate of this bug. ***
> [x] Enable annotation based null analysis > FQN of the annotation type to be used for marking nullable types _____ > FQN of the annotation type to be used for marking non-null types _____ > FQN of the annotation type to be used for making non-null the default ____ These options should deny the user from specifying a simple name
These new preferences belong in the 'Potential Programming Problems' group. However, to avoid increasing the entropy further in this group (see bug 335249) we might want to create a new group 'Null Analysis' and add the new options to this new group and also move the 2 existing options here.
(In reply to comment #4) > These new preferences belong in the 'Potential Programming Problems' group. > However, to avoid increasing the entropy further in this group (see bug 335249) > we might want to create a new group 'Null Analysis' and add the new options to > this new group Sounds good to me. Would this mean the problems' categoryID should be changed, too? I see some correspondence but I can't easily find many code locations that actually use the categoryID, with two exceptions: CAT_BUILDPATH is queried in a few locations and several tests do check the category. Are categoryIDs supposed to exactly match the structure in the UI? > and also move the 2 existing options here. I see 3 configurable warnings plus "Include 'assert' in null analysis" They'd all go to the new group, right?
(In reply to comment #5) > Would this mean the problems' categoryID should be changed, too? > I see some correspondence but I can't easily find many code locations that > actually use the categoryID, with two exceptions: CAT_BUILDPATH is queried > in a few locations and several tests do check the category. > Are categoryIDs supposed to exactly match the structure in the UI? Doh! I always forget 'Problems View > Group by > Java Problem Type'. The categoryIDs are used for this grouping, and I think it is best to have a one to one correspondence between the preference page and the problems view. Hence the categoryIDs would have to be changed. Markus, your call on creating a new group. > > and also move the 2 existing options here. > > I see 3 configurable warnings plus "Include 'assert' in null analysis" > They'd all go to the new group, right? Yup, I would move this too. The current placement of this option does look a bit odd to me.
Update according to bug 186342 comment 194: (In reply to comment #0) > Reporting: > Violation of null specification [Error/Warning/Ignore] this option is changed to Violation of null specification [Error/Warning]
Is this in plan for M4 ? While the more elaborate support in the form of quick fixes and such can come later, it would be great if basic enablement of the underlying capability is there in M4.
> Is this in plan for M4 ? Yes, I'm working on this. Sorry, forgot to set the milestone.
Pushed a first version, see commit 2d2440c7f27a364ebf0a9e6d074280fa6c54ff45. The editable annotation text fields need more work: - better labels - button to reset defaults - Browse... button - syntax validation - maybe type resolution to show a warning if the annotation is not (yet) resolvable -- but a missing type should not be blocking Maybe we should even move this into a separate dialog and just show a checkbox with a link: [x] Use default org.eclipse.jdt.annotation.* annotations (Configure...) When the user clicks the link or tries to deselect the checkbox, we open a dialog that can contain more explanatory text.
(In reply to comment #10) > The editable annotation text fields need more work: > - better labels > - button to reset defaults > - Browse... button > - syntax validation > - maybe type resolution to show a warning if the annotation is not (yet) > resolvable -- but a missing type should not be blocking > > Maybe we should even move this into a separate dialog and just show a checkbox > with a link: - I think the text fields looks OK, and I would avoid creating another dialog. (Length of the preference page is certainly no longer an issue given that we have a filter box on top). - Do we really need a browse button? Can't we simply have content assist in the text field like the 'Superclass' field in the 'New Java Class' Wizard? > node= fFilteredPrefTree.addCheckBox(inner, label, PREF_ANNOTATION_NULL_ANALYSIS, enabledDisabled, defaultIndent, section, false); The last argument should be 'true' so that while filtering all the preferences under 'Enable annotation-based null analysis' are shown if anyone of them matches the filter text. Also, the 3 checkbox preferences look a bit ugly together. I would move 'Include assert...' below 'Enable annotation-based null analysis' block and 'Use non-null by default' below the three text fields.
(In reply to comment #6) > Doh! I always forget 'Problems View > Group by > Java Problem Type'. The > categoryIDs are used for this grouping, and I think it is best to have a one to > one correspondence between the preference page and the problems view. Hence the > categoryIDs would have to be changed. Markus, your call on creating a new > group. Markus, we also need new categoryID for the new 'Null Analysis Group'.
Content assist is not easy to add, since there's no project available in the workspace preference page. I just added some more description, a link to reset the default annotations, and I reshuffled the options a bit. 'Include assert...' as last option didn't look good. I'm not yet sure if we really need a new CategorizedProblem category. Yes, the categories roughly correspond to the option groups, but it's not a 1:1 mapping. We can do this later if we find it would really be helpful. Fixed with commit 018e4d6c13b846005b7fda362b74dc53fe789c73.
Looks good now, and the explanation does make things clear :)
(In reply to comment #3) > > [x] Enable annotation based null analysis > > FQN of the annotation type to be used for marking nullable types _____ > > FQN of the annotation type to be used for marking non-null types _____ > > FQN of the annotation type to be used for making non-null the default ____ > > These options should deny the user from specifying a simple name I think this still needs to be handled, because currently I can set a simple name in the text fields. Otherwise it looks good. Thanks a ton! :)
Stephan, please test drive this using I20111202-0800 or later and record your observations. TIA.
(In reply to comment #16) > Stephan, please test drive this using I20111202-0800 or later and > record your observations. TIA. Finally I have a build with the latest version (and we know why we didn't have a successful build earlier :-/ ). The new preference section looks very good to me and works as expected. A heads-up for the documentation of these options: One of the options has a context-sensitive label, depending on how the page is opened: [ ] use non-null as project-wide default [ ] use non-null as workspace-wide default I don't think this is reflected correctly, yet. (In reply to comment #15) > (In reply to comment #3) > > > [x] Enable annotation based null analysis > > > FQN of the annotation type to be used for marking nullable types _____ > > > FQN of the annotation type to be used for marking non-null types _____ > > > FQN of the annotation type to be used for making non-null the default ____ > > > > These options should deny the user from specifying a simple name > > I think this still needs to be handled, because currently I can set a simple > name in the text fields. More generally: in the core we decided to not perform any lookup of configured annotation types before they are actually used in the code. Since the UI doesn't do any validation either (I understand this would only work on the project level) users can now break null-analysis by a simple typo in the preferences without being alerted. Maybe we should rethink responsibility for a desirable validation here. Unqualified names are really just a special case that was relevant for the implementation at some point but doesn't require special treatment any longer. The problem is: user specifies in the preferences: org.mycomp.Nonnull and writes code using @org.mycomp.NonNull and still gets no errors/warnings. > Otherwise it looks good. Thanks a ton! :) from me, too!
(In reply to comment #17) > The new preference section looks very good to me and works as expected. > > A heads-up for the documentation of these options: > One of the options has a context-sensitive label, depending on how the page > is opened: > [ ] use non-null as project-wide default > [ ] use non-null as workspace-wide default > > I don't think this is reflected correctly, yet. Verified the options using 3.8 build I20111205-1800. The 4.2 build I20111205-1810 does not have the latest changes as it most likely picked up a slightly older 3.8 build. Stephan, maybe you used this?
(In reply to comment #18) > > The 4.2 build I20111205-1810 does not have the latest changes as it most likely > picked up a slightly older 3.8 build. Stephan, maybe you used this? 4.2 I20111205-2330 will pick up 3.8 I20111205-1800 PW
(In reply to comment #18) > (In reply to comment #17) > > The new preference section looks very good to me and works as expected. > > > > A heads-up for the documentation of these options: > > One of the options has a context-sensitive label, depending on how the page > > is opened: > > [ ] use non-null as project-wide default > > [ ] use non-null as workspace-wide default > > > > I don't think this is reflected correctly, yet. > > Verified the options using 3.8 build I20111205-1800. > > The 4.2 build I20111205-1810 does not have the latest changes as it most likely > picked up a slightly older 3.8 build. Stephan, maybe you used this? I meant to say: "I don't think this is correctly reflected in the documentation, yet." (cf. bug 364820 comment 24, second bullet) Implementation looks good.
(In reply to comment #17) > Maybe we should rethink responsibility for a desirable validation here. > Unqualified names are really just a special case that was relevant for the > implementation at some point but doesn't require special treatment any longer. > The problem is: user specifies in the preferences: org.mycomp.Nonnull and > writes code using @org.mycomp.NonNull and still gets no errors/warnings. Stephan, can you please include this as well in the follow up bugs? Thanks!
> > The problem is: user specifies in the preferences: org.mycomp.Nonnull and > > writes code using @org.mycomp.NonNull and still gets no errors/warnings. > Stephan, can you please include this as well in the follow up bugs? Thanks! I filed bug 365775 for syntax validation in the dialog. For unresolved annotation types, I would expect a compile error on each affected project (only if the main option is enabled).
(In reply to comment #22) > > > The problem is: user specifies in the preferences: org.mycomp.Nonnull and > > > writes code using @org.mycomp.NonNull and still gets no errors/warnings. > > Stephan, can you please include this as well in the follow up bugs? Thanks! > > I filed bug 365775 for syntax validation in the dialog. Thanks. > For unresolved annotation types, I would expect a compile error on each > affected project (only if the main option is enabled). That's what we did, until we started to optimize by avoiding to touch the annotation types before referenced. See bug 365764 for follow-up.
