Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -80,17 +80,37 @@ public void initialize(Context context) {

private static List<LatestExecutedBlock> collectLatestExecutedBlocks(ControlFlowGraph cfg) {
List<LatestExecutedBlock> collectedBlocks = new ArrayList<>();
Set<CfgBlock> reachableBlocks = reachableBlocks(cfg.start());
for (CfgBlock predecessor : cfg.end().predecessors()) {
if (!reachableBlocks.contains(predecessor)) {
continue;
}
if (predecessor instanceof PythonCfgBranchingBlock pythonCfgBranchingBlock) {
collectBranchingBlock(collectedBlocks, pythonCfgBranchingBlock);
} else if (!endsWithElementKind(predecessor, Kind.RAISE_STMT)) {
collectBranchingBlock(collectedBlocks, pythonCfgBranchingBlock, reachableBlocks);
} else {
collectedBlocks.add(new LatestExecutedBlock(predecessor));
}
}
Comment thread
gitar-bot[bot] marked this conversation as resolved.
return collectedBlocks;
}

private static void collectBranchingBlock(List<LatestExecutedBlock> collectedBlocks, PythonCfgBranchingBlock branchingBlock) {
private static Set<CfgBlock> reachableBlocks(CfgBlock start) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we don't extract this method to existing CfgUtils (and reuse it in unreachableBlocks)?

Set<CfgBlock> reachable = new HashSet<>();
Deque<CfgBlock> 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<LatestExecutedBlock> collectedBlocks, PythonCfgBranchingBlock branchingBlock, Set<CfgBlock> reachableBlocks) {
Tree branchingTree = branchingBlock.branchingTree();
if (branchingTree.is(Kind.TRY_STMT)) {
TryStatement tryStatement = (TryStatement) branchingTree;
Expand All @@ -100,16 +120,19 @@ private static void collectBranchingBlock(List<LatestExecutedBlock> 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<LatestExecutedBlock> collectedBlocks, PythonCfgBranchingBlock branchingBlock) {
private static void collectBlocksHavingReturnBeforeExceptOrFinallyBlock(List<LatestExecutedBlock> collectedBlocks, PythonCfgBranchingBlock branchingBlock, Set<CfgBlock> 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);
} else if (endsWithElementKind(predecessor, Kind.RETURN_STMT)) {
collectBlocksHavingReturnBeforeExceptOrFinallyBlock(collectedBlocks, pythonCfgBranchingBlock, reachableBlocks);
} else if (endsWithElementKind(predecessor, Kind.RETURN_STMT) || endsWithElementKind(predecessor, Kind.RAISE_STMT)) {
collectedBlocks.add(new LatestExecutedBlock(predecessor));
}
}
Expand Down
49 changes: 45 additions & 4 deletions python-checks/src/test/resources/checks/invariantReturn.py
Original file line number Diff line number Diff line change
Expand Up @@ -433,15 +433,56 @@ 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, obj):
if already_valid:
return obj
if invalid:
raise ValueError()
return obj

def f_unreachable_raise_does_not_prevent_reporting(a, b): # Noncompliant
if a:
return b
return b
raise ValueError()

def f_raise_in_try_with_finally(early, invalid, value):
if early:
return value
try:
if invalid:
raise ValueError()
finally:
pass
return value

def f_raise_in_try_with_except(early, invalid, value):
if early:
return value
try:
if invalid:
raise ValueError()
except TypeError:
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
Expand Down