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

Rule TRY302 raised while the re-raise changes the error #4548

Closed
153957 opened this issue May 20, 2023 · 5 comments · Fixed by #4559
Closed

Rule TRY302 raised while the re-raise changes the error #4548

153957 opened this issue May 20, 2023 · 5 comments · Fixed by #4559
Labels
bug Something isn't working

Comments

@153957
Copy link
Contributor

153957 commented May 20, 2023

With code like the example below I got this unexpected error:

TRY302 Remove exception handler; error is immediately re-raised

Although the error is immediately re-raised, it is modified, the causes are removed.

Code to reproduce:

try:
    self.assertEqual(cells, interlinked)
except AssertionError as error:
    # Catch and raise again without causes
    raise error from None

Enabled ALL settings.
Version: ruff 0.0.269

@JonathanPlasse
Copy link
Contributor

Good catch!

@charliermarsh charliermarsh added the bug Something isn't working label May 20, 2023
@JonathanPlasse
Copy link
Contributor

Would you like to work on it?

@153957
Copy link
Contributor Author

153957 commented May 21, 2023

I'd like to, but have no experience writing Rust, so probably can't without some pointers.

I assume fixing this requires changes to the logic here: https://github.com/charliermarsh/ruff/blob/fc63c6f2e260760963c5fdea2d9d4e0cf38ec58a/crates/ruff/src/rules/tryceratops/rules/useless_try_except.rs#L52-L55
Somehow checking it there is a from after the raise statement, i.e. following the expression for which the name is checked, if that is the case just return None;.

And add a new test case(s) here: https://github.com/charliermarsh/ruff/blob/fc63c6f2e260760963c5fdea2d9d4e0cf38ec58a/crates/ruff/resources/test/fixtures/tryceratops/TRY302.py#L4

@JonathanPlasse
Copy link
Contributor

JonathanPlasse commented May 21, 2023

You can look up cause in ast::StmtRaise.
It should be only changing the pattern matching.

@153957
Copy link
Contributor Author

153957 commented May 21, 2023

Thank you for the hint, PR is up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants