diff --git a/CHANGELOG.md b/CHANGELOG.md index d40eb9105..b073fce66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Exceptions from empty structures (e.g., `if`) in `LoopExecutingAtMostOnce` and `RedundantJump`. +- False positives from case statements in `LoopExecutingAtMostOnce`. +- False positives from nested finally-except blocks in `RedundantJump`. + ## [1.14.1] - 2025-03-05 ### Fixed 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 08e28f335..f1d99e559 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 @@ -18,13 +18,12 @@ */ package au.com.integradev.delphi.checks; -import au.com.integradev.delphi.antlr.ast.node.AnonymousMethodNodeImpl; -import au.com.integradev.delphi.antlr.ast.node.RoutineImplementationNodeImpl; import au.com.integradev.delphi.cfg.ControlFlowGraphFactory; import au.com.integradev.delphi.cfg.api.Block; 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.utils.ControlFlowGraphUtils; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Deque; @@ -33,22 +32,16 @@ import java.util.Optional; import java.util.Queue; import java.util.Set; -import java.util.function.Supplier; import org.sonar.check.Rule; -import org.sonar.plugins.communitydelphi.api.ast.CompoundStatementNode; -import org.sonar.plugins.communitydelphi.api.ast.DelphiAst; import org.sonar.plugins.communitydelphi.api.ast.DelphiNode; -import org.sonar.plugins.communitydelphi.api.ast.FinalizationSectionNode; import org.sonar.plugins.communitydelphi.api.ast.ForInStatementNode; import org.sonar.plugins.communitydelphi.api.ast.ForStatementNode; import org.sonar.plugins.communitydelphi.api.ast.ForToStatementNode; import org.sonar.plugins.communitydelphi.api.ast.GotoStatementNode; import org.sonar.plugins.communitydelphi.api.ast.IfStatementNode; -import org.sonar.plugins.communitydelphi.api.ast.InitializationSectionNode; import org.sonar.plugins.communitydelphi.api.ast.NameReferenceNode; import org.sonar.plugins.communitydelphi.api.ast.RaiseStatementNode; import org.sonar.plugins.communitydelphi.api.ast.RepeatStatementNode; -import org.sonar.plugins.communitydelphi.api.ast.StatementListNode; import org.sonar.plugins.communitydelphi.api.ast.StatementNode; import org.sonar.plugins.communitydelphi.api.ast.WhileStatementNode; import org.sonar.plugins.communitydelphi.api.check.DelphiCheck; @@ -282,34 +275,11 @@ private static boolean isDescendant(DelphiNode descendant, DelphiNode target) { return false; } - private static Supplier getCFGSupplier(DelphiNode node) { - if (node instanceof RoutineImplementationNodeImpl) { - return ((RoutineImplementationNodeImpl) node)::getControlFlowGraph; - } - if (node instanceof AnonymousMethodNodeImpl) { - return ((AnonymousMethodNodeImpl) node)::getControlFlowGraph; - } - if (node instanceof CompoundStatementNode && node.getParent() instanceof DelphiAst) { - return () -> ControlFlowGraphFactory.create((CompoundStatementNode) node); - } - if (node instanceof StatementListNode - && (node.getParent() instanceof InitializationSectionNode - || node.getParent() instanceof FinalizationSectionNode)) { - return () -> ControlFlowGraphFactory.create((StatementListNode) node); - } - return null; - } - private static ControlFlowGraph getCFG(DelphiNode loop) { - DelphiNode parent = loop.getParent(); - Supplier cfgSupplier = getCFGSupplier(parent); - while (parent != null && cfgSupplier == null) { - parent = parent.getParent(); - cfgSupplier = getCFGSupplier(parent); - } - if (cfgSupplier != null) { - return cfgSupplier.get(); + ControlFlowGraph cfg = ControlFlowGraphUtils.findContainingCFG(loop); + if (cfg == null) { + return ControlFlowGraphFactory.create(loop.findChildrenOfType(StatementNode.class)); } - return ControlFlowGraphFactory.create(loop.findChildrenOfType(StatementNode.class)); + return cfg; } } 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 f5660d606..d564ed4b1 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 @@ -18,18 +18,20 @@ */ package au.com.integradev.delphi.checks; -import au.com.integradev.delphi.antlr.ast.node.RoutineImplementationNodeImpl; -import au.com.integradev.delphi.cfg.ControlFlowGraphFactory; import au.com.integradev.delphi.cfg.api.Block; import au.com.integradev.delphi.cfg.api.ControlFlowGraph; import au.com.integradev.delphi.cfg.api.Finally; +import au.com.integradev.delphi.cfg.api.Terminated; import au.com.integradev.delphi.cfg.api.UnconditionalJump; +import au.com.integradev.delphi.utils.ControlFlowGraphUtils; +import java.util.ArrayDeque; +import java.util.Deque; import org.sonar.check.Rule; -import org.sonar.plugins.communitydelphi.api.ast.AnonymousMethodNode; import org.sonar.plugins.communitydelphi.api.ast.ArgumentListNode; import org.sonar.plugins.communitydelphi.api.ast.DelphiNode; +import org.sonar.plugins.communitydelphi.api.ast.FinallyBlockNode; import org.sonar.plugins.communitydelphi.api.ast.NameReferenceNode; -import org.sonar.plugins.communitydelphi.api.ast.RoutineImplementationNode; +import org.sonar.plugins.communitydelphi.api.ast.TryStatementNode; import org.sonar.plugins.communitydelphi.api.check.DelphiCheck; import org.sonar.plugins.communitydelphi.api.check.DelphiCheckContext; import org.sonar.plugins.communitydelphi.api.symbol.declaration.NameDeclaration; @@ -39,103 +41,115 @@ public class RedundantJumpCheck extends DelphiCheck { private static final String MESSAGE = "Remove this redundant jump."; + private final Deque tryFinallyStatements = new ArrayDeque<>(); + @Override - public DelphiCheckContext visit(RoutineImplementationNode routine, DelphiCheckContext context) { - ControlFlowGraph cfg = ((RoutineImplementationNodeImpl) routine).getControlFlowGraph(); - if (cfg != null) { - cfg.getBlocks().forEach(block -> checkBlock(block, context)); + public DelphiCheckContext visit(TryStatementNode node, DelphiCheckContext data) { + boolean finallyBlock = node.hasFinallyBlock(); + if (!finallyBlock) { + return super.visit(node, data); } - - return super.visit(routine, context); + this.tryFinallyStatements.push(node); + super.visit(node.getStatementList(), data); + this.tryFinallyStatements.pop(); + return super.visit(node.getFinallyBlock(), data); } @Override - public DelphiCheckContext visit(AnonymousMethodNode routine, DelphiCheckContext context) { - ControlFlowGraph cfg = ControlFlowGraphFactory.create(routine.getStatementBlock()); - cfg.getBlocks().forEach(block -> checkBlock(block, context)); + public DelphiCheckContext visit(NameReferenceNode node, DelphiCheckContext data) { + RoutineNameDeclaration routineNameDeclaration = getRoutineNameDeclaration(node); + if (routineNameDeclaration != null + && (isContinue(routineNameDeclaration) + || isExitWithoutArgs(routineNameDeclaration, node))) { + checkJumpNode(node, data); + return data; + } + return super.visit(node, data); + } - return super.visit(routine, context); + private static RoutineNameDeclaration getRoutineNameDeclaration(NameReferenceNode node) { + NameDeclaration nameDeclaration = node.getNameDeclaration(); + if (nameDeclaration instanceof RoutineNameDeclaration) { + return (RoutineNameDeclaration) nameDeclaration; + } + return null; } - private void checkBlock(Block block, DelphiCheckContext context) { - if (!(block instanceof UnconditionalJump)) { - return; + private static boolean isContinue(RoutineNameDeclaration node) { + String name = node.fullyQualifiedName(); + return name.equals("System.Continue"); + } + + private static boolean isExitWithoutArgs(RoutineNameDeclaration node, DelphiNode statement) { + String name = node.fullyQualifiedName(); + if (!name.equals("System.Exit")) { + return false; } - UnconditionalJump jump = (UnconditionalJump) block; - DelphiNode terminator = jump.getTerminator(); + var argumentList = statement.getParent().getFirstChildOfType(ArgumentListNode.class); + int arguments = argumentList == null ? 0 : argumentList.getArgumentNodes().size(); + + return arguments == 0; + } + + private static Block findBlockWithTerminator(ControlFlowGraph cfg, DelphiNode node) { + for (Block block : cfg.getBlocks()) { + if ((block instanceof Terminated) && ((Terminated) block).getTerminator() == node) { + return block; + } + } + return null; + } - RoutineNameDeclaration routineNameDeclaration = getRoutineNameDeclaration(terminator); - if (routineNameDeclaration == null) { + private void checkJumpNode(NameReferenceNode node, DelphiCheckContext context) { + ControlFlowGraph cfg = ControlFlowGraphUtils.findContainingCFG(node); + if (cfg == null) { return; } - if (!isContinueOrExit(routineNameDeclaration) - || isExitWithExpression(routineNameDeclaration, terminator)) { + Block terminatedBlock = findBlockWithTerminator(cfg, node); + if (!(terminatedBlock instanceof UnconditionalJump)) { + // can't be a redundant jump without a jump return; } + UnconditionalJump jump = (UnconditionalJump) terminatedBlock; Block successor = jump.getSuccessor(); if (!successor.equals(jump.getSuccessorIfRemoved())) { + // without the jump, the successor block would be different return; } - Block finallyBlock = getFinallyBlock(block); - if (finallyBlock != null) { - if (onlyFinallyBlocksBeforeEnd(finallyBlock)) { - reportIssue(context, terminator, MESSAGE); - } - return; + if (isViolation(cfg)) { + reportIssue(context, jump.getTerminator(), MESSAGE); } - - reportIssue(context, terminator, MESSAGE); - } - - private static Block getFinallyBlock(Block block) { - return block.getSuccessors().stream() - .filter(Finally.class::isInstance) - .findFirst() - .orElse(null); } - private static boolean onlyFinallyBlocksBeforeEnd(Block finallyBlock) { - while (finallyBlock.getSuccessors().size() == 1) { - Block finallySuccessor = finallyBlock.getSuccessors().iterator().next(); - if (!(finallySuccessor instanceof Finally)) { + private boolean isViolation(ControlFlowGraph cfg) { + for (TryStatementNode tryFinally : this.tryFinallyStatements) { + Finally finallyBlock = findFinallyBlock(cfg, tryFinally.getFinallyBlock()); + if (finallyBlock == null) { + // if the finally block cannot be found, we have traversed outside the scope of the cfg break; } - finallyBlock = finallySuccessor; + if (!finallyBlock.getSuccessor().equals(finallyBlock.getExceptionSuccessor())) { + // multiple paths after the finally corresponds to code that would be skipped from the jump + return false; + } } - return finallyBlock.getSuccessors().size() == 1 - && finallyBlock.getSuccessors().iterator().next().getSuccessors().isEmpty(); + // if no invalidating try-finally blocks are found, the use is a violation + return true; } - private static RoutineNameDeclaration getRoutineNameDeclaration(DelphiNode node) { - NameDeclaration nameDeclaration = null; - if (node instanceof NameReferenceNode) { - nameDeclaration = ((NameReferenceNode) node).getNameDeclaration(); + private static Finally findFinallyBlock(ControlFlowGraph cfg, FinallyBlockNode element) { + if (element == null) { + return null; } - if (nameDeclaration instanceof RoutineNameDeclaration) { - return (RoutineNameDeclaration) nameDeclaration; + for (Block block : cfg.getBlocks()) { + if ((block instanceof Finally) && ((Finally) block).getTerminator().equals(element)) { + return (Finally) block; + } } return null; } - - private static boolean isContinueOrExit(RoutineNameDeclaration routineNameDeclaration) { - String fullyQualifiedName = routineNameDeclaration.fullyQualifiedName(); - return fullyQualifiedName.equals("System.Continue") || fullyQualifiedName.equals("System.Exit"); - } - - private static boolean isExitWithExpression( - RoutineNameDeclaration routineNameDeclaration, DelphiNode statement) { - String fullyQualifiedName = routineNameDeclaration.fullyQualifiedName(); - if (!fullyQualifiedName.equals("System.Exit")) { - return false; - } - - var argumentList = statement.getParent().getFirstChildOfType(ArgumentListNode.class); - int arguments = argumentList == null ? 0 : argumentList.getArgumentNodes().size(); - - return arguments > 0; - } } diff --git a/delphi-checks/src/main/java/au/com/integradev/delphi/utils/ControlFlowGraphUtils.java b/delphi-checks/src/main/java/au/com/integradev/delphi/utils/ControlFlowGraphUtils.java new file mode 100644 index 000000000..06ae2cc0e --- /dev/null +++ b/delphi-checks/src/main/java/au/com/integradev/delphi/utils/ControlFlowGraphUtils.java @@ -0,0 +1,66 @@ +/* + * Sonar Delphi Plugin + * Copyright (C) 2025 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.utils; + +import au.com.integradev.delphi.antlr.ast.node.AnonymousMethodNodeImpl; +import au.com.integradev.delphi.antlr.ast.node.RoutineImplementationNodeImpl; +import au.com.integradev.delphi.cfg.ControlFlowGraphFactory; +import au.com.integradev.delphi.cfg.api.ControlFlowGraph; +import java.util.function.Supplier; +import org.sonar.plugins.communitydelphi.api.ast.CompoundStatementNode; +import org.sonar.plugins.communitydelphi.api.ast.DelphiAst; +import org.sonar.plugins.communitydelphi.api.ast.DelphiNode; +import org.sonar.plugins.communitydelphi.api.ast.FinalizationSectionNode; +import org.sonar.plugins.communitydelphi.api.ast.InitializationSectionNode; +import org.sonar.plugins.communitydelphi.api.ast.StatementListNode; + +public final class ControlFlowGraphUtils { + private ControlFlowGraphUtils() { + // Utility class + } + + private static Supplier getCFGSupplier(DelphiNode node) { + if (node instanceof RoutineImplementationNodeImpl) { + return ((RoutineImplementationNodeImpl) node)::getControlFlowGraph; + } + if (node instanceof AnonymousMethodNodeImpl) { + return ((AnonymousMethodNodeImpl) node)::getControlFlowGraph; + } + if (node instanceof CompoundStatementNode && node.getParent() instanceof DelphiAst) { + return () -> ControlFlowGraphFactory.create((CompoundStatementNode) node); + } + if (node instanceof StatementListNode + && (node.getParent() instanceof InitializationSectionNode + || node.getParent() instanceof FinalizationSectionNode)) { + return () -> ControlFlowGraphFactory.create((StatementListNode) node); + } + return null; + } + + public static ControlFlowGraph findContainingCFG(DelphiNode node) { + while (node != null) { + Supplier cfgSupplier = getCFGSupplier(node); + if (cfgSupplier != null) { + return cfgSupplier.get(); + } + node = node.getParent(); + } + return null; + } +} diff --git a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/LoopExecutingAtMostOnceCheckTest.java b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/LoopExecutingAtMostOnceCheckTest.java index 3136136af..e02a7e32f 100644 --- a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/LoopExecutingAtMostOnceCheckTest.java +++ b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/LoopExecutingAtMostOnceCheckTest.java @@ -425,6 +425,46 @@ void testIfContinueElseExitShouldNotAddIssue() { .verifyNoIssues(); } + @Test + void testCaseBreakShouldNotAddIssue() { + DelphiTestUnitBuilder unitBuilder = + new DelphiTestUnitBuilder() + .appendImpl("procedure Test;") + .appendImpl("begin") + .appendImpl(" while A do begin // Compliant") + .appendImpl(" case True of") + .appendImpl(" True: Break; // Compliant") + .appendImpl(" end;") + .appendImpl(" end;") + .appendImpl("end;"); + + CheckVerifier.newVerifier() + .withCheck(new LoopExecutingAtMostOnceCheck()) + .onFile(unitBuilder) + .verifyNoIssues(); + } + + @Test + void testCaseBreakElseBreakShouldAddIssue() { + DelphiTestUnitBuilder unitBuilder = + new DelphiTestUnitBuilder() + .appendImpl("procedure Test;") + .appendImpl("begin") + .appendImpl(" while A do begin // Noncompliant (2) (4)") + .appendImpl(" case True of") + .appendImpl(" True: Break; // Secondary") + .appendImpl(" else") + .appendImpl(" Break; // Secondary") + .appendImpl(" end;") + .appendImpl(" end;") + .appendImpl("end;"); + + CheckVerifier.newVerifier() + .withCheck(new LoopExecutingAtMostOnceCheck()) + .onFile(unitBuilder) + .verifyIssues(); + } + @Test void testIfNestedShouldNotAddIssue() { DelphiTestUnitBuilder unitBuilder = diff --git a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/RedundantJumpCheckTest.java b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/RedundantJumpCheckTest.java index d9bb22007..722311096 100644 --- a/delphi-checks/src/test/java/au/com/integradev/delphi/checks/RedundantJumpCheckTest.java +++ b/delphi-checks/src/test/java/au/com/integradev/delphi/checks/RedundantJumpCheckTest.java @@ -262,6 +262,11 @@ void testExitInTryFinallyAtEndShouldAddIssue() { List.of("try", " if True then Exit; // Noncompliant", "finally", " Foo1;", "end;"), true); } + @Test + void testExitInExceptShouldAddIssue() { + doExitTest(List.of("try", " Exit; // Noncompliant", "except", " Foo2;", "end;"), true); + } + @Test void testExitInNestedTryFinallyAtEndShouldAddIssue() { doExitTest( @@ -330,6 +335,27 @@ void testExceptExitShouldNotAddIssue() { false); } + @Test + void testExceptExitWithStatementAfterShouldNotAddIssue() { + doExitTest( + List.of( + "try", + " try", + " A;", + " except", + " on E: Exception do begin", + " Exit;", + " end;", + " end;", + "finally;", + " if B then begin", + " C;", + " end;", + "end;", + "C;"), + false); + } + @Test void testForLoopAfterConditionalTryFinallyExitShouldNotAddIssue() { doExitTest( @@ -363,6 +389,42 @@ void testConditionalExitInForLoopTryFinallyShouldNotAddIssue() { false); } + @Test + void testRedundantTryFinallyNestedAnonymousMethodShouldAddIssue() { + doExitTest( + List.of( + "try", + " var A := procedure", + " begin", + " try", + " Exit; // Noncompliant", + " finally", + " end;", + " end;", + "finally", + "end;", + "A;"), + true); + } + + @Test + void testNonRedundantTryFinallyNestedAnonymousMethodShouldNotAddIssue() { + doExitTest( + List.of( + "try", + " var A := procedure", + " begin", + " try", + " Exit;", + " finally", + " end;", + " A;", + " end;", + "finally", + "end;"), + false); + } + @Test void testAnonymousMethodExitShouldAddIssue() { doExitTest( diff --git a/delphi-checks/src/test/java/au/com/integradev/delphi/utils/ControlFlowGraphUtilsTest.java b/delphi-checks/src/test/java/au/com/integradev/delphi/utils/ControlFlowGraphUtilsTest.java new file mode 100644 index 000000000..75a0aac54 --- /dev/null +++ b/delphi-checks/src/test/java/au/com/integradev/delphi/utils/ControlFlowGraphUtilsTest.java @@ -0,0 +1,106 @@ +/* + * Sonar Delphi Plugin + * Copyright (C) 2025 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.utils; + +import static org.assertj.core.api.Assertions.*; + +import au.com.integradev.delphi.builders.DelphiTestProgramBuilder; +import au.com.integradev.delphi.builders.DelphiTestUnitBuilder; +import au.com.integradev.delphi.cfg.api.ControlFlowGraph; +import au.com.integradev.delphi.file.DelphiFile; +import org.junit.jupiter.api.Test; +import org.sonar.plugins.communitydelphi.api.ast.DelphiNode; +import org.sonar.plugins.communitydelphi.api.ast.NameReferenceNode; + +class ControlFlowGraphUtilsTest { + private static final String NAME_TO_FIND = "ABC123"; + + private static void testFindCfg(DelphiFile unit) { + DelphiNode node = + unit.getAst().findDescendantsOfType(NameReferenceNode.class).stream() + .filter(n -> n.getIdentifier().getImage().equals(NAME_TO_FIND)) + .findFirst() + .orElseThrow(); + + ControlFlowGraph cfg = ControlFlowGraphUtils.findContainingCFG(node); + assertThat(cfg).isNotNull(); + assertThat(cfg.getBlocks().stream().filter(b -> b.getElements().contains(node)).count()) + .isEqualTo(1); + } + + @Test + void testFindCfgInRoutine() { + testFindCfg( + new DelphiTestUnitBuilder() + .appendImpl("procedure Test;") + .appendImpl("begin") + .appendImpl(String.format(" %s;", NAME_TO_FIND)) + .appendImpl("end;") + .delphiFile()); + } + + @Test + void testFindCfgInAnonymousRoutine() { + testFindCfg( + new DelphiTestUnitBuilder() + .appendImpl("procedure Test;") + .appendImpl("begin") + .appendImpl(String.format(" %s;", NAME_TO_FIND)) + .appendImpl(String.format(" var Proc := procedure begin %s; end;", NAME_TO_FIND)) + .appendImpl(String.format(" %s;", NAME_TO_FIND)) + .appendImpl("end;") + .delphiFile()); + } + + @Test + void testFindCfgInUnitBegin() { + testFindCfg( + new DelphiTestUnitBuilder() + .appendImpl("begin") + .appendImpl(String.format(" %s;", NAME_TO_FIND)) + .delphiFile()); + } + + @Test + void testFindCfgInInitialization() { + testFindCfg( + new DelphiTestUnitBuilder() + .appendImpl("initialization") + .appendImpl(String.format(" %s;", NAME_TO_FIND)) + .delphiFile()); + } + + @Test + void testFindCfgInFinalization() { + testFindCfg( + new DelphiTestUnitBuilder() + .appendImpl("initialization") + .appendImpl("finalization") + .appendImpl(String.format(" %s;", NAME_TO_FIND)) + .delphiFile()); + } + + @Test + void testFindCfgInProgram() { + testFindCfg( + new DelphiTestProgramBuilder() + .appendImpl(String.format(" %s;", NAME_TO_FIND)) + .delphiFile()); + } +} diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphDebug.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphDebug.java index 91f8e8892..f23512da9 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphDebug.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/ControlFlowGraphDebug.java @@ -83,7 +83,7 @@ private static void appendElement(StringBuilder buffer, int index, DelphiNode no } private static String getBlockString(Block block) { - return "B" + ((BlockImpl) block).getId(); + return "B" + ((BlockImpl) block).getId() + " - " + ((BlockImpl) block).getBlockType(); } private static Optional getAs(Block block, Class clazz) { 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 3b09b225d..e4e7c09a5 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 @@ -524,11 +524,16 @@ public ControlFlowGraphBuilder visit(TryStatementNode node, ControlFlowGraphBuil private ControlFlowGraphBuilder buildTryFinally( TryStatementNode node, ControlFlowGraphBuilder builder) { + // to ensure after a `finally` exceptional paths still exist + handleExceptionalPaths(builder); + builder.addBlockBeforeCurrent(); // Finally FinallyBlockNode finallyNode = node.getFinallyBlock(); builder.addBlock( - ProtoBlockFactory.finallyBlock(builder.getCurrentBlock(), builder.getExitBlock())); + ProtoBlockFactory.finallyBlock( + node.getFinallyBlock(), builder.getCurrentBlock(), builder.getExitBlock())); + build(finallyNode.getStatementList(), builder); builder.pushLoopContext(builder.getCurrentBlock(), builder.getCurrentBlock()); builder.pushExitBlock(builder.getCurrentBlock()); diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Branch.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Branch.java index 93f26eb25..ed02f7e9c 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Branch.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Branch.java @@ -18,6 +18,8 @@ */ package au.com.integradev.delphi.cfg.api; +import java.util.Collections; +import java.util.HashSet; import java.util.Set; /** A block where the control flow is dictated by a boolean condition, e.g., {@code if} */ @@ -38,6 +40,9 @@ public interface Branch extends Block, Terminated { @Override default Set getSuccessors() { - return Set.of(getTrueBlock(), getFalseBlock()); + Set successors = new HashSet<>(); + successors.add(getTrueBlock()); + successors.add(getFalseBlock()); + return Collections.unmodifiableSet(successors); } } diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Cases.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Cases.java index abc0d4ba9..22af28216 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Cases.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Cases.java @@ -18,6 +18,8 @@ */ package au.com.integradev.delphi.cfg.api; +import java.util.Collections; +import java.util.HashSet; import java.util.Set; /** A block for the {@code case} statement's behaviour of a possible successor for each arm */ @@ -38,6 +40,8 @@ public interface Cases extends Block, Terminated { @Override default Set getSuccessors() { - return getCaseSuccessors(); + Set successors = new HashSet<>(getCaseSuccessors()); + successors.add(getFallthroughSuccessor()); + return Collections.unmodifiableSet(successors); } } diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Finally.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Finally.java index b73884cbf..8bd3f044a 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Finally.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/api/Finally.java @@ -23,7 +23,7 @@ import java.util.Set; /** A {@code finally} block whose control flow depends on the existence of previous exceptions */ -public interface Finally extends Block { +public interface Finally extends Block, Terminated { /** * Next block in the {@link ControlFlowGraph} without exceptional circumstances * diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/block/BlockImpl.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/block/BlockImpl.java index a39d0936e..79d08c15d 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/block/BlockImpl.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/cfg/block/BlockImpl.java @@ -67,4 +67,6 @@ protected static Block getNewTarget(Block subject, Block inactiveBlock, Block ta } public abstract String getDescription(); + + public abstract String getBlockType(); } 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 0e93e5507..8027800e8 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 @@ -33,6 +33,7 @@ import java.util.Set; import java.util.stream.Collectors; import org.sonar.plugins.communitydelphi.api.ast.DelphiNode; +import org.sonar.plugins.communitydelphi.api.ast.FinallyBlockNode; public final class ProtoBlockFactory { private ProtoBlockFactory() { @@ -56,11 +57,13 @@ public static ProtoBlock branch( .setData(terminator, blocks.get(trueBlock), blocks.get(falseBlock))); } - public static ProtoBlock finallyBlock(ProtoBlock successor, ProtoBlock finallySuccessor) { + public static ProtoBlock finallyBlock( + FinallyBlockNode terminator, ProtoBlock successor, ProtoBlock finallySuccessor) { return new ProtoBlock( FinallyImpl::new, (blocks, block) -> - ((FinallyImpl) block).setData(blocks.get(successor), blocks.get(finallySuccessor))); + ((FinallyImpl) block) + .setData(terminator, blocks.get(successor), blocks.get(finallySuccessor))); } public static ProtoBlock linear(ProtoBlock successor) { @@ -143,6 +146,11 @@ public String getDescription() { "%n\tjumps to: %s%n\texceptions to: %s", getBlockString(successor), getBlocksString(exceptions)); } + + @Override + public String getBlockType() { + return "UnknownException"; + } } static class CasesImpl extends BlockImpl implements Cases { @@ -194,6 +202,11 @@ public String getDescription() { "%n\tcases to: %s%n\tfallthrough to: %s", getBlocksString(cases), getBlockString(fallthrough)); } + + @Override + public String getBlockType() { + return "Cases"; + } } static class UnconditionalJumpImpl extends BlockImpl implements UnconditionalJump { @@ -243,6 +256,11 @@ public String getDescription() { "%n\tjumps to: %s%n\twithout jump to: %s", getBlockString(target), getBlockString(withoutJump)); } + + @Override + public String getBlockType() { + return "UnconditionalJump"; + } } static class LinearImpl extends BlockImpl implements Linear { @@ -270,19 +288,26 @@ public void replaceInactiveSuccessor(Block inactiveBlock, Block target) { public String getDescription() { return String.format("%n\tjumps to: %s", getBlockString(successor)); } + + @Override + public String getBlockType() { + return "Linear"; + } } static class FinallyImpl extends BlockImpl implements Finally { private Block successor; private Block exceptionSuccessor; + private Terminator terminator; protected FinallyImpl(List elements) { super(elements); } - public void setData(Block successor, Block exitSuccessor) { + public void setData(FinallyBlockNode node, Block successor, Block exitSuccessor) { this.successor = successor; this.exceptionSuccessor = exitSuccessor; + this.terminator = new Terminator(node); } @Override @@ -295,6 +320,16 @@ public Block getExceptionSuccessor() { return exceptionSuccessor; } + @Override + public DelphiNode getTerminator() { + return terminator.getTerminatorNode(); + } + + @Override + public TerminatorKind getTerminatorKind() { + return terminator.getKind(); + } + @Override public void replaceInactiveSuccessor(Block inactiveBlock, Block target) { this.successor = getNewTarget(this.successor, inactiveBlock, target); @@ -307,6 +342,11 @@ public String getDescription() { "%n\tjumps to: %s%n\texits to: %s", getBlockString(successor), getBlockString(exceptionSuccessor)); } + + @Override + public String getBlockType() { + return "Finally"; + } } static class HaltImpl extends BlockImpl implements Halt { @@ -339,6 +379,11 @@ public void replaceInactiveSuccessor(Block inactiveBlock, Block target) { public String getDescription() { return String.format("%n\tno successors"); } + + @Override + public String getBlockType() { + return "Halt"; + } } private static class TerminusImpl extends BlockImpl implements Terminus { @@ -356,6 +401,11 @@ public void replaceInactiveSuccessor(Block inactiveBlock, Block target) { public String getDescription() { return String.format("%n\t(Exit)"); } + + @Override + public String getBlockType() { + return "Terminus"; + } } static class BranchImpl extends BlockImpl implements Branch { @@ -405,5 +455,10 @@ public String getDescription() { "%n\tjumps to: %s(true) %s(false)", getBlockString(trueBlock), getBlockString(falseBlock)); } + + @Override + public String getBlockType() { + return "Branch"; + } } } 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 2c7a16c61..9ecadf525 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 @@ -59,6 +59,7 @@ import org.sonar.plugins.communitydelphi.api.ast.CaseStatementNode; import org.sonar.plugins.communitydelphi.api.ast.CommonDelphiNode; import org.sonar.plugins.communitydelphi.api.ast.DelphiNode; +import org.sonar.plugins.communitydelphi.api.ast.FinallyBlockNode; import org.sonar.plugins.communitydelphi.api.ast.ForInStatementNode; import org.sonar.plugins.communitydelphi.api.ast.ForToStatementNode; import org.sonar.plugins.communitydelphi.api.ast.GotoStatementNode; @@ -317,6 +318,28 @@ void testIfAnd() { block(element(NameReferenceNode.class, "Foo")).succeedsTo(0))); } + @Test + void testEmptyIf() { + test( + "if C then begin end; A;", + checker( + block(element(NameReferenceNode.class, "C")) + .branchesTo(1, 1) + .withTerminator(IfStatementNode.class), + block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + } + + @Test + void testEmptyIfElse() { + test( + "if C then begin end else begin end; A;", + checker( + block(element(NameReferenceNode.class, "C")) + .branchesTo(1, 1) + .withTerminator(IfStatementNode.class), + block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + } + @Test void testLocalVarDeclaration() { test( @@ -421,6 +444,42 @@ void testCaseStatementElse() { .succeedsToCases(2, 3, 4))); } + @Test + void testEmptyCase() { + test( + "case S of end; A;", + checker( + block(element(NameReferenceNode.class, "S")) + .withTerminator(CaseStatementNode.class) + .succeedsToCases(1), + block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + } + + @Test + void testEmptyCaseElse() { + test( + "case S of else end; A;", + checker( + block(element(NameReferenceNode.class, "S")) + .withTerminator(CaseStatementNode.class) + .succeedsToCases(1), + block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + } + + @Test + void testEmptyCaseArm() { + test( + "case S of S1:; S2:; end; A;", + checker( + block( + element(NameReferenceNode.class, "S"), + element(NameReferenceNode.class, "S1"), + element(NameReferenceNode.class, "S2")) + .withTerminator(CaseStatementNode.class) + .succeedsToCases(1, 1), + block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + } + @Test void testRepeat() { test( @@ -457,6 +516,17 @@ void testRepeatBreak() { .withTerminator(RepeatStatementNode.class))); } + @Test + void testEmptyRepeat() { + test( + "repeat until C; A;", + checker( + block(element(NameReferenceNode.class, "C")) + .branchesTo(1, 2) + .withTerminator(RepeatStatementNode.class), + block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + } + @Test void testWhile() { test( @@ -493,6 +563,17 @@ void testWhileBreak() { .withTerminator(StatementTerminator.BREAK))); } + @Test + void testEmptyWhile() { + test( + "while C do; A;", + checker( + block(element(NameReferenceNode.class, "C")) + .branchesTo(2, 1) + .withTerminator(WhileStatementNode.class), + block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + } + @Test void testForToVarDecl() { test( @@ -585,6 +666,19 @@ void testForToConditionalContinue() { .withTerminator(ForToStatementNode.class))); } + @Test + void testEmptyForTo() { + test( + "for I := F to T do; A;", + checker( + block(element(NameReferenceNode.class, "F")).succeedsTo(3), + block(element(NameReferenceNode.class, "T")).succeedsTo(2), + block(element(NameReferenceNode.class, "I")) + .branchesTo(2, 1) + .withTerminator(ForToStatementNode.class), + block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + } + @Test void testForInVarDecl() { test( @@ -671,6 +765,18 @@ void testForInConditionalContinue() { .withTerminator(ForInStatementNode.class))); } + @Test + void testEmptyForIn() { + test( + "for I in C do; A;", + checker( + block(element(NameReferenceNode.class, "C")).succeedsTo(2), + block(element(NameReferenceNode.class, "I")) + .branchesTo(2, 1) + .withTerminator(ForInStatementNode.class), + block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + } + @Test void testBreakOutsideOfLoop() { GraphChecker checker = checker(); @@ -696,6 +802,15 @@ void testWith() { block(element(NameReferenceNode.class, "Foo")).succeedsTo(0))); } + @Test + void testEmptyWith() { + test( + "with S do; A;", + checker( + block(element(NameReferenceNode.class, "S")).succeedsTo(1), + block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + } + @Test void testTryFinallyNoRaise() { test( @@ -704,7 +819,9 @@ void testTryFinallyNoRaise() { checker( block(element(TryStatementNode.class)).succeedsTo(2), block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(1, 1), - block(element(NameReferenceNode.class, "Bar")).succeedsToWithExit(0, 0))); + block(element(NameReferenceNode.class, "Bar")) + .withTerminator(FinallyBlockNode.class) + .succeedsToWithExit(0, 0))); } @Test @@ -716,7 +833,9 @@ void testTryFinallyRaise() { block(element(NameReferenceNode.class, "E")) .withTerminator(RaiseStatementNode.class, TerminatorKind.RAISE) .jumpsTo(1, 1), - block(element(NameReferenceNode.class, "Bar")).succeedsToWithExit(0, 0))); + block(element(NameReferenceNode.class, "Bar")) + .withTerminator(FinallyBlockNode.class) + .succeedsToWithExit(0, 0))); } @Test @@ -726,7 +845,9 @@ void testTryFinallyExit() { checker( block(element(TryStatementNode.class)).succeedsTo(2), terminator(StatementTerminator.EXIT).jumpsTo(1, 1), - block(element(NameReferenceNode.class, "Bar")).succeedsToWithExit(0, 0))); + block(element(NameReferenceNode.class, "Bar")) + .withTerminator(FinallyBlockNode.class) + .succeedsToWithExit(0, 0))); } @Test @@ -739,7 +860,9 @@ void testTryFinallyBreak() { .withTerminator(WhileStatementNode.class), block(element(TryStatementNode.class)).succeedsTo(2), terminator(StatementTerminator.BREAK).jumpsTo(1, 1), - block(element(NameReferenceNode.class, "Bar")).succeedsToWithExit(4, 0))); + block(element(NameReferenceNode.class, "Bar")) + .withTerminator(FinallyBlockNode.class) + .succeedsToWithExit(4, 0))); } @Test @@ -752,7 +875,9 @@ void testTryFinallyContinue() { .withTerminator(WhileStatementNode.class), block(element(TryStatementNode.class)).succeedsTo(2), terminator(StatementTerminator.CONTINUE).jumpsTo(1, 1), - block(element(NameReferenceNode.class, "Bar")).succeedsToWithExit(4, 0))); + block(element(NameReferenceNode.class, "Bar")) + .withTerminator(FinallyBlockNode.class) + .succeedsToWithExit(4, 0))); } @Test @@ -762,7 +887,9 @@ void testTryFinallyHalt() { checker( block(element(TryStatementNode.class)).succeedsTo(2), terminator(StatementTerminator.HALT).isSink(), - block(element(NameReferenceNode.class, "Bar")).succeedsToWithExit(0, 0))); + block(element(NameReferenceNode.class, "Bar")) + .withTerminator(FinallyBlockNode.class) + .succeedsToWithExit(0, 0))); } @Test @@ -780,7 +907,9 @@ void testTryContinueFinallyInLoop() { block(element(IntegerLiteralNode.class)).succeedsTo(1), block(element(TryStatementNode.class)).succeedsTo(3), terminator(StatementTerminator.CONTINUE).jumpsTo(2, 2), - block(element(NameReferenceNode.class, "Bar")).succeedsToWithExit(1, 0), + block(element(NameReferenceNode.class, "Bar")) + .withTerminator(FinallyBlockNode.class) + .succeedsToWithExit(1, 0), block(element(NameDeclarationNode.class, "A")) .branchesTo(4, 0) .withTerminator(ForToStatementNode.class))); @@ -801,7 +930,9 @@ void testTryBreakFinallyInLoop() { block(element(IntegerLiteralNode.class)).succeedsTo(1), block(element(TryStatementNode.class)).succeedsTo(3), terminator(StatementTerminator.BREAK).jumpsTo(2, 2), - block(element(NameReferenceNode.class, "Bar")).succeedsToWithExit(1, 0), + block(element(NameReferenceNode.class, "Bar")) + .withTerminator(FinallyBlockNode.class) + .succeedsToWithExit(1, 0), block(element(NameDeclarationNode.class, "A")) .branchesTo(4, 0) .withTerminator(ForToStatementNode.class))); @@ -822,7 +953,9 @@ void testTryExitFinallyInLoop() { block(element(IntegerLiteralNode.class)).succeedsTo(1), block(element(TryStatementNode.class)).succeedsTo(3), terminator(StatementTerminator.EXIT).jumpsTo(2, 2), - block(element(NameReferenceNode.class, "Bar")).succeedsToWithExit(1, 0), + block(element(NameReferenceNode.class, "Bar")) + .succeedsToWithExit(1, 0) + .withTerminator(FinallyBlockNode.class), block(element(NameDeclarationNode.class, "A")) .branchesTo(4, 0) .withTerminator(ForToStatementNode.class))); @@ -843,7 +976,9 @@ void testTryHaltFinallyInLoop() { block(element(IntegerLiteralNode.class)).succeedsTo(1), block(element(TryStatementNode.class)).succeedsTo(3), terminator(StatementTerminator.HALT).isSink(), - block(element(NameReferenceNode.class, "Bar")).succeedsToWithExit(1, 0), + block(element(NameReferenceNode.class, "Bar")) + .succeedsToWithExit(1, 0) + .withTerminator(FinallyBlockNode.class), block(element(NameDeclarationNode.class, "A")) .branchesTo(4, 0) .withTerminator(ForToStatementNode.class))); @@ -857,7 +992,9 @@ void testTryExceptNoRaise() { checker( block(element(TryStatementNode.class)).succeedsTo(2), block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(1, 1), - block(element(NameReferenceNode.class, "Bar")).succeedsToWithExit(0, 0))); + block(element(NameReferenceNode.class, "Bar")) + .withTerminator(FinallyBlockNode.class) + .succeedsToWithExit(0, 0))); } @Test @@ -1014,7 +1151,9 @@ void testNestedTryExceptFinally() { block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(1, 1, 2), block(element(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Bar")) .succeedsToWithExceptions(1, 1), - block(element(NameReferenceNode.class, "Baz")).succeedsToWithExit(0, 0))); + block(element(NameReferenceNode.class, "Baz")) + .withTerminator(FinallyBlockNode.class) + .succeedsToWithExit(0, 0))); } @Test @@ -1028,10 +1167,12 @@ void testNestedTryFinallyExcept() { "procedure Baz; begin end")), "try try Foo finally Bar end except on E: Exception do Baz end;", checker( - block(element(TryStatementNode.class)).succeedsTo(4), - block(element(TryStatementNode.class)).succeedsTo(3), - block(element(NameReferenceNode.class, "Foo")).succeedsToWithExceptions(2, 2), - block(element(NameReferenceNode.class, "Bar")).succeedsToWithExceptions(0, 0, 1), + 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(NameDeclarationNode.class, "E"), element(NameReferenceNode.class, "Baz")) .succeedsTo(0))); } @@ -1074,6 +1215,25 @@ void testTryExceptReRaise() { .succeedsToWithExceptions(0))); } + @Test + void testEmptyTryExcept() { + test( + "try except end; A", + checker( + block(element(TryStatementNode.class)).succeedsTo(1), + block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + } + + @Test + void testEmptyTryFinally() { + test( + "try finally end; A", + checker( + block(element(TryStatementNode.class)).succeedsTo(2), + terminator(FinallyBlockNode.class).succeedsToWithExit(1, 0), + block(element(NameReferenceNode.class, "A")).succeedsTo(0))); + } + @Test void testRaiseOutsideTry() { test( @@ -1320,6 +1480,14 @@ void testLabelSeparatesBlock() { .jumpsTo(2, 0))); } + @Test + void testEmptyLabel() { + test( + Map.of("label", List.of("A,B")), + "A: B: C;", + checker(block(element(NameReferenceNode.class, "C")).succeedsTo(0))); + } + @Test void testAnonymousRoutinesAreIgnored() { test( @@ -1425,7 +1593,7 @@ void testComplexConstructions() { .succeedsTo(2), block(element(TextLiteralNode.class, "'b'"), element(NameReferenceNode.class, "X")) .succeedsTo(2), - block().succeedsToWithExit(12, 0), + terminator(FinallyBlockNode.class).succeedsToWithExit(12, 0), block(element(NameReferenceNode.class, "Foo4")).succeedsTo(0))); } } 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 2e3f12175..9dfda38d4 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 @@ -85,7 +85,7 @@ public void check(final Block block) { if (terminatorChecker != null) { terminatorChecker.check(block); } else { - assertThat(block.getSuccessors()) + assertThat(block) .withFailMessage("%s should have its terminator specified", getBlockDisplay(block)) .isNotInstanceOf(Terminated.class); }