Skip to content

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

@github-actions
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
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