Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PLR0912 seems to overcount branches #11408

Closed
jaap3 opened this issue May 13, 2024 · 3 comments · Fixed by #11423
Closed

PLR0912 seems to overcount branches #11408

jaap3 opened this issue May 13, 2024 · 3 comments · Fixed by #11423
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers question Asking for support or clarification

Comments

@jaap3
Copy link
Contributor

jaap3 commented May 13, 2024

ruff 0.4.4 triggers PLR0912 Too many branches (13 > 12) on this code snippet:

def foo(bar_list, baz_list):
    if len(bar_list) != len(baz_list):
        raise ValueError("Both lists must have the same length")

    for bar in bar_list:
        if bar.foobar:
            try:
                handle_foobar_bar(bar)
            except SomeError:
                logger.exception("Error handling bar")
            else:
                success(bar)

    for baz in baz_list:
        try:
            f = baz.file.open("rb")
        except FileNotFoundError:
            logger.exception("File not found")
            continue

        with f:
            try:
                handle_baz_file(f)
            except SomeError as e:
                logger.exception("Error handling baz")
                if isinstance(e, SpecificError):
                    handle_specific_error(baz, e)
            else:
                success(baz)

I can only get to 13 branches by counting each for loop as a branch and including try/except as branches (related #11205).

Is this intended?

@zanieb
Copy link
Member

zanieb commented May 13, 2024

I believe this matches the upstream implementation e.g.

Not only if clauses are branches, also loops and try-except clauses have conditional characteristics and thus are counted as well.

https://pycodequ.al/docs/pylint-messages/r0912-too-many-branches.html

@zanieb zanieb added the question Asking for support or clarification label May 13, 2024
@jaap3
Copy link
Contributor Author

jaap3 commented May 13, 2024

@zanieb ah, thanks for pointing that out. I only looked at https://docs.astral.sh/ruff/rules/too-many-branches/ and https://pylint.readthedocs.io/en/stable/user_guide/messages/refactor/too-many-branches.html and didn't find any mention of loops or try/except there.

@zanieb
Copy link
Member

zanieb commented May 13, 2024

I'd welcome a PR improving that documentation. Idk why it's omitted.

@zanieb zanieb added documentation Improvements or additions to documentation good first issue Good for newcomers labels May 13, 2024
charliermarsh pushed a commit that referenced this issue May 16, 2024
## Summary

As discussed in issue #11408, PLR0912 has a broader definition of
"branches" than I expected. This updates the documentation to include
this definition.

I also updated the example to include several different types of
branches, while still maintaining dictionary lookup as an alternative
solution. (Crafting a realistic example was quite a challenge 😅).

Closes #11408.
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 good first issue Good for newcomers question Asking for support or clarification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants