Skip to content

Clarify R503 terminal-return scope (#197) - #215

Merged
leynos merged 1 commit into
mainfrom
issue-197-clarify-r503-require-terminal-returns-only-when-another-branch-returns-a-value
Aug 1, 2026
Merged

Clarify R503 terminal-return scope (#197)#215
leynos merged 1 commit into
mainfrom
issue-197-clarify-r503-require-terminal-returns-only-when-another-branch-returns-a-value

Conversation

@leynos

@leynos leynos commented Jul 30, 2026

Copy link
Copy Markdown
Owner

Summary

This branch clarifies the Python return guidance so R503 requires an explicit terminal return only when another control-flow path returns a value. It aligns R502 and R503 with R501 by using a bare return for an explicit None result, while allowing procedures whose only possible result is None to return implicitly.

Closes #197.

Review walkthrough

  • Review .rules/python-return.md for the aligned R502 example, the narrowed R503 heading and explanation, and the explicit exemption for all-None procedures.

Validation

  • make fmt: passed; unrelated formatter churn was removed.
  • make markdownlint: passed, including .rules/python-return.md.
  • make nixie: passed.
  • git diff --check: passed.
  • coderabbit review --agent: completed with zero findings.

References

Summary by Sourcery

Documentation:

  • Update python-return rule documentation to distinguish functions that may return a non-None value from procedures whose only possible result is None, including examples using bare return and refined wording for R502 and R503.

Scope R503 to functions that return a value along another path. Align R502
with R501 by using a bare `return` for the explicit `None` result while
leaving all-`None` procedures free to return implicitly.
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3b329ef0-7f31-4e91-afd2-2039ca58e18f

📥 Commits

Reviewing files that changed from the base of the PR and between ab56c1a and f14723f.

📒 Files selected for processing (1)
  • .rules/python-return.md
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • leynos/cmd-mox (auto-detected)
  • leynos/cuprum (auto-detected)
  • leynos/shared-actions (auto-detected)

Summary

  • Clarify R503 for mixed-result control flow.
  • Align R502 and R503 with R501 by using bare return for explicit None.
  • Allow implicit returns when a function can only return None.
  • Update R503 guidance and examples in .rules/python-return.md.

Validation

  • Run make fmt, make markdownlint, make nixie, and git diff --check.
  • Complete CodeRabbit review successfully.

Walkthrough

Update .rules/python-return.md so R503 requires a terminal return only when another path returns a value. Clarify that all-None functions do not need a redundant final return.

Changes

R503 guidance

Layer / File(s) Summary
Update R503 rule and example
.rules/python-return.md
R503 now permits a bare return for None paths. Its example uses a bare return and excludes functions whose only result is None from the terminal-return requirement.

Possibly related issues

Possibly related PRs

  • leynos/cuprum#216 — Updates .rules/python-return.md with the same R503 terminal-return clarification.

Suggested labels: Issue

Poem

R503 now reads clear,
Bare returns need not appear,
Unless values cross the flow,
All-None paths may simply go,
Guidance aligned, confusion gone.

🚥 Pre-merge checks | ✅ 20
✅ Passed checks (20 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the R503 scope clarification and links issue #197 as required.
Description check ✅ Passed The description accurately explains the documentation changes and their relationship to issue #197.
Linked Issues check ✅ Passed The changes satisfy all issue #197 objectives, including R502/R503 alignment and the exemption for all-None procedures.
Out of Scope Changes check ✅ Passed The changes are limited to the documented R502 and R503 guidance described in issue #197.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Testing (Overall) ✅ Passed Treat testing as not applicable: the commit changes only .rules/python-return.md and adds no executable functionality or behavioural code.
User-Facing Documentation ✅ Passed Pass this check: the commit changes only .rules/python-return.md, which documents developer coding standards; it adds no user-facing functionality or behaviour requiring docs/users-guide.md upd...
Developer Documentation ✅ Passed The patch changes only .rules/python-return.md guidance; it adds no API, architecture, tooling, build, roadmap, execplan, or locale changes that require developer-guide updates.
Module-Level Documentation ✅ Passed The PR changes only .rules/python-return.md and adds no Python module, so the module-level docstring requirement is not applicable.
Testing (Unit And Behavioural) ✅ Passed Assess the documentation-only change with Markdown validation; HEAD changes only .rules/python-return.md, so no unit, behavioural, or end-to-end tests are applicable.
Testing (Property / Proof) ✅ Passed The commit changes only .rules/python-return.md; it adds guidance and examples, not executable behaviour or a new input/state invariant requiring property or proof tests.
Testing (Compile-Time / Ui) ✅ Passed Pass this check: the commit changes only .rules/python-return.md; it adds no Rust or TypeScript compile-time behaviour and produces no structured or UI output requiring snapshot tests.
Unit Architecture ✅ Passed Pass this check: HEAD changes only .rules/python-return.md; it adds no units, dependencies, queries, commands, fallible operations, or side-effects.
Domain Architecture ✅ Passed Treat this check as passed: HEAD changes only .rules/python-return.md; no domain, adapter, transport, persistence, or infrastructure code changes exist.
Observability ✅ Passed Pass: the pull request changes only .rules/python-return.md; it adds no operational behaviour, so logging, metrics, tracing, and alerts are not required.
Security And Privacy ✅ Passed Pass the check: the commit changes only .rules/python-return.md; added lines contain return guidance only, with no secrets, credentials, permissions, inputs, or sensitive data.
Performance And Resource Use ✅ Passed The commit changes only .rules/python-return.md; it updates Markdown prose and examples, with no executable code, loops, I/O, allocations, or runtime paths.
Concurrency And State ✅ Passed The commit changes only .rules/python-return.md; it adds no executable code, shared state, async task, lock, ordering, or cancellation behaviour requiring concurrency review.
Architectural Complexity And Maintainability ✅ Passed Pass: HEAD changes only .rules/python-return.md; the diff adds no code, dependency, abstraction, layer, or architectural mechanism.
Rust Compiler Lint Integrity ✅ Passed The commit changes only .rules/python-return.md; git diff-tree reports zero .rs paths and the repository has no Rust files, so this Rust check is not applicable.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-197-clarify-r503-require-terminal-returns-only-when-another-branch-returns-a-value

Comment @coderabbitai help to get the list of available commands.

@sourcery-ai

sourcery-ai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Clarifies Python return rules R502 and R503 to distinguish when explicit terminal returns are required, aligning them with R501 and exempting functions that can only return None.

File-Level Changes

Change Details Files
Clarified wording and examples for Python return rules R502 and R503 to specify when explicit terminal returns (including bare returns) are required.
  • Updated R502 example to use a bare return to represent an explicit None result instead of returning a concrete value.
  • Rewrote R502 guidance text to emphasize explicit returns when any branch returns a value and to mention using bare return for None, consistent with R501.
  • Renamed and reworded the R503 section heading to focus on terminal returns only when another control-flow path returns a value.
  • Adjusted R503 code example to use a bare terminal return instead of returning a sentinel value, matching the clarified rule.
  • Expanded R503 explanatory text to distinguish functions that may return a value from those whose only possible result is None, explicitly exempting the latter from needing a terminal bare return.
.rules/python-return.md

Assessment against linked issues

Issue Objective Addressed Explanation
#197 Update R503 to scope the terminal-return requirement to functions that return a value in another branch or path.
#197 Align R501, R502, and R503 by preferring a bare return over return None and making the R502 and R503 prose/examples internally consistent.
#197 Ensure the rules do not require an explicit bare return for functions whose only possible result is None.

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

codescene-access[bot]

This comment was marked as outdated.

@codescene-access codescene-access Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No quality gates enabled for this code.

@leynos
leynos marked this pull request as ready for review August 1, 2026 09:54

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @leynos, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai coderabbitai Bot added the Issue label Aug 1, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f14723f800

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .rules/python-return.md
Comment on lines +40 to +41
Ensure all branches return explicitly if any branch returns a value. Use a bare
`return` when the other result is `None`, as required by R501.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Replace bare returns in mixed-value examples

When one branch returns x, a bare return is precisely what RET502 rejects: running ruff check --isolated --select RET501,RET502,RET503 on this example reports “RET502 Do not implicitly return None in function able to return non-None value.” R501 only removes explicit None when None is the function's only possible result, so both new “GOOD” examples fail the configured RET lint; use return None or another explicit value instead.

AGENTS.md reference: AGENTS.md:L199-L200

Useful? React with 👍 / 👎.

@leynos
leynos merged commit 8f046ae into main Aug 1, 2026
9 checks passed
@leynos
leynos deleted the issue-197-clarify-r503-require-terminal-returns-only-when-another-branch-returns-a-value branch August 1, 2026 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clarify R503: require terminal returns only when another branch returns a value

1 participant