Skip to content

Commit be4d69a

Browse files
benzonicobenzonico
committed
Fix quality flaw
1 parent 4e251f9 commit be4d69a

2 files changed

Lines changed: 53 additions & 49 deletions

File tree

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

Lines changed: 39 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -191,9 +191,7 @@ private void build(List<? extends Tree> trees) {
191191
private void build(Tree tree) {
192192
switch (tree.kind()) {
193193
case BLOCK:
194-
for (StatementTree statementTree : Lists.reverse(((BlockTree) tree).body())) {
195-
build(statementTree);
196-
}
194+
build(((BlockTree) tree).body());
197195
break;
198196
case RETURN_STATEMENT:
199197
buildReturnStatement((ReturnStatementTree) tree);
@@ -310,17 +308,6 @@ private void build(Tree tree) {
310308
case NEW_CLASS:
311309
buildNewClass((NewClassTree) tree);
312310
break;
313-
case IDENTIFIER:
314-
case INT_LITERAL:
315-
case LONG_LITERAL:
316-
case DOUBLE_LITERAL:
317-
case CHAR_LITERAL:
318-
case FLOAT_LITERAL:
319-
case STRING_LITERAL:
320-
case BOOLEAN_LITERAL:
321-
case NULL_LITERAL:
322-
currentBlock.elements.add(tree);
323-
break;
324311
case TYPE_CAST:
325312
buildTypeCast(tree);
326313
break;
@@ -330,19 +317,28 @@ private void build(Tree tree) {
330317
case NEW_ARRAY:
331318
buildNewArray((NewArrayTree) tree);
332319
break;
320+
// Java 8 constructions : ignored for now.
333321
case METHOD_REFERENCE:
334-
// Java 8 constructions : ignored for now.
335-
break;
322+
// assert can be ignored by VM so skip them for now.
336323
case ASSERT_STATEMENT:
337-
// assert can be ignored by VM so skip them for now.
338324
break;
325+
// store declarations as complete blocks.
339326
case EMPTY_STATEMENT:
340327
case CLASS:
341328
case ENUM:
342329
case ANNOTATION_TYPE:
343330
case INTERFACE:
344331
case LAMBDA_EXPRESSION:
345-
// store declarations as complete blocks.
332+
// simple instructions
333+
case IDENTIFIER:
334+
case INT_LITERAL:
335+
case LONG_LITERAL:
336+
case DOUBLE_LITERAL:
337+
case CHAR_LITERAL:
338+
case FLOAT_LITERAL:
339+
case STRING_LITERAL:
340+
case BOOLEAN_LITERAL:
341+
case NULL_LITERAL:
346342
currentBlock.elements.add(tree);
347343
break;
348344
default:
@@ -485,7 +481,7 @@ private void buildSwitchStatement(SwitchStatementTree tree) {
485481
for (CaseGroupTree caseGroupTree : Lists.reverse(switchStatementTree.cases())) {
486482
build(caseGroupTree.body());
487483
switches.getLast().successors.add(currentBlock);
488-
if (caseGroupTree != firstCase) {
484+
if (!caseGroupTree.equals(firstCase)) {
489485
// No block predecessing the first case group.
490486
currentBlock = createBlock(currentBlock);
491487
}
@@ -668,8 +664,7 @@ private void buildTypeCast(Tree tree) {
668664
build(typeCastTree.expression());
669665
}
670666

671-
private void buildInstanceOf(InstanceOfTree tree) {
672-
InstanceOfTree instanceOfTree = tree;
667+
private void buildInstanceOf(InstanceOfTree instanceOfTree) {
673668
currentBlock.elements.add(instanceOfTree);
674669
build(instanceOfTree.expression());
675670
}
@@ -691,26 +686,13 @@ private Block createUnconditionalJump(Tree terminator, @Nullable Block target) {
691686

692687
private void buildCondition(Tree syntaxNode, Block trueBlock, Block falseBlock) {
693688
switch (syntaxNode.kind()) {
694-
case CONDITIONAL_OR: {
695-
BinaryExpressionTree e = (BinaryExpressionTree) syntaxNode;
696-
// process RHS
697-
buildCondition(e.rightOperand(), trueBlock, falseBlock);
698-
falseBlock = currentBlock;
699-
// process LHS
700-
currentBlock = createBranch(e, trueBlock, falseBlock);
701-
buildCondition(e.leftOperand(), trueBlock, falseBlock);
689+
case CONDITIONAL_OR:
690+
buildConditionalOr((BinaryExpressionTree) syntaxNode, trueBlock, falseBlock);
702691
break;
703-
}
704-
case CONDITIONAL_AND: {
692+
case CONDITIONAL_AND:
705693
// process RHS
706-
BinaryExpressionTree e = (BinaryExpressionTree) syntaxNode;
707-
buildCondition(e.rightOperand(), trueBlock, falseBlock);
708-
trueBlock = currentBlock;
709-
// process LHS
710-
currentBlock = createBranch(e, trueBlock, falseBlock);
711-
buildCondition(e.leftOperand(), trueBlock, falseBlock);
694+
buildConditionalAnd((BinaryExpressionTree) syntaxNode, trueBlock, falseBlock);
712695
break;
713-
}
714696
// Skip syntactic sugar:
715697
case PARENTHESIZED_EXPRESSION:
716698
buildCondition(((ParenthesizedTree) syntaxNode).expression(), trueBlock, falseBlock);
@@ -721,6 +703,23 @@ private void buildCondition(Tree syntaxNode, Block trueBlock, Block falseBlock)
721703
}
722704
}
723705

706+
private void buildConditionalOr(BinaryExpressionTree conditionalOr, Block trueBlock, Block falseBlock) {
707+
// process RHS
708+
buildCondition(conditionalOr.rightOperand(), trueBlock, falseBlock);
709+
Block newFalseBlock = currentBlock;
710+
// process LHS
711+
currentBlock = createBranch(conditionalOr, trueBlock, newFalseBlock);
712+
buildCondition(conditionalOr.leftOperand(), trueBlock, newFalseBlock);
713+
}
714+
715+
private void buildConditionalAnd(BinaryExpressionTree conditionalAnd, Block trueBlock, Block falseBlock) {
716+
buildCondition(conditionalAnd.rightOperand(), trueBlock, falseBlock);
717+
Block newTrueBlock = currentBlock;
718+
// process LHS
719+
currentBlock = createBranch(conditionalAnd, newTrueBlock, falseBlock);
720+
buildCondition(conditionalAnd.leftOperand(), newTrueBlock, falseBlock);
721+
}
722+
724723
private Block createBranch(Tree terminator, Block trueBranch, Block falseBranch) {
725724
Block result = createBlock();
726725
result.terminator = terminator;
@@ -768,6 +767,8 @@ private static String syntaxNodeToDebugString(Tree syntaxNode) {
768767
case INT_LITERAL:
769768
sb.append(' ').append(((LiteralTree) syntaxNode).token().text());
770769
break;
770+
default:
771+
//no need to debug other syntaxNodes
771772
}
772773
return sb.toString();
773774
}

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

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,21 @@
2424
import com.google.common.collect.Sets;
2525
import org.sonar.plugins.java.api.semantic.Symbol;
2626
import org.sonar.plugins.java.api.tree.AssignmentExpressionTree;
27-
import org.sonar.plugins.java.api.tree.ClassTree;
2827
import org.sonar.plugins.java.api.tree.ExpressionTree;
2928
import org.sonar.plugins.java.api.tree.IdentifierTree;
3029
import org.sonar.plugins.java.api.tree.LambdaExpressionTree;
3130
import org.sonar.plugins.java.api.tree.NewClassTree;
3231
import org.sonar.plugins.java.api.tree.Tree;
3332
import org.sonar.plugins.java.api.tree.VariableTree;
3433

34+
import javax.annotation.Nullable;
3535
import java.io.PrintStream;
36+
import java.util.Collections;
3637
import java.util.Deque;
3738
import java.util.HashMap;
3839
import java.util.HashSet;
3940
import java.util.LinkedList;
41+
import java.util.List;
4042
import java.util.Map;
4143
import java.util.Set;
4244

@@ -120,7 +122,6 @@ private void processBlockElements(CFG.Block block, Set<Symbol> blockKill, Set<Sy
120122
switch (element.kind()) {
121123
case ASSIGNMENT:
122124
ExpressionTree lhs = ((AssignmentExpressionTree) element).variable();
123-
124125
if (lhs.is(Tree.Kind.IDENTIFIER)) {
125126
symbol = ((IdentifierTree) lhs).symbol();
126127
if (isLocalVariable(symbol)) {
@@ -141,17 +142,10 @@ private void processBlockElements(CFG.Block block, Set<Symbol> blockKill, Set<Sy
141142
blockGen.remove(((VariableTree) element).symbol());
142143
break;
143144
case LAMBDA_EXPRESSION:
144-
LocalVariableReadExtractor extractor = new LocalVariableReadExtractor(cfg.methodSymbol());
145-
((LambdaExpressionTree) element).body().accept(extractor);
146-
blockGen.addAll(extractor.usedVariables());
145+
blockGen.addAll(getUsedVariables(((LambdaExpressionTree) element).body(), cfg.methodSymbol()));
147146
break;
148147
case NEW_CLASS:
149-
ClassTree body = ((NewClassTree) element).classBody();
150-
if (body != null) {
151-
LocalVariableReadExtractor extractorFromClass = new LocalVariableReadExtractor(cfg.methodSymbol());
152-
body.accept(extractorFromClass);
153-
blockGen.addAll(extractorFromClass.usedVariables());
154-
}
148+
blockGen.addAll(getUsedVariables(((NewClassTree) element).classBody(), cfg.methodSymbol()));
155149
break;
156150
default:
157151
// Ignore other kind of elements, no change of gen/kill
@@ -163,6 +157,15 @@ private static boolean isLocalVariable(Symbol symbol) {
163157
return symbol.owner().isMethodSymbol();
164158
}
165159

160+
private static List<Symbol> getUsedVariables(@Nullable Tree syntaxNode, Symbol.MethodSymbol owner) {
161+
if(syntaxNode == null) {
162+
return Collections.emptyList();
163+
}
164+
LocalVariableReadExtractor extractorFromClass = new LocalVariableReadExtractor(owner);
165+
syntaxNode.accept(extractorFromClass);
166+
return extractorFromClass.usedVariables();
167+
}
168+
166169
public void debugTo(PrintStream outStream) {
167170
for (CFG.Block block : cfg.reversedBlocks()) {
168171
outStream.println("B" + block.id + " Live:");

0 commit comments

Comments
 (0)