Potential Null Pointer Access
-
I'm taking some time to get rid of stupid warnings in my Java code base so that warnings will be useful again. One of the more useful warnings are for potential null pointer accesses. However, I've got a lot of false positives in my code. Consider this equivalent and simplified example:
private boolean isNonNull( String foo ){ return foo != null; } public void foo(){ String foo = null; for( int i = 0; i < 3 ; ++i ){ if( isNonNull( foo ) ){ foo.compareTo( "xyz" ); } foo = "bar"; } }
The
foo.compareTo( "xyz" );
gets flagged by the compiler as a potential null pointer access. Obviously, it isn't. There doesn't seem to be a good way to get rid of this. Obviously I don't want to add additional null checks. I could add asserts, which won't get compiled into the production code, but that's also extra work and clutters up the code. The null related annotations don't cover this case.Did I miss some other way? What would you do?
(fixed condition)
-
@boomzilla Well, for starters, your condition is inverted. You're only calling the function if foo is null.
-
@asdf
isNonNull
returns true if it's null, so!isNonNull
switches it?It's not the best name...
edit: maybe not.
-
@asdf said in Potential Null Pointer Access:
@boomzilla Well, for starters, your condition is inverted. You're only calling the function if foo is null.
Fixed. Though that doesn't change the resulting warning (which should have been an error at that point given my compiler settings).
-
@loopback0 said in Potential Null Pointer Access:
@asdf
isNonNull
returns true if it's null, so!isNonNull
switches it?It's not the best name...
edit: maybe not.
Yeah...anonymization and playing around to see if it does anything there tripped me up.
-
@boomzilla said in Potential Null Pointer Access:
The null related annotations don't cover this case.
Hm. Back in C# land, Resharper has
ContractAnnotation
s which let you specify contracts on your methods:[ContractAnnotation("null => false; notnull => true")] private bool IsNonNull(string foo) { return foo != null; } //... string foo = null; if (IsNonNull(foo)) { foo.ToString(); //look ma, no warning! }
I haven't found anything equivalent for Eclipse, but perhaps there is some sort of extension that does that?
-
@Maciejasjmj said in Potential Null Pointer Access:
I haven't found anything equivalent for Eclipse, but perhaps there is some sort of extension that does that?
Yeah, that's the sort of thing I'm hoping someone will point out.
-
How equivalent is this? Right now, your method is so trivial that I would fix the problem by eliminating it and just inlining the != check.
-
@boomzilla said in Potential Null Pointer Access:
Did I miss some other way?
https://www.jetbrains.com/help/idea/2016.3/contract-annotations.html
@Contract("null -> false; !null -> true") private boolean isNonNull( String foo ){ return foo != null; }
Don't know how widespread their support is.
-
@masonwheeler said in Potential Null Pointer Access:
How equivalent is this? Right now, your method is so trivial that I would fix the problem by eliminating it and just inlining the != check.
There's another check, but that relies on the object not being null and this is absolutely not going to be inlined in the zillions of places where it (and other similar methods) are used.
-
@boomzilla said in Potential Null Pointer Access:
Did I miss some other way? What would you do?
Initialize foo to point to something other than null. Does Java have some equivalent to C's line number macro? Stringizing that and pointing foo to it would get rid of the compile time warnings, while still leaving you with some semi-meaningful way to debug WTF is going on when one of your "it's really, truly can't be null" assumptions eventually does fail at runtime.
-
@flabdablet That really doesn't work in Java (or C#) as the type safety rules prevent that sort of thing. OTOH, the
new
operator is guaranteed to give non-null
if it doesn't throw an exception, and that helps a lot, as does the exactly defined order of operation. Java's a lot easier to statically-analyse than C.
-
@dkf said in Potential Null Pointer Access:
Don't know how widespread their support is.
Hmm...doesn't seem to work with Eclipse.
-
@Maciejasjmj said in Potential Null Pointer Access:
I haven't found anything equivalent for Eclipse
Well, there is JML, but I have no idea how good or bad its Eclipse integration is.
-
@boomzilla maybe idiotic of me, but:
(new String("xyz")).compareTo(foo)
Ugly, and probably syntaxtically wrong too (on mobile and too long without programming), but you get the idea...
IIRC, as long as it is strings you've already lost once you throw in a literal like that in Java -- the String object will be created for you anyways...
-
@boomzilla said in Potential Null Pointer Access:
this is absolutely not going to be inlined in the zillions of places where it (and other similar methods) are used
No macro expansion in the Java compilation chain either. Clear deficiency.
-
@Mikael_Svahnberg Syntactically correct, but dumb as heck when you can do this:
"xyz".compareTo(foo)
Doesn't deal with the general problem though. Alas, it doesn't seem that any (freely-available) Eclipse plugins support anything much better than the most basic
@Nullable
and@NonNull
.
-
@Mikael_Svahnberg said in Potential Null Pointer Access:
@boomzilla maybe idiotic of me, but:
(new String("xyz")).compareTo(foo)
Ugly, and probably syntaxtically wrong too (on mobile and too long without programming), but you get the idea...
IIRC, as long as it is strings you've already lost once you throw in a literal like that in Java -- the String object will be created for you anyways...Java allows
"xyz".compareTo(foo)
syntax.This SO answer recommends the following null-safe replacement for
compareTo()
:public static boolean compare(String str1, String str2) { return (str1 == null ? str2 == null : str1.equals(str2)); }
If you're doing an ordering comparison, you can modify this a bit to indicate how nulls should be ordered.
-
@boomzilla said in Potential Null Pointer Access:
@dkf said in Potential Null Pointer Access:
Don't know how widespread their support is.
Hmm...doesn't seem to work with Eclipse.
Yea, looks like there's no support for stuff like that in Eclipse.
IntelliJ, for example, allows you to
@SuppressWarnings
or//suppress
for a single line - it looks like the best you can get in Eclipse is suppressing an entire file of all warnings.
-
@flabdablet Yeah, a good metaprogramming solution can be really amazing. (Or a huge pain, if abused.)
-
@sloosecannon
I guess @boomzilla will have to come to the dark side, then.
-
@asdf Only if he gets a sudden craving for s...
-
@asdf said in Potential Null Pointer Access:
@Maciejasjmj said in Potential Null Pointer Access:
I haven't found anything equivalent for Eclipse
Well, there is JML, but I have no idea how good or bad its Eclipse integration is.
Looks more like Code Contracts than R#'s contract annotations (which only really support null, notnull, true, false and halt, but they help immensely when writing Debug.Assert style methods telling R# that yes, you've checked this shit via means beyond its feeble recognition).
And since Code Contracts are pretty terribly broken as far as their usability goes, I don't have much trust in the Java implementation either.
-
@sloosecannon said in Potential Null Pointer Access:
@boomzilla said in Potential Null Pointer Access:
@dkf said in Potential Null Pointer Access:
Don't know how widespread their support is.
Hmm...doesn't seem to work with Eclipse.
Yea, looks like there's no support for stuff like that in Eclipse.
IntelliJ, for example, allows you to
@SuppressWarnings
or//suppress
for a single line - it looks like the best you can get in Eclipse is suppressing an entire file of all warnings.Eclipse supports
@SuppressWarnings
- I'm sure it's one of the things it suggests on the...umm... suggestion popup?
Does the suggestion popup have an official name?
-
@loopback0 said in Potential Null Pointer Access:
@sloosecannon said in Potential Null Pointer Access:
@boomzilla said in Potential Null Pointer Access:
@dkf said in Potential Null Pointer Access:
Don't know how widespread their support is.
Hmm...doesn't seem to work with Eclipse.
Yea, looks like there's no support for stuff like that in Eclipse.
IntelliJ, for example, allows you to
@SuppressWarnings
or//suppress
for a single line - it looks like the best you can get in Eclipse is suppressing an entire file of all warnings.Eclipse supports
@SuppressWarnings
- I'm sure it's one of the things it suggests on the...umm... suggestion popup?
Does the suggestion popup have an official name?Then why not do that?
-
@sloosecannon said in Potential Null Pointer Access:
@boomzilla said in Potential Null Pointer Access:
@dkf said in Potential Null Pointer Access:
Don't know how widespread their support is.
Hmm...doesn't seem to work with Eclipse.
Yea, looks like there's no support for stuff like that in Eclipse.
IntelliJ, for example, allows you to
@SuppressWarnings
or//suppress
for a single line - it looks like the best you can get in Eclipse is suppressing an entire file of all warnings.Eclipse supports the
@SuppressWarnings({string-name-of-warning {, more-warnings {,...}}})
annotation for variables, functions, and classes.@SuppressWarnings("null")
suppresses warnings relative to null analysis.
-
-
@sloosecannon said in Potential Null Pointer Access:
@loopback0 said in Potential Null Pointer Access:
@sloosecannon said in Potential Null Pointer Access:
@boomzilla said in Potential Null Pointer Access:
@dkf said in Potential Null Pointer Access:
Don't know how widespread their support is.
Hmm...doesn't seem to work with Eclipse.
Yea, looks like there's no support for stuff like that in Eclipse.
IntelliJ, for example, allows you to
@SuppressWarnings
or//suppress
for a single line - it looks like the best you can get in Eclipse is suppressing an entire file of all warnings.Eclipse supports
@SuppressWarnings
- I'm sure it's one of the things it suggests on the...umm... suggestion popup?
Does the suggestion popup have an official name?Then why not do that?
Asking the wrong person
-
@sloosecannon said in Potential Null Pointer Access:
Then why not do that?
I'd rather use the
assert
hack, since that's more granular and wouldn't suppress stuff that really needs to be warned about (like...caused by future modifications).
-
I know that
javax.validation
(part of the JavaEE spec) has constraints such as@NotNull
, but I have no idea if Eclipse supports it or not.Edit: Probably not as it's for runtime validation.
-
@powerlord It does, but that isn't helpful in the cases I'm looking at.
-
I don't suppose Java 8's Optional would be of help here?
-
@tharpa Not really. The null checking is already being done, just not in a way that the compiler understands. It's something like
StringUtils.isEmpty()
where it returns true if the string is either null or an empty string. But the compiler doesn't realize that null has already been tested and acted upon, so it throws a false positive warning.There are ways to change the coding, but I was hoping there was some way to give the compiler a hint, like the contracts stuff mentioned above.
-
@boomzilla said in Potential Null Pointer Access:
@tharpa Not really. The null checking is already being done, just not in a way that the compiler understands. It's something like
StringUtils.isEmpty()
where it returns true if the string is either null or an empty string. But the compiler doesn't realize that null has already been tested and acted upon, so it throws a false positive warning.There are ways to change the coding, but I was hoping there was some way to give the compiler a hint, like the contracts stuff mentioned above.
I have to give the point to .Net on this one. In .Net, just the fact that you've assigned it as null tells the compiler not to give you a warning on it.
-
@tharpa said in Potential Null Pointer Access:
I have to give the point to .Net on this one. In .Net, just the fact that you've assigned it as null tells the compiler not to give you a warning on it.
If you're correct (and I have a difficult time believing that you are) then .Net is terrible, unless you mean that it gives you an error for an obvious null pointer access, which java does (or can do) too.
-
@tharpa First, ".Net" is not a language and it doesn't have a compiler. Are you talking about C#?
Second, assuming you're talking about C#, that's a different thing. The C# compiler will error (not warn) if you try to use a variable that hasn't been assigned to. This is about a warning if you use a nullable value, assuming it's not null, without having checked.
The C# compiler doesn't have that warning, but you can get it added to Visual Studio with Resharper.
-
@masonwheeler said in Potential Null Pointer Access:
This is about a warning if you use a nullable value, assuming it's not null, without having checked.
Actually, it's about getting the compiler to recognize that you did check it for null, just not in the current method.
-
-
@masonwheeler Hey, it's my question so it's in my interest to keep you guys on topic.
-
@boomzilla C# compiler lead explains why interprocedural analysis is tricky and some false positives are inevitable. TLDR: doing it perfectly can be reduced to the Halting Problem. All the same points are equally applicable to Java.
-
@masonwheeler said in Potential Null Pointer Access:
C# compiler lead explains why interprocedural analysis is tricky and some false positives are inevitable. TLDR: doing it perfectly can be reduced to the Halting Problem. All the same points are equally applicable to Java.
Oh, yeah, I've done this sort of stuff and I understand how tricky it can be. But while you obviously can't solve everything you can still solve a lot of things that are trivial to still pretty tricky. This one should be towards the trivial end which I'd expect any static analysis tool that wasn't total crap to be able to pick up on.
But I was also aware that stuff like the
@Contract
annotations were things, even if I didn't know of one that would suit my specific purpose.
-
Is it possible to initialize foo with a sentinel value?
Not to be confused with sentimental value....which is what I suppose @flabdablet was proposing.
-
@Zecc Yeah...that's one alternative. And it's what I was technically using
null
for. But doing something like that would require changing multiple places in the code.
-
@boomzilla said in Potential Null Pointer Access:
Oh, yeah, I've done this sort of stuff and I understand how tricky it can be.
Strictly, the problem is that the standard analysis engines are being run on each
functionmethod in isolation, whereas this is really an inter-procedural problem; there's usually no automatic sharing of analysis info, since once you go down that route there's a danger of the amount of work to analyse things getting silly. (It's approximately the same reason why link-time optimization can get crazy-expensive with C and C++.) The@Contract
annotation (and its C#-equivalent) are ways forward, since they allow a programmer to split the reasoning graph, but they require the reasoner to be aware of what the annotations mean. Right now, it looks like it's only one IDE that really has that sort of thing, and it isn't Eclipse.A pity, this is a nice feature but I really don't like the way JetBrains's IDEs work. They don't suit me in all sorts of little ways. :(
-
@dkf said in Potential Null Pointer Access:
whereas this is really an inter-procedural problem
True, though the other method in this case is small enough that it could reasonably be inlined.
-
@boomzilla said in Potential Null Pointer Access:
True, though the other method in this case is small enough that it could reasonably be inlined.
Yes, but the strategy used by both Java and C# is to postpone that until JIT. In most applications, that helps a lot as it lets the system know what concrete classes are in use. But in this case, it's way too late for you.
-
Is this something you can trivially rewrite to start the loop with the assignment instead of the null check? Not having to do a null check at all would probably fix your problem.
The sample code can be rewritten as
public void foo(){ String foo; for( int i = 1; i < 3 ; ++i ){ foo = "bar"; foo.compareTo( "xyz" ); } foo = "bar"; // if the string literal somehow has side effects }
-
@ben_lubar said in Potential Null Pointer Access:
The sample code can be rewritten as
foo = "bar";
I assume in the actual real code there's something useful being done with the value of
foo
inside thefor
body and with the result ofcompareTo
inside the if block.As the sample code is now, it looks like @boomzilla wants foo to be null initially just so it can skip the if block the first time around the loop. I see you changed the initializer to
i = 1
.My question now is why not just change the conditional to check for
i != 0
?
I suppose there are other statements that may setfoo
to null inside the loop?
-
@ben_lubar said in Potential Null Pointer Access:
Is this something you can trivially rewrite to start the loop with the assignment instead of the null check?
I'm guessing that the answer is “no” because this is representative of the more complex real code.
-
For a quick and dirty hack, if you're using Guava, you can do
String foo = Strings.nullToEmpty(null);
Or inside the check, which will stop the compiler complaining (Netbeans, at least).