Community
Participate
Working Groups
The new problem reporting support for null pointers is extremely handy and has found a number of problems that need attention. But it is also over aggressive about reporting problems that aren't actual problems. This simple example illustrates simple versions of such over aggressive problem reporting: public class Example { boolean condition() { return true; } void assignmentAndAccessGuardedBySameGuard() { Object x = null; if (condition()) { x = new Object(); } if (condition()) { System.out.println(x.hashCode()); } } void equivalentGuard(List<?> list) { Object x = null; boolean y = false; for (Object z : list) { if (z != null) { x = z; y = true; } } if (y) { System.out.println(x.hashCode()); } } void anotherEquivalentGuard(String s) { int length = s == null ? 0 : s.length(); if (length != 0) { System.out.println(s.charAt(length - 0)); } } } If problems are reported for code that's logically correct, the support for the problem reporting becomes less useful...
Ed - what exactly are you seeing which you do not like ? The idea is that with the new pref separation, definite threats are separated from potential threats. If you are complaining about false definite threats, then this is clearly an issue we need to consider. If only, a false potential threats, then we have a few scenarii which we are aware of in this space, and there is no easy fix without introducing ways of tagging the code (likely using new annotations, ala JSR305).
Philippe, I'm not sure what you mean by "new pref separation". Is there some preference control beyond M5eh that I don't see with M5eh? Suppose I wanted to treat definite threats as an error, while I wanted to ignore potential threats or perhaps treat them only as a warning. Is that possible?
It just got added since M5, you should see it in last integration build I believe.
Ed, the newest null related options were released for 3.3 M6 indeed (bug 170704 for JDT Core, bug 175496 for JDT UI), hence they are available starting with I20070228-0930. They allow for separate error levels to be set on 'Null reference', 'Potential null reference' and 'Redundant null check'. The options names in the UI may still change (see bug 175830) but the semantics should remain the same. Having said that, copying your example with comments: public class Example { boolean condition() { return true; } void assignmentAndAccessGuardedBySameGuard() { Object x = null; if (condition()) { x = new Object(); } if (condition()) { System.out.println(x.hashCode()); // a) x may be null } } void equivalentGuard(List<?> list) { Object x = null; boolean y = false; for (Object z : list) { if (z != null) { x = z; y = true; } } if (y) { System.out.println(x.hashCode()); // b) x may be null } } void anotherEquivalentGuard(String s) { int length = s == null ? 0 : s.length(); if (length != 0) { System.out.println(s.charAt(length - 0)); // c) s may be null } } } and considering each case separately: - c) is a case of variables correlation in which length controls s; while the most general cases of variables correlation is out of scope for performance reasons (the compiler cannot afford the CPU and memory that more aggressive static analysis tools devote to such problems), we have listed some patterns that we may address at some point in time (not in 3.3 though); this peculiar case is tracked by bug 125319; - b) is a variables correlation case again; it is more complex than any of the patterns we consider, because the relationship between y and x being null (or not) is typically conditioned by their presence in the same block and involves scenario playing of some sorts (or other techniques similarly complex); I do not foresee such class of analysis making its way in JDT plans; - a) is even more agressive; while the code of condition is readily available to the reader if the code is complete, a class may extend Example and get condition() return different values over time; even if we made condition final, we would need to determine that it always returns true and prune the flow of control graph accordingly; again, I fear that this is out of JDT scope for the foreseable future. As Philippe suggested above, the 'maybe null' series is far less affirmative about the runtime behavior that the 'can only be null' (resp. 'cannot be null') series of warnings. In the former context, we emit a warning whenever the code shades doubts upon the considered variable. In the latter context, we try hard to only complain on situations where we *know* the null status of a given variable, but under the constraint that we do not perform general variables correlation, scenario exploration, or inter-procedural analysis. Please do not hesitate to fill in new bugs if you find situations in which we do not fulfill those objectives.
Guys, I was surprised to get a warning about this one: public AdapterFactory getFactoryForTypes(Collection<?> types) { FactoryLoop: for (AdapterFactory factory : adapterFactories) { if (factory instanceof ComposedAdapterFactory) { factory = ((ComposedAdapterFactory)factory).getFactoryForTypes(types); if (factory != null) { return factory; } } else { for (Object type : types) { if (!factory.isFactoryForType(type)) { continue FactoryLoop; } } return factory; } } if (adapterFactoryDescriptorRegistry != null) { Descriptor descriptor = adapterFactoryDescriptorRegistry.getDescriptor(types); if (descriptor != null) { AdapterFactory result = descriptor.createAdapterFactory(); addAdapterFactory(result); return result; } } return delegatedGetFactoryForTypes(types); } The null check *after* the assignment in the "then" branch should really not have any impact on the "else" branch. Is this something that could be fixed? I'll change my code to avoid this, so it's not a really important thing for me. I'm very happy with the "for sure" problem and the "potential" problem split we have now...
Ed, seems we miss the fact that the for changes factory at each iteration. Would you please open a specific bug for that case? I would then close this one again, since it discusses the issue in a wider perspective. Simplified test case: void foo(List<Object> l, boolean b) { for (Object o: l) { if (b) { if (o != null) { return; } } else { System.out.println(o.toString()); // warning here } } } Contrast with: void bar(List<Object> l, boolean b) { Iterator<Object> i = l.iterator(); Object o; while (i.hasNext()) { o = i.next(); if (b) { if (o != null) { return; } } else { System.out.println(o.toString()); // silent } } }
Bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=178895 was opened. The general problem can't be fixed.
