WIP(general feedback appreciated): fix/refactor(control-flow): Switch rework and multi-node statement changes

spoon-control-flow/src/main/java/fr/inria/controlflow/BranchKind.java

@@ -30,6 +30,8 @@ public enum BranchKind {
 	FINALLY,     // Represents the start of a finally block
 	BRANCH,      // Represents a branch
 	STATEMENT,   // Represents an statement
+	STATEMENT_END, // Represents the end of a multinode statement
+	EXPRESSION, // Represents an expression such as a case constant
 	BLOCK_BEGIN, // Represents the begining of a block
 	BLOCK_END,   // Represents the end of a block
 	CONVERGE,    // The exit node of all branches. Depending on the analysis it may be convenient to leave them

spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java

@@ -21,6 +21,8 @@
  */
 package fr.inria.controlflow;
 
+import spoon.reflect.code.CaseKind;
+import spoon.reflect.code.CtAbstractSwitch;
 import spoon.reflect.code.CtAnnotationFieldAccess;
 import spoon.reflect.code.CtArrayRead;
 import spoon.reflect.code.CtArrayWrite;
@@ -116,14 +118,17 @@
 import java.lang.annotation.Annotation;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Set;
 import java.util.Stack;
+import java.util.stream.Collectors;
 
 /**
  * Builds the control graph for a given snippet of code
- *
+ * <p>
  * Created by marodrig on 13/10/2015.
  */
 public class ControlFlowBuilder extends CtAbstractVisitor {
+	public static final String CONTROL_FLOW_ORIGINAL_STATEMENT = "CONTROL_FLOW_ORIGINAL_STATEMENT";
 
 	ControlFlowGraph result = new ControlFlowGraph(ControlFlowEdge.class);
 
@@ -222,7 +227,7 @@ private void visitConditional(CtElement parent, CtConditional conditional) {
 
 	/**
 	 * Returns the first graph node representing the statement s construction.
-	 *
+	 * <p>
 	 * Usually an statement is represented by many blocks and branches.
 	 * This method returns the first of those blocks/branches.
 	 *
@@ -281,7 +286,6 @@ private void defaultAction(BranchKind kind, CtStatement st) {
 
 	/**
 	 * Register the label of the statement
-	 *
 	 */
 	private void registerStatementLabel(CtStatement st) {
 		if (st.getLabel() == null || st.getLabel().isEmpty()) {
@@ -319,8 +323,8 @@ private void tryAddEdge(ControlFlowNode source, ControlFlowNode target, boolean
 		boolean isContinue = source != null && source.getStatement() instanceof CtContinue;
 
 		if (source != null && target != null
-			&& !result.containsEdge(source, target)
-			&& (isLooping || breakDance || !(isBreak || isContinue))) {
+				&& !result.containsEdge(source, target)
+				&& (isLooping || breakDance || !(isBreak || isContinue))) {
 			ControlFlowEdge e = result.addEdge(source, target);
 			e.setBackEdge(isLooping);
 		}
@@ -390,7 +394,13 @@ public <T> void visitCtBinaryOperator(CtBinaryOperator<T> operator) {
 
 	}
 
-	private <R> void travelStatementList(List<CtStatement> statements) {
+	/**
+	 * Add a list of statements as a block to the current CFG.
+	 *
+	 * @param statements The list of statements
+	 * @return The start node of the block
+	 */
+	private ControlFlowNode travelStatementList(List<CtStatement> statements) {
 		ControlFlowNode begin = new ControlFlowNode(null, result, BranchKind.BLOCK_BEGIN);
 		tryAddEdge(lastNode, begin);
 		lastNode = begin;
@@ -402,6 +412,7 @@ private <R> void travelStatementList(List<CtStatement> statements) {
 		ControlFlowNode end = new ControlFlowNode(null, result, BranchKind.BLOCK_END);
 		tryAddEdge(lastNode, end);
 		lastNode = end;
+		return begin;
 	}
 
 	@Override
@@ -652,6 +663,8 @@ public <T> void visitCtLocalVariable(CtLocalVariable<T> localVariable) {
 		registerStatementLabel(localVariable);
 		if (localVariable.getDefaultExpression() instanceof CtConditional) {
 			visitConditional(localVariable, (CtConditional) localVariable.getDefaultExpression());
+		} else if (localVariable.getDefaultExpression() instanceof CtSwitchExpression<T, ?> switchExpression) {
+			handleCtAbstractSwitch(localVariable, switchExpression);
 		} else {
 			defaultAction(BranchKind.STATEMENT, localVariable);
 		}
@@ -736,10 +749,14 @@ public <T> void visitCtParameterReference(CtParameterReference<T> reference) {
 	@Override
 	public <R> void visitCtReturn(CtReturn<R> returnStatement) {
 		registerStatementLabel(returnStatement);
-		ControlFlowNode n = new ControlFlowNode(returnStatement, result, BranchKind.STATEMENT);
-		tryAddEdge(lastNode, n);
-		tryAddEdge(n, exitNode);
-		lastNode = null; //Special case in which this node does not connect with the next, because is a return
+		if (returnStatement.getReturnedExpression() instanceof CtSwitchExpression<R, ?> switchExpression) {
+			handleCtAbstractSwitch(returnStatement, switchExpression);
+		} else {
+			ControlFlowNode n = new ControlFlowNode(returnStatement, result, BranchKind.STATEMENT);
+			tryAddEdge(lastNode, n);
+			tryAddEdge(n, exitNode);
+			lastNode = null; //Special case in which this node does not connect with the next, because is a return
+		}
 	}
 
 	@Override
@@ -758,42 +775,96 @@ public <S> void visitCtCase(CtCase<S> caseStatement) {
 
 	@Override
 	public <S> void visitCtSwitch(CtSwitch<S> switchStatement) {
-		registerStatementLabel(switchStatement);
-		//Push the condition
-		ControlFlowNode switchNode = new ControlFlowNode(switchStatement.getSelector(), result, BranchKind.BRANCH);
-		tryAddEdge(lastNode, switchNode);
+		handleCtAbstractSwitch(switchStatement, switchStatement);
+	}
+
+	public <S> void handleCtAbstractSwitch(CtStatement containingStatement, CtAbstractSwitch<S> abstractSwitch) {
+		ControlFlowNode statementNode = new ControlFlowNode(containingStatement, result, BranchKind.STATEMENT);
+		tryAddEdge(lastNode, statementNode);
+		lastNode = statementNode;
+
+		ControlFlowNode switchBlockBegin = new ControlFlowNode(null, result, BranchKind.BLOCK_BEGIN);
+		tryAddEdge(lastNode, switchBlockBegin);
+		lastNode = switchBlockBegin;
+
+		ControlFlowNode selectorNode = new ControlFlowNode(abstractSwitch.getSelector(), result, BranchKind.BRANCH);
+		tryAddEdge(lastNode, selectorNode);
+		lastNode = selectorNode;
 
-		//Create a convergence node for all the branches to converge after this
 		ControlFlowNode convergenceNode = new ControlFlowNode(null, result, BranchKind.CONVERGE);
-		//Push the convergence node so all non labeled breaks jumps there
 		breakingBad.push(convergenceNode);
 
-		lastNode = switchNode;
-		for (CtCase caseStatement : switchStatement.getCases()) {
+		ControlFlowNode fallThroughNode = null;
+		for (CtCase<?> switchCase : abstractSwitch.getCases()) {
+			boolean isEnhanced = switchCase.getCaseKind() == CaseKind.ARROW;
+
+			ControlFlowNode caseConvergenceNode = new ControlFlowNode(null, result, BranchKind.CONVERGE);
+			lastNode = caseConvergenceNode;
+			tryAddEdge(fallThroughNode, caseConvergenceNode);
 
-			//Visit Case
-			registerStatementLabel(caseStatement);
-			ControlFlowNode cn = new ControlFlowNode(caseStatement.getCaseExpression(), result, BranchKind.STATEMENT);
-			tryAddEdge(lastNode, cn);
-			if (lastNode != switchNode) {
-				tryAddEdge(switchNode, cn);
+			for (CtExpression<?> expression : switchCase.getCaseExpressions()) {
+				ControlFlowNode node = new ControlFlowNode(expression, result, BranchKind.EXPRESSION);
+				node.setTag(switchCase.getGuard());
+				tryAddEdge(selectorNode, node);
+				tryAddEdge(node, caseConvergenceNode);
 			}
-			lastNode = cn;
-			travelStatementList(caseStatement.getStatements());
-			if (lastNode.getStatement() instanceof CtBreak) {
-				lastNode = switchNode;
+			if (switchCase.getIncludesDefault() || switchCase.getCaseExpressions().isEmpty()) {
+				tryAddEdge(selectorNode, caseConvergenceNode);
+			}
+
+			for (CtStatement statement : switchCase.getStatements()) {
+				registerStatementLabel(statement);
+				statement.accept(this);
+			}
+
+			if (!isEnhanced) {
+				fallThroughNode = lastNode;
+			} else {
+				tryAddEdge(lastNode, convergenceNode);
 			}
 		}
-		tryAddEdge(lastNode, convergenceNode);
+		tryAddEdge(fallThroughNode, convergenceNode);
+
+		if (!checkExhaustive(abstractSwitch)) {
+			tryAddEdge(selectorNode, convergenceNode);
+		}
 
-		//Return as last node the convergence node
-		lastNode = convergenceNode;
 		breakingBad.pop();
+
+		ControlFlowNode switchBlockEnd = new ControlFlowNode(null, result, BranchKind.BLOCK_END);
+		tryAddEdge(convergenceNode, switchBlockEnd);
+		lastNode = switchBlockEnd;
+
+		ControlFlowNode statementEndNode = new ControlFlowNode(containingStatement, result, BranchKind.STATEMENT_END);
+		statementNode.setEndTo(statementEndNode);
+		tryAddEdge(lastNode, statementEndNode);
+		lastNode = statementEndNode;
 	}
 
-	@Override
-	public <T, S> void visitCtSwitchExpression(CtSwitchExpression<T, S> switchExpression) {
-		//TODO: missing, implementation needed
+	private boolean checkExhaustive(CtAbstractSwitch<?> switchElement) {
+		if (switchElement instanceof CtSwitchExpression<?, ?>) { // A successfully compiled switch expression is always exhaustive
+			return true;
+		}
+
+		boolean hasDefault = switchElement.getCases().stream().anyMatch(cases -> cases.getIncludesDefault() || cases.getCaseExpressions().isEmpty());
+		if (hasDefault) {
+			return true;
+		}
+
+		if (switchElement.getSelector().getType().isEnum()) {
+			Set<CtExpression<?>> caseExpressions = switchElement.getCases().stream().flatMap(cases -> cases.getCaseExpressions().stream()).collect(Collectors.toSet());
+			CtEnum<?> enumType = (CtEnum<?>) switchElement.getSelector().getType().getTypeDeclaration();
+			Set<String> enumConstantNames = enumType.getEnumValues().stream().map(CtEnumValue::getSimpleName).collect(Collectors.toSet());
+			Set<String> referencedEnumConstants = caseExpressions.stream().map(expression -> ((CtFieldRead<?>) expression).getVariable().getSimpleName()).collect(Collectors.toSet());
+
+			return referencedEnumConstants.containsAll(enumConstantNames);
+		}
+
+		CtTypeReference<?> selectorTypeReference = switchElement.getSelector().getType();
+
+		boolean isEnhanced = !selectorTypeReference.isPrimitive() && !selectorTypeReference.equals(selectorTypeReference.getFactory().Type().createReference(String.class));
+
+		return isEnhanced;
 	}
 
 	@Override
@@ -975,7 +1046,7 @@ public void visitCtTypeMemberWildcardImportReference(CtTypeMemberWildcardImportR
 
 	@Override
 	public void visitCtYieldStatement(CtYieldStatement ctYieldStatement) {
-
+		defaultAction(BranchKind.STATEMENT, ctYieldStatement);
 	}
 
 	@Override

spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowNode.java

@@ -21,14 +21,22 @@
  */
 package fr.inria.controlflow;
 
+import spoon.reflect.code.CtAbstractSwitch;
+import spoon.reflect.code.CtConditional;
+import spoon.reflect.code.CtDo;
+import spoon.reflect.code.CtFor;
+import spoon.reflect.code.CtIf;
+import spoon.reflect.code.CtLocalVariable;
+import spoon.reflect.code.CtReturn;
+import spoon.reflect.code.CtWhile;
 import spoon.reflect.declaration.CtElement;
 
 import java.util.ArrayList;
 import java.util.List;
 
 /**
  * A node of the control flow
- *
+ * <p>
  * Created by marodrig on 13/10/2015.
  */
 public class ControlFlowNode {
@@ -56,6 +64,10 @@ public void setKind(BranchKind kind) {
 	 * Statement that is going to be pointed to by this node
 	 */
 	CtElement statement;
+	/**
+	 * Whether this Node finishes the statement. See conditionals or switch expressions for an example where it might not.
+	 */
+	private boolean isStatementEnd;
 
 	List<Value> input;
 
@@ -73,14 +85,16 @@ public ControlFlowNode(CtElement statement, ControlFlowGraph parent, BranchKind
 		this.kind = kind;
 		this.parent = parent;
 		this.statement = statement;
+		this.isStatementEnd = true;
 		++count;
 		id = count;
 	}
 
 
 	public ControlFlowNode(CtElement statement, ControlFlowGraph parent) {
-		this.statement = statement;
 		this.parent = parent;
+		this.statement = statement;
+		this.isStatementEnd = true;
 		++count;
 		id = count;
 	}
@@ -147,7 +161,7 @@ public List<ControlFlowNode> prev() {
 	}
 
 	public List<Value> getOutput() {
-		if (output == null)  {
+		if (output == null) {
 			transfer();
 		}
 		return output;
@@ -161,6 +175,17 @@ public void setStatement(CtElement statement) {
 		this.statement = statement;
 	}
 
+	public boolean getIsStatementEnd() {
+		return isStatementEnd;
+	}
+
+	public void setEndTo(ControlFlowNode end) {
+		this.isStatementEnd = false;
+		setTag(end);
+		end.setTag(this);
+		end.isStatementEnd = true;
+	}
+
 	public List<Value> getInput() {
 		return input;
 	}
@@ -187,10 +212,47 @@ public void setTag(Object tag) {
 
 	@Override
 	public String toString() {
-		if (statement != null) {
-			return id + " - " + statement.toString();
-		} else {
+		if (statement == null) {
 			return kind + "_" + id;
+		} else if (kind == BranchKind.STATEMENT_END || !isStatementEnd) {
+			String prefix;
+			if (isStatementEnd && tag instanceof ControlFlowNode start) {
+				prefix = "END of " + start.id + ": " + id + " - ";
+			} else {
+				prefix = id + " - ";
+			}
+			if (statement instanceof CtAbstractSwitch<?> switchStatement) {
+				return prefix + "switch (" + switchStatement.getSelector() + ")";
+			} else if (statement instanceof CtReturn<?> returnStatement && returnStatement.getReturnedExpression() instanceof CtAbstractSwitch<?> switchExpression) {
+				return prefix + "return switch (" + switchExpression.getSelector() + ")";
+			} else if (statement instanceof CtLocalVariable<?> localVariable && localVariable.getDefaultExpression() instanceof CtAbstractSwitch<?> switchExpression) {
+				CtLocalVariable<?> renderVariable = localVariable.clone();
+				renderVariable.setDefaultExpression(null);
+				return prefix + renderVariable + " = switch (" + switchExpression.getSelector() + ")";
+			} else if (statement instanceof CtConditional<?> conditional) {
+				return prefix + conditional.getCondition() + " ?";
+			} else if (statement instanceof CtIf ifStatement) {
+				CtIf renderStatement = ifStatement.clone();
+				renderStatement.setThenStatement(null);
+				renderStatement.setElseStatement(null);
+				return prefix + "if (" + ifStatement.getCondition() + ")";
+			} else if (statement instanceof CtDo && !isStatementEnd) {
+				return prefix + "do";
+			} else if (statement instanceof CtDo doWhileLoop) {
+				return prefix + "while (" + doWhileLoop.getLoopingExpression() + ")";
+			} else if (statement instanceof CtWhile whileLoop) {
+				CtWhile renderLoop = whileLoop.clone();
+				renderLoop.setBody(null);
+				return prefix + renderLoop;
+			} else if (statement instanceof CtFor forLoop) {
+				CtFor renderLoop = forLoop.clone();
+				renderLoop.setBody(null);
+				return prefix + renderLoop;
+			} else {
+				return prefix + statement;
+			}
+		} else {
+			return id + " - " + statement;
 		}
 	}
 

spoon-control-flow/src/main/java/fr/inria/dataflow/InitializedVariables.java

@@ -152,8 +152,8 @@ private InitFactors initialized(ControlFlowNode n, HashMap<ControlFlowNode, Init
 
 
 		Set<CtVariableReference> defN = includeDefinedInNode ? defined(n) : new HashSet<CtVariableReference>();
-		//[Used_n - Def_n]
-		Set<CtVariableReference> usedN = includeDefinedInNode ? used(n) : new HashSet<CtVariableReference>();
+		//[Used_n - Def_n] - Only get used variables for single node statements. Multi node statements might define new ones that aren't in scope anymore
+		Set<CtVariableReference> usedN = (includeDefinedInNode && n.getIsStatementEnd()) ? used(n) : new HashSet<CtVariableReference>();
 		usedN.removeAll(defN);
 
 		InitFactors result = new InitFactors();

spoon-control-flow/src/test/java/fr/inria/controlflow/AllBranchesReturnTest.java

@@ -121,7 +121,7 @@ public void testSegment(AbstractProcessor processor) throws Exception {
 		//        "nestedIfSomeNotReturning", false);
 
 		Factory factory = new SpoonMetaFactory().buildNewFactory(
-				this.getClass().getResource("/control-flow").toURI().getPath(), 7);
+				this.getClass().getResource("/control-flow").toURI().getPath(), 21);
 		ProcessingManager pm = new QueueProcessingManager(factory);
 		pm.addProcessor(processor);
 		pm.process(factory.getModel().getRootPackage());