fixup! Add ControlFlowGraph and implementation

Author: jgardn3r

delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java

@@ -71,10 +71,10 @@ public ControlFlowGraph build() {
       block.updateBlockData(map);
     }
 
-    ControlFlowGraph cfg =
+    ControlFlowGraphImpl cfg =
         new ControlFlowGraphImpl(
-                map.get(currentBlock), map.get(exitBlocks.peek()), new ArrayList<>(map.values()))
-            .pruned();
+            map.get(currentBlock), map.get(exitBlocks.peek()), new ArrayList<>(map.values()));
+    cfg.prune();
 
     populatePredecessors(cfg);
     populateIds(cfg);
@@ -203,7 +203,7 @@ public ProtoBlock getCatchTarget(Type exceptionType) {
         .orElse(getExitBlock());
   }
 
-  private boolean isCompatibleType(Type exceptionType, Type catchType) {
+  private static boolean isCompatibleType(Type exceptionType, Type catchType) {
     return exceptionType.is(catchType) || exceptionType.isDescendantOf(catchType);
   }
 

delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphImpl.java

@@ -57,7 +57,7 @@ public List<Block> getBlocks() {
   }
 
   /// Removes redundant blocks from the graph and updates their neighbouring blocks.
-  public ControlFlowGraphImpl pruned() {
+  public void prune() {
     Set<Block> inactiveBlocks = new HashSet<>();
 
     do {

delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java

@@ -76,9 +76,9 @@
 import org.sonarsource.analyzer.commons.collections.ListUtils;
 
 /**
- * This visitor populates the {@code ControlFlowGraphBuilder} to construct a control flow
- * graph. Generally, the statements and elements are traversed backward simplify the construction of
- * a directed graph. `Block` objects are typically are ordered in way they are evaluated.
+ * This visitor populates the {@link ControlFlowGraphBuilder} to construct a control flow graph.
+ * Generally, the statements and elements are traversed backward simplify the construction of a
+ * directed graph. `Block` objects are typically are ordered in way they are evaluated.
  */
 class ControlFlowGraphVisitor implements DelphiParserVisitor<ControlFlowGraphBuilder> {
 
@@ -139,8 +139,8 @@ public ControlFlowGraphBuilder visit(ArgumentNode node, ControlFlowGraphBuilder
   }
 
   /**
-   * {@code NameReferenceNode} has overloaded meanings in Delphi. The control flow intrinsics
-   * are handled individually.
+   * {@code NameReferenceNode} has overloaded meanings in Delphi. The control flow intrinsics are
+   * handled individually.
    */
   @Override
   public ControlFlowGraphBuilder visit(NameReferenceNode node, ControlFlowGraphBuilder builder) {
@@ -158,7 +158,7 @@ public ControlFlowGraphBuilder visit(NameReferenceNode node, ControlFlowGraphBui
       case "System.Break":
         return buildControlFlowStatement(node, builder.getBreakTarget(), builder);
       case "System.Halt":
-        builder.addBlock(newBlock().withTerminator(node));
+        builder.addBlock(newBlock().withProgramTerminator(node));
         return builder;
       case "System.Continue":
         return buildControlFlowStatement(node, builder.getContinueTarget(), builder);
@@ -171,7 +171,7 @@ public ControlFlowGraphBuilder visit(NameReferenceNode node, ControlFlowGraphBui
     return builder;
   }
 
-  private ControlFlowGraphBuilder buildControlFlowStatement(
+  private static ControlFlowGraphBuilder buildControlFlowStatement(
       DelphiNode node, ProtoBlock target, ControlFlowGraphBuilder builder) {
     if (target == null) {
       throw new IllegalStateException(
@@ -181,7 +181,7 @@ private ControlFlowGraphBuilder buildControlFlowStatement(
     return builder;
   }
 
-  private void handleExceptionalPaths(ControlFlowGraphBuilder builder) {
+  private static void handleExceptionalPaths(ControlFlowGraphBuilder builder) {
     if (!builder.inTryContext()) {
       // Only consider routines as potentially exceptional if in a `try` context.
       return;
@@ -568,11 +568,9 @@ private ControlFlowGraphBuilder buildTryFinally(
     builder.pushLoopContext(builder.getCurrentBlock(), builder.getCurrentBlock());
     builder.pushExitBlock(builder.getCurrentBlock());
 
-    ProtoBlock beforeFinally = builder.addBlockBeforeCurrent();
+    builder.addBlockBeforeCurrent();
 
     // Body
-    builder.setCurrentBlock(beforeFinally);
-
     builder.pushTryFinallyContext();
     build(node.getStatementList(), builder);
     builder.popTryContext();

delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Finally.java

@@ -22,7 +22,7 @@
 import java.util.HashSet;
 import java.util.Set;
 
-/** A {@code finally} block whose control flow depends on the existence previous exceptions */
+/** A {@code finally} block whose control flow depends on the existence of previous exceptions */
 public interface Finally extends Block {
   /**
    * Next block in the {@link ControlFlowGraph} without exceptional circumstances

delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Sink.java renamed to delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Halt.java

@@ -21,8 +21,8 @@
 import java.util.Collections;
 import java.util.Set;
 
-/** A block which has no successors by design, e.g., {@code Exit} in a routine */
-public interface Sink extends Block, Terminated {
+/** A block which represents a {@code Halt}, an abrupt process termination */
+public interface Halt extends Block, Terminated {
   @Override
   default Set<Block> getSuccessors() {
     return Collections.emptySet();

delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/block/ProtoBlockBuilder.java

@@ -22,8 +22,8 @@
 import au.com.integradev.delphi.cfg.api.Branch;
 import au.com.integradev.delphi.cfg.api.Cases;
 import au.com.integradev.delphi.cfg.api.Finally;
+import au.com.integradev.delphi.cfg.api.Halt;
 import au.com.integradev.delphi.cfg.api.Linear;
-import au.com.integradev.delphi.cfg.api.Sink;
 import au.com.integradev.delphi.cfg.api.Terminus;
 import au.com.integradev.delphi.cfg.api.UnconditionalJump;
 import au.com.integradev.delphi.cfg.api.UnknownException;
@@ -50,9 +50,9 @@ public static ProtoBlockBuilder newBlock() {
     return new ProtoBlockBuilder();
   }
 
-  public ProtoBlockBuilder withTerminator(DelphiNode terminator) {
-    this.blockSupplier = SinkImpl::new;
-    this.dataSetter = (blocks, block) -> ((SinkImpl) block).setData(terminator);
+  public ProtoBlockBuilder withProgramTerminator(DelphiNode terminator) {
+    this.blockSupplier = HaltImpl::new;
+    this.dataSetter = (blocks, block) -> ((HaltImpl) block).setData(terminator);
     return this;
   }
 
@@ -314,10 +314,10 @@ public String getDescription() {
     }
   }
 
-  static class SinkImpl extends BlockImpl implements Sink {
+  static class HaltImpl extends BlockImpl implements Halt {
     private Terminator terminator;
 
-    protected SinkImpl(List<DelphiNode> elements) {
+    protected HaltImpl(List<DelphiNode> elements) {
       super(elements);
     }
 

delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java

@@ -80,17 +80,17 @@
 class ControlFlowGraphTest {
   private static final Logger LOG = LoggerFactory.getLogger(ControlFlowGraphTest.class);
 
-  private ControlFlowGraph buildCFG(String input) {
-    return buildCFG(Collections.emptyMap(), input);
+  private ControlFlowGraph buildCfg(String input) {
+    return buildCfg(Collections.emptyMap(), input);
   }
 
-  private ControlFlowGraph buildCFG(List<String> variables, String input) {
-    return buildCFG(Map.of("var", variables), input);
+  private ControlFlowGraph buildCfg(List<String> variables, String input) {
+    return buildCfg(Map.of("var", variables), input);
   }
 
-  private ControlFlowGraph buildCFG(Map<String, List<String>> sections, String input) {
+  private ControlFlowGraph buildCfg(Map<String, List<String>> sections, String input) {
     try {
-      var tempFile = File.createTempFile("CFGTest-", ".pas");
+      var tempFile = File.createTempFile("CfgTest-", ".pas");
       tempFile.deleteOnExit();
 
       StringBuilder content = new StringBuilder();
@@ -155,11 +155,11 @@ private void test(String input, GraphChecker checker) {
   }
 
   private void test(List<String> variables, String input, GraphChecker checker) {
-    checker.check(buildCFG(variables, input));
+    checker.check(buildCfg(variables, input));
   }
 
   private void test(Map<String, List<String>> sections, String input, GraphChecker checker) {
-    checker.check(buildCFG(sections, input));
+    checker.check(buildCfg(sections, input));
   }
 
   private static Path createStandardLibrary() {
@@ -225,15 +225,15 @@ private Consumer<DelphiNode> unaryOpTest(UnaryOperator operator) {
   }
 
   @Test
-  void testEmptyCFG() {
-    final ControlFlowGraph cfg = buildCFG("");
+  void testEmptyCfg() {
+    final ControlFlowGraph cfg = buildCfg("");
     checker().check(cfg);
     assertThat(cfg.getEntryBlock().getSuccessors()).as("entry is an exit").isEmpty();
   }
 
   @Test
-  void testSimplestCFG() {
-    final ControlFlowGraph cfg = buildCFG("Foo;");
+  void testSimplestCfg() {
+    final ControlFlowGraph cfg = buildCfg("Foo;");
     checker(block(element(NameReferenceNode.class, "Foo")).succeedsTo(0)).check(cfg);
     Block entry = cfg.getEntryBlock();
     assertThat(entry)
@@ -243,7 +243,7 @@ void testSimplestCFG() {
     assertThat(exit)
         .withFailMessage("Expecting entry block's successor to be the exit block")
         .isEqualTo(cfg.getExitBlock())
-        .withFailMessage("Expecting entry block's successor to be the of type Terminus.")
+        .withFailMessage("Expecting entry block's successor to be of type Terminus.")
         .isInstanceOf(Terminus.class);
   }
 
@@ -671,14 +671,16 @@ void testForInConditionalContinue() {
 
   @Test
   void testBreakOutsideOfLoop() {
-    assertThatThrownBy(() -> test("Break;", checker()))
+    GraphChecker checker = checker();
+    assertThatThrownBy(() -> test("Break;", checker))
         .withFailMessage("'Break' statement not in loop statement.")
         .isInstanceOf(IllegalStateException.class);
   }
 
   @Test
   void testContinueOutsideOfLoop() {
-    assertThatThrownBy(() -> test("Continue;", checker()))
+    GraphChecker checker = checker();
+    assertThatThrownBy(() -> test("Continue;", checker))
         .withFailMessage("'Continue' statement not in loop statement.")
         .isInstanceOf(IllegalStateException.class);
   }

delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/checker/BlockChecker.java

@@ -25,7 +25,7 @@
 import au.com.integradev.delphi.cfg.api.Cases;
 import au.com.integradev.delphi.cfg.api.Finally;
 import au.com.integradev.delphi.cfg.api.Linear;
-import au.com.integradev.delphi.cfg.api.Sink;
+import au.com.integradev.delphi.cfg.api.Halt;
 import au.com.integradev.delphi.cfg.api.Terminated;
 import au.com.integradev.delphi.cfg.api.UnconditionalJump;
 import au.com.integradev.delphi.cfg.api.UnknownException;
@@ -212,7 +212,7 @@ public BlockChecker succeedsToWithExceptions(int successor, int... unknownExcept
   }
 
   public BlockChecker isSink() {
-    successorChecker = new BlockDetailChecker(block -> assertBlockIsType(block, Sink.class));
+    successorChecker = new BlockDetailChecker(block -> assertBlockIsType(block, Halt.class));
     return this;
   }