lahodaj commented on code in PR #9450:
URL: https://github.com/apache/netbeans/pull/9450#discussion_r3454365130
##########
java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/NPECheckTest.java:
##########
@@ -1970,6 +2001,26 @@ enum E {A}
.assertWarnings("9:12-9:21:verifier:ERR_NotNull");
}
+ public void testIfAndInstanceOf() throws Exception {
+ HintTest.create()
+ .sourceLevel("21")
+ .input("""
+ package test;
+ public class Test {
+ public @NotNull Object test(Object o) {
+ if (o instanceof String s) {
+ return s;
+ } else {
+ return o; //can't say anything
Review Comment:
The `can't say anything` note is meant in the context of the null checker.
The null checker currently intentionally does not produce warnings if it
can't prove the variable can be `null`. E.g.:
```
Object o = methodWithUnmarkedReturnType();
o.toString(); //no warning here, as we can't prove o can be null
```
```
Object o = methodWithUnmarkedReturnType();
boolean b = o != null;
o.toString(); //warning here, as we can prove o can be null
```
Now, the question is: is `instanceof` strong enough signal that the variable
can be, "provably", `null`. The current checker says "yes and no". In some
cases, it will produce the warning, in other cases, it won't as it was disabled
under:
https://issues.apache.org/jira/browse/NETBEANS-407
It seems to cause too much inconsistencies, and it makes it increasingly
difficult to reason about the checker. It feels better to err on the side of
not saying anything, than annoying the user with false warnings. Alternatively,
we can rollback 407, and really say "`instanceof` is a test for `null`". But
the current state makes less and less sense to me.
In other words, can this remove a potentially useful warning? Yes, if that
user expects the warning. It also creates noise if the user does not expect the
warning.
My long-er term goal is to support JSpecify (I have some prototypes).
JSpecify allows to bulk-mark types as non-null explicitly (`@NullMarked`), or
nullable explicitly (`@NullUnmarked`) at which point the "automatic" "possible
null, but don't report" mostly ceases to exist.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
