Skip to content

docs: clarify C901 complexity example#25186

Open
ya-nsh wants to merge 1 commit into
astral-sh:mainfrom
ya-nsh:docs/c901-clarify-complexity-example
Open

docs: clarify C901 complexity example#25186
ya-nsh wants to merge 1 commit into
astral-sh:mainfrom
ya-nsh:docs/c901-clarify-complexity-example

Conversation

@ya-nsh
Copy link
Copy Markdown

@ya-nsh ya-nsh commented May 15, 2026

Summary

  • clarifies the C901 documentation example so the before/after guidance does not imply that guard clauses necessarily reduce McCabe complexity
  • replaces the previous nested-vs-guard-clause example with an example that moves decision-making into a focused helper

Fixes #25028.

Validation

  • git diff --check
  • sanity-checked the edited docs block with a short Python script

Note: I attempted the repository preflight command uvx prek run -a, but this environment does not have the Rust toolchain installed, so the rustfmt hook failed before running:

error: Failed to run hook `rustfmt`
  caused by: Run command `run system command` failed
  caused by: No such file or directory (os error 2)

This change only updates Rust doc comments for the generated rule documentation.

@astral-sh-bot astral-sh-bot Bot requested a review from ntBre May 15, 2026 20:08
@ntBre ntBre added the documentation Improvements or additions to documentation label May 15, 2026
Copy link
Copy Markdown
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

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

Thanks! I think this is a little better, but it feels a bit outside the spirit of the rule to me just to move the complexity into a helper function (which would actually make reading/reviewing the code more complex, in my opinion). Codex suggested something like the following, where we replace a sequence of if conditions with a dict lookup:

# Before: branch-heavy lookup
def normalize_status(status):
    if status == "new":
        return "queued"
    if status == "queued":
        return "running"
    if status == "running":
        return "done"
    if status == "failed":
        return "retry"
    if status == "cancelled":
        return "closed"
    return "unknown"


# After: the decision table is data, not control flow
STATUS_TRANSITIONS = {
    "new": "queued",
    "queued": "running",
    "running": "done",
    "failed": "retry",
    "cancelled": "closed",
}


def normalize_status_lookup(status):
    return STATUS_TRANSITIONS.get(status, "unknown")

That specifically is pretty verbose, but I kind of like something in that direction.

I also realized that the default value of max-complexity is 10, so neither the existing nor new input examples actually trigger C901 out of the box. I don't think we need to extend the length of the examples to meet that level of complexity, but we should probably note that somewhere (e.g. "These examples assume a lint.mccabe.max-complexity of ..."). I'll try to remember to comment on #18972 to mark this rule as an exception, assuming we go that route.

@ya-nsh ya-nsh force-pushed the docs/c901-clarify-complexity-example branch from 6d21567 to 3acd130 Compare May 16, 2026 05:38
@ya-nsh
Copy link
Copy Markdown
Author

ya-nsh commented May 16, 2026

Addressed in 3acd130: updated the C901 docs example to use the lookup-table direction you suggested and explicitly notes that the example assumes a max complexity of 5.

Validation: git diff --check- Parsed both Python snippets and confirmed the before example has complexity 6 while the lookup-table version has complexity 1

I also reran uvx prek run -a, but it still cannot complete in this environment because the rustfmt hook fails with No such file or directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

C901 complex-structure has unclear documentation

2 participants