Constant Conditions & "assert" QuickFix

I just found out that for cases where the inspection triggers on code that theoretically
could throw an NPE but never will because of application logic, there's a QuickFix to
insert an assert statement for the suspicious code. Very cool, but it doesn't get rid of
the "... may produce j.l.NullPointerException" warning. Though technically the NPE can
still occur, I think the warning should go away because the assert documents that this is
not supposed to happen for whatever reasons.

Another observation was that the "assert" QuickFix is not always shown (see method test2()
below) and I haven't fully understood yet why it isn't. Is this a bug or did I miss
anything here? IMO an assert QuickFix can be useful in any case, especially because it
would allow an even finer grained control over the inspection than just disabling it for
the whole statement.

Is this still work in progress, the desired behavior (if so, why?) or should I file a JIRA
request?

Thanks,
Sascha

public class Test {
public void test(@NotNull PsiElement element) {
if (checkSomething(element)) {
// This doesn't remove the NPE warning:
// assert ((PsiVariable)element.getParent()) != null;
if (((PsiVariable)element.getParent()).getNameIdentifier() != null) {
// ...
}
}
}

private void test2(@NotNull PsiElement element) {
// no assert quickfix available here...
if (element.getFirstChild().getText().length() > 0) {

}
}

private boolean checkSomething(@NotNull PsiElement element) {
return element.getParent() instanceof PsiVariable;
}
}

2 comments
Comment actions Permalink

No, it was completed long ago.
Please file a request.

Eugene.

"Sascha Weinreuter" <sascha.weinreuter@NOSPAM-cit.de> wrote in message
news:d9oqcu$r0s$1@is.intellij.net...
>I just found out that for cases where the inspection triggers on code that
>theoretically

could throw an NPE but never will because of application logic, there's a
QuickFix to
insert an assert statement for the suspicious code. Very cool, but it
doesn't get rid of
the "... may produce j.l.NullPointerException" warning. Though technically
the NPE can
still occur, I think the warning should go away because the assert
documents that this is
not supposed to happen for whatever reasons.

>

Another observation was that the "assert" QuickFix is not always shown
(see method test2()
below) and I haven't fully understood yet why it isn't. Is this a bug or
did I miss
anything here? IMO an assert QuickFix can be useful in any case,
especially because it
would allow an even finer grained control over the inspection than just
disabling it for
the whole statement.

>

Is this still work in progress, the desired behavior (if so, why?) or
should I file a JIRA
request?

>

Thanks,
Sascha

>

public class Test {
public void test(@NotNull PsiElement element) {
if (checkSomething(element)) {
// This doesn't remove the NPE warning:
// assert ((PsiVariable)element.getParent()) != null;
if (((PsiVariable)element.getParent()).getNameIdentifier() !=
null) {
// ...
}
}
}

>

private void test2(@NotNull PsiElement element) {
// no assert quickfix available here...
if (element.getFirstChild().getText().length() > 0) {

>

}
}

>

private boolean checkSomething(@NotNull PsiElement element) {
return element.getParent() instanceof PsiVariable;
}
}



0
Comment actions Permalink

Ok, experimenting a bit more made me understand the problem a bit better:

assert ((PsiVariable)element.getParent()) != null;
if (((PsiVariable)element.getParent()).getNameIdentifier() != null) {
// ...
}

does not work because "element.getParent()" could return different values for each
invocation. That's why the warning stays.

So actually problem #2 is the desired behavior because the returned values could change
and problem #1 is actually a bug because it offers an assert-QuickFix (that seems to be
triggered by the type cast) here.

Those can both be fixed by a) offering the QuickFix for any expression (either constant or
not) and b) to introduce a local variable for (potentially) non-constant expressions:
http://www.jetbrains.net/jira/browse/IDEA-3072

Sascha

0

Please sign in to leave a comment.