diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/LoopExecutingAtMostOnceCheck.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/LoopExecutingAtMostOnceCheck.java index f1d99e559..9afccd1c2 100644 --- a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/LoopExecutingAtMostOnceCheck.java +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/LoopExecutingAtMostOnceCheck.java @@ -23,6 +23,7 @@ import au.com.integradev.delphi.cfg.api.Branch; import au.com.integradev.delphi.cfg.api.ControlFlowGraph; import au.com.integradev.delphi.cfg.api.Terminated; +import au.com.integradev.delphi.cfg.api.Terminus; import au.com.integradev.delphi.utils.ControlFlowGraphUtils; import java.util.ArrayDeque; import java.util.ArrayList; @@ -244,7 +245,7 @@ private static boolean jumpsBeforeLoop(ControlFlowGraph cfg, Block loopBlock, De return true; } if ((search.getSuccessors().size() == 1 && search.getSuccessors().contains(jumpTarget)) - || search.equals(cfg.getExitBlock())) { + || search instanceof Terminus) { return false; } diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/NoreturnContractCheck.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/NoreturnContractCheck.java index b8fea04b1..c851e73f8 100644 --- a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/NoreturnContractCheck.java +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/NoreturnContractCheck.java @@ -20,8 +20,10 @@ import au.com.integradev.delphi.cfg.api.Block; import au.com.integradev.delphi.cfg.api.ControlFlowGraph; +import au.com.integradev.delphi.cfg.api.ExceptionalRoutineExit; import au.com.integradev.delphi.cfg.api.Finally; import au.com.integradev.delphi.cfg.api.PossibleException; +import au.com.integradev.delphi.cfg.api.RoutineExit; import au.com.integradev.delphi.cfg.api.Terminated; import au.com.integradev.delphi.cfg.api.UnknownException; import au.com.integradev.delphi.cfg.block.TerminatorKind; @@ -50,11 +52,11 @@ public DelphiCheckContext visit(RoutineImplementationNode routine, DelphiCheckCo private static boolean canReturnNormally(RoutineImplementationNode routine) { ControlFlowGraph cfg = ControlFlowGraphUtils.findContainingCFG(routine); - return cfg != null && canReturnNormally(cfg, cfg.getEntryBlock()); + return cfg != null && canReturnNormally(cfg.getEntryBlock()); } - private static boolean canReturnNormally(ControlFlowGraph cfg, Block block) { - if (block.equals(cfg.getExitBlock())) { + private static boolean canReturnNormally(Block block) { + if (block instanceof RoutineExit) { return true; } @@ -63,18 +65,20 @@ private static boolean canReturnNormally(ControlFlowGraph cfg, Block block) { if (isGuaranteedException(block)) { successors = block.getSuccessors().stream() - .filter(s -> notFinallyOrExit(cfg, s)) + .filter(NoreturnContractCheck::notFinallyOrExit) .collect(Collectors.toSet()); } else if (block instanceof PossibleException) { successors = new HashSet<>(); successors.add(((PossibleException) block).getSuccessor()); ((PossibleException) block) - .getExceptions().stream().filter(s -> notFinallyOrExit(cfg, s)).forEach(successors::add); + .getExceptions().stream() + .filter(NoreturnContractCheck::notFinallyOrExit) + .forEach(successors::add); } else { successors = block.getSuccessors(); } - return successors.stream().anyMatch(s -> canReturnNormally(cfg, s)); + return successors.stream().anyMatch(NoreturnContractCheck::canReturnNormally); } private static boolean isGuaranteedException(Block block) { @@ -83,7 +87,7 @@ private static boolean isGuaranteedException(Block block) { && ((Terminated) block).getTerminatorKind() == TerminatorKind.RAISE); } - private static boolean notFinallyOrExit(ControlFlowGraph cfg, Block block) { - return !(block instanceof Finally || block.equals(cfg.getExitBlock())); + private static boolean notFinallyOrExit(Block block) { + return !(block instanceof Finally || block instanceof ExceptionalRoutineExit); } } diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/RedundantJumpCheck.java b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/RedundantJumpCheck.java index d564ed4b1..f11c2f95b 100644 --- a/delphi-checks/src/main/java/au/com/integradev/delphi/checks/RedundantJumpCheck.java +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/checks/RedundantJumpCheck.java @@ -20,7 +20,9 @@ import au.com.integradev.delphi.cfg.api.Block; import au.com.integradev.delphi.cfg.api.ControlFlowGraph; +import au.com.integradev.delphi.cfg.api.ExceptionalRoutineExit; import au.com.integradev.delphi.cfg.api.Finally; +import au.com.integradev.delphi.cfg.api.RoutineExit; import au.com.integradev.delphi.cfg.api.Terminated; import au.com.integradev.delphi.cfg.api.UnconditionalJump; import au.com.integradev.delphi.utils.ControlFlowGraphUtils; @@ -134,7 +136,11 @@ private boolean isViolation(ControlFlowGraph cfg) { } if (!finallyBlock.getSuccessor().equals(finallyBlock.getExceptionSuccessor())) { // multiple paths after the finally corresponds to code that would be skipped from the jump - return false; + if (!(finallyBlock.getSuccessor() instanceof RoutineExit + && finallyBlock.getExceptionSuccessor() instanceof ExceptionalRoutineExit)) { + // if both of the paths are routine exits, then they are effectively equivalent + return false; + } } } // if no invalidating try-finally blocks are found, the use is a violation diff --git a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/NoreturnContractCheckTest.java b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/NoreturnContractCheckTest.java index 8ce3da7a2..18b125b37 100644 --- a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/NoreturnContractCheckTest.java +++ b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/NoreturnContractCheckTest.java @@ -341,7 +341,7 @@ void testNoreturnRoutineWithRaiseToIrrelevantExceptShouldAddIssue() { } @Test - void testFailOnUpgradeNoreturnRoutineWithRaiseToSpecificEmptyExceptShouldNotAddIssue() { + void testNoreturnRoutineWithRaiseToSpecificEmptyExceptShouldAddIssue() { CheckVerifier.newVerifier() .withCheck(new NoreturnContractCheck()) .onFile( @@ -349,7 +349,7 @@ void testFailOnUpgradeNoreturnRoutineWithRaiseToSpecificEmptyExceptShouldNotAddI .appendImpl("uses") .appendImpl(" System.SysUtils;") .appendImpl("") - .appendImpl("procedure RaiseToSpecificExcept; noreturn;") + .appendImpl("procedure RaiseToSpecificExcept; noreturn; // Noncompliant") .appendImpl("begin") .appendImpl(" try") .appendImpl(" raise Exception.Create('Error');") @@ -359,6 +359,6 @@ void testFailOnUpgradeNoreturnRoutineWithRaiseToSpecificEmptyExceptShouldNotAddI .appendImpl(" end;") .appendImpl(" end;") .appendImpl("end;")) - .verifyNoIssues(); + .verifyIssues(); } } diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java index ab795cbfa..b2d1a3352 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphBuilder.java @@ -20,6 +20,8 @@ import au.com.integradev.delphi.cfg.api.Block; import au.com.integradev.delphi.cfg.api.ControlFlowGraph; +import au.com.integradev.delphi.cfg.api.ExceptionalRoutineExit; +import au.com.integradev.delphi.cfg.api.RoutineExit; import au.com.integradev.delphi.cfg.block.BlockImpl; import au.com.integradev.delphi.cfg.block.ProtoBlock; import au.com.integradev.delphi.cfg.block.ProtoBlockFactory; @@ -47,6 +49,7 @@ public class ControlFlowGraphBuilder { private final List blocks = new ArrayList<>(); private final Deque exitBlocks = new ArrayDeque<>(); + private final Deque exceptionalExitBlocks = new ArrayDeque<>(); private final Deque breakTargets = new ArrayDeque<>(); private final Deque continueTargets = new ArrayDeque<>(); private final Map labelTargets = new HashMap<>(); @@ -57,9 +60,14 @@ public class ControlFlowGraphBuilder { @SuppressWarnings("this-escape") public ControlFlowGraphBuilder() { + ProtoBlock exceptionalExitBlock = ProtoBlockFactory.exceptionalExitBlock(); + addBlock(exceptionalExitBlock); + exceptionalExitBlocks.add(exceptionalExitBlock); + ProtoBlock exitBlock = ProtoBlockFactory.exitBlock(); addBlock(exitBlock); exitBlocks.add(exitBlock); + addBlockBefore(exitBlock); } @@ -73,8 +81,7 @@ public ControlFlowGraph build() { } ControlFlowGraphImpl cfg = - new ControlFlowGraphImpl( - map.get(currentBlock), map.get(exitBlocks.peek()), new ArrayList<>(map.values())); + new ControlFlowGraphImpl(map.get(currentBlock), new ArrayList<>(map.values())); cfg.prune(); populatePredecessors(cfg); @@ -93,8 +100,17 @@ private static void populatePredecessors(ControlFlowGraph cfg) { private static void populateIds(ControlFlowGraph cfg) { List blocks = Lists.reverse(cfg.getBlocks()); - for (int blockId = 0; blockId < blocks.size(); blockId++) { - ((BlockImpl) blocks.get(blockId)).setId(blockId); + int blockId = 0; + for (Block block : blocks) { + if (block instanceof RoutineExit) { + // Ensure `RoutineExit` is always id 0 + ((BlockImpl) block).setId(0); + } else if (block instanceof ExceptionalRoutineExit) { + // Ensure `ExceptionalRoutineExit` is always id -1 + ((BlockImpl) block).setId(-1); + } else { + ((BlockImpl) block).setId(++blockId); + } } } @@ -102,6 +118,10 @@ public ProtoBlock getExitBlock() { return exitBlocks.peek(); } + public ProtoBlock getExceptionalExitBlock() { + return exceptionalExitBlocks.peek(); + } + public ProtoBlock getBreakTarget() { return breakTargets.peek(); } @@ -110,6 +130,10 @@ public ProtoBlock getContinueTarget() { return continueTargets.peek(); } + public boolean isInLoop() { + return !continueTargets.isEmpty(); + } + public void pushLoopContext(ProtoBlock continueTarget, ProtoBlock breakTarget) { continueTargets.push(continueTarget); breakTargets.push(breakTarget); @@ -122,10 +146,12 @@ public void popLoopContext() { public void pushExitBlock(ProtoBlock target) { exitBlocks.push(target); + exceptionalExitBlocks.push(target); } public void popExitBlock() { exitBlocks.pop(); + exceptionalExitBlocks.pop(); } private static class UnresolvedLabel { @@ -193,7 +219,7 @@ public boolean inTryContext() { public ProtoBlock getCatchTarget(Type exceptionType) { if (tryContexts.isEmpty()) { - return getExitBlock(); + return getExceptionalExitBlock(); } TryContext tryContext = tryContexts.peek(); return tryContext.catches.keySet().stream() @@ -201,7 +227,7 @@ public ProtoBlock getCatchTarget(Type exceptionType) { .findFirst() .map(tryContext.catches::get) .or(() -> Optional.ofNullable(tryContext.elseBlock)) - .orElse(getExitBlock()); + .orElse(getExceptionalExitBlock()); } private static boolean isCompatibleType(Type exceptionType, Type catchType) { @@ -214,7 +240,7 @@ public Set getAllCatchTargets() { } TryContext context = tryContexts.peek(); Stream elseOrExit = - Stream.of(Optional.ofNullable(context.elseBlock).orElse(getExitBlock())); + Stream.of(Optional.ofNullable(context.elseBlock).orElse(getExceptionalExitBlock())); return Stream.concat(context.catches.values().stream(), elseOrExit) .filter(Objects::nonNull) .collect(Collectors.toSet()); diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphImpl.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphImpl.java index f5db511cd..c00ccdd9e 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphImpl.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphImpl.java @@ -20,6 +20,7 @@ import au.com.integradev.delphi.cfg.api.Block; import au.com.integradev.delphi.cfg.api.ControlFlowGraph; +import au.com.integradev.delphi.cfg.api.ExceptionalRoutineExit; import au.com.integradev.delphi.cfg.api.Terminated; import au.com.integradev.delphi.cfg.api.UnconditionalJump; import au.com.integradev.delphi.cfg.block.BlockImpl; @@ -32,12 +33,10 @@ public class ControlFlowGraphImpl implements ControlFlowGraph { private Block entry; - private final Block exit; private final List blocks; - public ControlFlowGraphImpl(Block entry, Block exit, List blocks) { + public ControlFlowGraphImpl(Block entry, List blocks) { this.entry = entry; - this.exit = exit; this.blocks = blocks; } @@ -46,11 +45,6 @@ public Block getEntryBlock() { return entry; } - @Override - public Block getExitBlock() { - return exit; - } - @Override public List getBlocks() { return Collections.unmodifiableList(Lists.reverse(blocks)); @@ -75,6 +69,20 @@ public void prune() { blocks.forEach(inactiveBlocks::remove); } while (!inactiveBlocks.isEmpty()); + + // Remove the exceptional exit if there are no paths to it + Block exceptionalExitBlock = + Lists.reverse(blocks).stream() + .filter(ExceptionalRoutineExit.class::isInstance) + .findFirst() + .orElseThrow( + () -> new IllegalStateException("Couldn't find the exceptional exit block")); + + if (blocks.stream() + .flatMap(block -> block.getSuccessors().stream()) + .noneMatch(successor -> successor.equals(exceptionalExitBlock))) { + blocks.remove(exceptionalExitBlock); + } } private boolean isInactive(Block block) { diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java index 99b3ed841..bf8240419 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphVisitor.java @@ -208,7 +208,7 @@ private static void handleExceptionalPaths(ControlFlowGraphBuilder builder) { private static Set getUnknownExceptionTargets(ControlFlowGraphBuilder builder) { Set exceptions = builder.getAllCatchTargets(); if (exceptions.isEmpty()) { - return Set.of(builder.getExitBlock()); + return Set.of(builder.getExceptionalExitBlock()); } return exceptions; } @@ -548,10 +548,13 @@ private ControlFlowGraphBuilder buildTryFinally( FinallyBlockNode finallyNode = node.getFinallyBlock(); builder.addBlock( ProtoBlockFactory.finallyBlock( - node.getFinallyBlock(), builder.getCurrentBlock(), builder.getExitBlock())); + node.getFinallyBlock(), builder.getCurrentBlock(), builder.getExceptionalExitBlock())); build(finallyNode.getStatementList(), builder); - builder.pushLoopContext(builder.getCurrentBlock(), builder.getCurrentBlock()); + boolean inLoop = builder.isInLoop(); + if (inLoop) { + builder.pushLoopContext(builder.getCurrentBlock(), builder.getCurrentBlock()); + } builder.pushExitBlock(builder.getCurrentBlock()); builder.addBlockBeforeCurrent(); @@ -565,6 +568,9 @@ private ControlFlowGraphBuilder buildTryFinally( builder.addElement(node); builder.popExitBlock(); + if (inLoop) { + builder.popLoopContext(); + } return builder; } diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/ControlFlowGraph.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/ControlFlowGraph.java index adf01982c..72a9e3e23 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/ControlFlowGraph.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/ControlFlowGraph.java @@ -29,13 +29,6 @@ public interface ControlFlowGraph { */ Block getEntryBlock(); - /** - * The final exit block of the control flow graph - * - * @return the final block of the control flow graph - */ - Block getExitBlock(); - /** * All the blocks within the control flow graph * diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/ExceptionalRoutineExit.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/ExceptionalRoutineExit.java new file mode 100644 index 000000000..e1c2c8a28 --- /dev/null +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/ExceptionalRoutineExit.java @@ -0,0 +1,22 @@ +/* + * Sonar Delphi Plugin + * Copyright (C) 2026 Integrated Application Development + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package au.com.integradev.delphi.cfg.api; + +/** A block representing an exceptional end of a routine, e.g., an uncaught exception */ +public interface ExceptionalRoutineExit extends Terminus {} diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/RoutineExit.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/RoutineExit.java new file mode 100644 index 000000000..f52510be9 --- /dev/null +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/RoutineExit.java @@ -0,0 +1,22 @@ +/* + * Sonar Delphi Plugin + * Copyright (C) 2026 Integrated Application Development + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package au.com.integradev.delphi.cfg.api; + +/** A block representing the end of a routine */ +public interface RoutineExit extends Terminus {} diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Terminus.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Terminus.java index f3db9210b..e2557a462 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Terminus.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Terminus.java @@ -21,7 +21,7 @@ import java.util.Collections; import java.util.Set; -/** A block which has no successors by nature, e.g., the end of a routine */ +/** A block that ends paths through a {@link ControlFlowGraph} */ public interface Terminus extends Block { @Override default Set getSuccessors() { diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/block/ProtoBlockFactory.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/block/ProtoBlockFactory.java index d8b82fb26..b1847b61b 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/block/ProtoBlockFactory.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/block/ProtoBlockFactory.java @@ -21,11 +21,12 @@ import au.com.integradev.delphi.cfg.api.Block; import au.com.integradev.delphi.cfg.api.Branch; import au.com.integradev.delphi.cfg.api.Cases; +import au.com.integradev.delphi.cfg.api.ExceptionalRoutineExit; 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.PossibleException; -import au.com.integradev.delphi.cfg.api.Terminus; +import au.com.integradev.delphi.cfg.api.RoutineExit; import au.com.integradev.delphi.cfg.api.UnconditionalJump; import au.com.integradev.delphi.cfg.api.UnknownException; import java.util.Collection; @@ -42,7 +43,11 @@ private ProtoBlockFactory() { } public static ProtoBlock exitBlock() { - return new ProtoBlock(TerminusImpl::new, (blocks, block) -> {}); + return new ProtoBlock(RoutineExitImpl::new, (blocks, block) -> {}); + } + + public static ProtoBlock exceptionalExitBlock() { + return new ProtoBlock(ExceptionalRoutineExitImpl::new, (blocks, block) -> {}); } public static ProtoBlock halt(DelphiNode terminator) { @@ -442,9 +447,9 @@ public String getBlockType() { } } - private static class TerminusImpl extends BlockImpl implements Terminus { + private static class RoutineExitImpl extends BlockImpl implements RoutineExit { - public TerminusImpl(List elements) { + public RoutineExitImpl(List elements) { super(elements); } @@ -460,7 +465,30 @@ public String getDescription() { @Override public String getBlockType() { - return "Terminus"; + return "RoutineExit"; + } + } + + private static class ExceptionalRoutineExitImpl extends BlockImpl + implements ExceptionalRoutineExit { + + public ExceptionalRoutineExitImpl(List elements) { + super(elements); + } + + @Override + public void replaceInactiveSuccessor(Block inactiveBlock, Block target) { + // Block has no successors + } + + @Override + public String getDescription() { + return String.format("%n\t(ExceptionalExit)"); + } + + @Override + public String getBlockType() { + return "ExceptionalExit"; } } diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java index 7c6571d5b..2092ba313 100644 --- a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/ControlFlowGraphTest.java @@ -29,7 +29,7 @@ import au.com.integradev.delphi.cfg.api.Block; import au.com.integradev.delphi.cfg.api.ControlFlowGraph; import au.com.integradev.delphi.cfg.api.Linear; -import au.com.integradev.delphi.cfg.api.Terminus; +import au.com.integradev.delphi.cfg.api.RoutineExit; import au.com.integradev.delphi.cfg.block.TerminatorKind; import au.com.integradev.delphi.cfg.checker.GraphChecker; import au.com.integradev.delphi.cfg.checker.StatementTerminator; @@ -53,6 +53,8 @@ import java.util.function.Consumer; import org.apache.commons.io.FileUtils; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.sonar.plugins.communitydelphi.api.ast.BinaryExpressionNode; @@ -83,6 +85,9 @@ class ControlFlowGraphTest { private static final Logger LOG = LoggerFactory.getLogger(ControlFlowGraphTest.class); + private static final int EXIT_ID = 0; + private static final int EXCEPTION_EXIT_ID = -1; + private ControlFlowGraph buildCfg(String input) { return buildCfg(Collections.emptyMap(), input); } @@ -239,7 +244,7 @@ void testEmptyCfg() { @Test void testSimplestCfg() { final ControlFlowGraph cfg = buildCfg("Foo;"); - checker(block(element(NameReferenceNode.class, "Foo")).succeedsTo(0)).check(cfg); + checker(block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID)).check(cfg); Block entry = cfg.getEntryBlock(); assertThat(entry) .withFailMessage("Expecting entry block to have single successor") @@ -247,9 +252,7 @@ void testSimplestCfg() { Block exit = entry.getSuccessors().iterator().next(); assertThat(exit) .withFailMessage("Expecting entry block's successor to be the exit block") - .isEqualTo(cfg.getExitBlock()) - .withFailMessage("Expecting entry block's successor to be of type Terminus.") - .isInstanceOf(Terminus.class); + .isInstanceOf(RoutineExit.class); } @Test @@ -258,9 +261,9 @@ void testIfThen() { "if A then Foo;", checker( block(element(NameReferenceNode.class, "A")) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(IfStatementNode.class), - block(element(NameReferenceNode.class, "Foo")).succeedsTo(0))); + block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID))); } @Test @@ -271,8 +274,8 @@ void testIfThenElse() { block(element(NameReferenceNode.class, "A")) .branchesTo(2, 1) .withTerminator(IfStatementNode.class), - block(element(NameReferenceNode.class, "Foo")).succeedsTo(0), - block(element(NameReferenceNode.class, "Bar")).succeedsTo(0))); + block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID), + block(element(NameReferenceNode.class, "Bar")).succeedsTo(EXIT_ID))); } @Test @@ -283,11 +286,11 @@ void testIfThenElseIf() { block(element(NameReferenceNode.class, "A")) .branchesTo(3, 2) .withTerminator(IfStatementNode.class), - block(element(NameReferenceNode.class, "Foo")).succeedsTo(0), + block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID), block(element(NameReferenceNode.class, "B")) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(IfStatementNode.class), - block(element(NameReferenceNode.class, "Bar")).succeedsTo(0))); + block(element(NameReferenceNode.class, "Bar")).succeedsTo(EXIT_ID))); } @Test @@ -300,9 +303,9 @@ void testIfOr() { .withTerminator(BinaryExpressionNode.class) .withTerminatorNodeCheck(binaryOpTest(BinaryOperator.OR)), block(element(NameReferenceNode.class, "B")) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(IfStatementNode.class), - block(element(NameReferenceNode.class, "Foo")).succeedsTo(0))); + block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID))); } @Test @@ -311,13 +314,13 @@ void testIfAnd() { "if A and B then Foo;", checker( block(element(NameReferenceNode.class, "A")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(BinaryExpressionNode.class) .withTerminatorNodeCheck(binaryOpTest(BinaryOperator.AND)), block(element(NameReferenceNode.class, "B")) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(IfStatementNode.class), - block(element(NameReferenceNode.class, "Foo")).succeedsTo(0))); + block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID))); } @Test @@ -328,7 +331,7 @@ void testEmptyIf() { block(element(NameReferenceNode.class, "C")) .branchesTo(1, 1) .withTerminator(IfStatementNode.class), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -339,14 +342,14 @@ void testEmptyIfElse() { block(element(NameReferenceNode.class, "C")) .branchesTo(1, 1) .withTerminator(IfStatementNode.class), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test void testLocalVarDeclaration() { test( "var Bar: TObject;", - checker(block(element(NameDeclarationNode.class, "Bar")).succeedsTo(0))); + checker(block(element(NameDeclarationNode.class, "Bar")).succeedsTo(EXIT_ID))); } @Test @@ -357,7 +360,7 @@ void testLocalVarListDeclaration() { block( element(NameDeclarationNode.class, "Foo"), element(NameDeclarationNode.class, "Bar")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -366,7 +369,7 @@ void testLocalVarValueDeclaration() { "var Bar: TObject := nil;", checker( block(element(NilLiteralNode.class), element(NameDeclarationNode.class, "Bar")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -375,7 +378,7 @@ void testUntypedConstDeclaration() { "const Foo = 0;", checker( block(element(IntegerLiteralNode.class), element(NameDeclarationNode.class, "Foo")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -384,7 +387,7 @@ void testTypedConstDeclaration() { "const Foo: Integer = 0;", checker( block(element(IntegerLiteralNode.class), element(NameDeclarationNode.class, "Foo")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -392,9 +395,9 @@ void testCaseStatement1Arm() { test( "case Foo of Bar: Bar1; end;", checker( - block(element(NameReferenceNode.class, "Bar1")).succeedsTo(0), + block(element(NameReferenceNode.class, "Bar1")).succeedsTo(EXIT_ID), block(element(NameReferenceNode.class, "Foo"), element(NameReferenceNode.class, "Bar")) - .succeedsToCases(0, 2) + .succeedsToCases(EXIT_ID, 2) .withTerminator(CaseStatementNode.class))); } @@ -403,14 +406,14 @@ void testCaseStatement2Arm() { test( "case Foo of Bar: Bar1; Baz: Baz1; end;", checker( - block(element(NameReferenceNode.class, "Bar1")).succeedsTo(0), - block(element(NameReferenceNode.class, "Baz1")).succeedsTo(0), + block(element(NameReferenceNode.class, "Bar1")).succeedsTo(EXIT_ID), + block(element(NameReferenceNode.class, "Baz1")).succeedsTo(EXIT_ID), block( element(NameReferenceNode.class, "Foo"), element(NameReferenceNode.class, "Bar"), element(NameReferenceNode.class, "Baz")) .withTerminator(CaseStatementNode.class) - .succeedsToCases(0, 2, 3))); + .succeedsToCases(EXIT_ID, 2, 3))); } @Test @@ -418,8 +421,8 @@ void testCaseStatementRangeArm() { test( "case Foo of 1..2: Bar1; 3..4: Baz1; end;", checker( - block(element(NameReferenceNode.class, "Bar1")).succeedsTo(0), - block(element(NameReferenceNode.class, "Baz1")).succeedsTo(0), + block(element(NameReferenceNode.class, "Bar1")).succeedsTo(EXIT_ID), + block(element(NameReferenceNode.class, "Baz1")).succeedsTo(EXIT_ID), block( element(NameReferenceNode.class, "Foo"), element(IntegerLiteralNode.class, "1"), @@ -427,7 +430,7 @@ void testCaseStatementRangeArm() { element(IntegerLiteralNode.class, "3"), element(IntegerLiteralNode.class, "4")) .withTerminator(CaseStatementNode.class) - .succeedsToCases(0, 2, 3))); + .succeedsToCases(EXIT_ID, 2, 3))); } @Test @@ -435,9 +438,9 @@ void testCaseStatementElse() { test( "case Foo of Bar: Bar1; Baz: Baz1; else Flarp; end;", checker( - block(element(NameReferenceNode.class, "Bar1")).succeedsTo(0), - block(element(NameReferenceNode.class, "Baz1")).succeedsTo(0), - block(element(NameReferenceNode.class, "Flarp")).succeedsTo(0), + block(element(NameReferenceNode.class, "Bar1")).succeedsTo(EXIT_ID), + block(element(NameReferenceNode.class, "Baz1")).succeedsTo(EXIT_ID), + block(element(NameReferenceNode.class, "Flarp")).succeedsTo(EXIT_ID), block( element(NameReferenceNode.class, "Foo"), element(NameReferenceNode.class, "Bar"), @@ -454,7 +457,7 @@ void testEmptyCase() { block(element(NameReferenceNode.class, "S")) .withTerminator(CaseStatementNode.class) .succeedsToCases(1), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -465,7 +468,7 @@ void testEmptyCaseElse() { block(element(NameReferenceNode.class, "S")) .withTerminator(CaseStatementNode.class) .succeedsToCases(1), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -479,7 +482,7 @@ void testEmptyCaseArm() { element(NameReferenceNode.class, "S2")) .withTerminator(CaseStatementNode.class) .succeedsToCases(1, 1), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -489,7 +492,7 @@ void testRepeat() { checker( block(element(NameReferenceNode.class, "Bar")).succeedsTo(1), block(element(NameReferenceNode.class, "Foo")) - .branchesTo(0, 2) + .branchesTo(EXIT_ID, 2) .withTerminator(RepeatStatementNode.class))); } @@ -501,7 +504,7 @@ void testRepeatContinue() { terminator(StatementTerminator.CONTINUE).jumpsTo(1, 2), block(element(NameReferenceNode.class, "Bar")).succeedsTo(1), block(element(NameReferenceNode.class, "Foo")) - .branchesTo(0, 3) + .branchesTo(EXIT_ID, 3) .withTerminator(RepeatStatementNode.class))); } @@ -511,10 +514,10 @@ void testRepeatBreak() { "repeat Bar; break; until Foo;", checker( block(element(NameReferenceNode.class, "Bar")) - .jumpsTo(0, 1) + .jumpsTo(EXIT_ID, 1) .withTerminator(StatementTerminator.BREAK), block(element(NameReferenceNode.class, "Foo")) - .branchesTo(0, 2) + .branchesTo(EXIT_ID, 2) .withTerminator(RepeatStatementNode.class))); } @@ -526,7 +529,7 @@ void testEmptyRepeat() { block(element(NameReferenceNode.class, "C")) .branchesTo(1, 2) .withTerminator(RepeatStatementNode.class), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -535,7 +538,7 @@ void testWhile() { "while Foo do Bar;", checker( block(element(NameReferenceNode.class, "Foo")) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(WhileStatementNode.class), block(element(NameReferenceNode.class, "Bar")).succeedsTo(2))); } @@ -546,7 +549,7 @@ void testWhileContinue() { "while Foo do begin Continue; Bar; end;", checker( block(element(NameReferenceNode.class, "Foo")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(WhileStatementNode.class), terminator(StatementTerminator.CONTINUE).jumpsTo(3, 1), block(element(NameReferenceNode.class, "Bar")).succeedsTo(3))); @@ -558,10 +561,10 @@ void testWhileBreak() { "while Foo do begin Bar; break; end;", checker( block(element(NameReferenceNode.class, "Foo")) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(WhileStatementNode.class), block(element(NameReferenceNode.class, "Bar")) - .jumpsTo(0, 2) + .jumpsTo(EXIT_ID, 2) .withTerminator(StatementTerminator.BREAK))); } @@ -573,7 +576,7 @@ void testEmptyWhile() { block(element(NameReferenceNode.class, "C")) .branchesTo(2, 1) .withTerminator(WhileStatementNode.class), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -585,7 +588,7 @@ void testForToVarDecl() { block(element(NameReferenceNode.class, "Bar")).succeedsTo(1), block(element(NameReferenceNode.class, "Baz")).succeedsTo(1), block(element(NameDeclarationNode.class, "I")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -598,7 +601,7 @@ void testForToVarRef() { block(element(NameReferenceNode.class, "Bar")).succeedsTo(1), block(element(NameReferenceNode.class, "Baz")).succeedsTo(1), block(element(NameReferenceNode.class, "I")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -609,9 +612,9 @@ void testForToBreak() { checker( block(element(NameReferenceNode.class, "Foo")).succeedsTo(3), block(element(NameReferenceNode.class, "Bar")).succeedsTo(1), - terminator(StatementTerminator.BREAK).jumpsTo(0, 1), + terminator(StatementTerminator.BREAK).jumpsTo(EXIT_ID, 1), block(element(NameReferenceNode.class, "I")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -629,9 +632,9 @@ void testForToConditionalBreak() { .withCheck(binaryOpTest(BinaryOperator.EQUAL))) .branchesTo(2, 1) .withTerminator(IfStatementNode.class), - terminator(StatementTerminator.BREAK).jumpsTo(0, 1), + terminator(StatementTerminator.BREAK).jumpsTo(EXIT_ID, 1), block(element(NameReferenceNode.class, "I")) - .branchesTo(3, 0) + .branchesTo(3, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -644,7 +647,7 @@ void testForToContinue() { block(element(NameReferenceNode.class, "Bar")).succeedsTo(1), terminator(StatementTerminator.CONTINUE).jumpsTo(1, 1), block(element(NameReferenceNode.class, "I")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -664,7 +667,7 @@ void testForToConditionalContinue() { .withTerminator(IfStatementNode.class), terminator(StatementTerminator.CONTINUE).jumpsTo(1, 1), block(element(NameReferenceNode.class, "I")) - .branchesTo(3, 0) + .branchesTo(3, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -678,7 +681,7 @@ void testEmptyForTo() { block(element(NameReferenceNode.class, "I")) .branchesTo(2, 1) .withTerminator(ForToStatementNode.class), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -689,7 +692,7 @@ void testForInVarDecl() { block(element(NameReferenceNode.class, "List")).succeedsTo(1), block(element(NameReferenceNode.class, "Foo")).succeedsTo(1), block(element(NameDeclarationNode.class, "I")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(ForInStatementNode.class))); } @@ -701,7 +704,7 @@ void testForInVarRef() { block(element(NameReferenceNode.class, "List")).succeedsTo(1), block(element(NameReferenceNode.class, "Foo")).succeedsTo(1), block(element(NameReferenceNode.class, "I")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(ForInStatementNode.class))); } @@ -711,9 +714,9 @@ void testForInBreak() { "for I in List do Break;", checker( block(element(NameReferenceNode.class, "List")).succeedsTo(1), - terminator(StatementTerminator.BREAK).jumpsTo(0, 1), + terminator(StatementTerminator.BREAK).jumpsTo(EXIT_ID, 1), block(element(NameReferenceNode.class, "I")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(ForInStatementNode.class))); } @@ -730,9 +733,9 @@ void testForInConditionalBreak() { .withCheck(binaryOpTest(BinaryOperator.EQUAL))) .branchesTo(2, 1) .withTerminator(IfStatementNode.class), - terminator(StatementTerminator.BREAK).jumpsTo(0, 1), + terminator(StatementTerminator.BREAK).jumpsTo(EXIT_ID, 1), block(element(NameReferenceNode.class, "I")) - .branchesTo(3, 0) + .branchesTo(3, EXIT_ID) .withTerminator(ForInStatementNode.class))); } @@ -744,7 +747,7 @@ void testForInContinue() { block(element(NameReferenceNode.class, "List")).succeedsTo(1), terminator(StatementTerminator.CONTINUE).jumpsTo(1, 1), block(element(NameReferenceNode.class, "I")) - .branchesTo(2, 0) + .branchesTo(2, EXIT_ID) .withTerminator(ForInStatementNode.class))); } @@ -763,7 +766,7 @@ void testForInConditionalContinue() { .withTerminator(IfStatementNode.class), terminator(StatementTerminator.CONTINUE).jumpsTo(1, 1), block(element(NameReferenceNode.class, "I")) - .branchesTo(3, 0) + .branchesTo(3, EXIT_ID) .withTerminator(ForInStatementNode.class))); } @@ -776,21 +779,65 @@ void testEmptyForIn() { block(element(NameReferenceNode.class, "I")) .branchesTo(2, 1) .withTerminator(ForInStatementNode.class), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); - } - - @Test - void testBreakOutsideOfLoop() { - GraphChecker checker = checker(); - assertThatThrownBy(() -> test("Break;", checker)) + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); + } + + @ParameterizedTest + @ValueSource( + strings = { + "Break", + // for to + "Break; for I := Foo to Bar do;", + "for I := Foo to Bar do; Break;", + // for in + "Break; for I in Foo do;", + "for I in Foo do; Break;", + // while + "Break; while Foo do;", + "while Foo do; Break;", + // repeat until + "Break; repeat until Foo;", + "repeat until Foo; Break;", + // try finally + "Break; try finally end;", + "try Break finally end;", + "try finally Break end;", + "try finally end; Break;", + "try try Break finally end finally end;", + "try try finally Break end finally end;", + }) + void testBreakOutsideOfLoop(String test) { + assertThatThrownBy(() -> buildCfg(test)) .withFailMessage("'Break' statement not in loop statement.") .isInstanceOf(IllegalStateException.class); } - @Test - void testContinueOutsideOfLoop() { - GraphChecker checker = checker(); - assertThatThrownBy(() -> test("Continue;", checker)) + @ParameterizedTest + @ValueSource( + strings = { + "Continue", + // for to + "Continue; for I := Foo to Bar do;", + "for I := Foo to Bar do; Continue;", + // for in + "Continue; for I in Foo do;", + "for I in Foo do; Continue;", + // while + "Continue; while Foo do;", + "while Foo do; Continue;", + // repeat until + "Continue; repeat until Foo;", + "repeat until Foo; Continue;", + // try finally + "Continue; try finally end;", + "try Continue finally end;", + "try finally Continue end;", + "try finally end; Continue;", + "try try Continue finally end finally end;", + "try try finally Continue end finally end;", + }) + void testContinueOutsideOfLoop(String test) { + assertThatThrownBy(() -> buildCfg(test)) .withFailMessage("'Continue' statement not in loop statement.") .isInstanceOf(IllegalStateException.class); } @@ -801,7 +848,7 @@ void testWith() { "with TObject.Create do Foo;", checker( block(element(NameReferenceNode.class, "TObject.Create")).succeedsTo(1), - block(element(NameReferenceNode.class, "Foo")).succeedsTo(0))); + block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID))); } @Test @@ -810,7 +857,7 @@ void testEmptyWith() { "with S do; A;", checker( block(element(NameReferenceNode.class, "S")).succeedsTo(1), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -823,7 +870,7 @@ void testTryFinallyNoRaise() { block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(1, 1), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(0, 0))); + .succeedsToWithExit(EXIT_ID, EXCEPTION_EXIT_ID))); } @Test @@ -837,7 +884,7 @@ void testTryFinallyRaise() { .jumpsTo(1, 1), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(0, 0))); + .succeedsToWithExit(EXIT_ID, EXCEPTION_EXIT_ID))); } @Test @@ -849,7 +896,7 @@ void testTryFinallyExit() { terminator(StatementTerminator.EXIT).jumpsTo(1, 1), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(0, 0))); + .succeedsToWithExit(EXIT_ID, EXCEPTION_EXIT_ID))); } @Test @@ -858,13 +905,13 @@ void testTryFinallyBreak() { "while True do try Break finally Bar end;", checker( block(element(NameReferenceNode.class, "True")) - .branchesTo(3, 0) + .branchesTo(3, EXIT_ID) .withTerminator(WhileStatementNode.class), block(element(TryStatementNode.class)).succeedsTo(2), terminator(StatementTerminator.BREAK).jumpsTo(1, 1), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(4, 0))); + .succeedsToWithExit(4, EXCEPTION_EXIT_ID))); } @Test @@ -873,13 +920,13 @@ void testTryFinallyContinue() { "while True do try Continue finally Bar end;", checker( block(element(NameReferenceNode.class, "True")) - .branchesTo(3, 0) + .branchesTo(3, EXIT_ID) .withTerminator(WhileStatementNode.class), block(element(TryStatementNode.class)).succeedsTo(2), terminator(StatementTerminator.CONTINUE).jumpsTo(1, 1), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(4, 0))); + .succeedsToWithExit(4, EXCEPTION_EXIT_ID))); } @Test @@ -891,7 +938,7 @@ void testTryFinallyHalt() { terminator(StatementTerminator.HALT).isSink(), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(0, 0))); + .succeedsToWithExit(EXIT_ID, EXCEPTION_EXIT_ID))); } @Test @@ -911,9 +958,9 @@ void testTryContinueFinallyInLoop() { terminator(StatementTerminator.CONTINUE).jumpsTo(2, 2), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(1, 0), + .succeedsToWithExit(1, EXCEPTION_EXIT_ID), block(element(NameDeclarationNode.class, "A")) - .branchesTo(4, 0) + .branchesTo(4, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -934,9 +981,9 @@ void testTryBreakFinallyInLoop() { terminator(StatementTerminator.BREAK).jumpsTo(2, 2), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(1, 0), + .succeedsToWithExit(1, EXCEPTION_EXIT_ID), block(element(NameDeclarationNode.class, "A")) - .branchesTo(4, 0) + .branchesTo(4, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -956,10 +1003,10 @@ void testTryExitFinallyInLoop() { block(element(TryStatementNode.class)).succeedsTo(3), terminator(StatementTerminator.EXIT).jumpsTo(2, 2), block(element(NameReferenceNode.class, "Bar")) - .succeedsToWithExit(1, 0) + .succeedsToWithExit(1, EXCEPTION_EXIT_ID) .withTerminator(FinallyBlockNode.class), block(element(NameDeclarationNode.class, "A")) - .branchesTo(4, 0) + .branchesTo(4, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -979,10 +1026,10 @@ void testTryHaltFinallyInLoop() { block(element(TryStatementNode.class)).succeedsTo(3), terminator(StatementTerminator.HALT).isSink(), block(element(NameReferenceNode.class, "Bar")) - .succeedsToWithExit(1, 0) + .succeedsToWithExit(1, EXCEPTION_EXIT_ID) .withTerminator(FinallyBlockNode.class), block(element(NameDeclarationNode.class, "A")) - .branchesTo(4, 0) + .branchesTo(4, EXIT_ID) .withTerminator(ForToStatementNode.class))); } @@ -996,7 +1043,7 @@ void testTryExceptNoRaise() { block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(1, 1), block(element(NameReferenceNode.class, "Bar")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(0, 0))); + .succeedsToWithExit(EXIT_ID, EXCEPTION_EXIT_ID))); } @Test @@ -1006,8 +1053,8 @@ void testTryBareExceptNoRaise() { "try Foo; except Bar; end;", checker( block(element(TryStatementNode.class)).succeedsTo(2), - block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(0, 1), - block(element(NameReferenceNode.class, "Bar")).succeedsTo(0))); + block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(EXIT_ID, 1), + block(element(NameReferenceNode.class, "Bar")).succeedsTo(EXIT_ID))); } @Test @@ -1022,11 +1069,12 @@ void testTryExceptOnNoRaise() { + "end;", checker( block(element(TryStatementNode.class)).succeedsTo(3), - block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(0, 0, 1, 2), + block(element(NameReferenceNode.class, "Foo")) + .succeedsToWithExceptions(EXIT_ID, EXCEPTION_EXIT_ID, 1, 2), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Bar")) - .succeedsTo(0), + .succeedsTo(EXIT_ID), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Baz")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1049,12 +1097,12 @@ void testTryExceptRaise1stCatch() { block( element(TextLiteralNode.class, "''"), element(NameReferenceNode.class, "EAbort.Create")) - .succeedsToWithExceptions(3, 0, 1, 2), - terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(2, 0), + .succeedsToWithExceptions(3, EXCEPTION_EXIT_ID, 1, 2), + terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(2, EXIT_ID), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Bar")) - .succeedsTo(0), + .succeedsTo(EXIT_ID), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Baz")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1077,12 +1125,12 @@ void testTryExceptRaise2ndCatch() { block( element(TextLiteralNode.class, "''"), element(NameReferenceNode.class, "Exception.Create")) - .succeedsToWithExceptions(3, 0, 1, 2), - terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(1, 0), + .succeedsToWithExceptions(3, EXCEPTION_EXIT_ID, 1, 2), + terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(1, EXIT_ID), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Bar")) - .succeedsTo(0), + .succeedsTo(EXIT_ID), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Baz")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1096,8 +1144,8 @@ void testTryBareExceptRaise() { element(TextLiteralNode.class, "''"), element(NameReferenceNode.class, "Exception.Create")) .succeedsToWithExceptions(2, 1), - terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(1, 0), - block(element(NameReferenceNode.class, "Bar")).succeedsTo(0))); + terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(1, EXIT_ID), + block(element(NameReferenceNode.class, "Bar")).succeedsTo(EXIT_ID))); } @Test @@ -1121,12 +1169,12 @@ void testTryExceptElseRaise() { block(element(TryStatementNode.class)).succeedsTo(5), block(element(NameReferenceNode.class, "TObject.Create")) .succeedsToWithExceptions(4, 1, 2, 3), - terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(1, 0), + terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(1, EXIT_ID), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Bar")) - .succeedsTo(0), + .succeedsTo(EXIT_ID), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Baz")) - .succeedsTo(0), - block(element(NameReferenceNode.class, "Flarp")).succeedsTo(0))); + .succeedsTo(EXIT_ID), + block(element(NameReferenceNode.class, "Flarp")).succeedsTo(EXIT_ID))); } @Test @@ -1155,7 +1203,7 @@ void testNestedTryExceptFinally() { .succeedsToWithExceptions(1, 1), block(element(NameReferenceNode.class, "Baz")) .withTerminator(FinallyBlockNode.class) - .succeedsToWithExit(0, 0))); + .succeedsToWithExit(EXIT_ID, EXCEPTION_EXIT_ID))); } @Test @@ -1172,11 +1220,12 @@ void testNestedTryFinallyExcept() { block(element(TryStatementNode.class)).succeedsTo(6), block(element(TryStatementNode.class)).succeedsTo(5), block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(4, 4), - block(element(NameReferenceNode.class, "Bar")).succeedsToWithExceptions(3, 0, 1), - terminator(FinallyBlockNode.class).succeedsToWithExit(2, 0), - block().succeedsToWithExceptions(0, 0, 1), + block(element(NameReferenceNode.class, "Bar")) + .succeedsToWithExceptions(3, EXCEPTION_EXIT_ID, 1), + terminator(FinallyBlockNode.class).succeedsToWithExit(2, EXCEPTION_EXIT_ID), + block().succeedsToWithExceptions(EXIT_ID, EXCEPTION_EXIT_ID, 1), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Baz")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1190,8 +1239,8 @@ void testTryBareExcept() { element(TextLiteralNode.class, "''"), element(NameReferenceNode.class, "Exception.Create")) .succeedsToWithExceptions(2, 1), - terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(1, 0), - block(element(NameReferenceNode.class, "Foo")).succeedsTo(0))); + terminator(RaiseStatementNode.class, TerminatorKind.RAISE).jumpsTo(1, EXIT_ID), + block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID))); } @Test @@ -1201,8 +1250,9 @@ void testTryBareExceptReRaise() { "try Foo except raise; end;", checker( block(element(TryStatementNode.class)).succeedsTo(2), - block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(0, 1), - terminator(RaiseStatementNode.class, TerminatorKind.RAISE).throwsTo(0))); + block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(EXIT_ID, 1), + terminator(RaiseStatementNode.class, TerminatorKind.RAISE) + .throwsTo(EXCEPTION_EXIT_ID))); } @Test @@ -1212,10 +1262,11 @@ void testTryExceptReRaise() { "try Foo except on E: Exception do raise; end;", checker( block(element(TryStatementNode.class)).succeedsTo(2), - block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(0, 0, 1), + block(element(NameReferenceNode.class, "Foo")) + .succeedsToWithExceptions(EXIT_ID, EXCEPTION_EXIT_ID, 1), block(element(NameDeclarationNode.class, "E")) .withTerminator(RaiseStatementNode.class, TerminatorKind.RAISE) - .throwsTo(0))); + .throwsTo(EXCEPTION_EXIT_ID))); } @Test @@ -1223,7 +1274,8 @@ void testNoreturnRoutineInvocation() { test( Map.of("", List.of("procedure Boom; noreturn; begin end")), "Boom;", - checker(terminator(NameReferenceNode.class, TerminatorKind.HALT).throwsTo(0))); + checker( + terminator(NameReferenceNode.class, TerminatorKind.HALT).throwsTo(EXCEPTION_EXIT_ID))); } @Test @@ -1234,7 +1286,7 @@ void testTryExceptNoreturnRoutineInvocation() { checker( block(element(TryStatementNode.class)).succeedsTo(2), terminator(NameReferenceNode.class, TerminatorKind.HALT).throwsTo(1), - block(element(NameReferenceNode.class, "Foo")).succeedsTo(0))); + block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID))); } @Test @@ -1243,7 +1295,7 @@ void testEmptyTryExcept() { "try except end; A", checker( block(element(TryStatementNode.class)).succeedsTo(1), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -1252,8 +1304,8 @@ void testEmptyTryFinally() { "try finally end; A", checker( block(element(TryStatementNode.class)).succeedsTo(2), - terminator(FinallyBlockNode.class).succeedsToWithExit(1, 0), - block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + terminator(FinallyBlockNode.class).succeedsToWithExit(1, EXCEPTION_EXIT_ID), + block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test @@ -1262,7 +1314,7 @@ void testRaiseOutsideTry() { "raise A;", checker( block(element(NameReferenceNode.class, "A")) - .jumpsTo(0, 0) + .jumpsTo(EXCEPTION_EXIT_ID, EXIT_ID) .withTerminator(RaiseStatementNode.class, TerminatorKind.RAISE))); } @@ -1272,7 +1324,7 @@ void testCompoundStatement() { "begin Foo; end; begin Bar; end;", checker( block(element(NameReferenceNode.class, "Foo"), element(NameReferenceNode.class, "Bar")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1285,7 +1337,7 @@ void testAssignmentAnd() { .branchesTo(2, 1) .withTerminator(BinaryExpressionNode.class), block(element(NameReferenceNode.class, "B")).succeedsTo(1), - block(element(NameReferenceNode.class, "Foo")).succeedsTo(0))); + block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID))); } @Test @@ -1299,7 +1351,7 @@ void testAssignmentPlus() { element(RealLiteralNode.class, "1.1"), element(BinaryExpressionNode.class), element(NameReferenceNode.class, "Foo")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1319,7 +1371,7 @@ void testAssignmentMultipleBinary() { .withCheck(binaryOpTest(BinaryOperator.MULTIPLY)), element(BinaryExpressionNode.class).withCheck(binaryOpTest(BinaryOperator.ADD)), element(NameReferenceNode.class, "Foo")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1334,7 +1386,7 @@ void testArrayConstructor() { element(IntegerLiteralNode.class, "4"), element(IntegerLiteralNode.class, "5"), element(NameReferenceNode.class, "Foo")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1348,9 +1400,9 @@ void testExitStatement() { element(NilLiteralNode.class), element(BinaryExpressionNode.class) .withCheck(binaryOpTest(BinaryOperator.EQUAL))) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(IfStatementNode.class), - terminator(StatementTerminator.EXIT).jumpsTo(0, 0))); + terminator(StatementTerminator.EXIT).jumpsTo(EXIT_ID, EXIT_ID))); } @Test @@ -1364,11 +1416,11 @@ void testExitValueStatement() { element(NilLiteralNode.class), element(BinaryExpressionNode.class) .withCheck(binaryOpTest(BinaryOperator.EQUAL))) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(IfStatementNode.class), block(element(NameReferenceNode.class, "Bar")) .withTerminator(StatementTerminator.EXIT) - .jumpsTo(0, 0))); + .jumpsTo(EXIT_ID, EXIT_ID))); } @Test @@ -1386,7 +1438,7 @@ void testExitCascadedOr() { .withTerminatorNodeCheck(binaryOpTest(BinaryOperator.OR)) .branchesTo(1, 2), block(element(NameReferenceNode.class, "C")).succeedsTo(1), - terminator(StatementTerminator.EXIT).jumpsTo(0, 0))); + terminator(StatementTerminator.EXIT).jumpsTo(EXIT_ID, EXIT_ID))); } @Test @@ -1404,7 +1456,7 @@ void testExitCascadedAnd() { .withTerminatorNodeCheck(binaryOpTest(BinaryOperator.AND)) .branchesTo(2, 1), block(element(NameReferenceNode.class, "C")).succeedsTo(1), - terminator(StatementTerminator.EXIT).jumpsTo(0, 0))); + terminator(StatementTerminator.EXIT).jumpsTo(EXIT_ID, EXIT_ID))); } @Test @@ -1428,12 +1480,14 @@ void testExitComplexBoolean() { .withTerminatorNodeCheck(binaryOpTest(BinaryOperator.AND)) .branchesTo(2, 1), block(element(NameReferenceNode.class, "B")).succeedsTo(1), - terminator(StatementTerminator.EXIT).jumpsTo(0, 0))); + terminator(StatementTerminator.EXIT).jumpsTo(EXIT_ID, EXIT_ID))); } @Test void testBareInherited() { - test("inherited;", checker(block(element(CommonDelphiNode.class, "inherited")).succeedsTo(0))); + test( + "inherited;", + checker(block(element(CommonDelphiNode.class, "inherited")).succeedsTo(EXIT_ID))); } @Test @@ -1444,7 +1498,7 @@ void testNamedInheritedNoArgs() { block( element(CommonDelphiNode.class, "inherited"), element(NameReferenceNode.class, "Foo")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1458,7 +1512,7 @@ void testInherited() { element(NameReferenceNode.class, "A"), element(NameReferenceNode.class, "B"), element(NameReferenceNode.class, "C")) - .succeedsTo(0))); + .succeedsTo(EXIT_ID))); } @Test @@ -1468,9 +1522,9 @@ void testSucceedingLabelGoto() { "if B then goto A; A:", checker( block(element(NameReferenceNode.class, "B")) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(IfStatementNode.class), - terminator(GotoStatementNode.class, TerminatorKind.GOTO).jumpsTo(0, 0))); + terminator(GotoStatementNode.class, TerminatorKind.GOTO).jumpsTo(EXIT_ID, EXIT_ID))); } @Test @@ -1480,11 +1534,11 @@ void testPrecedingLabelGoto() { "A: if B then goto A;", checker( block(element(NameReferenceNode.class, "B")) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(IfStatementNode.class), block(element(NameReferenceNode.class, "A")) .withTerminator(GotoStatementNode.class, TerminatorKind.GOTO) - .jumpsTo(2, 0))); + .jumpsTo(2, EXIT_ID))); } @Test @@ -1495,11 +1549,11 @@ void testLabelSeparatesBlock() { checker( block(element(NameReferenceNode.class, "Foo")).succeedsTo(2), block(element(NameReferenceNode.class, "Bar"), element(NameReferenceNode.class, "B")) - .branchesTo(1, 0) + .branchesTo(1, EXIT_ID) .withTerminator(IfStatementNode.class), block(element(NameReferenceNode.class, "A")) .withTerminator(GotoStatementNode.class, TerminatorKind.GOTO) - .jumpsTo(2, 0))); + .jumpsTo(2, EXIT_ID))); } @Test @@ -1507,21 +1561,21 @@ void testEmptyLabel() { test( Map.of("label", List.of("A,B")), "A: B: C;", - checker(block(element(NameReferenceNode.class, "C")).succeedsTo(0))); + checker(block(element(NameReferenceNode.class, "C")).succeedsTo(EXIT_ID))); } @Test void testAnonymousRoutinesAreIgnored() { test( "A := procedure begin Foo; Bar; end;", - checker(block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + checker(block(element(NameReferenceNode.class, "A")).succeedsTo(EXIT_ID))); } @Test void testInlineAssemblyIsIgnored() { test( "Foo; asm XOR EAX, EAX end;", - checker(block(element(NameReferenceNode.class, "Foo")).succeedsTo(0))); + checker(block(element(NameReferenceNode.class, "Foo")).succeedsTo(EXIT_ID))); } @Test @@ -1615,7 +1669,7 @@ void testComplexConstructions() { .succeedsTo(2), block(element(TextLiteralNode.class, "'b'"), element(NameReferenceNode.class, "X")) .succeedsTo(2), - terminator(FinallyBlockNode.class).succeedsToWithExit(12, 0), - block(element(NameReferenceNode.class, "Foo4")).succeedsTo(0))); + terminator(FinallyBlockNode.class).succeedsToWithExit(12, EXCEPTION_EXIT_ID), + block(element(NameReferenceNode.class, "Foo4")).succeedsTo(EXIT_ID))); } } diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/checker/BlockChecker.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/checker/BlockChecker.java index 48c9b7d0b..102fd2243 100644 --- a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/checker/BlockChecker.java +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/checker/BlockChecker.java @@ -79,7 +79,7 @@ public void check(final Block block) { .check(block.getElements().get(elementId)); } assertThat(successorChecker) - .withFailMessage("%s should have its successors declared", getBlockDisplay(block)) + .as("%s should have its successors declared", getBlockDisplay(block)) .isNotNull(); successorChecker.check(block); @@ -87,7 +87,7 @@ public void check(final Block block) { terminatorChecker.check(block); } else { assertThat(block) - .withFailMessage("%s should have its terminator specified", getBlockDisplay(block)) + .as("%s should have its terminator specified", getBlockDisplay(block)) .isNotInstanceOf(Terminated.class); } terminatorNodeChecks.forEach(check -> check.check(block)); @@ -99,8 +99,7 @@ public BlockChecker succeedsTo(int successor) { block -> { Linear branch = assertBlockIsType(block, Linear.class); assertThat(getBlockId(branch.getSuccessor())) - .withFailMessage( - getBlockDisplay(block) + " is expected to have successor of B" + successor) + .as(getBlockDisplay(block) + " is expected to have successor of B" + successor) .isEqualTo(successor); }); return this; @@ -112,11 +111,10 @@ public BlockChecker succeedsToWithExit(int successor, int exitSuccessor) { block -> { Finally branch = assertBlockIsType(block, Finally.class); assertThat(getBlockId(branch.getSuccessor())) - .withFailMessage( - getBlockDisplay(block) + " is expected to have successor of B" + successor) + .as(getBlockDisplay(block) + " is expected to have successor of B" + successor) .isEqualTo(successor); assertThat(getBlockId(branch.getExceptionSuccessor())) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to have exit successor of B" + exitSuccessor) @@ -131,13 +129,13 @@ public BlockChecker branchesTo(int trueBlock, int falseBlock) { block -> { Branch branch = assertBlockIsType(block, Branch.class); assertThat(getBlockId(branch.getTrueBlock())) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to have true successor of B" + trueBlock) .isEqualTo(trueBlock); assertThat(getBlockId(branch.getFalseBlock())) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to have false successor of B" + falseBlock) @@ -152,11 +150,10 @@ public BlockChecker jumpsTo(int successor, int successorWithoutJump) { block -> { UnconditionalJump branch = assertBlockIsType(block, UnconditionalJump.class); assertThat(getBlockId(branch.getSuccessor())) - .withFailMessage( - getBlockDisplay(block) + " is expected to have successor of B" + successor) + .as(getBlockDisplay(block) + " is expected to have successor of B" + successor) .isEqualTo(successor); assertThat(getBlockId(branch.getSuccessorIfRemoved())) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to have successor without jump of B" + successorWithoutJump) @@ -176,7 +173,7 @@ public BlockChecker succeedsToCases(int fallthrough, int... cases) { .map(BlockChecker::getBlockId) .collect(Collectors.toSet()); assertThat(caseIds) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to have case successors of [" + expectedCases.stream() @@ -185,7 +182,7 @@ public BlockChecker succeedsToCases(int fallthrough, int... cases) { + "]") .containsExactlyInAnyOrderElementsOf(expectedCases); assertThat(getBlockId(caseSuccessor.getFallthroughSuccessor())) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to have fallthrough successor of B" + fallthrough) @@ -201,15 +198,14 @@ public BlockChecker succeedsToWithExceptions(int successor, int... unknownExcept block -> { PossibleException branch = assertBlockIsType(block, PossibleException.class); assertThat(getBlockId(branch.getSuccessor())) - .withFailMessage( - getBlockDisplay(block) + " is expected to have successor of B" + successor) + .as(getBlockDisplay(block) + " is expected to have successor of B" + successor) .isEqualTo(successor); Set blockIds = branch.getExceptions().stream() .map(BlockChecker::getBlockId) .collect(Collectors.toSet()); assertThat(blockIds) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to have exception successors of [" + exceptions.stream() @@ -232,7 +228,7 @@ public BlockChecker throwsTo(int... unknownExceptions) { .map(BlockChecker::getBlockId) .collect(Collectors.toSet()); assertThat(blockIds) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to have exception successors of [" + exceptions.stream() @@ -264,26 +260,24 @@ public BlockChecker withTerminator(StatementTerminator terminator) { block -> { Terminated terminated = assertBlockTerminated(block); assertThat(terminated.getTerminatorKind()) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to be terminated with kind " + terminator.getTerminatorKind()) .isEqualTo(terminator.getTerminatorKind()); assertThat(terminated.getTerminator()) - .withFailMessage( - getBlockDisplay(block) + " is expected to be terminated with name reference") + .as(getBlockDisplay(block) + " is expected to be terminated with name reference") .isInstanceOf(NameReferenceNode.class); NameReferenceNode nameReferenceNode = (NameReferenceNode) terminated.getTerminator(); assertThat(nameReferenceNode.getLastName().getNameDeclaration()) - .withFailMessage( - getBlockDisplay(block) + " is expected to be terminated with routine") + .as(getBlockDisplay(block) + " is expected to be terminated with routine") .isInstanceOf(RoutineNameDeclaration.class); assertThat( ((RoutineNameDeclaration) nameReferenceNode.getLastName().getNameDeclaration()) .fullyQualifiedName()) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to be terminated with " + terminator.getRoutineName()) @@ -299,14 +293,13 @@ public BlockChecker withTerminator( block -> { Terminated terminated = assertBlockTerminated(block); assertThat(terminated.getTerminator()) - .withFailMessage( + .as( getBlockDisplay(block) + " is expected to be terminated with " + terminatorClass.getTypeName()) .isInstanceOf(terminatorClass); assertThat(terminated.getTerminatorKind()) - .withFailMessage( - getBlockDisplay(block) + " is expected to be terminated with kind " + kind) + .as(getBlockDisplay(block) + " is expected to be terminated with kind " + kind) .isEqualTo(kind); }); return this; @@ -324,7 +317,7 @@ public BlockChecker withTerminatorNodeCheck(Consumer extraChecker) { private Terminated assertBlockTerminated(Block block) { assertThat(block) - .withFailMessage(getBlockDisplay(block) + " is expected to be terminated") + .as(getBlockDisplay(block) + " is expected to be terminated") .isInstanceOf(Terminated.class); return (Terminated) block; } diff --git a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/checker/GraphChecker.java b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/checker/GraphChecker.java index b33b303b8..ee7da8b5f 100644 --- a/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/checker/GraphChecker.java +++ b/delphi-frontend/src/test/java/au/com/integradev/delphi/cfg/checker/GraphChecker.java @@ -22,6 +22,8 @@ import au.com.integradev.delphi.cfg.api.Block; import au.com.integradev.delphi.cfg.api.ControlFlowGraph; +import au.com.integradev.delphi.cfg.api.ExceptionalRoutineExit; +import au.com.integradev.delphi.cfg.api.RoutineExit; import au.com.integradev.delphi.cfg.block.BlockImpl; import java.util.ArrayList; import java.util.Collections; @@ -41,19 +43,28 @@ private GraphChecker(BlockChecker... checkers) { } public void check(final ControlFlowGraph cfg) { - assertThat(cfg.getBlocks()).as("block count").hasSize(checkers.size() + 1); + List blocks = new ArrayList<>(cfg.getBlocks()); + List exitBlocks = new ArrayList<>(); + for (int i = blocks.size() - 1; i >= 0; i--) { + Block block = blocks.get(i); + if (block instanceof RoutineExit || block instanceof ExceptionalRoutineExit) { + exitBlocks.add(block); + blocks.remove(i); + } + } + + assertThat(blocks).as("block count").hasSize(checkers.size()); final Iterator checkerIterator = checkers.iterator(); - List blocks = new ArrayList<>(cfg.getBlocks()); - final Block exitBlock = blocks.remove(blocks.size() - 1); for (Block block : blocks) { checkerIterator.next().check(block); int blockId = ((BlockImpl) block).getId(); checkLinkedBlocks("Successor of B" + blockId, cfg.getBlocks(), block.getSuccessors()); checkLinkedBlocks("Predecessor of B" + blockId, cfg.getBlocks(), block.getPredecessors()); } - assertThat(exitBlock.getElements()).isEmpty(); - assertThat(exitBlock.getSuccessors()).isEmpty(); + + assertThat(exitBlocks.stream().map(Block::getElements)).allMatch(List::isEmpty); + assertThat(exitBlocks.stream().map(Block::getSuccessors)).allMatch(Set::isEmpty); assertThat(cfg.getBlocks()) .withFailMessage("CFG entry block is no longer in the list of blocks!") .contains(cfg.getEntryBlock());