Skip to content

Commit b6dc73e

Browse files
benzonicobenzonico
committed
SONARJAVA-1399 Execute naively catch blocks
1 parent 03476bf commit b6dc73e

5 files changed

Lines changed: 101 additions & 43 deletions

File tree

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

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,6 @@
22
'com.google.guava:guava:src/com/google/common/cache/LocalCache.java':[
33
4053,
44
],
5-
'com.google.guava:guava:src/com/google/common/collect/ComputingConcurrentHashMap.java':[
6-
339,
7-
],
8-
'com.google.guava:guava:src/com/google/common/collect/Queues.java':[
9-
329,
10-
],
11-
'com.google.guava:guava:src/com/google/common/io/Closer.java':[
12-
224,
13-
],
145
'com.google.guava:guava:src/com/google/common/reflect/TypeVisitor.java':[
156
92,
167
],
@@ -24,9 +15,6 @@
2415
452,
2516
476,
2617
],
27-
'com.google.guava:guava:src/com/google/common/util/concurrent/MoreExecutors.java':[
28-
735,
29-
],
3018
'com.google.guava:guava:src/com/google/common/util/concurrent/SerializingExecutor.java':[
3119
147,
3220
],

‎java-squid/src/main/java/org/sonar/java/cfg/CFG.java‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.sonar.plugins.java.api.tree.BreakStatementTree;
3333
import org.sonar.plugins.java.api.tree.CaseGroupTree;
3434
import org.sonar.plugins.java.api.tree.CaseLabelTree;
35+
import org.sonar.plugins.java.api.tree.CatchTree;
3536
import org.sonar.plugins.java.api.tree.ConditionalExpressionTree;
3637
import org.sonar.plugins.java.api.tree.ContinueStatementTree;
3738
import org.sonar.plugins.java.api.tree.DoWhileStatementTree;
@@ -193,18 +194,19 @@ public Tree terminator() {
193194
}
194195

