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

[flake8-return] Consider exception suppress for unnecessary assignment #9673

Conversation

mikeleppane
Copy link
Contributor

Summary

This review contains a fix for RET504 (unnecessary-assign)

The problem is that Ruff suggests combining a return statement inside contextlib.suppress. Even though it is an unsafe fix it might lead to an invalid code that is not equivalent to the original one.

See: #5909

Test Plan

cargo test

Copy link
Contributor

github-actions bot commented Jan 29, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Copy link
Member

@charliermarsh charliermarsh 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 moved this logic into the visitor, since as-is, I think this would've missed with statements that weren't at the end of the function.

@charliermarsh charliermarsh merged commit ad2cfa3 into astral-sh:main Jan 29, 2024
17 checks passed
zanieb pushed a commit that referenced this pull request Jan 29, 2024
#9673)

## Summary

This review contains a fix for
[RET504](https://docs.astral.sh/ruff/rules/unnecessary-assign/)
(unnecessary-assign)

The problem is that Ruff suggests combining a return statement inside
contextlib.suppress. Even though it is an unsafe fix it might lead to an
invalid code that is not equivalent to the original one.

See: #5909

## Test Plan

```bash
cargo test
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants