SONARPY-4544 Count terminal raises as exits

[erwan-leforestier-sonarsource]

Summary

Treat terminal raise CFG edges as exits for S3516, preventing reports on validator-style methods that either return a common value or raise.

Changes

• Preserve terminal raise predecessors when collecting S3516 exit paths.
• Cover both the existing raised exit and a return/raise/return validator pattern.

Functional Validation

Artifact: sonarpy-4544-fv.zip

Once the file is attached to the PR description, unzip and run:
./run.sh

Expected output:

******************* MASTER *******************
Analyzing "sample.py"...
Results:
    - Rule "S3516" -> L.2
****** Branch "fix/sonarpy-4544-treat-terminal-raise-as-exit" ******
Analyzing "sample.py"...
Results:
    (none)

[asya-vorobeva]

LGTM, with a minor suggestion.

[asya-vorobeva]

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

[guillaume-dequenne]

Hey there,
FYI @erwan-leforestier-sonarsource and @erwan-leforestier-sonarsource, this repository is a public mirror. You should never open PRs or merge changes directly against it.

[gitar-bot]

Code Review ✅ Approved 1 resolved / 1 findings

Treats terminal raise control flow graph edges as exits for rule S3516, addressing the missing reachability filter on except/finally predecessors. No issues found.

✅ 1 resolved

✅ Edge Case: Reachability filter not applied to except/finally predecessors

📄 python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java:83-93 📄 python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java:127-137
In collectLatestExecutedBlocks the reachableBlocks set gates only the direct predecessors of cfg.end() (line 85). The recursive collectBlocksHavingReturnBeforeExceptOrFinallyBlock walks branchingBlock.predecessors() and adds any block ending in RETURN_STMT/RAISE_STMT without re-checking reachability. If a reachable except/finally branching block has an unreachable predecessor that ends in a raise, that raise would still be collected and could suppress an otherwise valid S3516 report (false negative). This is hard to construct with normal control flow, so it is low risk; consider passing the reachableBlocks set into the recursion and skipping unreachable predecessors for consistency.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}