Skip to content

Commit a77456f

Browse files
WohopsWohops
committed
SONARJAVA-1213 UselessConditionCheck now handle bitwise operators
1 parent 65c0a5c commit a77456f

4 files changed

Lines changed: 232 additions & 7 deletions

File tree

‎its/ruling/src/test/resources/expected/squid-S2583.json‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,6 @@
9292
299,
9393
388,
9494
],
95-
'project:com/sun/org/apache/xerces/internal/impl/xs/models/XSDFACM.java':[
96-
1117,
97-
],
9895
'project:com/sun/org/apache/xerces/internal/parsers/ObjectFactory.java':[
9996
150,
10097
],

‎java-checks/src/main/java/org/sonar/java/symexec/SymbolicEvaluator.java‎

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,12 @@ ExecutionState instantiateExecutionState(ExecutionState parentState) {
128128
abstract static class BaseExpressionVisitor extends BaseTreeVisitor {
129129
@Override
130130
public final void visitBinaryExpression(BinaryExpressionTree tree) {
131-
if (tree.is(Tree.Kind.CONDITIONAL_AND)) {
131+
if (tree.is(Tree.Kind.CONDITIONAL_AND, Tree.Kind.AND)) {
132132
evaluateConditionalAnd(tree);
133-
} else if (tree.is(Tree.Kind.CONDITIONAL_OR)) {
133+
} else if (tree.is(Tree.Kind.CONDITIONAL_OR, Tree.Kind.OR)) {
134134
evaluateConditionalOr(tree);
135+
} else if (tree.is(Tree.Kind.XOR)) {
136+
evaluateXor(tree);
135137
} else if (tree.is(Tree.Kind.EQUAL_TO)) {
136138
evaluateRelationalOperator(tree, SymbolicRelation.EQUAL_TO);
137139
} else if (tree.is(Tree.Kind.GREATER_THAN)) {
@@ -151,6 +153,8 @@ public final void visitBinaryExpression(BinaryExpressionTree tree) {
151153

152154
abstract void evaluateConditionalOr(BinaryExpressionTree tree);
153155

156+
abstract void evaluateXor(BinaryExpressionTree tree);
157+
154158
abstract void evaluateRelationalOperator(BinaryExpressionTree tree, SymbolicRelation operator);
155159

156160
@CheckForNull
@@ -337,6 +341,23 @@ void evaluateConditionalOr(BinaryExpressionTree tree) {
337341
}
338342
}
339343

344+
@Override
345+
void evaluateXor(BinaryExpressionTree tree) {
346+
PackedStates leftResult = evaluateCondition(currentState, tree.leftOperand());
347+
for (ExecutionState leftState : leftResult.trueStates) {
348+
PackedStates rightResult = evaluateCondition(leftState, tree.rightOperand());
349+
currentResult.trueStates.addAll(rightResult.falseStates);
350+
currentResult.falseStates.addAll(rightResult.trueStates);
351+
currentResult.unknownStates.addAll(rightResult.unknownStates);
352+
}
353+
for (ExecutionState leftState : leftResult.falseStates) {
354+
PackedStates rightResult = evaluateCondition(leftState, tree.rightOperand());
355+
currentResult.trueStates.addAll(rightResult.trueStates);
356+
currentResult.falseStates.addAll(rightResult.falseStates);
357+
currentResult.unknownStates.addAll(rightResult.unknownStates);
358+
}
359+
}
360+
340361
@Override
341362
void evaluateRelationalOperator(BinaryExpressionTree tree, SymbolicRelation operator) {
342363
SymbolicValue leftValue = retrieveSymbolicValue(tree.leftOperand());
@@ -492,6 +513,25 @@ void evaluateConditionalOr(BinaryExpressionTree tree) {
492513
currentState.mergeRelations(Iterables.concat(leftStates.falseStates, leftStates.trueStates));
493514
}
494515

516+
@Override
517+
void evaluateXor(BinaryExpressionTree tree) {
518+
PackedStates leftStates = evaluateCondition(currentState, tree.leftOperand());
519+
currentResult = leftStates.getBooleanConstraint();
520+
if (currentResult == SymbolicBooleanConstraint.FALSE) {
521+
currentResult = null;
522+
for (ExecutionState state : leftStates.falseStates) {
523+
SymbolicBooleanConstraint newResult = evaluateExpression(state, tree.rightOperand());
524+
currentResult = newResult.union(currentResult);
525+
}
526+
} else if (currentResult == SymbolicBooleanConstraint.TRUE) {
527+
currentResult = null;
528+
for (ExecutionState state : leftStates.trueStates) {
529+
SymbolicBooleanConstraint newResult = evaluateExpression(state, tree.rightOperand());
530+
currentResult = newResult.negate().union(currentResult);
531+
}
532+
}
533+
}
534+
495535
@Override
496536
void evaluateRelationalOperator(BinaryExpressionTree tree, SymbolicRelation operator) {
497537
SymbolicValue leftValue = retrieveSymbolicValue(tree.leftOperand());

‎java-checks/src/test/files/checks/UselessConditionCheck.java‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,27 @@ public void conditional_and(boolean parameter1, boolean parameter2) {
4343
if (parameter1 && parameter2) { // Compliant, unknown
4444
}
4545
}
46+
47+
public void bitwise_and(boolean parameter1, boolean parameter2) {
48+
if (false & false) { // Noncompliant {{Change this condition so that it does not always evaluate to "false"}}
49+
}
50+
if (false & true) { // Noncompliant {{Change this condition so that it does not always evaluate to "false"}}
51+
}
52+
if (false & parameter2) { // Noncompliant {{Change this condition so that it does not always evaluate to "false"}}
53+
}
54+
if (true & false) { // Noncompliant {{Change this condition so that it does not always evaluate to "false"}}
55+
}
56+
if (true & true) { // Noncompliant {{Change this condition so that it does not always evaluate to "true"}}
57+
}
58+
if (true & parameter2) { // Compliant, unknown
59+
}
60+
if (parameter1 & false) { // Noncompliant {{Change this condition so that it does not always evaluate to "false"}}
61+
}
62+
if (parameter1 & true) { // Compliant, unknown
63+
}
64+
if (parameter1 & parameter2) { // Compliant, unknown
65+
}
66+
}
4667

4768
public void conditional_or(boolean parameter1, boolean parameter2) {
4869
if (false || false) { // Noncompliant {{Change this condition so that it does not always evaluate to "false"}}
@@ -64,6 +85,48 @@ public void conditional_or(boolean parameter1, boolean parameter2) {
6485
if (parameter1 || parameter2) { // Compliant, unknown
6586
}
6687
}
88+
89+
public void bitwise_or(boolean parameter1, boolean parameter2) {
90+
if (false | false) { // Noncompliant {{Change this condition so that it does not always evaluate to "false"}}
91+
}
92+
if (false | true) { // Noncompliant {{Change this condition so that it does not always evaluate to "true"}}
93+
}
94+
if (false | parameter2) { // Compliant, unknown
95+
}
96+
if (true | false) { // Noncompliant {{Change this condition so that it does not always evaluate to "true"}}
97+
}
98+
if (true | true) { // Noncompliant {{Change this condition so that it does not always evaluate to "true"}}
99+
}
100+
if (true | parameter2) { // Noncompliant {{Change this condition so that it does not always evaluate to "true"}}
101+
}
102+
if (parameter1 | false) { // Compliant, unknown
103+
}
104+
if (parameter1 | true) { // Noncompliant {{Change this condition so that it does not always evaluate to "true"}}
105+
}
106+
if (parameter1 | parameter2) { // Compliant, unknown
107+
}
108+
}
109+
110+
public void conditional_bitwise_xor(boolean parameter1, boolean parameter2) {
111+
if (false ^ false) { // Noncompliant {{Change this condition so that it does not always evaluate to "false"}}
112+
}
113+
if (false ^ true) { // Noncompliant {{Change this condition so that it does not always evaluate to "true"}}
114+
}
115+
if (false ^ parameter2) { // Compliant, unknown
116+
}
117+
if (true ^ false) { // Noncompliant {{Change this condition so that it does not always evaluate to "true"}}
118+
}
119+
if (true ^ true) { // Noncompliant {{Change this condition so that it does not always evaluate to "false"}}
120+
}
121+
if (true ^ parameter2) { // Compliant, unknown
122+
}
123+
if (parameter1 ^ false) { // Compliant, unknown
124+
}
125+
if (parameter1 ^ true) { // Compliant, unknown
126+
}
127+
if (parameter1 ^ parameter2) { // Compliant, unknown
128+
}
129+
}
67130

68131
public void identifier_field() {
69132
if (field == false && field == true) { // Compliant
@@ -90,8 +153,16 @@ public void identifier_parameter(boolean parameter) {
90153
}
91154
if (parameter && !parameter) { // Noncompliant {{Change this condition so that it does not always evaluate to "false"}}
92155
}
156+
if (parameter & !parameter) { // Noncompliant {{Change this condition so that it does not always evaluate to "false"}}
157+
}
93158
if (parameter || !parameter) { // Noncompliant {{Change this condition so that it does not always evaluate to "true"}}
94159
}
160+
if (parameter | !parameter) { // Noncompliant {{Change this condition so that it does not always evaluate to "true"}}
161+
}
162+
if (parameter ^ parameter) { // Noncompliant {{Change this condition so that it does not always evaluate to "false"}}
163+
}
164+
if (parameter ^ !parameter) { // Noncompliant {{Change this condition so that it does not always evaluate to "true"}}
165+
}
95166
}
96167

97168
public void instanceOf() {
@@ -943,6 +1014,15 @@ public void ternary(boolean condition) {
9431014
if (false ? condition : true) { // Noncompliant {{Change this condition so that it does not always evaluate to "true"}}
9441015
}
9451016
}
1017+
1018+
public void ternary_with_bitwise_operators() {
1019+
boolean b1 = false;
1020+
boolean b2 = false;
1021+
1022+
int value;
1023+
value = (b1 ^ b2) ? 1 : 2; // False Negative - Not handled
1024+
value = (b1 ^ !b2) ? 1 : 2; // False Negative - Not handled
1025+
}
9461026

9471027
public overrun(boolean[] a) { // analysis aborted due to too many execution states
9481028
if (a[0] && a[0] && a[0] && a[0] && a[0] && a[0] && a[0] && a[0] && a[0] && a[0] && a[0] && a[0] && a[0]) {

‎java-checks/src/test/java/org/sonar/java/symexec/SymbolicEvaluatorTest.java‎

Lines changed: 110 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,31 @@ public void test_condition_conditional_and() {
9898
assertOutputStates(evaluateCondition("local1 && local2"), 1, 2, 0);
9999
}
100100

101+
@Test
102+
public void test_condition_bitwise_and() {
103+
// evaluation
104+
assertThat(evaluateCondition("false & false").isAlwaysFalse()).isTrue();
105+
assertThat(evaluateCondition("false & true").isAlwaysFalse()).isTrue();
106+
assertThat(evaluateCondition("false & local2").isAlwaysFalse()).isTrue();
107+
assertThat(evaluateCondition("true & false").isAlwaysFalse()).isTrue();
108+
assertThat(evaluateCondition("true & true").isAlwaysTrue()).isTrue();
109+
assertThat(evaluateCondition("true & local2").isUnknown()).isTrue();
110+
assertThat(evaluateCondition("local1 & false").isAlwaysFalse()).isTrue();
111+
assertThat(evaluateCondition("local1 & true").isUnknown()).isTrue();
112+
assertThat(evaluateCondition("local1 & local").isUnknown()).isTrue();
113+
114+
// number of spawned states
115+
assertOutputStates(evaluateCondition("false & false"), 0, 1, 0);
116+
assertOutputStates(evaluateCondition("false & true"), 0, 1, 0);
117+
assertOutputStates(evaluateCondition("false & local2"), 0, 1, 0);
118+
assertOutputStates(evaluateCondition("true & false"), 0, 1, 0);
119+
assertOutputStates(evaluateCondition("true & true"), 1, 0, 0);
120+
assertOutputStates(evaluateCondition("true & local2"), 1, 1, 0);
121+
assertOutputStates(evaluateCondition("local1 & false"), 0, 2, 0);
122+
assertOutputStates(evaluateCondition("local1 & true"), 1, 1, 0);
123+
assertOutputStates(evaluateCondition("local1 & local2"), 1, 2, 0);
124+
}
125+
101126
@Test
102127
public void test_condition_conditional_or() {
103128
// evaluation
@@ -124,7 +149,57 @@ public void test_condition_conditional_or() {
124149
}
125150

126151
@Test
127-
public void test_expression_conditional_and() {
152+
public void test_condition_bitwise_or() {
153+
// evaluation
154+
assertThat(evaluateCondition("false | false").isAlwaysFalse()).isTrue();
155+
assertThat(evaluateCondition("false | true").isAlwaysTrue()).isTrue();
156+
assertThat(evaluateCondition("false | local2").isUnknown()).isTrue();
157+
assertThat(evaluateCondition("true | false").isAlwaysTrue()).isTrue();
158+
assertThat(evaluateCondition("true | true").isAlwaysTrue()).isTrue();
159+
assertThat(evaluateCondition("true | local2").isAlwaysTrue()).isTrue();
160+
assertThat(evaluateCondition("local1 | false").isUnknown()).isTrue();
161+
assertThat(evaluateCondition("local1 | true").isAlwaysTrue()).isTrue();
162+
assertThat(evaluateCondition("local1 | local").isUnknown()).isTrue();
163+
164+
// number of spawned states
165+
assertOutputStates(evaluateCondition("false | false"), 0, 1, 0);
166+
assertOutputStates(evaluateCondition("false | true"), 1, 0, 0);
167+
assertOutputStates(evaluateCondition("false | local2"), 1, 1, 0);
168+
assertOutputStates(evaluateCondition("true | false"), 1, 0, 0);
169+
assertOutputStates(evaluateCondition("true | true"), 1, 0, 0);
170+
assertOutputStates(evaluateCondition("true | local2"), 1, 0, 0);
171+
assertOutputStates(evaluateCondition("local1 | false"), 1, 1, 0);
172+
assertOutputStates(evaluateCondition("local1 | true"), 2, 0, 0);
173+
assertOutputStates(evaluateCondition("local1 | local2"), 2, 1, 0);
174+
}
175+
176+
@Test
177+
public void test_condition_xor() {
178+
// evaluation
179+
assertThat(evaluateCondition("false ^ false").isAlwaysFalse()).isTrue();
180+
assertThat(evaluateCondition("false ^ true").isAlwaysTrue()).isTrue();
181+
assertThat(evaluateCondition("false ^ local2").isUnknown()).isTrue();
182+
assertThat(evaluateCondition("true ^ false").isAlwaysTrue()).isTrue();
183+
assertThat(evaluateCondition("true ^ true").isAlwaysFalse()).isTrue();
184+
assertThat(evaluateCondition("true ^ local2").isUnknown()).isTrue();
185+
assertThat(evaluateCondition("local1 ^ false").isUnknown()).isTrue();
186+
assertThat(evaluateCondition("local1 ^ true").isUnknown()).isTrue();
187+
assertThat(evaluateCondition("local1 ^ local").isUnknown()).isTrue();
188+
189+
// number of spawned states
190+
assertOutputStates(evaluateCondition("false ^ false"), 0, 1, 0);
191+
assertOutputStates(evaluateCondition("false ^ true"), 1, 0, 0);
192+
assertOutputStates(evaluateCondition("false ^ local2"), 1, 1, 0);
193+
assertOutputStates(evaluateCondition("true ^ false"), 1, 0, 0);
194+
assertOutputStates(evaluateCondition("true ^ true"), 0, 1, 0);
195+
assertOutputStates(evaluateCondition("true ^ local2"), 1, 1, 0);
196+
assertOutputStates(evaluateCondition("local1 ^ false"), 1, 1, 0);
197+
assertOutputStates(evaluateCondition("local1 ^ true"), 1, 1, 0);
198+
assertOutputStates(evaluateCondition("local1 ^ local2"), 2, 2, 0);
199+
}
200+
201+
@Test
202+
public void test_expression_and() {
128203
assertThat(evaluateExpression("false && false")).isSameAs(FALSE);
129204
assertThat(evaluateExpression("false && true")).isSameAs(FALSE);
130205
assertThat(evaluateExpression("false && local2")).isSameAs(FALSE);
@@ -134,10 +209,20 @@ public void test_expression_conditional_and() {
134209
assertThat(evaluateExpression("local1 && false")).isSameAs(FALSE);
135210
assertThat(evaluateExpression("local1 && true")).isSameAs(UNKNOWN);
136211
assertThat(evaluateExpression("local1 && local")).isSameAs(UNKNOWN);
212+
213+
assertThat(evaluateExpression("false & false")).isSameAs(FALSE);
214+
assertThat(evaluateExpression("false & true")).isSameAs(FALSE);
215+
assertThat(evaluateExpression("false & local2")).isSameAs(FALSE);
216+
assertThat(evaluateExpression("true & false")).isSameAs(FALSE);
217+
assertThat(evaluateExpression("true & true")).isSameAs(TRUE);
218+
assertThat(evaluateExpression("true & local2")).isSameAs(UNKNOWN);
219+
assertThat(evaluateExpression("local1 & false")).isSameAs(FALSE);
220+
assertThat(evaluateExpression("local1 & true")).isSameAs(UNKNOWN);
221+
assertThat(evaluateExpression("local1 & local")).isSameAs(UNKNOWN);
137222
}
138223

139224
@Test
140-
public void test_expression_conditional_or() {
225+
public void test_expression_or() {
141226
assertThat(evaluateExpression("false || false")).isSameAs(FALSE);
142227
assertThat(evaluateExpression("false || true")).isSameAs(TRUE);
143228
assertThat(evaluateExpression("false || local2")).isSameAs(UNKNOWN);
@@ -147,6 +232,29 @@ public void test_expression_conditional_or() {
147232
assertThat(evaluateExpression("local1 || false")).isSameAs(UNKNOWN);
148233
assertThat(evaluateExpression("local1 || true")).isSameAs(TRUE);
149234
assertThat(evaluateExpression("local1 || local")).isSameAs(UNKNOWN);
235+
236+
assertThat(evaluateExpression("false | false")).isSameAs(FALSE);
237+
assertThat(evaluateExpression("false | true")).isSameAs(TRUE);
238+
assertThat(evaluateExpression("false | local2")).isSameAs(UNKNOWN);
239+
assertThat(evaluateExpression("true | false")).isSameAs(TRUE);
240+
assertThat(evaluateExpression("true | true")).isSameAs(TRUE);
241+
assertThat(evaluateExpression("true | local2")).isSameAs(TRUE);
242+
assertThat(evaluateExpression("local1 | false")).isSameAs(UNKNOWN);
243+
assertThat(evaluateExpression("local1 | true")).isSameAs(TRUE);
244+
assertThat(evaluateExpression("local1 | local")).isSameAs(UNKNOWN);
245+
}
246+
247+
@Test
248+
public void test_expression_xor() {
249+
assertThat(evaluateExpression("false ^ false")).isSameAs(FALSE);
250+
assertThat(evaluateExpression("false ^ true")).isSameAs(TRUE);
251+
assertThat(evaluateExpression("false ^ local2")).isSameAs(UNKNOWN);
252+
assertThat(evaluateExpression("true ^ false")).isSameAs(TRUE);
253+
assertThat(evaluateExpression("true ^ true")).isSameAs(FALSE);
254+
assertThat(evaluateExpression("true ^ local2")).isSameAs(UNKNOWN);
255+
assertThat(evaluateExpression("local1 ^ false")).isSameAs(UNKNOWN);
256+
assertThat(evaluateExpression("local1 ^ true")).isSameAs(UNKNOWN);
257+
assertThat(evaluateExpression("local1 ^ local")).isSameAs(UNKNOWN);
150258
}
151259

152260
@Test

0 commit comments

Comments
 (0)