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

SIM108 for return statements #13898

Open
joppi588 opened this issue Oct 23, 2024 · 3 comments
Open

SIM108 for return statements #13898

joppi588 opened this issue Oct 23, 2024 · 3 comments
Labels
question Asking for support or clarification

Comments

@joppi588
Copy link

joppi588 commented Oct 23, 2024

For the following code snippet ruff detects a violation of SIM505:

def foo_sim108(bar):
    if bar is not None:
        xyz = bar
    else:
        xyz = "bar"
    return xyz

and proposes

def foo_sim108(bar):
    xyz=bar if bar is not None else "bar"
    return xyz

Usually I would omit xyz and write

def foo_ret505(bar):
    if bar is not None:
        return bar
    else:
        return "bar"

which is simplified by RET505 to

def foo(bar):
    if bar is not None:
        return bar
    return "bar"

However the shortest version would be

def simplified_foo(bar):
    return bar if bar is not None else "bar"

For me it is a special case for SIM108 (return means that a value is assigned to the function output), but ruff does not propose this solution.
Is this intended?
If yes, I would be happy to have this behavior described in the SIM108 documentation.
If no, I would love to have it implemented.

Keywords: SIM108, RET505
Ruff version: 0.7.0

@joppi588 joppi588 changed the title SIM108 SIM108 for return statements Oct 23, 2024
@MichaReiser
Copy link
Member

Hi @joppi588

That makes sense to me.

Yes, this is intentional because there's RET504 that detects the unnecessary assignment to xyz before returning (playground). Keeping the rules separate is that users are in control on whether they prefer the assignment before the return or not.

Running ruff check --fix should directly fix

def foo_sim108(bar):
    if bar is not None:
        xyz = bar
    else:
        xyz = "bar"
    return xyz

to

def simplified_foo(bar):
    return bar if bar is not None else "bar"

if you have both rules enabled.

@MichaReiser MichaReiser added the question Asking for support or clarification label Oct 24, 2024
@joppi588
Copy link
Author

joppi588 commented Oct 24, 2024

Thanks @MichaReiser , I see.

However simplifying

def foo(bar):
    if bar is not None:
        return bar
    return "bar"

to

def simplified_foo(bar):
    return bar if bar is not None else "bar"

would require a new rule, right?

@MichaReiser
Copy link
Member

would require a new rule, right?

Yes, but it would be a very opinionated rule because it could result in very long expressions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Asking for support or clarification
Projects
None yet
Development

No branches or pull requests

2 participants