Community
Participate
Working Groups
20070226 if (l == null) { l.get(0) } The current error message is: 'The variable l can only be null; it was either set to null or checked for null when last used' That doesn't seem to say what's the problem. I'd expect something like: 'Null pointer access. The variable 'l' has been either set to null or checked for null when last used' Same for the potential null pointer access. 'Potential null pointer access.'
Makes sense (as bug 175571 does). Will see if I can get something along these lines out for tomorrow, or else this will wait for the following I-Build (we have quite a few messages in tests).
Created attachment 59851 [details] DO NOT RELEASE: experimental patch and test cases We have a couple of problems here. First, as reported in bug 175571, two cases are better represented by the current error message than they would be with the suggested new messages. Second, if we fix the messages, we'd better also fix the problem IDs (need two more, and renaming the existing ones would not be overkill), which is an API change.
Philippe, what do you think?
Since we improved null diagnosis, we should also improve the messages to make it more understandable. I don't like the way problemIDs are being evolved. Reading the comments in the code looks scary and confusing. #392 is not at the right place, it must be in sequence at its place. /** @since 3.1 */ int LocalVariableCannotBeNull = Internal + 397; // since 3.3: semantics are LocalVariableRedundantCheckOnNonNull /** @since 3.1 */ int LocalVariableCanOnlyBeNull = Internal + 398; // since 3.3: split with LocalVariableRedundantCheckOnNull depending on context // this id shares the null dereference errors with LocalVariableMayBeNull /** @since 3.2 */ int LocalVariableMayBeNull = Internal + 399; /** @since 3.3 */ int LocalVariableRedundantCheckOnNull = Internal + 392; We should reuse existing IDs with their proper semantics, then fork one particular diagnosis to use a new ID: LocalVariableRedundantNullCheck.
More finegrained problem ids are of course always good (for quick fix ect.). I wouldn't mind if you deprecate the previous ids and give the new one as specific names as possible. Now is a good moment for this. Like with the compiler messages, I think the IDs should name the problem, not the result of some analysis. There's probably many locations where a 'varibale can only be null'. That doesn't make it a problem, unless it is accessed.
Created attachment 60059 [details] Suggested fix + test cases This new proposal adds better messages for comparisons that yield negative results (aka o = null; if (o != null)...) and changes the problem IDs to align them on clearer semantics. Will seek PMC approval on those basis.
+1 for the API change. Kent - can you pls double check the proposed message, and make sure they align with our other messages ?
Released for 3.3 M6. (Kent, if you detect anything plain wrong with messages/IDs please let me know and I'll fix it asap.)
Verified for 3.3 M6 using build I20070320-0010.
