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

"RUF047" (needless-else) conflicts with pylint R5601 (confusing-consecutive-elif), Can't suppress the rule inline. #15781

Open
IgnatKudriavtsev opened this issue Jan 28, 2025 · 2 comments
Labels
bug Something isn't working suppression Related to supression of violations e.g. noqa

Comments

@IgnatKudriavtsev
Copy link

IgnatKudriavtsev commented Jan 28, 2025

Description

ruff 0.9.3

Ruff preview rule RUF047 (needless-else) complains on the code that has an intentionally added empty else branch as one of recommended by pylint ways to fix the rule confusing-consecutive-elif / R5601:

Description:

Used when an elif statement follows right after an indented block which itself ends with if or elif. It may not be obvious if the elif statement was willingly or mistakenly unindented. Extracting the indented if statement into a separate function might avoid confusion and prevent errors.

Problematic code:

def myfunc(shall_continue: bool, shall_exit: bool):
    if shall_continue:
        if input("Are you sure?") == "y":
            print("Moving on.")
    elif shall_exit:  # [confusing-consecutive-elif]
        print("Exiting.")

Correct code:

# Option 1: add explicit 'else'
def myfunc(shall_continue: bool, shall_exit: bool):
    if shall_continue:
        if input("Are you sure?") == "y":
            print("Moving on.")
        else:
            pass
    elif shall_exit:
        print("Exiting.")

...

Currently we need to either suppress ruff or pylint rules for such cases.

Actually, I'm not sure what is the bigger problem here: the fact the ruff complains on the problem or the fact that I can't suppress it locally.

Local suppression of the ruff locally in this case doesn't work because of RUF100 [*] Unused 'noqa' directive (unused: 'RUF047')

@AlexWaygood
Copy link
Member

Hmm, so there are two issues here.

The first issue is about RUF047 conflicting with R5601. This isn't about two Ruff rules conflicting. This is about R5601 in the original pylint linter making a conflicting recommendation with one of Ruff's rules.

Ruff doesn't currently have a reimplementation of R5601 in its pylint category. The rule was considered in #10103, but was rejected for now. In general, we try to guarantee that no two Ruff rules will offer conflicting recommendations, but I don't think we make any guarantees about being able to run Ruff alongside other linters.

The second issue is about not being able to suppress RUF047. I can reproduce that in the Ruff playground. That looks like a bug to me!

@AlexWaygood AlexWaygood added bug Something isn't working suppression Related to supression of violations e.g. noqa labels Jan 28, 2025
@InSyncWithFoo
Copy link
Contributor

InSyncWithFoo commented Jan 28, 2025

This is an edge case, as else branches with any kinds of comments, pragma or not, are never reported. Thus, I think it is more preferable to either suppress R5601 or add a (normal) comment explaining why that empty branch is necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working suppression Related to supression of violations e.g. noqa
Projects
None yet
Development

No branches or pull requests

3 participants