Skip to content

Commit

Permalink
Fixing handling of 'case null'.
Browse files Browse the repository at this point in the history
  • Loading branch information
lahodaj committed Nov 22, 2024
1 parent 41483e9 commit 722ad29
Show file tree
Hide file tree
Showing 2 changed files with 196 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -227,13 +227,44 @@ public static ErrorDescription unboxingConditional(HintContext ctx) {
}


@TriggerPattern("switch ($select) { case $cases$; }")
@TriggerTreeKind({Kind.SWITCH, Kind.SWITCH_EXPRESSION})
public static ErrorDescription switchExpression(HintContext ctx) {
TreePath select = ctx.getVariables().get("$select");
ExpressionTree selector;
List<? extends CaseTree> cases;
Tree swtch = ctx.getPath().getLeaf();

switch (swtch.getKind()) {
case SWITCH -> {
SwitchTree st = (SwitchTree) swtch;
selector = st.getExpression();
cases = st.getCases();
}
case SWITCH_EXPRESSION -> {
SwitchExpressionTree st = (SwitchExpressionTree) swtch;
selector = st.getExpression();
cases = st.getCases();
}
default -> throw new IllegalStateException("Unexpected tree kind: " + swtch.getKind());
}

TreePath select = new TreePath(ctx.getPath(), selector);
TypeMirror m = ctx.getInfo().getTrees().getTypeMirror(select);
if (m == null || m.getKind() != TypeKind.DECLARED) {
return null;
}

boolean hasNullCase = cases.stream()
.flatMap(ct -> ct.getLabels().stream())
.filter(clt -> clt.getKind() == Kind.CONSTANT_CASE_LABEL)
.map(ctl -> ((ConstantCaseLabelTree) ctl).getConstantExpression())
.filter(expr -> expr.getKind() == Kind.NULL_LITERAL)
.findAny()
.isPresent();

if (hasNullCase) {
return null;
}

State r = computeExpressionsState(ctx).get(select.getLeaf());
if (r == NULL || r == NULL_HYPOTHETICAL) {
String displayName = NbBundle.getMessage(NPECheck.class, "ERR_DereferencingNull");
Expand Down Expand Up @@ -1284,21 +1315,41 @@ public State visitSwitchExpression(SwitchExpressionTree node, Void p) {
private void handleGeneralizedSwitch(Tree switchTree, ExpressionTree expression, List<? extends CaseTree> cases) {
scan(expression, null);

Element selectorElement = info.getTrees().getElement(new TreePath(getCurrentPath(), expression));
VariableElement selectorVariable =
isVariableElement(selectorElement) && !hasDefiniteValue((VariableElement) selectorElement) ? (VariableElement) selectorElement
: null;

Map<VariableElement, State> origVariable2State = new HashMap<>(variable2State);

boolean exhaustive = false;

for (CaseTree ct : cases) {
mergeIntoVariable2State(origVariable2State);

if (ct.getExpression() == null) {
exhaustive = true;
boolean hasNull = false;

for (CaseLabelTree clt : ct.getLabels()) {
switch (clt.getKind()) {
case DEFAULT_CASE_LABEL -> exhaustive = true;
case CONSTANT_CASE_LABEL -> {
if (((ConstantCaseLabelTree) clt).getConstantExpression().getKind() == Kind.NULL_LITERAL) {
hasNull = true;
}
}
}
}

if (selectorVariable != null) {
variable2State.put(selectorVariable, hasNull ? State.NULL_HYPOTHETICAL : State.NOT_NULL_HYPOTHETICAL);
}

State caseResult = scan(ct, null);

if (ct.getCaseKind() == CaseKind.RULE) {
pendingYields.add(caseResult);
if (ct.getBody() != null && ExpressionTree.class.isAssignableFrom(ct.getBody().getKind().asInterface())) {
pendingYields.add(caseResult);
}
breakTo(switchTree);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1830,6 +1830,146 @@ public void testObjectsRequireNonNullElse() throws Exception {
.assertNotContainsWarnings("Possibly Dereferencing null");
}

public void testSwitchCaseNull() throws Exception {
HintTest.create()
.sourceLevel("21")
.input("""
package test;
public class Test {
private void test(@NullAllowed E e) {
switch (e) {
case null:
System.err.println(e.name());
break;
default:
if (e == null) {}
System.err.println(e.name());
break;
}
}
@interface NullAllowed {}
enum E {A}
}
""")
.run(NPECheck.class)
.assertWarnings("5:37-5:41:verifier:DN",
"8:20-8:29:verifier:ERR_NotNull");
}

public void testCaseNullIsANullTest1() throws Exception {
HintTest.create()
.sourceLevel("21")
.input("""
package test;
public class Test {
private void test(E e) {
switch (e) {
case null:
break;
default:
break;
}
e.name();
}
enum E {A}
}
""")
.run(NPECheck.class)
.assertWarnings("9:10-9:14:verifier:Possibly Dereferencing null");
}

public void testCaseNullIsANullTest2() throws Exception {
HintTest.create()
.sourceLevel("21")
.input("""
package test;
public class Test {
private void test(E e) {
switch (e) {
case null:
return ;
default:
break;
}
if (e != null) {
e.name();
}
}
enum E {A}
}
""")
.run(NPECheck.class)
.assertWarnings("9:12-9:21:verifier:ERR_NotNull");
}

public void testSwitchExpressionCaseNull() throws Exception {
HintTest.create()
.sourceLevel("21")
.input("""
package test;
public class Test {
private String test(@NullAllowed E e) {
return switch (e) {
case null:
System.err.println(e.name());
yield "";
default:
if (e == null) {}
System.err.println(e.name());
yield "";
};
}
@interface NullAllowed {}
enum E {A}
}
""")
.run(NPECheck.class)
.assertWarnings("5:37-5:41:verifier:DN",
"8:20-8:29:verifier:ERR_NotNull");
}

public void testSwitchExpressionNullSelector() throws Exception {
HintTest.create()
.sourceLevel("21")
.input("""
package test;
public class Test {
private String test(@NullAllowed E e) {
return switch (e) {
default:
yield "";
};
}
@interface NullAllowed {}
enum E {A}
}
""")
.run(NPECheck.class)
.assertWarnings("3:15-3:27:verifier:Possibly Dereferencing null");
}

public void testSwitchExpressionYieldedNonNull() throws Exception {
HintTest.create()
.sourceLevel("21")
.input("""
package test;
public class Test {
private void test(int i) {
String y = switch (i) {
case 0 -> "";
default -> {
yield "";
}
};
if (y != null) {}
}
enum E {A}
}
""")
.run(NPECheck.class)
.assertWarnings("9:12-9:21:verifier:ERR_NotNull");
}

private void performAnalysisTest(String fileName, String code, String... golden) throws Exception {
HintTest.create()
.input(fileName, code)
Expand Down

0 comments on commit 722ad29

Please sign in to comment.