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

SIM103 return condition directly incorrect error message #10843

Closed
IronCore864 opened this issue Apr 9, 2024 · 3 comments · Fixed by #10854
Closed

SIM103 return condition directly incorrect error message #10843

IronCore864 opened this issue Apr 9, 2024 · 3 comments · Fixed by #10854
Assignees
Labels
question Asking for support or clarification

Comments

@IronCore864
Copy link

I have searched SIM103 in the open issues and didn't find anything related, so creating this one.

It seems the error message/prompt for SIM103 can be incorrect in some cases.

My code base is quite large, but if I copy the single function out for testing, I could not reproduce. It only happens on the large code base:

lint: commands[0]> ruff check --preview
ops/testing.py:3469:9: SIM103 Return the condition `keys is not None and notice.key not in keys` directly
     |
3467 |           if types is not None and notice.type not in types:
3468 |               return False
3469 |           if keys is not None and notice.key not in keys:
     |  _________^
3470 | |             return False
3471 | |         return True
     | |___________________^ SIM103
     |
     = help: Inline condition

Found 1 error.
No fixes available (1 hidden fix can be enabled with the `--unsafe-fixes` option).

I think the prompt/error message is not correct. How could:

if condition:
    return False

return True

be replaced by return condition? The message should be something like "return not condition instead", not "return condition instead". If the user copy/paste the suggestion it'd be wrong.

Only

if condition:
    return True

return False

can be replaced directly by return condition instead.

With latest ruff version 0.3.5.

@IronCore864 IronCore864 changed the title SIM103 return condition directly bug SIM103 return condition directly incorrect error message Apr 9, 2024
@charliermarsh
Copy link
Member

In general, you can replace:

if condition:
    return False
return True

With:

return not condition

So in your case, it could be:

return not (keys is not None and notice.key not in keys)

Or, simplified:

return keys is None or notice.key in keys

@charliermarsh charliermarsh added the question Asking for support or clarification label Apr 9, 2024
@carljm
Copy link
Contributor

carljm commented Apr 9, 2024

I think the OP is pointing out that the diagnostic message sort of implies you can replace the code with return condition, whereas actually in this case you need return not condition, and the diagnostic doesn't clarify this distinction.

@charliermarsh
Copy link
Member

Makes sense -- I'll invert it.

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

Successfully merging a pull request may close this issue.

3 participants