Summary: | [null] avoid code pattern that unnecessarily signal potential null in JDT/Core code | ||||||
---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Stephan Herrmann <stephan.herrmann> | ||||
Component: | Core | Assignee: | JDT-Core-Inbox <jdt-core-inbox> | ||||
Status: | CLOSED WONTFIX | QA Contact: | |||||
Severity: | enhancement | ||||||
Priority: | P3 | CC: | amj87.iitr, daniel_megert, gautier.desaintmartinlacaze, jarthana | ||||
Version: | 3.7 | ||||||
Target Milestone: | --- | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
See Also: |
https://git.eclipse.org/r/131529 https://git.eclipse.org/r/131530 https://git.eclipse.org/r/c/jdt/eclipse.jdt.core/+/131529 |
||||||
Whiteboard: | |||||||
Attachments: |
|
Description
Stephan Herrmann
2011-01-23 11:03:47 EST
See bug 127575 for another reason that makes it hard to enable the preference (speaking for JDT UI here). Created attachment 189975 [details]
two samples
Here are two samples of what I'm talking about:
Inside ConstructorDeclaration it's the pure form of the pattern.
In class Main a little more hand-crafting is needed.
The same can be said for fields as well. Turning on the option to include fields will show up similar warnings. I see two action items: 1. clean up our code so it can be understood by our null analysis (this bug) 2. make tutorials of some kind disseminating to users what we learned from (1) Getting experience with this kind of cleanup is getting more and more relevant. I agree that in terms of efficiency of the code, we can find fault with both patterns mentioned in comment #0. However, what is worrying me is the 362 warnings. To top it, we don't have a good explanation for the user as to why he should change the code. Too much on my plate for 4.6. Bulk deferral to 4.7 Ran out of time for 4.7. Bulk move to 4.8. New Gerrit change created: https://git.eclipse.org/r/131529 New Gerrit change created: https://git.eclipse.org/r/131530 Maybe it is necessary to repeat the purpose of this bug: The idea is not to "fix" actual potential bugs, but to help the compiler so it doesn't report potential null references. There are some strategies besides Stephanâs suggestion in comment #0: - Early param null checks if(array==null) return; - Map null arguments to empty, preallocated instances static final Type[] EMPTY_TYPE_ARRAY={}; if(array==null) array=EMPTY_TYPE_ARRAY; - Expect params to be null-null by changing invocations: Never start with null in the first place, but use preallocated empty arrays (NOTE: only for internal methods possilbe, not for existing APIs). (It would be good if we could already add annotations to document the expectations, even if annotation based null analysis is still of by default) - add asserts (doesnt work in stephans example, but in similar situations) if(length != 0) { assert array != null; ... } So sometimes the code may get a bit more ugly by these added checks. But note that once arrays are guaranteed to be non-null, for loops can often be replaced by enhanced loops, which actually increases readability. (In reply to Till Brychcy from comment #11) +1 I do support the provided patches but suggest to keep them back until we are branch-free in JDT (i.e., right after merging BETA_JAVA_12 into master). |