Skip to content

Commit

Permalink
Fix quality flaw: fix coverage on conditions when flow message target…
Browse files Browse the repository at this point in the history
…s ternary (#1491)
  • Loading branch information
Wohops authored May 24, 2017
1 parent 662682d commit b4d6f2a
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -385,8 +385,7 @@ List<JavaFileScannerContext.Location> flowFromLearnedAssociation(LearnedAssociat
for (Constraint constraint : constraints) {
String symbolName = learnedAssociation.symbol().name();
String msg;
if ((ObjectConstraint.NULL == constraint && assigningNullFromTernary(node))
|| (assigningFromMethodInvocation(node) && assignedFromYieldWithUncertainResult(constraint, node))) {
if (assigningNullFromTernary(node) || (assigningFromMethodInvocation(node) && assignedFromYieldWithUncertainResult(constraint, node))) {
msg = IMPLIES_CAN_BE_MSG;
} else {
msg = IMPLIES_IS_MSG;
Expand Down Expand Up @@ -443,7 +442,7 @@ private boolean isTernaryWithNullBranch(@Nullable ExpressionTree expressionTree)
ExpressionTree expr = ExpressionUtils.skipParentheses(expressionTree);
if (expr.is(Tree.Kind.CONDITIONAL_EXPRESSION)) {
ConditionalExpressionTree cet = (ConditionalExpressionTree) expr;
return ExpressionUtils.isNullLiteral(cet.trueExpression()) || ExpressionUtils.isNullLiteral(cet.falseExpression());
return ExpressionUtils.isNullLiteral(cet.trueExpression()) ^ ExpressionUtils.isNullLiteral(cet.falseExpression());
}
return false;
}
Expand Down
24 changes: 23 additions & 1 deletion java-frontend/src/test/files/se/FlowMessagesBranch.java
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
abstract class A {
class A {

void foo(Object a, Object b) {
if (a == null // flow@npe1 {{Implies 'a' can be null.}}
Expand All @@ -23,4 +23,26 @@ void qix(Object a) {
o.toString(); // flow@npe3 {{'o' is dereferenced.}}
}

void gul(Object a) {
Object o = a != null ? null : null; // flow@npe4 {{Implies 'o' is null.}}
// Noncompliant@+1 [[flows=npe4]] {{A "NullPointerException" could be thrown; "o" is nullable here.}}
o.toString(); // flow@npe4 {{'o' is dereferenced.}}
}

void dag(Object a) {
Object o = a == null ? null : a.toString(); // flow@npe5 {{Implies 'o' can be null.}}
// Noncompliant@+1 [[flows=npe5]] {{A "NullPointerException" could be thrown; "o" is nullable here.}}
o.toString(); // flow@npe5 {{'o' is dereferenced.}}
}

void cro(Object a) {
Object o = a != null ? a.toString() : foo(); // flow@npe6 {{'foo()' returns null.}} flow@npe6 {{Implies 'o' is null.}}
// Noncompliant@+1 [[flows=npe6]] {{A "NullPointerException" could be thrown; "o" is nullable here.}}
o.toString(); // flow@npe6 {{'o' is dereferenced.}}
}

static Object foo() {
return null;
}

}

0 comments on commit b4d6f2a

Please sign in to comment.