Skip to content

Clarify R503 requires terminal returns only when another branch returns a value (#197) - #216

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

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

Conversation

@buzzybee-df12

Copy link
Copy Markdown
Collaborator

Closes #197

R503 currently says "Don't rely on implicit None—always return something at the end." This conflicts with the narrower intent expressed by R502 and led to an invalid request to add return to an all-None helper.

State the rule as:

Don't rely on implicit None if the function may return a value elsewhere—always return something at the end.

This retains the safeguard for mixed value/None control-flow while avoiding redundant bare returns for procedures.

R503 previously said every function should end with an explicit return.
That conflicted with the narrower intent of R502 and led to requests for
bare `return` in procedures whose only possible result is `None`.

Scope the terminal-return requirement to functions that may return a value
elsewhere, and add a `GOOD` example showing all-`None` procedures do not
need a final `return`.
@coderabbitai

coderabbitai Bot commented Jul 21, 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: da0b27ac-dd74-419f-a5db-765c65ae721c

📥 Commits

Reviewing files that changed from the base of the PR and between 4ac9e76 and 000275b.

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

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

  • leynos/shared-actions (auto-detected)
  • leynos/pylint-pypy-shim (auto-detected)
  • leynos/whitaker (auto-detected)

Summary

  • Clarified R503 to require a terminal explicit return only when another branch may return a value.
  • Added a GOOD example showing that procedures returning only None do not need a final bare return.
  • Updated the rule’s bad example and guidance to distinguish mixed value/None control flow from all-None procedures.

Walkthrough

Update R503 documentation to require a terminal explicit return when other paths may return a value, while documenting that implicit None remains valid when no non-None result is possible.

Changes

R503 documentation clarification

Layer / File(s) Summary
Clarify terminal-return guidance
.rules/python-return.md
Revise the R503 heading and BAD example, then add guidance and a GOOD example distinguishing value-returning branches from functions that can only return None.

Possibly related issues

  • leynos/lading#197 — Clarifies the same R503 terminal-return requirement in .rules/python-return.md.
  • leynos/agent-template-python#40 — Updates R503 guidance and examples for functions that may return a value.

Suggested labels: Issue

Poem

R503 found a path astray,
A terminal return now leads the way.
Values land where branches turn,
Bare None stays when none can earn.
Docs now guide each flow aright.

🚥 Pre-merge checks | ✅ 18 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR updates R503 docs, not the Whitaker wiring changes required by #197. Implement the Makefile, CI, rust/dylint.toml, and AGENTS.md Whitaker updates required by #197.
Out of Scope Changes check ⚠️ Warning The R503 documentation change is unrelated to the Whitaker lint wiring scope in #197. Split the R503 docs update into a separate PR or retarget this PR to the correct issue.
✅ Passed checks (18 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the R503 documentation clarification and includes the linked issue reference.
Description check ✅ Passed The description matches the R503 rule clarification and stays on topic.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Testing (Overall) ✅ Passed Only a documentation file changed; no runtime behaviour or new functionality was introduced, so no test coverage was required.
User-Facing Documentation ✅ Passed Pass: classify this as internal rule guidance only; no user-facing behaviour changed, so docs/users-guide.md need not move.
Developer Documentation ✅ Passed PASS: only .rules/python-return.md changed; no internal API, build, architecture, roadmap, or locale-synced docs were touched.
Module-Level Documentation ✅ Passed PASS: The patch only edits .rules/python-return.md, a documentation file; no code modules were added or modified, so the module-docstring rule is unaffected.
Testing (Unit And Behavioural) ✅ Passed Docs-only R503 wording change; the patch touches no executable code or observable workflow, so no new unit or behavioural tests were required.
Testing (Property / Proof) ✅ Passed Only .rules/python-return.md changed; no executable invariant, state transition, or lemma was introduced.
Testing (Compile-Time / Ui) ✅ Passed Docs-only R503 wording change; no compile-time or UI behaviour changed, so trybuild/snapshot coverage is not applicable.
Unit Architecture ✅ Passed Only .rules/python-return.md changed; this is a documentation tweak with no code, dependencies, state, or side-effects to violate the architecture boundaries.
Domain Architecture ✅ Passed Only .rules/python-return.md changed; no domain, adapter, transport, persistence, or infrastructure code was touched.
Observability ✅ Passed Treat this as not applicable: only a markdown style guide changed, with no runtime observability surfaces added or altered.
Security And Privacy ✅ Passed Only .rules/python-return.md changed; it is generic lint documentation with no secrets, auth, permissions, or sensitive data exposure.
Performance And Resource Use ✅ Passed Mark PASS: this patch only edits markdown guidance; no executable code, loops, I/O, or allocations change.
Concurrency And State ✅ Passed Only .rules/python-return.md changed; no shared state, async, locking, or ordering logic was touched.
Architectural Complexity And Maintainability ✅ Passed Docs-only R503 clarification; no new abstractions, layers, or dependencies were introduced.
Rust Compiler Lint Integrity ✅ Passed PASS: The PR changes only .rules/python-return.md; no Rust code, lint suppressions, or clone-heavy ownership changes are introduced.
✨ 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 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sorry @buzzybee-df12, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@buzzybee-df12
buzzybee-df12 marked this pull request as ready for review July 21, 2026 17:34
@coderabbitai coderabbitai Bot added the Issue label Jul 21, 2026

@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 merged commit 7bd671f into main Jul 21, 2026
22 checks passed
@leynos
leynos deleted the issue-197-clarify-r503-require-terminal-returns-only-when-another-branch-returns-a-value branch July 21, 2026 21:12
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.

2 participants