195196
public boolean isInactive() {
196-
return terminator == null && elements.isEmpty();
197+
return terminator == null && elements.isEmpty() && successors.size() == 1;
197198
}
199+
198200
private void prune(Block inactiveBlock) {
199201
if (inactiveBlock.equals(trueBlock)) {
200202
if (inactiveBlock.successors.size() != 1) {
201-
throw new IllegalStateException("True successor must be replaced by a uniuqe successor!");
203+
throw new IllegalStateException("True successor must be replaced by a unique successor!");
202204
}
203205
trueBlock = inactiveBlock.successors.iterator().next();
204206
}
205207
if (inactiveBlock.equals(falseBlock)) {
206208
if (inactiveBlock.successors.size() != 1) {
207-
throw new IllegalStateException("True successor must be replaced by a uniuqe successor!");
209+
throw new IllegalStateException("False successor must be replaced by a unique successor!");
208210
}
209211
falseBlock = inactiveBlock.successors.iterator().next();
210212
}
@@ -723,12 +725,23 @@ private void buildForStatement(ForStatementTree tree) {
723725
private void buildTryStatement(TryStatementTree tryStatementTree) {
724726
// FIXME only path with no failure constructed for now, (not taking try with resources into consideration).
725727
currentBlock = createBlock(currentBlock);
726-
BlockTree finallyBlock = tryStatementTree.finallyBlock();
727-
if (finallyBlock != null) {
728-
build(finallyBlock);
729-
}
730-
currentBlock = createBlock(currentBlock);
728+
BlockTree finallyBlockTree = tryStatementTree.finallyBlock();
729+
if (finallyBlockTree != null) {
730+
build(finallyBlockTree);
731+
}
732+
Block finallyOrEndBlock = currentBlock;
733+
Block beforeFinally = createBlock(currentBlock);
734+
List<Block> catches = new ArrayList<>();
735+
for (CatchTree catchTree : tryStatementTree.catches()) {
736+
currentBlock = createBlock(finallyOrEndBlock);
737+
build(catchTree.block());
738+
catches.add(currentBlock);
739+
}
740+
currentBlock = beforeFinally;
731741
build(tryStatementTree.block());
742+
for (Block catchBlock : catches) {
743+
currentBlock.addSuccessor(catchBlock);
744+
}
732745
build((List<? extends Tree>) tryStatementTree.resources());
733746
currentBlock = createBlock(currentBlock);
734747
currentBlock.elements.add(tryStatementTree);

‎java-squid/src/test/files/se/ConditionAlwaysTrueOrFalseCheck.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1047,16 +1047,16 @@ public void try_catch() {
10471047
b = true;
10481048
c = true;
10491049
} catch (Exception e) {
1050-
if (a) { //false negative : evaluate catch blocks Noncompliant {{Change this condition so that it does not always evaluate to "false"}}
1050+
if (a) { // Noncompliant {{Change this condition so that it does not always evaluate to "false"}}
10511051
}
1052-
if (b) { // Compliant
1052+
if (b) { // Noncompliant {{Change this condition so that it does not always evaluate to "true"}}
10531053
}
10541054
c = true;
10551055
d = true;
10561056
} catch (Exception e) {
1057-
if (a) { //false negative : evaluate catch blocks Noncompliant {{Change this condition so that it does not always evaluate to "false"}}
1057+
if (a) { // Noncompliant {{Change this condition so that it does not always evaluate to "false"}}
10581058
}
1059-
if (c) { // Compliant
1059+
if (c) { // Noncompliant {{Change this condition so that it does not always evaluate to "true"}}
10601060
}
10611061
d = true;
10621062
}
@@ -1066,8 +1066,7 @@ public void try_catch() {
10661066
}
10671067
if (c) { // Noncompliant {{Change this condition so that it does not always evaluate to "true"}}
10681068
}
1069-
//false positive !
1070-
if (d) { // Noncompliant {{Change this condition so that it does not always evaluate to "false"}}
1069+
if (d) {
10711070
}
10721071
}
10731072

‎java-squid/src/test/java/org/sonar/java/cfg/CFGTest.java‎

Lines changed: 73 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,13 @@ private static class BlockChecker {
114114
private int ifTrue = -1;
115115
private int ifFalse = -1;
116116

117+
BlockChecker(final int... ids) {
118+
if( ids.length <= 1) {
119+
throw new IllegalArgumentException("creating a block with only one successors should not be possible!");
120+
}
121+
successors(ids);
122+
}
123+
117124
BlockChecker(final Tree.Kind kind, final int... ids) {
118125
successors(ids);
119126
terminator(kind);
@@ -956,26 +963,76 @@ public void prefix_operators() {
956963

957964
@Test
958965
public void try_statement() {
959-
final CFG cfg = buildCFG("void fun() {try {System.out.println('');} finally { System.out.println(''); }}");
960-
final CFGChecker cfgChecker = checker(
961-
block(
962-
element(Tree.Kind.TRY_STATEMENT)
966+
CFG cfg = buildCFG("void fun() {try {System.out.println('');} finally { System.out.println(''); }}");
967+
CFGChecker cfgChecker = checker(
968+
block(
969+
element(Tree.Kind.TRY_STATEMENT)
963970
).successors(2),
964-
block(
965-
element(Tree.Kind.CHAR_LITERAL, "''"),
966-
element(Tree.Kind.IDENTIFIER, "System"),
967-
element(Tree.Kind.MEMBER_SELECT),
968-
element(Tree.Kind.MEMBER_SELECT),
969-
element(Tree.Kind.METHOD_INVOCATION)
971+
block(
972+
element(Tree.Kind.CHAR_LITERAL, "''"),
973+
element(Tree.Kind.IDENTIFIER, "System"),
974+
element(Tree.Kind.MEMBER_SELECT),
975+
element(Tree.Kind.MEMBER_SELECT),
976+
element(Tree.Kind.METHOD_INVOCATION)
970977
).successors(1),
971-
block(
972-
element(Tree.Kind.CHAR_LITERAL, "''"),
973-
element(Tree.Kind.IDENTIFIER, "System"),
974-
element(Tree.Kind.MEMBER_SELECT),
975-
element(Tree.Kind.MEMBER_SELECT),
976-
element(Tree.Kind.METHOD_INVOCATION)
978+
block(
979+
element(Tree.Kind.CHAR_LITERAL, "''"),
980+
element(Tree.Kind.IDENTIFIER, "System"),
981+
element(Tree.Kind.MEMBER_SELECT),
982+
element(Tree.Kind.MEMBER_SELECT),
983+
element(Tree.Kind.METHOD_INVOCATION)
977984
).successors(0));
978985
cfgChecker.check(cfg);
986+
cfg = buildCFG("void fun() {try {System.out.println('');} catch(IllegalArgumentException e) { foo('iae');} catch(Exception e){foo('e');}" +
987+
" finally { System.out.println('finally'); }}");
988+
cfgChecker = checker(
989+
block(
990+
element(Tree.Kind.TRY_STATEMENT)
991+
).successors(2),
992+
block(
993+
element(Tree.Kind.CHAR_LITERAL, "'e'"),
994+
element(Tree.Kind.IDENTIFIER, "foo"),
995+
element(Tree.Kind.METHOD_INVOCATION)
996+
).successors(1),
997+
block(
998+
element(Tree.Kind.CHAR_LITERAL, "'iae'"),
999+
element(Tree.Kind.IDENTIFIER, "foo"),
1000+
element(Tree.Kind.METHOD_INVOCATION)
1001+
).successors(1),
1002+
block(
1003+
element(Tree.Kind.CHAR_LITERAL, "''"),
1004+
element(Tree.Kind.IDENTIFIER, "System"),
1005+
element(Tree.Kind.MEMBER_SELECT),
1006+
element(Tree.Kind.MEMBER_SELECT),
1007+
element(Tree.Kind.METHOD_INVOCATION)
1008+
).successors(1, 3, 4),
1009+
block(
1010+
element(Tree.Kind.CHAR_LITERAL, "'finally'"),
1011+
element(Tree.Kind.IDENTIFIER, "System"),
1012+
element(Tree.Kind.MEMBER_SELECT),
1013+
element(Tree.Kind.MEMBER_SELECT),
1014+
element(Tree.Kind.METHOD_INVOCATION)
1015+
).successors(0)
1016+
);
1017+
cfgChecker.check(cfg);
1018+
cfg = buildCFG(
1019+
" private void f() {\n" +
1020+
" try {\n" +
1021+
" } catch (Exception e) {\n" +
1022+
" if (e instanceof IOException) { \n" +
1023+
" }\n}}");
1024+
cfgChecker = checker(
1025+
block(
1026+
element(Tree.Kind.TRY_STATEMENT)
1027+
).successors(1),
1028+
block(
1029+
element(Tree.Kind.IDENTIFIER, "e"),
1030+
element(Tree.Kind.INSTANCE_OF)
1031+
).terminator(Tree.Kind.IF_STATEMENT).ifTrue(0).ifFalse(0),
1032+
new BlockChecker(0, 2) // paritcular case of a block with multiple successors but no instructions.
1033+
);
1034+
cfgChecker.check(cfg);
1035+
9791036
}
9801037

9811038
@Test

‎java-squid/src/test/java/org/sonar/java/model/CFGDebug.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ public static String toString(CFG.Block block) {
7878
buffer.append(successor.id());
7979
if (successor == block.trueBlock()) {
8080
buffer.append("(true)");
81-
} else if (successor == block.falseBlock()) {
81+
}
82+
if (successor == block.falseBlock()) {
8283
buffer.append("(false)");
8384
}
8485
}

0 commit comments

Comments
 (0)