Skip to content

[pyupgrade] Add resource.error as deprecated alias of OSError (UP024) #17933

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
May 14, 2025
Merged

[pyupgrade] Add resource.error as deprecated alias of OSError (UP024) #17933

merged 3 commits into from
May 14, 2025

Conversation

DimitriPapadopoulos
Copy link
Contributor

@DimitriPapadopoulos DimitriPapadopoulos commented May 8, 2025 •
edited
Loading

Summary

Partially addresses #17935.

resource.error is a deprecated alias of OSError.

Changed in version 3.3: Following PEP 3151, this class was made an alias of OSError.

Add it to the list of OSError aliases found by os-error-alias (UP024).

Test Plan

Sorry, I usually don't program in Rust. Could you at least point me to the test I would need to modify?

@DimitriPapadopoulos DimitriPapadopoulos changed the title Add another alias of OSError: ressource.error alias Add another alias of OSError: ressource.error May 8, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 8, 2025 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@DimitriPapadopoulos DimitriPapadopoulos changed the title Add another alias of OSError: ressource.error Add deprecated alias of OSError: ressource.error May 8, 2025
@DimitriPapadopoulos DimitriPapadopoulos changed the title Add deprecated alias of OSError: ressource.error [pyupgrade] Add deprecated alias of OSError: ressource.error May 8, 2025
@MichaReiser MichaReiser requested a review from ntBre May 9, 2025 06:32
ntBre
ntBre reviewed May 9, 2025
View reviewed changes
Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, this makes sense to me! For a test, you'll want to add a new UP024 test fixture like this:

ruff/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP024_3.py

Lines 1 to 8 in ec2fdad

class SocketError(Exception):
pass
try:
raise SocketError()
except SocketError:
pass

It looks like UP024_5.py is the next available name (I linked to 3 since it's a bit more minimal of an example than 4).

And then you can add a new test_case here:

ruff/crates/ruff_linter/src/rules/pyupgrade/mod.rs

Lines 48 to 53 in ec2fdad

#[test_case(Rule::OSErrorAlias, Path::new("UP024_0.py"))]
#[test_case(Rule::OSErrorAlias, Path::new("UP024_1.py"))]
#[test_case(Rule::OSErrorAlias, Path::new("UP024_2.py"))]
#[test_case(Rule::OSErrorAlias, Path::new("UP024_3.py"))]
#[test_case(Rule::OSErrorAlias, Path::new("UP024_4.py"))]
#[test_case(Rule::OpenAlias, Path::new("UP020.py"))]

That should be all you need to generate new snapshots, which you can update with cargo-insta. The prerequisites section in the CONTRIBUTING.md file has some links for setting that up, if you need it. I'm happy to help if you run into any issues! I think this is good to go once we have a simple test case :)

@DimitriPapadopoulos
Copy link
Contributor Author

DimitriPapadopoulos commented May 10, 2025 •
edited
Loading

I chose to modify the existing test instead:

ruff/crates/ruff_linter/resources/test/fixtures/pyupgrade/UP024_2.py

Lines 8 to 16 in ec2fdad

# Testing the modules
import socket, mmap, select
raise socket.error
raise mmap.error
raise select.error
raise socket.error()
raise mmap.error(1)
raise select.error(1, 2)

Unfortunately, the whole toolchain eats too much disk space, I cannot run tests on my computer. I'll try on a machine with more disk space next week.

@MichaReiser MichaReiser added the rule Implementing or modifying a lint rule label May 12, 2025
@ntBre
Copy link
Contributor

ntBre commented May 12, 2025

Thanks for adding the tests. Let me know if you want me to update the snapshots for you.

@DimitriPapadopoulos
Copy link
Contributor Author

Snapshots updated.

ntBre
ntBre approved these changes May 14, 2025
View reviewed changes
Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ntBre ntBre changed the title [pyupgrade] Add deprecated alias of OSError: ressource.error [pyupgrade] Add resource.error as deprecated alias of OSError (UP024) May 14, 2025
@ntBre ntBre merged commit 1b4f7de into astral-sh:main May 14, 2025
34 checks passed
@DimitriPapadopoulos DimitriPapadopoulos deleted the patch-1 branch May 14, 2025 18:24
@BrewTestBot BrewTestBot mentioned this pull request May 15, 2025
Merged
Glyphack pushed a commit to Glyphack/ruff that referenced this pull request May 21, 2025
@DimitriPapadopoulos @Glyphack
[pyupgrade] Add resource.error as deprecated alias of OSError (…
fa4c66b 
…`UP024`) (astral-sh#17933)

## Summary

Partially addresses astral-sh#17935.


[`resource.error`](https://docs.python.org/3/library/resource.html#resource.error)
is a deprecated alias of
[`OSError`](https://docs.python.org/3/library/exceptions.html#OSError).
> _Changed in version 3.3:_ Following [**PEP
3151**](https://peps.python.org/pep-3151/), this class was made an alias
of
[`OSError`](https://docs.python.org/3/library/exceptions.html#OSError).

Add it to the list of `OSError` aliases found by [os-error-alias
(UP024)](https://docs.astral.sh/ruff/rules/os-error-alias/#os-error-alias-up024).

## Test Plan

Sorry, I usually don't program in Rust. Could you at least point me to
the test I would need to modify?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ntBre ntBre ntBre approved these changes

Assignees
No one assigned
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DimitriPapadopoulos @ntBre @MichaReiser