From 6362bce7b3f1caac1442fb32da31b905992e1c33 Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Fri, 5 Apr 2024 00:23:54 +0200 Subject: [PATCH 01/24] Add test case for correct switch behaviour with implicit default case --- .../controlflow/ControlFlowPathHelper.java | 128 +++++++ .../inria/controlflow/ExceptionFlowTests.java | 320 ++++++------------ .../ForwardFlowBuilderVisitorTest.java | 12 + 3 files changed, 242 insertions(+), 218 deletions(-) create mode 100644 spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java new file mode 100644 index 00000000000..46e77a47648 --- /dev/null +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java @@ -0,0 +1,128 @@ +package fr.inria.controlflow; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +public class ControlFlowPathHelper { + /** + * Memoization of paths. + */ + Map>> pathsMemo = new HashMap<>(); + + /** + * Get the set of possible paths to the exit node from a given starting node. + * + * @param node Starting node + * @return Set of possible paths + */ + private List> paths(ControlFlowNode node) { + if (pathsMemo.containsKey(node)) { + return pathsMemo.get(node); + } + + List> result = new ArrayList<>(); + + for (ControlFlowNode nextNode : node.next()) { + result.add(new ArrayList<>(Arrays.asList(node, nextNode))); + } + + result = paths(result); + pathsMemo.put(node, result); + return result; + } + + /** + * Get the set of possible paths to the exit node given a set of potentially incomplete paths. + * + * @param prior Set of potentially incomplete paths + * @return Set of possible paths + */ + private List> paths(List> prior) { + List> result = new ArrayList<>(); + boolean extended = false; + + for (List path : prior) { + ControlFlowNode lastNode = path.get(path.size() - 1); + + if (lastNode.getKind() == BranchKind.EXIT) { + result.add(new ArrayList<>(path)); + } else { + for (ControlFlowNode nextNode : lastNode.next()) { + extended = true; + List thisPath = new ArrayList<>(path); + thisPath.add(nextNode); + result.add(thisPath); + } + } + } + + if (extended) { + return paths(result); + } else { + return result; + } + } + + /** + * Check whether a path contains a catch block node. + * + * @param nodes Path to check + * @return True if path contains a catch block node, false otherwise + */ + private boolean containsCatchBlockNode(List nodes) { + return nodes.stream().anyMatch(node -> node.getKind() == BranchKind.CATCH); + } + + /** + * Check whether a node has a path to the exit node that does not enter a catch block. + * + * @param node Node to check + * @return True if node has path to exit that does not enter any catch block, false otherwise + */ + boolean canReachExitWithoutEnteringCatchBlock(ControlFlowNode node) { + return paths(node).stream().anyMatch(xs -> !containsCatchBlockNode(xs)); + } + + /** + * Check whether a node has a path to another node. + * + * @param source Starting node + * @param target Target node + * @return True if there is a path from source to target, false otherwise + */ + boolean canReachNode(ControlFlowNode source, ControlFlowNode target) { + return paths(source).stream().anyMatch(xs -> xs.contains(target)); + } + + /** + * Check whether a node can reach the exit without crossing a certain node. + * + * @param source Starting node + * @param avoid Avoid node + * @return True if there exists a path between source and exit that does not include avoid, false otherwise + */ + boolean canAvoidNode(ControlFlowNode source, ControlFlowNode avoid) { + return !paths(source).stream().allMatch(xs -> xs.contains(avoid)); + } + + /** + * Find a node in a ControlFlowGraph by matching on the string representation of the statement + * stored in the node (if any). + * + * @param graph Graph to search + * @param s String to match against statement + * @return First node found with statement matching string, or null if none was found + */ + ControlFlowNode findNodeByString(ControlFlowGraph graph, String s) { + for (ControlFlowNode node : graph.vertexSet()) { + if (node.getStatement() != null && node.getStatement().toString().equals(s)) { + return node; + } + } + + return null; + } +} diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ExceptionFlowTests.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ExceptionFlowTests.java index 18dbcc678db..677142565cd 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ExceptionFlowTests.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ExceptionFlowTests.java @@ -12,6 +12,9 @@ import static org.junit.jupiter.api.Assertions.assertTrue; public class ExceptionFlowTests { + + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + @Test public void testBasicSingle() { @@ -36,24 +39,24 @@ public void testBasicSingle() { builder.build(method); ControlFlowGraph cfg = builder.getResult(); - ControlFlowNode x = findNodeByString(cfg, "x()"); - ControlFlowNode a = findNodeByString(cfg, "a()"); - ControlFlowNode b = findNodeByString(cfg, "b()"); - ControlFlowNode c = findNodeByString(cfg, "c()"); - ControlFlowNode bang = findNodeByString(cfg, "bang()"); + ControlFlowNode x = pathHelper.findNodeByString(cfg, "x()"); + ControlFlowNode a = pathHelper.findNodeByString(cfg, "a()"); + ControlFlowNode b = pathHelper.findNodeByString(cfg, "b()"); + ControlFlowNode c = pathHelper.findNodeByString(cfg, "c()"); + ControlFlowNode bang = pathHelper.findNodeByString(cfg, "bang()"); - assertFalse(canReachNode(x, bang)); + assertFalse(pathHelper.canReachNode(x, bang)); - assertTrue(canReachExitWithoutEnteringCatchBlock(x)); - assertTrue(canReachExitWithoutEnteringCatchBlock(a)); - assertTrue(canReachExitWithoutEnteringCatchBlock(b)); - assertTrue(canReachExitWithoutEnteringCatchBlock(c)); + assertTrue(pathHelper.canReachExitWithoutEnteringCatchBlock(x)); + assertTrue(pathHelper.canReachExitWithoutEnteringCatchBlock(a)); + assertTrue(pathHelper.canReachExitWithoutEnteringCatchBlock(b)); + assertTrue(pathHelper.canReachExitWithoutEnteringCatchBlock(c)); - assertTrue(canReachNode(a, bang)); - assertTrue(canReachNode(a, b)); - assertTrue(canReachNode(b, bang)); - assertTrue(canReachNode(b, c)); - assertTrue(canReachNode(c, bang)); + assertTrue(pathHelper.canReachNode(a, bang)); + assertTrue(pathHelper.canReachNode(a, b)); + assertTrue(pathHelper.canReachNode(b, bang)); + assertTrue(pathHelper.canReachNode(b, c)); + assertTrue(pathHelper.canReachNode(c, bang)); } @Test @@ -83,23 +86,23 @@ public void testBasicDouble() { builder.build(method); ControlFlowGraph cfg = builder.getResult(); - ControlFlowNode x = findNodeByString(cfg, "x()"); - ControlFlowNode a = findNodeByString(cfg, "a()"); - ControlFlowNode b = findNodeByString(cfg, "b()"); - ControlFlowNode bang = findNodeByString(cfg, "bang()"); - ControlFlowNode boom = findNodeByString(cfg, "boom()"); + ControlFlowNode x = pathHelper.findNodeByString(cfg, "x()"); + ControlFlowNode a = pathHelper.findNodeByString(cfg, "a()"); + ControlFlowNode b = pathHelper.findNodeByString(cfg, "b()"); + ControlFlowNode bang = pathHelper.findNodeByString(cfg, "bang()"); + ControlFlowNode boom = pathHelper.findNodeByString(cfg, "boom()"); - assertFalse(canReachNode(x, bang)); - assertTrue(canReachNode(x, boom)); + assertFalse(pathHelper.canReachNode(x, bang)); + assertTrue(pathHelper.canReachNode(x, boom)); - assertTrue(canReachExitWithoutEnteringCatchBlock(x)); - assertTrue(canReachExitWithoutEnteringCatchBlock(a)); - assertTrue(canReachExitWithoutEnteringCatchBlock(b)); + assertTrue(pathHelper.canReachExitWithoutEnteringCatchBlock(x)); + assertTrue(pathHelper.canReachExitWithoutEnteringCatchBlock(a)); + assertTrue(pathHelper.canReachExitWithoutEnteringCatchBlock(b)); - assertTrue(canReachNode(a, bang)); + assertTrue(pathHelper.canReachNode(a, bang)); - assertTrue(canReachNode(b, boom)); - assertFalse(canReachNode(b, bang)); + assertTrue(pathHelper.canReachNode(b, boom)); + assertFalse(pathHelper.canReachNode(b, bang)); } @Test @@ -129,23 +132,23 @@ public void testBasicNested() { builder.build(method); ControlFlowGraph cfg = builder.getResult(); - ControlFlowNode a = findNodeByString(cfg, "a()"); - ControlFlowNode b = findNodeByString(cfg, "b()"); - ControlFlowNode c = findNodeByString(cfg, "c()"); - ControlFlowNode boom = findNodeByString(cfg, "boom()"); - ControlFlowNode bang = findNodeByString(cfg, "bang()"); - - assertTrue(canReachExitWithoutEnteringCatchBlock(a)); - assertTrue(canReachExitWithoutEnteringCatchBlock(b)); - assertTrue(canReachExitWithoutEnteringCatchBlock(c)); - assertTrue(canReachExitWithoutEnteringCatchBlock(boom)); - - assertTrue(canReachNode(a, boom)); - assertTrue(canReachNode(a, bang)); - assertTrue(canReachNode(b, boom)); - assertTrue(canReachNode(b, bang)); - assertFalse(canReachNode(c, boom)); - assertTrue(canReachNode(c, bang)); + ControlFlowNode a = pathHelper.findNodeByString(cfg, "a()"); + ControlFlowNode b = pathHelper.findNodeByString(cfg, "b()"); + ControlFlowNode c = pathHelper.findNodeByString(cfg, "c()"); + ControlFlowNode boom = pathHelper.findNodeByString(cfg, "boom()"); + ControlFlowNode bang = pathHelper.findNodeByString(cfg, "bang()"); + + assertTrue(pathHelper.canReachExitWithoutEnteringCatchBlock(a)); + assertTrue(pathHelper.canReachExitWithoutEnteringCatchBlock(b)); + assertTrue(pathHelper.canReachExitWithoutEnteringCatchBlock(c)); + assertTrue(pathHelper.canReachExitWithoutEnteringCatchBlock(boom)); + + assertTrue(pathHelper.canReachNode(a, boom)); + assertTrue(pathHelper.canReachNode(a, bang)); + assertTrue(pathHelper.canReachNode(b, boom)); + assertTrue(pathHelper.canReachNode(b, bang)); + assertFalse(pathHelper.canReachNode(c, boom)); + assertTrue(pathHelper.canReachNode(c, bang)); } @Test @@ -175,23 +178,23 @@ public void testMultipleCatchers() { builder.build(method); ControlFlowGraph cfg = builder.getResult(); - ControlFlowNode top = findNodeByString(cfg, "top()"); - ControlFlowNode a = findNodeByString(cfg, "a()"); - ControlFlowNode b = findNodeByString(cfg, "b()"); - ControlFlowNode c = findNodeByString(cfg, "c()"); + ControlFlowNode top = pathHelper.findNodeByString(cfg, "top()"); + ControlFlowNode a = pathHelper.findNodeByString(cfg, "a()"); + ControlFlowNode b = pathHelper.findNodeByString(cfg, "b()"); + ControlFlowNode c = pathHelper.findNodeByString(cfg, "c()"); - assertTrue(canReachExitWithoutEnteringCatchBlock(top)); - assertTrue(canReachExitWithoutEnteringCatchBlock(a)); + assertTrue(pathHelper.canReachExitWithoutEnteringCatchBlock(top)); + assertTrue(pathHelper.canReachExitWithoutEnteringCatchBlock(a)); - assertTrue(canReachNode(top, a)); - assertTrue(canReachNode(top, b)); - assertTrue(canReachNode(top, c)); + assertTrue(pathHelper.canReachNode(top, a)); + assertTrue(pathHelper.canReachNode(top, b)); + assertTrue(pathHelper.canReachNode(top, c)); - assertTrue(canReachNode(a, b)); - assertTrue(canReachNode(a, c)); + assertTrue(pathHelper.canReachNode(a, b)); + assertTrue(pathHelper.canReachNode(a, c)); - assertFalse(canReachNode(b, c)); - assertFalse(canReachNode(c, b)); + assertFalse(pathHelper.canReachNode(b, c)); + assertFalse(pathHelper.canReachNode(c, b)); } @Test @@ -220,16 +223,16 @@ public void testThrowStatement() { builder.build(method); ControlFlowGraph cfg = builder.getResult(); - assertNull(findNodeByString(cfg, "unreachable()")); + assertNull(pathHelper.findNodeByString(cfg, "unreachable()")); - ControlFlowNode throwstmt = findNodeByString(cfg, "throw new RuntimeException()"); - ControlFlowNode boom = findNodeByString(cfg, "boom()"); - ControlFlowNode bang = findNodeByString(cfg, "bang()"); + ControlFlowNode throwstmt = pathHelper.findNodeByString(cfg, "throw new RuntimeException()"); + ControlFlowNode boom = pathHelper.findNodeByString(cfg, "boom()"); + ControlFlowNode bang = pathHelper.findNodeByString(cfg, "bang()"); - assertTrue(canReachNode(throwstmt, boom)); - assertTrue(canReachNode(throwstmt, bang)); + assertTrue(pathHelper.canReachNode(throwstmt, boom)); + assertTrue(pathHelper.canReachNode(throwstmt, bang)); - assertFalse(canReachExitWithoutEnteringCatchBlock(throwstmt)); + assertFalse(pathHelper.canReachExitWithoutEnteringCatchBlock(throwstmt)); } @Test @@ -253,7 +256,7 @@ public void testAddPathsForEmptyTryBlocksDisabled() { builder.build(method); ControlFlowGraph cfg = builder.getResult(); - assertNull(findNodeByString(cfg, "bang()")); + assertNull(pathHelper.findNodeByString(cfg, "bang()")); } @Test @@ -278,7 +281,7 @@ public void testAddPathsForEmptyTryBlocksEnabled() { builder.build(method); ControlFlowGraph cfg = builder.getResult(); - assertNotNull(findNodeByString(cfg, "bang()")); + assertNotNull(pathHelper.findNodeByString(cfg, "bang()")); } @Test @@ -397,18 +400,18 @@ public void testFinalizer() { builder.build(method); ControlFlowGraph cfg = builder.getResult(); - ControlFlowNode top = findNodeByString(cfg, "top()"); - ControlFlowNode a = findNodeByString(cfg, "a()"); - ControlFlowNode b = findNodeByString(cfg, "b()"); - ControlFlowNode c = findNodeByString(cfg, "c()"); - - assertFalse(canAvoidNode(top, c)); - assertTrue(canReachNode(top, b)); - assertTrue(canAvoidNode(top, b)); - assertFalse(canAvoidNode(a, c)); - assertTrue(canReachNode(a, b)); - assertTrue(canAvoidNode(a, b)); - assertFalse(canAvoidNode(b, c)); + ControlFlowNode top = pathHelper.findNodeByString(cfg, "top()"); + ControlFlowNode a = pathHelper.findNodeByString(cfg, "a()"); + ControlFlowNode b = pathHelper.findNodeByString(cfg, "b()"); + ControlFlowNode c = pathHelper.findNodeByString(cfg, "c()"); + + assertFalse(pathHelper.canAvoidNode(top, c)); + assertTrue(pathHelper.canReachNode(top, b)); + assertTrue(pathHelper.canAvoidNode(top, b)); + assertFalse(pathHelper.canAvoidNode(a, c)); + assertTrue(pathHelper.canReachNode(a, b)); + assertTrue(pathHelper.canAvoidNode(a, b)); + assertFalse(pathHelper.canAvoidNode(b, c)); } @Test @@ -436,13 +439,13 @@ public void testCatchlessTry() { builder.build(method); ControlFlowGraph cfg = builder.getResult(); - ControlFlowNode top = findNodeByString(cfg, "top()"); - ControlFlowNode a = findNodeByString(cfg, "a()"); - ControlFlowNode b = findNodeByString(cfg, "b()"); + ControlFlowNode top = pathHelper.findNodeByString(cfg, "top()"); + ControlFlowNode a = pathHelper.findNodeByString(cfg, "a()"); + ControlFlowNode b = pathHelper.findNodeByString(cfg, "b()"); - assertFalse(canAvoidNode(top, a)); - assertFalse(canAvoidNode(top, b)); - assertFalse(canAvoidNode(a, b)); + assertFalse(pathHelper.canAvoidNode(top, a)); + assertFalse(pathHelper.canAvoidNode(top, b)); + assertFalse(pathHelper.canAvoidNode(a, b)); } @Test @@ -476,16 +479,16 @@ public void testMultipleCatchersWithFinalizer() { builder.build(method); ControlFlowGraph cfg = builder.getResult(); - ControlFlowNode top = findNodeByString(cfg, "top()"); - ControlFlowNode a = findNodeByString(cfg, "a()"); - ControlFlowNode b = findNodeByString(cfg, "b()"); - ControlFlowNode c = findNodeByString(cfg, "c()"); - ControlFlowNode breathe = findNodeByString(cfg, "breathe()"); + ControlFlowNode top = pathHelper.findNodeByString(cfg, "top()"); + ControlFlowNode a = pathHelper.findNodeByString(cfg, "a()"); + ControlFlowNode b = pathHelper.findNodeByString(cfg, "b()"); + ControlFlowNode c = pathHelper.findNodeByString(cfg, "c()"); + ControlFlowNode breathe = pathHelper.findNodeByString(cfg, "breathe()"); - assertFalse(canAvoidNode(top, breathe)); - assertFalse(canAvoidNode(a, breathe)); - assertFalse(canAvoidNode(b, breathe)); - assertFalse(canAvoidNode(c, breathe)); + assertFalse(pathHelper.canAvoidNode(top, breathe)); + assertFalse(pathHelper.canAvoidNode(a, breathe)); + assertFalse(pathHelper.canAvoidNode(b, breathe)); + assertFalse(pathHelper.canAvoidNode(c, breathe)); } @Test @@ -518,130 +521,11 @@ public void testFinalizerReturnStatementWithSimplifyingOption() { ControlFlowGraph cfg = builder.getResult(); System.out.println(cfg.toGraphVisText()); - ControlFlowNode ret = findNodeByString(cfg, "return"); - ControlFlowNode a = findNodeByString(cfg, "a()"); - ControlFlowNode b = findNodeByString(cfg, "b()"); - - assertTrue(canAvoidNode(ret, a)); - assertTrue(canAvoidNode(ret, b)); - } - - /** - * Memoization of paths. - */ - Map>> pathsMemo = new HashMap<>(); - - /** - * Get the set of possible paths to the exit node from a given starting node. - * - * @param node Starting node - * @return Set of possible paths - */ - private List> paths(ControlFlowNode node) { - if (pathsMemo.containsKey(node)) { - return pathsMemo.get(node); - } - - List> result = new ArrayList<>(); - - for (ControlFlowNode nextNode : node.next()) { - result.add(new ArrayList<>(Arrays.asList(node, nextNode))); - } - - result = paths(result); - pathsMemo.put(node, result); - return result; - } - - /** - * Get the set of possible paths to the exit node given a set of potentially incomplete paths. - * - * @param prior Set of potentially incomplete paths - * @return Set of possible paths - */ - private List> paths(List> prior) { - List> result = new ArrayList<>(); - boolean extended = false; - - for (List path : prior) { - ControlFlowNode lastNode = path.get(path.size() - 1); - - if (lastNode.getKind() == BranchKind.EXIT) { - result.add(new ArrayList<>(path)); - } else { - for (ControlFlowNode nextNode : lastNode.next()) { - extended = true; - List thisPath = new ArrayList<>(path); - thisPath.add(nextNode); - result.add(thisPath); - } - } - } - - if (extended) { - return paths(result); - } else { - return result; - } - } - - /** - * Check whether a path contains a catch block node. - * - * @param nodes Path to check - * @return True if path contains a catch block node, false otherwise - */ - private boolean containsCatchBlockNode(List nodes) { - return nodes.stream().anyMatch(node -> node.getKind() == BranchKind.CATCH); - } - - /** - * Check whether a node has a path to the exit node that does not enter a catch block. - * - * @param node Node to check - * @return True if node has path to exit that does not enter any catch block, false otherwise - */ - private boolean canReachExitWithoutEnteringCatchBlock(ControlFlowNode node) { - return paths(node).stream().anyMatch(xs -> !containsCatchBlockNode(xs)); - } - - /** - * Check whether a node has a path to another node. - * - * @param source Starting node - * @param target Target node - * @return True if there is a path from source to target, false otherwise - */ - private boolean canReachNode(ControlFlowNode source, ControlFlowNode target) { - return paths(source).stream().anyMatch(xs -> xs.contains(target)); - } - - /** - * Check whether a node can reach the exit without crossing a certain node. - * - * @param source Starting node - * @param target Target node - * @return True if there exists a path between source and exit that does not include target, false otherwise - */ - private boolean canAvoidNode(ControlFlowNode source, ControlFlowNode target) { - return !paths(source).stream().allMatch(xs -> xs.contains(target)); - } + ControlFlowNode ret = pathHelper.findNodeByString(cfg, "return"); + ControlFlowNode a = pathHelper.findNodeByString(cfg, "a()"); + ControlFlowNode b = pathHelper.findNodeByString(cfg, "b()"); - /** - * Find a node in a ControlFlowGraph by matching on the string representation of the statement - * stored in the node (if any). - * - * @param graph Graph to search - * @param s String to match against statement - * @return First node found with statement matching string, or null if none was found - */ - private ControlFlowNode findNodeByString(ControlFlowGraph graph, String s) { - for (ControlFlowNode node : graph.vertexSet()) { - if (node.getStatement() != null && node.getStatement().toString().equals(s)) { - return node; - } - } - - return null; + assertTrue(pathHelper.canAvoidNode(ret, a)); + assertTrue(pathHelper.canAvoidNode(ret, b)); } } diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java index 2ddc71abbd1..eaac851c095 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java @@ -35,6 +35,7 @@ import static fr.inria.controlflow.BranchKind.*; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertFalse; /** @@ -172,6 +173,17 @@ public void testSwitchFallThrough() throws Exception { testMethod("lastCaseFallThrough", false, 1, 4, 12); } + @Test + public void testSwitchImplicitDefault() throws Exception { + ControlFlowGraph graph = testMethod("lastCaseFallThrough", false, 1, 4, 12); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entryNode = pathHelper.findNodeByString(graph, "int b = 0"); + ControlFlowNode caseNode = pathHelper.findNodeByString(graph, "b = 1"); + boolean canAvoid = pathHelper.canAvoidNode(entryNode, caseNode); + assertTrue(canAvoid); + } + //Test some mixed conditions @Test public void testSimple() throws Exception { From 0d775f57b1e959fdebb0a7521cb813dfe7adee67 Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Fri, 5 Apr 2024 00:45:57 +0200 Subject: [PATCH 02/24] Handle implicit default case in control flow graph --- .../java/fr/inria/controlflow/ControlFlowBuilder.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java index fbf881d0215..c18e437a9ef 100644 --- a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java +++ b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java @@ -768,12 +768,14 @@ public void visitCtSwitch(CtSwitch switchStatement) { //Push the convergence node so all non labeled breaks jumps there breakingBad.push(convergenceNode); + boolean hasDefaultCase = false; lastNode = switchNode; - for (CtCase caseStatement : switchStatement.getCases()) { + for (CtCase caseStatement : switchStatement.getCases()) { //Visit Case registerStatementLabel(caseStatement); ControlFlowNode cn = new ControlFlowNode(caseStatement.getCaseExpression(), result, BranchKind.STATEMENT); + hasDefaultCase |= caseStatement.getCaseExpressions().isEmpty(); tryAddEdge(lastNode, cn); if (lastNode != switchNode) { tryAddEdge(switchNode, cn); @@ -786,6 +788,10 @@ public void visitCtSwitch(CtSwitch switchStatement) { } tryAddEdge(lastNode, convergenceNode); + if (!hasDefaultCase) { + tryAddEdge(switchNode, convergenceNode); + } + //Return as last node the convergence node lastNode = convergenceNode; breakingBad.pop(); From 3d5b626c8c5653c4685b817ea0bfefaa0044da2d Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Fri, 5 Apr 2024 00:56:06 +0200 Subject: [PATCH 03/24] Add test for correct handling of case statement with multiple expressions --- .../fr/inria/controlflow/ControlFlowPathHelper.java | 2 +- .../controlflow/ForwardFlowBuilderVisitorTest.java | 11 +++++++++++ .../control-flow/ControlFlowArithmetic.java | 12 ++++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java index 46e77a47648..3b7bc50bf47 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java @@ -18,7 +18,7 @@ public class ControlFlowPathHelper { * @param node Starting node * @return Set of possible paths */ - private List> paths(ControlFlowNode node) { + List> paths(ControlFlowNode node) { if (pathsMemo.containsKey(node)) { return pathsMemo.get(node); } diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java index eaac851c095..4f45156844d 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java @@ -31,6 +31,7 @@ import spoon.support.QueueProcessingManager; import java.io.PrintWriter; +import java.util.List; import java.net.URISyntaxException; import static fr.inria.controlflow.BranchKind.*; @@ -184,6 +185,16 @@ public void testSwitchImplicitDefault() throws Exception { assertTrue(canAvoid); } + @Test + public void testMultipleCaseExpressions() throws Exception { + ControlFlowGraph graph = testMethod("multipleCaseExpressions", true, 1, 8, 17); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode startNode = pathHelper.findNodeByString(graph, "int b = 0"); + List> paths = pathHelper.paths(startNode); + assertTrue(paths.size() > 2); + } + //Test some mixed conditions @Test public void testSimple() throws Exception { diff --git a/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java b/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java index 776f57bb71b..16489fafa98 100644 --- a/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java +++ b/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java @@ -377,6 +377,18 @@ public int lastCaseFallThrough(int a) { return b; } + public int multipleCaseExpressions(int a) { + int b = 0; + switch (a) { + case 1, 2: + b = 1; + break; + default: + break; + } + return b; + } + //All lines will be tested in this method public int simple(int a) { a = a + a / 2; From c706605d6a167b07ee43ee97e2ae02b176fe26f5 Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Fri, 5 Apr 2024 01:28:41 +0200 Subject: [PATCH 04/24] Handle multiple case expressions As a side effect, fall throughs now go to the next block, not the next case expression --- .../inria/controlflow/ControlFlowBuilder.java | 43 +++++++++++++++---- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java index c18e437a9ef..4641818f777 100644 --- a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java +++ b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java @@ -114,6 +114,7 @@ import spoon.reflect.visitor.CtAbstractVisitor; import java.lang.annotation.Annotation; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Stack; @@ -390,7 +391,12 @@ public void visitCtBinaryOperator(CtBinaryOperator operator) { } - private void travelStatementList(List 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 statements) { ControlFlowNode begin = new ControlFlowNode(null, result, BranchKind.BLOCK_BEGIN); tryAddEdge(lastNode, begin); lastNode = begin; @@ -402,6 +408,7 @@ private void travelStatementList(List statements) { ControlFlowNode end = new ControlFlowNode(null, result, BranchKind.BLOCK_END); tryAddEdge(lastNode, end); lastNode = end; + return begin; } @Override @@ -774,14 +781,34 @@ public void visitCtSwitch(CtSwitch switchStatement) { //Visit Case registerStatementLabel(caseStatement); - ControlFlowNode cn = new ControlFlowNode(caseStatement.getCaseExpression(), result, BranchKind.STATEMENT); - hasDefaultCase |= caseStatement.getCaseExpressions().isEmpty(); - tryAddEdge(lastNode, cn); - if (lastNode != switchNode) { - tryAddEdge(switchNode, cn); + var caseExpressions = caseStatement.getCaseExpressions(); + ArrayList caseExpressionNodes = new ArrayList<>(); + for (CtExpression expression: caseExpressions) { + ControlFlowNode caseNode = new ControlFlowNode(expression, result, BranchKind.STATEMENT); + caseExpressionNodes.add(caseNode); + tryAddEdge(switchNode, caseNode); + } + + if (caseExpressionNodes.isEmpty()) { + hasDefaultCase = true; + ControlFlowNode defaultNode = new ControlFlowNode(null, result, BranchKind.STATEMENT); + caseExpressionNodes.add(defaultNode); + tryAddEdge(switchNode, defaultNode); + } + + ControlFlowNode fallThroughEnd = null; + if(lastNode != switchNode) { + fallThroughEnd = lastNode; } - lastNode = cn; - travelStatementList(caseStatement.getStatements()); + lastNode = null; + + ControlFlowNode blockStart = travelStatementList(caseStatement.getStatements()); + tryAddEdge(fallThroughEnd, blockStart); + + for (ControlFlowNode expressionNode : caseExpressionNodes) { + tryAddEdge(expressionNode, blockStart); + } + if (lastNode.getStatement() instanceof CtBreak) { lastNode = switchNode; } From ee5d7e7f6e270b9738050573b150aeb1c1a1f98f Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Fri, 5 Apr 2024 02:06:55 +0200 Subject: [PATCH 05/24] Control Flow helper top level javaDoc --- .../java/fr/inria/controlflow/ControlFlowPathHelper.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java index 3b7bc50bf47..9e5edff5e96 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java @@ -6,11 +6,14 @@ import java.util.List; import java.util.Map; +/** + * A helper class for analyzing paths in the control flow graph + */ public class ControlFlowPathHelper { /** * Memoization of paths. */ - Map>> pathsMemo = new HashMap<>(); + private final Map>> pathsMemo = new HashMap<>(); /** * Get the set of possible paths to the exit node from a given starting node. From 8b7f6092e95fb926d959712829f05a6516a7a315 Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Fri, 5 Apr 2024 02:09:33 +0200 Subject: [PATCH 06/24] Messages for `assertTrue`s --- .../fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java index 4f45156844d..6c12f71b98c 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java @@ -182,7 +182,7 @@ public void testSwitchImplicitDefault() throws Exception { ControlFlowNode entryNode = pathHelper.findNodeByString(graph, "int b = 0"); ControlFlowNode caseNode = pathHelper.findNodeByString(graph, "b = 1"); boolean canAvoid = pathHelper.canAvoidNode(entryNode, caseNode); - assertTrue(canAvoid); + assertTrue(canAvoid, "Path for implicit default case missing"); } @Test @@ -192,7 +192,7 @@ public void testMultipleCaseExpressions() throws Exception { ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); ControlFlowNode startNode = pathHelper.findNodeByString(graph, "int b = 0"); List> paths = pathHelper.paths(startNode); - assertTrue(paths.size() > 2); + assertTrue(paths.size() > 2, "Not enough paths. Possibly missing different paths from multiple expressions for a case"); } //Test some mixed conditions From 433c1c216f01d180dec08f3e7c192de6f7ac94f5 Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Fri, 5 Apr 2024 12:23:59 +0200 Subject: [PATCH 07/24] Make Checkstyle happy --- .../main/java/fr/inria/controlflow/ControlFlowBuilder.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java index 4641818f777..3afc870b557 100644 --- a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java +++ b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java @@ -783,7 +783,7 @@ public void visitCtSwitch(CtSwitch switchStatement) { registerStatementLabel(caseStatement); var caseExpressions = caseStatement.getCaseExpressions(); ArrayList caseExpressionNodes = new ArrayList<>(); - for (CtExpression expression: caseExpressions) { + for (CtExpression expression : caseExpressions) { ControlFlowNode caseNode = new ControlFlowNode(expression, result, BranchKind.STATEMENT); caseExpressionNodes.add(caseNode); tryAddEdge(switchNode, caseNode); @@ -797,7 +797,7 @@ public void visitCtSwitch(CtSwitch switchStatement) { } ControlFlowNode fallThroughEnd = null; - if(lastNode != switchNode) { + if (lastNode != switchNode) { fallThroughEnd = lastNode; } lastNode = null; From 27375a8d33e9c7914383a6e5ada4074c887f316f Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Fri, 5 Apr 2024 12:37:42 +0200 Subject: [PATCH 08/24] Properly represent return type in javadoc --- .../java/fr/inria/controlflow/ControlFlowPathHelper.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java index 9e5edff5e96..1ccaf7787cd 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java @@ -16,10 +16,10 @@ public class ControlFlowPathHelper { private final Map>> pathsMemo = new HashMap<>(); /** - * Get the set of possible paths to the exit node from a given starting node. + * Get a list of possible paths to the exit node from a given starting node. * * @param node Starting node - * @return Set of possible paths + * @return List of possible paths */ List> paths(ControlFlowNode node) { if (pathsMemo.containsKey(node)) { @@ -38,10 +38,10 @@ List> paths(ControlFlowNode node) { } /** - * Get the set of possible paths to the exit node given a set of potentially incomplete paths. + * Get a list of possible paths to the exit node given a set of potentially incomplete paths. * * @param prior Set of potentially incomplete paths - * @return Set of possible paths + * @return List of possible paths */ private List> paths(List> prior) { List> result = new ArrayList<>(); From 4033929581fdf9abc18aee64b098344365a49a93 Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Sat, 6 Apr 2024 12:25:02 +0200 Subject: [PATCH 09/24] Add (failing) tests for correct switch control flow behaviour --- .../ForwardFlowBuilderVisitorTest.java | 172 +++++++++++++++--- .../control-flow/ControlFlowArithmetic.java | 72 ++++++++ 2 files changed, 215 insertions(+), 29 deletions(-) diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java index 6c12f71b98c..d70fd0afe83 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java @@ -22,6 +22,7 @@ package fr.inria.controlflow; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import spoon.processing.AbstractProcessor; import spoon.processing.ProcessingManager; @@ -48,7 +49,7 @@ public static ControlFlowGraph buildGraph(String folder, final String methodName throws Exception { final ControlFlowBuilder visitor = new ControlFlowBuilder(); - Factory factory = new SpoonMetaFactory().buildNewFactory(folder, 5); + Factory factory = new SpoonMetaFactory().buildNewFactory(folder, 21); ProcessingManager pm = new QueueProcessingManager(factory); pm.addProcessor(new AbstractProcessor() { @Override @@ -162,37 +163,150 @@ public void testifThenElseBlock() throws Exception { testMethod("simple", false, 0, 2, 6); } - //Test some mixed conditions - @Test - public void testSwitch() throws Exception { - testMethod("switchTest", false, 1, 13, 29); - } + @Nested + class SwitchTests { + @Test + public void testSwitch() throws Exception { + testMethod("switchTest", false, 1, 13, 29); + } - //Test fall-through of last switch case - @Test - public void testSwitchFallThrough() throws Exception { - testMethod("lastCaseFallThrough", false, 1, 4, 12); - } + //Test fall-through of last switch case + @Test + public void testSwitchFallThrough() throws Exception { + testMethod("lastCaseFallThrough", false, 1, 4, 12); + } - @Test - public void testSwitchImplicitDefault() throws Exception { - ControlFlowGraph graph = testMethod("lastCaseFallThrough", false, 1, 4, 12); - graph.simplify(); - ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); - ControlFlowNode entryNode = pathHelper.findNodeByString(graph, "int b = 0"); - ControlFlowNode caseNode = pathHelper.findNodeByString(graph, "b = 1"); - boolean canAvoid = pathHelper.canAvoidNode(entryNode, caseNode); - assertTrue(canAvoid, "Path for implicit default case missing"); - } + @Test + public void testSwitchImplicitDefault() throws Exception { + ControlFlowGraph graph = testMethod("lastCaseFallThrough", false, 1, 4, 12); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entryNode = pathHelper.findNodeByString(graph, "int b = 0"); + ControlFlowNode caseNode = pathHelper.findNodeByString(graph, "b = 1"); + boolean canAvoid = pathHelper.canAvoidNode(entryNode, caseNode); + assertTrue(canAvoid, "Path for implicit default case missing"); + } + + @Test + public void testMultipleCaseExpressions() throws Exception { + ControlFlowGraph graph = testMethod("multipleCaseExpressions", true, 1, 8, 17); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode startNode = pathHelper.findNodeByString(graph, "int b = 0"); + List> paths = pathHelper.paths(startNode); + assertTrue(paths.size() > 2, "Not enough paths. Possibly missing different paths from multiple expressions for a case"); + } + + @Test + public void testExpressionOldSwitch() throws Exception { + ControlFlowGraph graph = testMethod("oldSwitchExpression", true, null, null, null); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); + List> paths = pathHelper.paths(entryNode); + assertEquals(3, paths.size()); + } + + @Test + public void testExpressionOldExhaustive() throws Exception { + ControlFlowGraph graph = testMethod("oldSwitchExpressionExhaustive", true, null, null, null); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); + List> paths = pathHelper.paths(entryNode); + assertEquals(2, paths.size()); + } + + @Test + public void testSimpleEnhancedSwitch() throws Exception { + ControlFlowGraph graph = testMethod("enhancedSwitchSimple", true, null, 5, null); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); + List> paths = pathHelper.paths(entryNode); + assertEquals(3, paths.size()); + } + + @Test + public void testEnhancedSwitchImplicitDefault() throws Exception { + ControlFlowGraph graph = testMethod("enhancedSwitchImplicitDefault", true, null, null, null); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); + List> paths = pathHelper.paths(entryNode); + assertEquals(2, paths.size()); + } + + @Test + public void testEnhancedSwitchMultipleExpressions() throws Exception { + ControlFlowGraph graph = testMethod("enhancedSwitchMultipleExpressions", true, null, null, null); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); + List> paths = pathHelper.paths(entryNode); + assertEquals(3, paths.size()); + } + + @Test + public void testEnhancedSwitchNullDefault() throws Exception { + ControlFlowGraph graph = testMethod("enhancedSwitchNullDefault", true, null, null, null); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); + List> paths = pathHelper.paths(entryNode); + assertEquals(2, paths.size()); + } + + @Test + public void testEnhancedSwitchExpressionExhaustive() throws Exception { + ControlFlowGraph graph = testMethod("enhancedSwitchExhaustiveExpression", true, null, null, null); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); + List> paths = pathHelper.paths(entryNode); + assertEquals(2, paths.size()); + } + + @Test + public void testEnhancedSwitchExhaustive() throws Exception { + ControlFlowGraph graph = testMethod("enhancedSwitchExhaustive", true, null, null, null); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); + List> paths = pathHelper.paths(entryNode); + assertEquals(2, paths.size()); + } + + @Test + public void testSimpleSealedHierarchyExhaustive() throws Exception { + ControlFlowGraph graph = testMethod("enhancedSwitchSealedExhaustive", true, null, null, null); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); + List> paths = pathHelper.paths(entryNode); + assertEquals(2, paths.size()); + } + + @Test + public void testComplexSealedHierarchyExhaustive() throws Exception { + ControlFlowGraph graph = testMethod("enhancedSwitchMultilevelSealedExhaustive", true, null, null, null); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); + List> paths = pathHelper.paths(entryNode); + assertEquals(2, paths.size()); + } + + @Test + public void testEnhancedSwitchExhaustiveParametrization() throws Exception { + ControlFlowGraph graph = testMethod("enhancedSwitchExhaustiveParametrization", true, null, null, null); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); + List> paths = pathHelper.paths(entryNode); + assertEquals(2, paths.size()); + } - @Test - public void testMultipleCaseExpressions() throws Exception { - ControlFlowGraph graph = testMethod("multipleCaseExpressions", true, 1, 8, 17); - graph.simplify(); - ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); - ControlFlowNode startNode = pathHelper.findNodeByString(graph, "int b = 0"); - List> paths = pathHelper.paths(startNode); - assertTrue(paths.size() > 2, "Not enough paths. Possibly missing different paths from multiple expressions for a case"); } //Test some mixed conditions diff --git a/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java b/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java index 16489fafa98..be7e4c86340 100644 --- a/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java +++ b/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java @@ -389,6 +389,78 @@ public int multipleCaseExpressions(int a) { return b; } + enum Test { + TYPE_1, TYPE_2 + } + + public void oldSwitchExpression () { + int a = switch (1) { + case 1: yield 2; + case 2: + int b = 2; + yield 3; + default: + yield 1; + }; + } + + public void oldSwitchExpressionExhaustive() { + int a = switch (Test.TYPE_1) { + case TYPE_1: yield 1; + case TYPE_2: yield 2; + }; + } + + public void enhancedSwitchSimple() { + int a = 1; + switch (a) { + case 1 -> System.out.println("hi from case 1"); + case 2 -> { + System.out.println("Hello from case 2"); + } + default -> System.out.println("Hello from default"); + } + } + + public void enhancedSwitchImplicitDefault() { + int a = 1; + int b = 0; + switch (a) { + case 1 -> b = 1; + } + } + + public void enhancedSwitchMultipleExpressions() { + switch (3) { + case 1, 2 -> { + int b = 1; + } + default -> {} + } + } + + public void enhancedSwitchNullDefault(Object arg) { + switch (arg) { + case null, default -> { + int a = 1; + } + } + } + + public void enhancedSwitchExhaustiveExpression () { + int a = switch (Test.TYPE_1) { + case TYPE_1 -> 1; + case TYPE_2 -> 2; + }; + } + + public void enhandedSwitchExhaustive() { + switch (Test.TYPE_1) { + case TYPE_1 -> {} + case TYPE_2 -> {} + } + } + //All lines will be tested in this method public int simple(int a) { a = a + a / 2; From 0ff991810dc93c6151110aca021ed3acaeb13afe Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Sat, 6 Apr 2024 16:19:43 +0200 Subject: [PATCH 10/24] First step merged switch rewrite --- .../inria/controlflow/ControlFlowBuilder.java | 111 ++++++++++-------- .../ForwardFlowBuilderVisitorTest.java | 2 +- 2 files changed, 64 insertions(+), 49 deletions(-) diff --git a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java index 3afc870b557..1c07721b08c 100644 --- a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java +++ b/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; @@ -118,10 +120,11 @@ import java.util.HashMap; import java.util.List; import java.util.Stack; +import java.util.function.Function; /** * Builds the control graph for a given snippet of code - * + *

* Created by marodrig on 13/10/2015. */ public class ControlFlowBuilder extends CtAbstractVisitor { @@ -223,7 +226,7 @@ private void visitConditional(CtElement parent, CtConditional conditional) { /** * Returns the first graph node representing the statement s construction. - * + *

* Usually an statement is represented by many blocks and branches. * This method returns the first of those blocks/branches. * @@ -282,7 +285,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()) { @@ -320,8 +322,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); } @@ -393,6 +395,7 @@ public void visitCtBinaryOperator(CtBinaryOperator operator) { /** * 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 */ @@ -765,68 +768,80 @@ public void visitCtCase(CtCase caseStatement) { @Override public void visitCtSwitch(CtSwitch switchStatement) { - registerStatementLabel(switchStatement); - //Push the condition - ControlFlowNode switchNode = new ControlFlowNode(switchStatement.getSelector(), result, BranchKind.BRANCH); - tryAddEdge(lastNode, switchNode); + handleCtAbstractSwitch(switchStatement, null); + } + + @Override + public void visitCtSwitchExpression(CtSwitchExpression switchExpression) { + handleCtAbstractSwitch(switchExpression, null); + } + + public void handleCtAbstractSwitch(CtAbstractSwitch abstractSwitch, Function> yieldTransformer) { + 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); - boolean hasDefaultCase = false; - lastNode = switchNode; - for (CtCase caseStatement : switchStatement.getCases()) { - - //Visit Case - registerStatementLabel(caseStatement); - var caseExpressions = caseStatement.getCaseExpressions(); - ArrayList caseExpressionNodes = new ArrayList<>(); - for (CtExpression expression : caseExpressions) { - ControlFlowNode caseNode = new ControlFlowNode(expression, result, BranchKind.STATEMENT); - caseExpressionNodes.add(caseNode); - tryAddEdge(switchNode, caseNode); + ControlFlowNode fallThroughNode = null; + for (CtCase switchCase : abstractSwitch.getCases()) { + List caseExpressionNodes = new ArrayList<>(); + for (CtExpression expression : switchCase.getCaseExpressions()) { + ControlFlowNode node = new ControlFlowNode(expression, result, BranchKind.STATEMENT); + node.setTag(switchCase.getGuard()); + tryAddEdge(selectorNode, node); + caseExpressionNodes.add(node); } - - if (caseExpressionNodes.isEmpty()) { - hasDefaultCase = true; - ControlFlowNode defaultNode = new ControlFlowNode(null, result, BranchKind.STATEMENT); - caseExpressionNodes.add(defaultNode); - tryAddEdge(switchNode, defaultNode); + if (switchCase.getIncludesDefault() || switchCase.getCaseExpressions().isEmpty()) { + caseExpressionNodes.add(selectorNode); } - ControlFlowNode fallThroughEnd = null; - if (lastNode != switchNode) { - fallThroughEnd = lastNode; + List statements = switchCase.getStatements(); + List transformedStatements = new ArrayList<>(); + for (CtStatement statement : statements) { + if (statement instanceof CtYieldStatement yieldStatement) { + transformedStatements.addAll(yieldTransformer.apply(yieldStatement)); + } else { + transformedStatements.add(statement); + } } - lastNode = null; - ControlFlowNode blockStart = travelStatementList(caseStatement.getStatements()); - tryAddEdge(fallThroughEnd, blockStart); - - for (ControlFlowNode expressionNode : caseExpressionNodes) { - tryAddEdge(expressionNode, blockStart); + lastNode = null; // We want full control over the edges + ControlFlowNode blockStart = travelStatementList(transformedStatements); + for (ControlFlowNode caseExpressionNode : caseExpressionNodes) { + tryAddEdge(caseExpressionNode, blockStart); } + tryAddEdge(fallThroughNode, blockStart); - if (lastNode.getStatement() instanceof CtBreak) { - lastNode = switchNode; + if (switchCase.getCaseKind() == CaseKind.COLON) { + fallThroughNode = lastNode; + } else { + tryAddEdge(lastNode, convergenceNode); } } - tryAddEdge(lastNode, convergenceNode); + tryAddEdge(fallThroughNode, convergenceNode); - if (!hasDefaultCase) { - tryAddEdge(switchNode, convergenceNode); + if (!checkExhaustive(abstractSwitch)) { + tryAddEdge(selectorNode, convergenceNode); } - //Return as last node the convergence node - lastNode = convergenceNode; breakingBad.pop(); + lastNode = convergenceNode; } - @Override - public void visitCtSwitchExpression(CtSwitchExpression 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(CtCase::getIncludesDefault); + if (hasDefault) { + return true; + } + + // TODO: More complete exhaustive check + return false; } @Override diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java index d70fd0afe83..58b3530785c 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java @@ -167,7 +167,7 @@ public void testifThenElseBlock() throws Exception { class SwitchTests { @Test public void testSwitch() throws Exception { - testMethod("switchTest", false, 1, 13, 29); + testMethod("switchTest", false, 1, 12, 28); } //Test fall-through of last switch case From 6c24dc05adf3b3b8783d14b07e40d06d197a80a2 Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Sat, 6 Apr 2024 18:17:29 +0200 Subject: [PATCH 11/24] Rework graph layout (with case labels no separate blocks are created anymore and handle return switch () {} expression --- .../inria/controlflow/ControlFlowBuilder.java | 80 ++++++++++++------- .../ForwardFlowBuilderVisitorTest.java | 22 +++-- .../control-flow/ControlFlowArithmetic.java | 17 +++- 3 files changed, 83 insertions(+), 36 deletions(-) diff --git a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java index 1c07721b08c..6c1552e8a1c 100644 --- a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java +++ b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java @@ -116,11 +116,10 @@ import spoon.reflect.visitor.CtAbstractVisitor; import java.lang.annotation.Annotation; -import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Stack; -import java.util.function.Function; +import java.util.function.Consumer; /** * Builds the control graph for a given snippet of code @@ -128,6 +127,7 @@ * Created by marodrig on 13/10/2015. */ public class ControlFlowBuilder extends CtAbstractVisitor { + public static final String CONTROL_FLOW_SWITCH_YIELD_STATEMENT = "CONTROL_FLOW_SWITCH_YIELD_STATEMENT"; ControlFlowGraph result = new ControlFlowGraph(ControlFlowEdge.class); @@ -746,10 +746,14 @@ public void visitCtParameterReference(CtParameterReference reference) { @Override public void visitCtReturn(CtReturn 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 switchExpression) { + handleReturningSwitchExpression(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 @@ -771,50 +775,67 @@ public void visitCtSwitch(CtSwitch switchStatement) { handleCtAbstractSwitch(switchStatement, null); } - @Override - public void visitCtSwitchExpression(CtSwitchExpression switchExpression) { - handleCtAbstractSwitch(switchExpression, null); + public void handleReturningSwitchExpression(CtReturn ctReturn, CtSwitchExpression switchExpression) { + handleCtAbstractSwitch(switchExpression, yieldStatement -> { + CtReturn syntheticReturn = ctReturn.clone(); + syntheticReturn.putMetadata(CONTROL_FLOW_SWITCH_YIELD_STATEMENT, yieldStatement); + syntheticReturn.setReturnedExpression((CtExpression) yieldStatement.getExpression()); + + syntheticReturn.accept(this); + }); } - public void handleCtAbstractSwitch(CtAbstractSwitch abstractSwitch, Function> yieldTransformer) { + public void handleCtAbstractSwitch(CtAbstractSwitch abstractSwitch, Consumer yieldAction) { ControlFlowNode selectorNode = new ControlFlowNode(abstractSwitch.getSelector(), result, BranchKind.BRANCH); + selectorNode.setTag(abstractSwitch); tryAddEdge(lastNode, selectorNode); - lastNode = selectorNode; + ControlFlowNode switchBlockBegin = new ControlFlowNode(null, result, BranchKind.BLOCK_BEGIN); + tryAddEdge(selectorNode, switchBlockBegin); + lastNode = switchBlockBegin; ControlFlowNode convergenceNode = new ControlFlowNode(null, result, BranchKind.CONVERGE); breakingBad.push(convergenceNode); ControlFlowNode fallThroughNode = null; for (CtCase switchCase : abstractSwitch.getCases()) { - List caseExpressionNodes = new ArrayList<>(); + boolean isEnhanced = switchCase.getCaseKind() == CaseKind.ARROW; + + ControlFlowNode caseConvergenceNode = new ControlFlowNode(null, result, BranchKind.CONVERGE); + lastNode = caseConvergenceNode; + tryAddEdge(fallThroughNode, caseConvergenceNode); + for (CtExpression expression : switchCase.getCaseExpressions()) { ControlFlowNode node = new ControlFlowNode(expression, result, BranchKind.STATEMENT); node.setTag(switchCase.getGuard()); - tryAddEdge(selectorNode, node); - caseExpressionNodes.add(node); + tryAddEdge(switchBlockBegin, node); + tryAddEdge(node, caseConvergenceNode); } if (switchCase.getIncludesDefault() || switchCase.getCaseExpressions().isEmpty()) { - caseExpressionNodes.add(selectorNode); + tryAddEdge(switchBlockBegin, caseConvergenceNode); + } + + if (isEnhanced) { + ControlFlowNode begin = new ControlFlowNode(null, result, BranchKind.BLOCK_BEGIN); + tryAddEdge(lastNode, begin); + lastNode = begin; } - List statements = switchCase.getStatements(); - List transformedStatements = new ArrayList<>(); - for (CtStatement statement : statements) { + for (CtStatement statement : switchCase.getStatements()) { if (statement instanceof CtYieldStatement yieldStatement) { - transformedStatements.addAll(yieldTransformer.apply(yieldStatement)); + yieldAction.accept(yieldStatement); } else { - transformedStatements.add(statement); + registerStatementLabel(statement); + statement.accept(this); } } - lastNode = null; // We want full control over the edges - ControlFlowNode blockStart = travelStatementList(transformedStatements); - for (ControlFlowNode caseExpressionNode : caseExpressionNodes) { - tryAddEdge(caseExpressionNode, blockStart); + if (isEnhanced) { + ControlFlowNode end = new ControlFlowNode(null, result, BranchKind.BLOCK_END); + tryAddEdge(lastNode, end); + lastNode = end; } - tryAddEdge(fallThroughNode, blockStart); - if (switchCase.getCaseKind() == CaseKind.COLON) { + if (!isEnhanced) { fallThroughNode = lastNode; } else { tryAddEdge(lastNode, convergenceNode); @@ -823,11 +844,14 @@ public void handleCtAbstractSwitch(CtAbstractSwitch abstractSwitch, Funct tryAddEdge(fallThroughNode, convergenceNode); if (!checkExhaustive(abstractSwitch)) { - tryAddEdge(selectorNode, convergenceNode); + tryAddEdge(switchBlockBegin, convergenceNode); } breakingBad.pop(); - lastNode = convergenceNode; + + ControlFlowNode switchBlockEnd = new ControlFlowNode(null, result, BranchKind.BLOCK_END); + tryAddEdge(convergenceNode, switchBlockEnd); + lastNode = switchBlockEnd; } private boolean checkExhaustive(CtAbstractSwitch switchElement) { diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java index 58b3530785c..9f3373ac7ea 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java @@ -167,18 +167,18 @@ public void testifThenElseBlock() throws Exception { class SwitchTests { @Test public void testSwitch() throws Exception { - testMethod("switchTest", false, 1, 12, 28); + testMethod("switchTest", true, 1, 12, 19); } //Test fall-through of last switch case @Test public void testSwitchFallThrough() throws Exception { - testMethod("lastCaseFallThrough", false, 1, 4, 12); + testMethod("lastCaseFallThrough", true, 1, 4, 11); } @Test public void testSwitchImplicitDefault() throws Exception { - ControlFlowGraph graph = testMethod("lastCaseFallThrough", false, 1, 4, 12); + ControlFlowGraph graph = testMethod("lastCaseFallThrough", false, null, null, null); graph.simplify(); ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); ControlFlowNode entryNode = pathHelper.findNodeByString(graph, "int b = 0"); @@ -189,7 +189,7 @@ public void testSwitchImplicitDefault() throws Exception { @Test public void testMultipleCaseExpressions() throws Exception { - ControlFlowGraph graph = testMethod("multipleCaseExpressions", true, 1, 8, 17); + ControlFlowGraph graph = testMethod("multipleCaseExpressions", true, 1, 7, 14); graph.simplify(); ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); ControlFlowNode startNode = pathHelper.findNodeByString(graph, "int b = 0"); @@ -198,8 +198,18 @@ public void testMultipleCaseExpressions() throws Exception { } @Test - public void testExpressionOldSwitch() throws Exception { - ControlFlowGraph graph = testMethod("oldSwitchExpression", true, null, null, null); + public void testAssignExpressionOldSwitch() throws Exception { + ControlFlowGraph graph = testMethod("oldSwitchAssignExpression", true, null, null, null); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); + List> paths = pathHelper.paths(entryNode); + assertEquals(3, paths.size()); + } + + @Test + public void testReturnExpressionOldSwitch() throws Exception { + ControlFlowGraph graph = testMethod("oldSwitchReturnExpression", true, null, null, null); graph.simplify(); ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); diff --git a/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java b/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java index be7e4c86340..8537adc17c2 100644 --- a/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java +++ b/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java @@ -393,7 +393,7 @@ enum Test { TYPE_1, TYPE_2 } - public void oldSwitchExpression () { + public void oldSwitchAssignExpression () { int a = switch (1) { case 1: yield 2; case 2: @@ -404,6 +404,17 @@ public void oldSwitchExpression () { }; } + public int oldSwitchReturnExpression () { + return switch (1) { + case 1: yield 2; + case 2: + int b = 2; + yield 3; + default: + yield 1; + }; + } + public void oldSwitchExpressionExhaustive() { int a = switch (Test.TYPE_1) { case TYPE_1: yield 1; @@ -426,7 +437,9 @@ public void enhancedSwitchImplicitDefault() { int a = 1; int b = 0; switch (a) { - case 1 -> b = 1; + case 1 -> { + b = 1; + } } } From b27f6b8d7449aac351df7234fcdc13e5be15fa2f Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Sat, 6 Apr 2024 19:53:13 +0200 Subject: [PATCH 12/24] Rework handling of multinode statements They now have a start and an end node. Should be ported to other multinode constructs like if, loops, conditionals, ... --- .../java/fr/inria/controlflow/BranchKind.java | 2 + .../inria/controlflow/ControlFlowBuilder.java | 46 +++++------- .../fr/inria/controlflow/ControlFlowNode.java | 74 +++++++++++++++++-- 3 files changed, 90 insertions(+), 32 deletions(-) diff --git a/spoon-control-flow/src/main/java/fr/inria/controlflow/BranchKind.java b/spoon-control-flow/src/main/java/fr/inria/controlflow/BranchKind.java index e406a0630f0..bfd39c7e32c 100644 --- a/spoon-control-flow/src/main/java/fr/inria/controlflow/BranchKind.java +++ b/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 diff --git a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java index 6c1552e8a1c..192a768304f 100644 --- a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java +++ b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java @@ -119,7 +119,6 @@ import java.util.HashMap; import java.util.List; import java.util.Stack; -import java.util.function.Consumer; /** * Builds the control graph for a given snippet of code @@ -127,7 +126,7 @@ * Created by marodrig on 13/10/2015. */ public class ControlFlowBuilder extends CtAbstractVisitor { - public static final String CONTROL_FLOW_SWITCH_YIELD_STATEMENT = "CONTROL_FLOW_SWITCH_YIELD_STATEMENT"; + public static final String CONTROL_FLOW_ORIGINAL_STATEMENT = "CONTROL_FLOW_ORIGINAL_STATEMENT"; ControlFlowGraph result = new ControlFlowGraph(ControlFlowEdge.class); @@ -662,6 +661,8 @@ public void visitCtLocalVariable(CtLocalVariable localVariable) { registerStatementLabel(localVariable); if (localVariable.getDefaultExpression() instanceof CtConditional) { visitConditional(localVariable, (CtConditional) localVariable.getDefaultExpression()); + } else if (localVariable.getDefaultExpression() instanceof CtSwitchExpression switchExpression) { + handleCtAbstractSwitch(localVariable, switchExpression); } else { defaultAction(BranchKind.STATEMENT, localVariable); } @@ -747,7 +748,7 @@ public void visitCtParameterReference(CtParameterReference reference) { public void visitCtReturn(CtReturn returnStatement) { registerStatementLabel(returnStatement); if (returnStatement.getReturnedExpression() instanceof CtSwitchExpression switchExpression) { - handleReturningSwitchExpression(returnStatement, switchExpression); + handleCtAbstractSwitch(returnStatement, switchExpression); } else { ControlFlowNode n = new ControlFlowNode(returnStatement, result, BranchKind.STATEMENT); tryAddEdge(lastNode, n); @@ -772,25 +773,19 @@ public void visitCtCase(CtCase caseStatement) { @Override public void visitCtSwitch(CtSwitch switchStatement) { - handleCtAbstractSwitch(switchStatement, null); + handleCtAbstractSwitch(switchStatement, switchStatement); } - public void handleReturningSwitchExpression(CtReturn ctReturn, CtSwitchExpression switchExpression) { - handleCtAbstractSwitch(switchExpression, yieldStatement -> { - CtReturn syntheticReturn = ctReturn.clone(); - syntheticReturn.putMetadata(CONTROL_FLOW_SWITCH_YIELD_STATEMENT, yieldStatement); - syntheticReturn.setReturnedExpression((CtExpression) yieldStatement.getExpression()); + public void handleCtAbstractSwitch(CtStatement containingStatement, CtAbstractSwitch abstractSwitch) { + ControlFlowNode statementNode = new ControlFlowNode(containingStatement, result, BranchKind.STATEMENT); + tryAddEdge(lastNode, statementNode); - syntheticReturn.accept(this); - }); - } + ControlFlowNode selectorNode = new ControlFlowNode(abstractSwitch.getSelector(), result, BranchKind.EXPRESSION); + tryAddEdge(statementNode, selectorNode); + lastNode = selectorNode; - public void handleCtAbstractSwitch(CtAbstractSwitch abstractSwitch, Consumer yieldAction) { - ControlFlowNode selectorNode = new ControlFlowNode(abstractSwitch.getSelector(), result, BranchKind.BRANCH); - selectorNode.setTag(abstractSwitch); - tryAddEdge(lastNode, selectorNode); ControlFlowNode switchBlockBegin = new ControlFlowNode(null, result, BranchKind.BLOCK_BEGIN); - tryAddEdge(selectorNode, switchBlockBegin); + tryAddEdge(lastNode, switchBlockBegin); lastNode = switchBlockBegin; ControlFlowNode convergenceNode = new ControlFlowNode(null, result, BranchKind.CONVERGE); @@ -805,7 +800,7 @@ public void handleCtAbstractSwitch(CtAbstractSwitch abstractSwitch, Consu tryAddEdge(fallThroughNode, caseConvergenceNode); for (CtExpression expression : switchCase.getCaseExpressions()) { - ControlFlowNode node = new ControlFlowNode(expression, result, BranchKind.STATEMENT); + ControlFlowNode node = new ControlFlowNode(expression, result, BranchKind.EXPRESSION); node.setTag(switchCase.getGuard()); tryAddEdge(switchBlockBegin, node); tryAddEdge(node, caseConvergenceNode); @@ -819,16 +814,10 @@ public void handleCtAbstractSwitch(CtAbstractSwitch abstractSwitch, Consu tryAddEdge(lastNode, begin); lastNode = begin; } - for (CtStatement statement : switchCase.getStatements()) { - if (statement instanceof CtYieldStatement yieldStatement) { - yieldAction.accept(yieldStatement); - } else { - registerStatementLabel(statement); - statement.accept(this); - } + registerStatementLabel(statement); + statement.accept(this); } - if (isEnhanced) { ControlFlowNode end = new ControlFlowNode(null, result, BranchKind.BLOCK_END); tryAddEdge(lastNode, end); @@ -852,6 +841,11 @@ public void handleCtAbstractSwitch(CtAbstractSwitch abstractSwitch, Consu 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; } private boolean checkExhaustive(CtAbstractSwitch switchElement) { diff --git a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowNode.java b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowNode.java index e189cb3315a..87392305770 100644 --- a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowNode.java +++ b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowNode.java @@ -21,6 +21,14 @@ */ 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; @@ -28,7 +36,7 @@ /** * A node of the control flow - * + *

* 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 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 prev() { } public List 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 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; } } From 211bf204233dc18e7984ddf22c7ce7b288f0be98 Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Sat, 6 Apr 2024 20:53:20 +0200 Subject: [PATCH 13/24] Adjust tests to test structure instead of concrete node counts, change switch selector node to branch --- .../inria/controlflow/ControlFlowBuilder.java | 4 +-- .../controlflow/ControlFlowPathHelper.java | 12 ++++++++ .../ForwardFlowBuilderVisitorTest.java | 28 +++++++++++++++++-- .../control-flow/ControlFlowArithmetic.java | 2 +- 4 files changed, 40 insertions(+), 6 deletions(-) diff --git a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java index 192a768304f..64baf3dddb5 100644 --- a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java +++ b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java @@ -780,7 +780,7 @@ public void handleCtAbstractSwitch(CtStatement containingStatement, CtAbstra ControlFlowNode statementNode = new ControlFlowNode(containingStatement, result, BranchKind.STATEMENT); tryAddEdge(lastNode, statementNode); - ControlFlowNode selectorNode = new ControlFlowNode(abstractSwitch.getSelector(), result, BranchKind.EXPRESSION); + ControlFlowNode selectorNode = new ControlFlowNode(abstractSwitch.getSelector(), result, BranchKind.BRANCH); tryAddEdge(statementNode, selectorNode); lastNode = selectorNode; @@ -853,7 +853,7 @@ private boolean checkExhaustive(CtAbstractSwitch switchElement) { return true; } - boolean hasDefault = switchElement.getCases().stream().anyMatch(CtCase::getIncludesDefault); + boolean hasDefault = switchElement.getCases().stream().anyMatch(cases -> cases.getIncludesDefault() || cases.getCaseExpressions().isEmpty()); if (hasDefault) { return true; } diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java index 1ccaf7787cd..2571b1dc62f 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ControlFlowPathHelper.java @@ -37,6 +37,18 @@ List> paths(ControlFlowNode node) { return result; } + /** + * Get a list of possible paths to the exit node from a given starting node. + * + * @param from Starting node + * @param to End node + * @return List of possible paths + */ + List> pathsBetween(ControlFlowNode from, ControlFlowNode to) { + List> exitPaths = paths(from); + return exitPaths.stream().filter(path -> path.contains(to)).toList(); + } + /** * Get a list of possible paths to the exit node given a set of potentially incomplete paths. * diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java index 9f3373ac7ea..04008a3e7d2 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java @@ -167,13 +167,35 @@ public void testifThenElseBlock() throws Exception { class SwitchTests { @Test public void testSwitch() throws Exception { - testMethod("switchTest", true, 1, 12, 19); + ControlFlowGraph graph = testMethod("switchTest", true, 1, null, null); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entry = graph.findNodesOfKind(BEGIN).get(0); + + ControlFlowNode return0 = pathHelper.findNodeByString(graph, "return 0"); + assertEquals(1, pathHelper.pathsBetween(entry, return0).size()); + + ControlFlowNode case2 = pathHelper.findNodeByString(graph, "b = a * 2"); + assertEquals(1, pathHelper.pathsBetween(entry, case2).size()); + + ControlFlowNode case34 = pathHelper.findNodeByString(graph, "b = a * 9"); + assertEquals(2, pathHelper.pathsBetween(entry, case34).size()); + + ControlFlowNode defaultReturn = pathHelper.findNodeByString(graph, "return 1"); + assertEquals(1, pathHelper.pathsBetween(entry, defaultReturn).size()); + + ControlFlowNode afterSwitchReturn = pathHelper.findNodeByString(graph, "return b"); + assertEquals(3, pathHelper.pathsBetween(entry, afterSwitchReturn).size()); + + assertEquals(5, pathHelper.pathsBetween(entry, graph.getExitNode()).size()); } //Test fall-through of last switch case @Test public void testSwitchFallThrough() throws Exception { - testMethod("lastCaseFallThrough", true, 1, 4, 11); + ControlFlowGraph graph = testMethod("lastCaseFallThrough", true, null, null, null); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entry = graph.findNodesOfKind(BEGIN).get(0); + assertEquals(2, pathHelper.paths(entry).size()); } @Test @@ -189,7 +211,7 @@ public void testSwitchImplicitDefault() throws Exception { @Test public void testMultipleCaseExpressions() throws Exception { - ControlFlowGraph graph = testMethod("multipleCaseExpressions", true, 1, 7, 14); + ControlFlowGraph graph = testMethod("multipleCaseExpressions", true, null, null, null); graph.simplify(); ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); ControlFlowNode startNode = pathHelper.findNodeByString(graph, "int b = 0"); diff --git a/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java b/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java index 8537adc17c2..2054bc1f03f 100644 --- a/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java +++ b/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java @@ -363,7 +363,7 @@ public int switchTest(int a) { b = a * 9; break; default: - return 0; + return 1; } return b; } From 0326485583278489ee911d016ffa64e1bb3133af Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Sat, 6 Apr 2024 22:59:13 +0200 Subject: [PATCH 14/24] Remove unnecessary block wrapping --- .../fr/inria/controlflow/ControlFlowBuilder.java | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java index 64baf3dddb5..3609662c3c6 100644 --- a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java +++ b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java @@ -809,20 +809,10 @@ public void handleCtAbstractSwitch(CtStatement containingStatement, CtAbstra tryAddEdge(switchBlockBegin, caseConvergenceNode); } - if (isEnhanced) { - ControlFlowNode begin = new ControlFlowNode(null, result, BranchKind.BLOCK_BEGIN); - tryAddEdge(lastNode, begin); - lastNode = begin; - } for (CtStatement statement : switchCase.getStatements()) { registerStatementLabel(statement); statement.accept(this); } - if (isEnhanced) { - ControlFlowNode end = new ControlFlowNode(null, result, BranchKind.BLOCK_END); - tryAddEdge(lastNode, end); - lastNode = end; - } if (!isEnhanced) { fallThroughNode = lastNode; @@ -1041,7 +1031,7 @@ public void visitCtTypeMemberWildcardImportReference(CtTypeMemberWildcardImportR @Override public void visitCtYieldStatement(CtYieldStatement ctYieldStatement) { - + defaultAction(BranchKind.STATEMENT, ctYieldStatement); } @Override From f3b6591c3286facc149fa6f3fc3f48281786ea41 Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Sat, 6 Apr 2024 23:02:15 +0200 Subject: [PATCH 15/24] Temporarily adapt test to spoon bug --- .../fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java index 04008a3e7d2..370c8ba89ac 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java @@ -251,7 +251,8 @@ public void testExpressionOldExhaustive() throws Exception { @Test public void testSimpleEnhancedSwitch() throws Exception { - ControlFlowGraph graph = testMethod("enhancedSwitchSimple", true, null, 5, null); + // TODO: This is a spoon bug that imagines a break statement. It should ideally be only 6 statements + ControlFlowGraph graph = testMethod("enhancedSwitchSimple", true, null, 6, null); graph.simplify(); ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); From 10ab861afdd30745c00bb8ec0ce044f07d66fd51 Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Sun, 7 Apr 2024 00:01:22 +0200 Subject: [PATCH 16/24] Swap block begin and selector --- .../fr/inria/controlflow/ControlFlowBuilder.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java index 3609662c3c6..439d2675a02 100644 --- a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java +++ b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java @@ -779,15 +779,16 @@ public void visitCtSwitch(CtSwitch switchStatement) { public void handleCtAbstractSwitch(CtStatement containingStatement, CtAbstractSwitch abstractSwitch) { ControlFlowNode statementNode = new ControlFlowNode(containingStatement, result, BranchKind.STATEMENT); tryAddEdge(lastNode, statementNode); - - ControlFlowNode selectorNode = new ControlFlowNode(abstractSwitch.getSelector(), result, BranchKind.BRANCH); - tryAddEdge(statementNode, selectorNode); - lastNode = selectorNode; + 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; + ControlFlowNode convergenceNode = new ControlFlowNode(null, result, BranchKind.CONVERGE); breakingBad.push(convergenceNode); @@ -802,11 +803,11 @@ public void handleCtAbstractSwitch(CtStatement containingStatement, CtAbstra for (CtExpression expression : switchCase.getCaseExpressions()) { ControlFlowNode node = new ControlFlowNode(expression, result, BranchKind.EXPRESSION); node.setTag(switchCase.getGuard()); - tryAddEdge(switchBlockBegin, node); + tryAddEdge(selectorNode, node); tryAddEdge(node, caseConvergenceNode); } if (switchCase.getIncludesDefault() || switchCase.getCaseExpressions().isEmpty()) { - tryAddEdge(switchBlockBegin, caseConvergenceNode); + tryAddEdge(selectorNode, caseConvergenceNode); } for (CtStatement statement : switchCase.getStatements()) { @@ -823,7 +824,7 @@ public void handleCtAbstractSwitch(CtStatement containingStatement, CtAbstra tryAddEdge(fallThroughNode, convergenceNode); if (!checkExhaustive(abstractSwitch)) { - tryAddEdge(switchBlockBegin, convergenceNode); + tryAddEdge(selectorNode, convergenceNode); } breakingBad.pop(); From 06123b28f4e001d9461406b08f7cdeaccbcbe336 Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Sun, 7 Apr 2024 11:08:05 +0200 Subject: [PATCH 17/24] Fix test method name --- .../src/test/resources/control-flow/ControlFlowArithmetic.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java b/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java index 2054bc1f03f..b6cc7f4de81 100644 --- a/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java +++ b/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java @@ -467,7 +467,7 @@ public void enhancedSwitchExhaustiveExpression () { }; } - public void enhandedSwitchExhaustive() { + public void enhancedSwitchExhaustive() { switch (Test.TYPE_1) { case TYPE_1 -> {} case TYPE_2 -> {} From bf03f1f6882d3bed72d09d60f8e808b6235b9690 Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Sun, 7 Apr 2024 11:21:29 +0200 Subject: [PATCH 18/24] Detect exhaustive enum switches --- .../fr/inria/controlflow/ControlFlowBuilder.java | 13 +++++++++++++ .../controlflow/ForwardFlowBuilderVisitorTest.java | 14 ++++++++++++-- .../control-flow/ControlFlowArithmetic.java | 8 +++++++- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java index 439d2675a02..5ad12709031 100644 --- a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java +++ b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java @@ -118,7 +118,9 @@ 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 @@ -849,6 +851,17 @@ private boolean checkExhaustive(CtAbstractSwitch switchElement) { return true; } + Set> caseExpressions = switchElement.getCases().stream().flatMap(cases -> cases.getCaseExpressions().stream()).collect(Collectors.toSet()); + if (switchElement.getSelector().getType().isEnum()) { + CtEnum enumType = (CtEnum) switchElement.getSelector().getType().getTypeDeclaration(); + Set enumConstantNames = enumType.getEnumValues().stream().map(CtEnumValue::getSimpleName).collect(Collectors.toSet()); + Set referencedEnumConstants = caseExpressions.stream().map(expression -> ((CtFieldRead) expression).getVariable().getSimpleName()).collect(Collectors.toSet()); + + if (referencedEnumConstants.containsAll(enumConstantNames)) { + return true; + } + } + // TODO: More complete exhaustive check return false; } diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java index 370c8ba89ac..1643c570d77 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java @@ -301,8 +301,18 @@ public void testEnhancedSwitchExpressionExhaustive() throws Exception { } @Test - public void testEnhancedSwitchExhaustive() throws Exception { - ControlFlowGraph graph = testMethod("enhancedSwitchExhaustive", true, null, null, null); + public void testEnhancedSwitchExhaustiveEnum() throws Exception { + ControlFlowGraph graph = testMethod("enhancedSwitchExhaustiveEnum", true, null, null, null); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); + List> paths = pathHelper.paths(entryNode); + assertEquals(2, paths.size()); + } + + @Test + public void testEnhancedSwitchNonExhaustiveEnum() throws Exception { + ControlFlowGraph graph = testMethod("enhancedSwitchNonExhaustiveEnum", true, null, null, null); graph.simplify(); ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); diff --git a/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java b/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java index b6cc7f4de81..aeca9e782ad 100644 --- a/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java +++ b/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java @@ -467,13 +467,19 @@ public void enhancedSwitchExhaustiveExpression () { }; } - public void enhancedSwitchExhaustive() { + public void enhancedSwitchExhaustiveEnum() { switch (Test.TYPE_1) { case TYPE_1 -> {} case TYPE_2 -> {} } } + public void enhancedSwitchNonExhaustiveEnum() { + switch (Test.TYPE_1) { + case TYPE_2 -> {} + } + } + //All lines will be tested in this method public int simple(int a) { a = a + a / 2; From cc473a12456ebb19dd71e65fea197674caea86d5 Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Sun, 7 Apr 2024 11:34:36 +0200 Subject: [PATCH 19/24] Add missing code for exhaustiveness tests --- .../ForwardFlowBuilderVisitorTest.java | 2 +- .../control-flow/ControlFlowArithmetic.java | 72 +++++++++++++++---- 2 files changed, 61 insertions(+), 13 deletions(-) diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java index 1643c570d77..15ad66e97f4 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java @@ -347,7 +347,7 @@ public void testEnhancedSwitchExhaustiveParametrization() throws Exception { ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); List> paths = pathHelper.paths(entryNode); - assertEquals(2, paths.size()); + assertEquals(1, paths.size()); } } diff --git a/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java b/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java index aeca9e782ad..eee0999726e 100644 --- a/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java +++ b/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java @@ -393,9 +393,10 @@ enum Test { TYPE_1, TYPE_2 } - public void oldSwitchAssignExpression () { + public void oldSwitchAssignExpression() { int a = switch (1) { - case 1: yield 2; + case 1: + yield 2; case 2: int b = 2; yield 3; @@ -404,9 +405,10 @@ public void oldSwitchAssignExpression () { }; } - public int oldSwitchReturnExpression () { + public int oldSwitchReturnExpression() { return switch (1) { - case 1: yield 2; + case 1: + yield 2; case 2: int b = 2; yield 3; @@ -417,8 +419,10 @@ public int oldSwitchReturnExpression () { public void oldSwitchExpressionExhaustive() { int a = switch (Test.TYPE_1) { - case TYPE_1: yield 1; - case TYPE_2: yield 2; + case TYPE_1: + yield 1; + case TYPE_2: + yield 2; }; } @@ -448,7 +452,8 @@ public void enhancedSwitchMultipleExpressions() { case 1, 2 -> { int b = 1; } - default -> {} + default -> { + } } } @@ -460,7 +465,7 @@ public void enhancedSwitchNullDefault(Object arg) { } } - public void enhancedSwitchExhaustiveExpression () { + public void enhancedSwitchExhaustiveExpression() { int a = switch (Test.TYPE_1) { case TYPE_1 -> 1; case TYPE_2 -> 2; @@ -469,17 +474,60 @@ public void enhancedSwitchExhaustiveExpression () { public void enhancedSwitchExhaustiveEnum() { switch (Test.TYPE_1) { - case TYPE_1 -> {} - case TYPE_2 -> {} + case TYPE_1 -> { + } + case TYPE_2 -> { + } } } public void enhancedSwitchNonExhaustiveEnum() { switch (Test.TYPE_1) { - case TYPE_2 -> {} + case TYPE_2 -> { + } } } + sealed interface I permits R, A { + } + + record R(int j) implements I { + } + + sealed class A implements I permits B, C { + } + + final class B extends A { + } + + final class C extends A { + } + + static int enhancedSwitchSealedExhaustive(I i) { + switch (i) { + case A a -> {} + case R r -> {} + }; + } + + static int enhancedSwitchMultilevelSealedExhaustive(I i) { + switch (i) { + case B b -> {} + case C b -> {} + case R r -> {} + }; + } + + sealed interface J permits D, E {} + final class D implements J {} + final class E implements J {} + + static int enhancedSwitchExhaustiveParametrization(J ji) { + switch(ji) { // Exhaustive! + case E e -> 42; + }; + } + //All lines will be tested in this method public int simple(int a) { a = a + a / 2; @@ -525,7 +573,7 @@ boolean m3() { } public void complex1(double phase, double source, double target, double baseIncrement, boolean starved, int i, - double current, double[] outputs, double[] amplitudes, double[] rates) { + double current, double[] outputs, double[] amplitudes, double[] rates) { if ((phase) >= 1.0) { while ((phase) >= 1.0) { source = target; From 1b736acf87fc0c5f7f8d138913a7cc1a460a3ab7 Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Sun, 7 Apr 2024 12:32:16 +0200 Subject: [PATCH 20/24] Add test case for a constant true guard --- .../controlflow/ForwardFlowBuilderVisitorTest.java | 10 ++++++++++ .../resources/control-flow/ControlFlowArithmetic.java | 7 +++++++ 2 files changed, 17 insertions(+) diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java index 15ad66e97f4..bf9c515e1e3 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java @@ -330,6 +330,16 @@ public void testSimpleSealedHierarchyExhaustive() throws Exception { assertEquals(2, paths.size()); } + @Test + public void testConstantGuardExhaustive() throws Exception { + ControlFlowGraph graph = testMethod("enhancedSwitchExhaustiveConstantTrueGuard", true, null, null, null); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); + List> paths = pathHelper.paths(entryNode); + assertEquals(2, paths.size()); + } + @Test public void testComplexSealedHierarchyExhaustive() throws Exception { ControlFlowGraph graph = testMethod("enhancedSwitchMultilevelSealedExhaustive", true, null, null, null); diff --git a/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java b/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java index eee0999726e..0dd04c5734c 100644 --- a/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java +++ b/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java @@ -510,6 +510,13 @@ static int enhancedSwitchSealedExhaustive(I i) { }; } + static int enhancedSwitchExhaustiveConstantTrueGuard(I i) { + switch (i) { + case A a when 1 < 2 -> {} + case R r -> {} + }; + } + static int enhancedSwitchMultilevelSealedExhaustive(I i) { switch (i) { case B b -> {} From 262ac4ab04affdd1d3d8aae4c9406c0e9823aaaa Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Sun, 7 Apr 2024 21:08:11 +0200 Subject: [PATCH 21/24] Fix tests and complete exhaustiveness for java 21+ --- .../inria/controlflow/ControlFlowBuilder.java | 13 ++++++----- .../ForwardFlowBuilderVisitorTest.java | 22 +++++++++--------- .../control-flow/ControlFlowArithmetic.java | 23 ++++++++++--------- 3 files changed, 30 insertions(+), 28 deletions(-) diff --git a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java index 5ad12709031..2d7902a4465 100644 --- a/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java +++ b/spoon-control-flow/src/main/java/fr/inria/controlflow/ControlFlowBuilder.java @@ -851,19 +851,20 @@ private boolean checkExhaustive(CtAbstractSwitch switchElement) { return true; } - Set> caseExpressions = switchElement.getCases().stream().flatMap(cases -> cases.getCaseExpressions().stream()).collect(Collectors.toSet()); if (switchElement.getSelector().getType().isEnum()) { + Set> caseExpressions = switchElement.getCases().stream().flatMap(cases -> cases.getCaseExpressions().stream()).collect(Collectors.toSet()); CtEnum enumType = (CtEnum) switchElement.getSelector().getType().getTypeDeclaration(); Set enumConstantNames = enumType.getEnumValues().stream().map(CtEnumValue::getSimpleName).collect(Collectors.toSet()); Set referencedEnumConstants = caseExpressions.stream().map(expression -> ((CtFieldRead) expression).getVariable().getSimpleName()).collect(Collectors.toSet()); - if (referencedEnumConstants.containsAll(enumConstantNames)) { - return true; - } + return referencedEnumConstants.containsAll(enumConstantNames); } - // TODO: More complete exhaustive check - return false; + CtTypeReference selectorTypeReference = switchElement.getSelector().getType(); + + boolean isEnhanced = !selectorTypeReference.isPrimitive() && !selectorTypeReference.equals(selectorTypeReference.getFactory().Type().createReference(String.class)); + + return isEnhanced; } @Override diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java index bf9c515e1e3..215f738611b 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java @@ -270,6 +270,16 @@ public void testEnhancedSwitchImplicitDefault() throws Exception { assertEquals(2, paths.size()); } + @Test + public void testEnhancedSwitchImplicitDefaultString() throws Exception { + ControlFlowGraph graph = testMethod("enhancedSwitchImplicitDefaultString", true, null, null, null); + graph.simplify(); + ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); + ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); + List> paths = pathHelper.paths(entryNode); + assertEquals(2, paths.size()); + } + @Test public void testEnhancedSwitchMultipleExpressions() throws Exception { ControlFlowGraph graph = testMethod("enhancedSwitchMultipleExpressions", true, null, null, null); @@ -347,17 +357,7 @@ public void testComplexSealedHierarchyExhaustive() throws Exception { ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); List> paths = pathHelper.paths(entryNode); - assertEquals(2, paths.size()); - } - - @Test - public void testEnhancedSwitchExhaustiveParametrization() throws Exception { - ControlFlowGraph graph = testMethod("enhancedSwitchExhaustiveParametrization", true, null, null, null); - graph.simplify(); - ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); - ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0); - List> paths = pathHelper.paths(entryNode); - assertEquals(1, paths.size()); + assertEquals(3, paths.size()); } } diff --git a/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java b/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java index 0dd04c5734c..2e079471dc8 100644 --- a/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java +++ b/spoon-control-flow/src/test/resources/control-flow/ControlFlowArithmetic.java @@ -447,6 +447,16 @@ public void enhancedSwitchImplicitDefault() { } } + public void enhancedSwitchImplicitDefaultString() { + int a = 1; + int b = 0; + switch ("a") { + case "1" -> { + b = 1; + } + } + } + public void enhancedSwitchMultipleExpressions() { switch (3) { case 1, 2 -> { @@ -503,11 +513,11 @@ final class B extends A { final class C extends A { } - static int enhancedSwitchSealedExhaustive(I i) { + static void enhancedSwitchSealedExhaustive(I i) { switch (i) { case A a -> {} case R r -> {} - }; + } } static int enhancedSwitchExhaustiveConstantTrueGuard(I i) { @@ -525,15 +535,6 @@ static int enhancedSwitchMultilevelSealedExhaustive(I i) { }; } - sealed interface J permits D, E {} - final class D implements J {} - final class E implements J {} - - static int enhancedSwitchExhaustiveParametrization(J ji) { - switch(ji) { // Exhaustive! - case E e -> 42; - }; - } //All lines will be tested in this method public int simple(int a) { From 2caab712178a7f9a3c68b15a990ac634fd177172 Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Wed, 10 Apr 2024 22:53:25 +0200 Subject: [PATCH 22/24] Update tests --- .../test/java/fr/inria/controlflow/AllBranchesReturnTest.java | 2 +- .../fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/AllBranchesReturnTest.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/AllBranchesReturnTest.java index ef1ea725f6b..b36d40e2889 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/AllBranchesReturnTest.java +++ b/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()); diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java index 215f738611b..3baaf00b14f 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java @@ -527,7 +527,7 @@ public void testInvocation() throws Exception { public void testConstructor() throws URISyntaxException, ClassNotFoundException, IllegalAccessException, InstantiationException { ControlFlowBuilder visitor = new ControlFlowBuilder(); - Factory factory = new SpoonMetaFactory().buildNewFactory(this.getClass().getResource("/control-flow").toURI().getPath(), 17); + Factory factory = new SpoonMetaFactory().buildNewFactory(this.getClass().getResource("/control-flow").toURI().getPath(), 21); ProcessingManager pm = new QueueProcessingManager(factory); pm.addProcessor(new AbstractProcessor>() { @Override From 3c8e0784bac4939252dd1d2afa83ac2a7be59ca6 Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Wed, 10 Apr 2024 23:23:09 +0200 Subject: [PATCH 23/24] Handle multi node statements in InitializedVariables --- .../src/main/java/fr/inria/dataflow/InitializedVariables.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spoon-control-flow/src/main/java/fr/inria/dataflow/InitializedVariables.java b/spoon-control-flow/src/main/java/fr/inria/dataflow/InitializedVariables.java index da16864ab53..4f25bcb6db1 100644 --- a/spoon-control-flow/src/main/java/fr/inria/dataflow/InitializedVariables.java +++ b/spoon-control-flow/src/main/java/fr/inria/dataflow/InitializedVariables.java @@ -152,8 +152,8 @@ private InitFactors initialized(ControlFlowNode n, HashMap defN = includeDefinedInNode ? defined(n) : new HashSet(); - //[Used_n - Def_n] - Set usedN = includeDefinedInNode ? used(n) : new HashSet(); + //[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 usedN = (includeDefinedInNode && n.getIsStatementEnd()) ? used(n) : new HashSet(); usedN.removeAll(defN); InitFactors result = new InitFactors(); From 881371baf5cc054930de6c24619d62bd9cb8b16b Mon Sep 17 00:00:00 2001 From: Mr-Pine Date: Wed, 10 Apr 2024 23:34:22 +0200 Subject: [PATCH 24/24] Remove bug workaround --- .../fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java index 3baaf00b14f..c8a89549b5d 100644 --- a/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java +++ b/spoon-control-flow/src/test/java/fr/inria/controlflow/ForwardFlowBuilderVisitorTest.java @@ -251,8 +251,7 @@ public void testExpressionOldExhaustive() throws Exception { @Test public void testSimpleEnhancedSwitch() throws Exception { - // TODO: This is a spoon bug that imagines a break statement. It should ideally be only 6 statements - ControlFlowGraph graph = testMethod("enhancedSwitchSimple", true, null, 6, null); + ControlFlowGraph graph = testMethod("enhancedSwitchSimple", true, null, 5, null); graph.simplify(); ControlFlowPathHelper pathHelper = new ControlFlowPathHelper(); ControlFlowNode entryNode = graph.findNodesOfKind(BEGIN).get(0);