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

False positive for shutil.rmtree for python 3.12 #12103

Closed
wch opened this issue Jun 6, 2024 · 2 comments · Fixed by #12116
Closed

False positive for shutil.rmtree for python 3.12 #12103

wch opened this issue Jun 6, 2024 · 2 comments · Fixed by #12116
Labels
priority: regression Something that worked before doesn't work anymore stubs: false positive Type checkers report false errors

Comments

@wch
Copy link
Contributor

wch commented Jun 6, 2024

Consider the following file:

import shutil

shutil.rmtree("abcd")

If I run pyright 1.1.366 on it, it reports an error:

❯ pyright --version
pyright 1.1.366
❯ pyright test.py --pythonversion 3.12
/Users/winston/temp/test.py
  /Users/winston/temp/test.py:3:8 - error: The function "rmtree" is deprecated
    The `onerror` parameter is deprecated and will be removed in Python 3.14. Use `onexc` instead. (reportDeprecated)
1 error, 0 warnings, 0 informations 

However, I was not able to reproduce the issue using mypy (the custom typeshed dir was a git clone of this repository at main, b3fc7e2):

❯ mypy --python-version 3.12 --custom-typeshed-dir typeshed test.py
Success: no issues found in 1 source file

I originally filed this issue on the pyright repository (microsoft/pyright#8087), but was told that pyright is working correctly. If that's the case, that would mean it's an issue with shutil.pyi type stub.

@AlexWaygood
Copy link
Member

AlexWaygood commented Jun 6, 2024

I was not able to reproduce the issue using mypy

Pyright is producing this error because we decorate some overloads of the function with @deprecated. Mypy doesn't support @deprecated yet, so it's expected that it wouldn't produce the same error.

The onerror parameter for rmtree is indeed deprecated, but python/cpython#112659 removed the deprecation warning at runtime and postponed removal of the parameter indefinitely, on the grounds that the deprecation warnings were too noisy and achieved little (see python/cpython#112645). Given that it is still deprecated, though, maybe we should still continue to use the decorator in the stubs.

If we do keep it, though, we should apply it in a more intelligent way. Currently you don't even need to pass an argument to onerror for the overload we mark as deprecated to be chosen by the type checker:

@deprecated("The `onerror` parameter is deprecated and will be removed in Python 3.14. Use `onexc` instead.")

Thanks for the report! Agree there's a bug here.

@AlexWaygood
Copy link
Member

Looks like this regression was caused by the update in 50cbca4 removing the first overload from rmtree (cc. @Avasam)

@AlexWaygood AlexWaygood added the priority: regression Something that worked before doesn't work anymore label Jun 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: regression Something that worked before doesn't work anymore stubs: false positive Type checkers report false errors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants