From 758c2031a795a208e6ed54cb03af7ecb25087321 Mon Sep 17 00:00:00 2001 From: Erwan Le Forestier Date: Mon, 17 Aug 2026 11:34:55 +0200 Subject: [PATCH 1/6] SONARPY-4544 Count terminal raises as exits --- .../sonar/python/checks/InvariantReturnCheck.java | 10 +++++----- .../src/test/resources/checks/invariantReturn.py | 12 ++++++++---- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java b/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java index 476e0b2ea..e7f453915 100644 --- a/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java +++ b/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java @@ -83,7 +83,7 @@ private static List collectLatestExecutedBlocks(ControlFlow for (CfgBlock predecessor : cfg.end().predecessors()) { if (predecessor instanceof PythonCfgBranchingBlock pythonCfgBranchingBlock) { collectBranchingBlock(collectedBlocks, pythonCfgBranchingBlock); - } else if (!endsWithElementKind(predecessor, Kind.RAISE_STMT)) { + } else { collectedBlocks.add(new LatestExecutedBlock(predecessor)); } } @@ -211,6 +211,10 @@ private static Tree findUniquePreviousBinding(LatestExecutedBlock context, Symbo return bindings.size() == 1 ? bindings.iterator().next() : null; } + private static boolean endsWithElementKind(CfgBlock block, Kind kind) { + return lastElement(block, kind) != null; + } + @Nullable private static Tree findLastBinding(List elements, Symbol identifier) { for (int i = elements.size() - 1; i >= 0; i--) { @@ -251,10 +255,6 @@ private static boolean couldBeModified(Name name) { return true; } - private static boolean endsWithElementKind(CfgBlock block, Kind kind) { - return lastElement(block, kind) != null; - } - @Nullable private static Tree lastElement(CfgBlock block, Kind kind) { List elements = block.elements(); diff --git a/python-checks/src/test/resources/checks/invariantReturn.py b/python-checks/src/test/resources/checks/invariantReturn.py index 3648cb887..9bbc5e0f4 100644 --- a/python-checks/src/test/resources/checks/invariantReturn.py +++ b/python-checks/src/test/resources/checks/invariantReturn.py @@ -433,15 +433,19 @@ def f_raise_is_not_a_return(a, b): raise b return b -def f_function_exit_through_raise_should_be_ignored(a, b, c): # Noncompliant -# ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +def f_function_exit_through_raise_prevents_reporting(a, b, c): if a: return b -# ^^^^^^^^< if c: raise return b -# ^^^^^^^^< + +def f_early_return_with_raise_guard(already_valid, invalid, self): + if already_valid: + return self + if invalid: + raise ValueError() + return self def f_same_binding_through_multiple_paths(a): # Noncompliant d = 3 From 68e0eae983504e750ea1a0620bced4a71bdedc33 Mon Sep 17 00:00:00 2001 From: Erwan Le Forestier Date: Mon, 17 Aug 2026 14:05:08 +0200 Subject: [PATCH 2/6] SONARPY-4544 Ignore unreachable raise blocks when collecting exits --- .../python/checks/InvariantReturnCheck.java | 20 +++++++++++++++++++ .../test/resources/checks/invariantReturn.py | 6 ++++++ 2 files changed, 26 insertions(+) diff --git a/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java b/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java index e7f453915..8487b8809 100644 --- a/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java +++ b/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java @@ -80,7 +80,11 @@ public void initialize(Context context) { private static List collectLatestExecutedBlocks(ControlFlowGraph cfg) { List collectedBlocks = new ArrayList<>(); + Set reachableBlocks = reachableBlocks(cfg.start()); for (CfgBlock predecessor : cfg.end().predecessors()) { + if (!reachableBlocks.contains(predecessor)) { + continue; + } if (predecessor instanceof PythonCfgBranchingBlock pythonCfgBranchingBlock) { collectBranchingBlock(collectedBlocks, pythonCfgBranchingBlock); } else { @@ -90,6 +94,22 @@ private static List collectLatestExecutedBlocks(ControlFlow return collectedBlocks; } + private static Set reachableBlocks(CfgBlock start) { + Set reachable = new HashSet<>(); + Deque blockToVisit = new ArrayDeque<>(); + blockToVisit.push(start); + reachable.add(start); + while (!blockToVisit.isEmpty()) { + CfgBlock block = blockToVisit.pop(); + for (CfgBlock successor : block.successors()) { + if (reachable.add(successor)) { + blockToVisit.push(successor); + } + } + } + return reachable; + } + private static void collectBranchingBlock(List collectedBlocks, PythonCfgBranchingBlock branchingBlock) { Tree branchingTree = branchingBlock.branchingTree(); if (branchingTree.is(Kind.TRY_STMT)) { diff --git a/python-checks/src/test/resources/checks/invariantReturn.py b/python-checks/src/test/resources/checks/invariantReturn.py index 9bbc5e0f4..433e30f00 100644 --- a/python-checks/src/test/resources/checks/invariantReturn.py +++ b/python-checks/src/test/resources/checks/invariantReturn.py @@ -447,6 +447,12 @@ def f_early_return_with_raise_guard(already_valid, invalid, self): raise ValueError() return self +def f_unreachable_raise_does_not_prevent_reporting(a, b): # Noncompliant + if a: + return b + return b + raise ValueError() + def f_same_binding_through_multiple_paths(a): # Noncompliant d = 3 try: From c42a1d1f5b6d92938250703ad2bafc733fa1386b Mon Sep 17 00:00:00 2001 From: Erwan Le Forestier Date: Mon, 17 Aug 2026 14:10:09 +0200 Subject: [PATCH 3/6] SONARPY-4544 Treat raises through except/finally branches as exits --- .../python/checks/InvariantReturnCheck.java | 2 +- .../test/resources/checks/invariantReturn.py | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java b/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java index 8487b8809..b47e5432b 100644 --- a/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java +++ b/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java @@ -129,7 +129,7 @@ private static void collectBlocksHavingReturnBeforeExceptOrFinallyBlock(List Date: Mon, 17 Aug 2026 14:12:00 +0200 Subject: [PATCH 4/6] SONARPY-4544 Keep endsWithElementKind next to lastElement --- .../org/sonar/python/checks/InvariantReturnCheck.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java b/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java index b47e5432b..92544af4e 100644 --- a/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java +++ b/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java @@ -231,10 +231,6 @@ private static Tree findUniquePreviousBinding(LatestExecutedBlock context, Symbo return bindings.size() == 1 ? bindings.iterator().next() : null; } - private static boolean endsWithElementKind(CfgBlock block, Kind kind) { - return lastElement(block, kind) != null; - } - @Nullable private static Tree findLastBinding(List elements, Symbol identifier) { for (int i = elements.size() - 1; i >= 0; i--) { @@ -275,6 +271,10 @@ private static boolean couldBeModified(Name name) { return true; } + private static boolean endsWithElementKind(CfgBlock block, Kind kind) { + return lastElement(block, kind) != null; + } + @Nullable private static Tree lastElement(CfgBlock block, Kind kind) { List elements = block.elements(); From 0f2f196c54711b076f3debd9cefeac6d556af6a1 Mon Sep 17 00:00:00 2001 From: Erwan Le Forestier Date: Mon, 17 Aug 2026 14:12:25 +0200 Subject: [PATCH 5/6] SONARPY-4544 Rename misleading self parameter in test fixture --- python-checks/src/test/resources/checks/invariantReturn.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/python-checks/src/test/resources/checks/invariantReturn.py b/python-checks/src/test/resources/checks/invariantReturn.py index f40db9717..d0598ed8d 100644 --- a/python-checks/src/test/resources/checks/invariantReturn.py +++ b/python-checks/src/test/resources/checks/invariantReturn.py @@ -440,12 +440,12 @@ def f_function_exit_through_raise_prevents_reporting(a, b, c): raise return b -def f_early_return_with_raise_guard(already_valid, invalid, self): +def f_early_return_with_raise_guard(already_valid, invalid, obj): if already_valid: - return self + return obj if invalid: raise ValueError() - return self + return obj def f_unreachable_raise_does_not_prevent_reporting(a, b): # Noncompliant if a: From b4fb6ef5b563604b71f7a1586a3b1ebe5998ff64 Mon Sep 17 00:00:00 2001 From: Erwan Le Forestier Date: Mon, 17 Aug 2026 14:19:02 +0200 Subject: [PATCH 6/6] SONARPY-4544 Apply reachability filter to except/finally predecessors --- .../sonar/python/checks/InvariantReturnCheck.java | 13 ++++++++----- .../src/test/resources/checks/invariantReturn.py | 11 +++++++++++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java b/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java index 92544af4e..ee65063cc 100644 --- a/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java +++ b/python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java @@ -86,7 +86,7 @@ private static List collectLatestExecutedBlocks(ControlFlow continue; } if (predecessor instanceof PythonCfgBranchingBlock pythonCfgBranchingBlock) { - collectBranchingBlock(collectedBlocks, pythonCfgBranchingBlock); + collectBranchingBlock(collectedBlocks, pythonCfgBranchingBlock, reachableBlocks); } else { collectedBlocks.add(new LatestExecutedBlock(predecessor)); } @@ -110,7 +110,7 @@ private static Set reachableBlocks(CfgBlock start) { return reachable; } - private static void collectBranchingBlock(List collectedBlocks, PythonCfgBranchingBlock branchingBlock) { + private static void collectBranchingBlock(List collectedBlocks, PythonCfgBranchingBlock branchingBlock, Set reachableBlocks) { Tree branchingTree = branchingBlock.branchingTree(); if (branchingTree.is(Kind.TRY_STMT)) { TryStatement tryStatement = (TryStatement) branchingTree; @@ -120,15 +120,18 @@ private static void collectBranchingBlock(List collectedBlo } else if (branchingTree.is(Kind.IF_STMT) || branchingTree instanceof Pattern) { collectedBlocks.add(new LatestExecutedBlock(branchingBlock)); } else { - collectBlocksHavingReturnBeforeExceptOrFinallyBlock(collectedBlocks, branchingBlock); + collectBlocksHavingReturnBeforeExceptOrFinallyBlock(collectedBlocks, branchingBlock, reachableBlocks); } } - private static void collectBlocksHavingReturnBeforeExceptOrFinallyBlock(List collectedBlocks, PythonCfgBranchingBlock branchingBlock) { + private static void collectBlocksHavingReturnBeforeExceptOrFinallyBlock(List collectedBlocks, PythonCfgBranchingBlock branchingBlock, Set reachableBlocks) { if (branchingBlock.branchingTree().is(Kind.EXCEPT_CLAUSE, Kind.FINALLY_CLAUSE)) { for (CfgBlock predecessor : branchingBlock.predecessors()) { + if (!reachableBlocks.contains(predecessor)) { + continue; + } if (predecessor instanceof PythonCfgBranchingBlock pythonCfgBranchingBlock) { - collectBlocksHavingReturnBeforeExceptOrFinallyBlock(collectedBlocks, pythonCfgBranchingBlock); + collectBlocksHavingReturnBeforeExceptOrFinallyBlock(collectedBlocks, pythonCfgBranchingBlock, reachableBlocks); } else if (endsWithElementKind(predecessor, Kind.RETURN_STMT) || endsWithElementKind(predecessor, Kind.RAISE_STMT)) { collectedBlocks.add(new LatestExecutedBlock(predecessor)); } diff --git a/python-checks/src/test/resources/checks/invariantReturn.py b/python-checks/src/test/resources/checks/invariantReturn.py index d0598ed8d..14408417d 100644 --- a/python-checks/src/test/resources/checks/invariantReturn.py +++ b/python-checks/src/test/resources/checks/invariantReturn.py @@ -473,6 +473,17 @@ def f_raise_in_try_with_except(early, invalid, value): cleanup() return value +def f_unreachable_raise_before_finally_does_not_prevent_reporting(a, b): # Noncompliant + try: + if a: + return b + else: + return b + raise ValueError() + finally: + pass + return b + def f_same_binding_through_multiple_paths(a): # Noncompliant d = 3 try: