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 with pyright 1.1.366 #8087

Closed
wch opened this issue Jun 6, 2024 · 7 comments
Closed

False positive for shutil.rmtree for python 3.12 with pyright 1.1.366 #8087

wch opened this issue Jun 6, 2024 · 7 comments
Labels
as designed Not a bug, working as intended bug Something isn't working

Comments

@wch
Copy link

wch commented Jun 6, 2024

Consider the following file:

import shutil

shutil.rmtree("abcd")

If I run pyright 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 

I believe this is due in part to a recent change to the type stub, but it might also be due to how pyright matches overloads. The message seems to be intended for calls that provide a value for the onerror parameter, but in the actual code, that isn't happening.

@wch wch added the bug Something isn't working label Jun 6, 2024
@schloerke
Copy link

I don't believe this is an error as it's prepping for a python 3.14 update where the function will be removed.

We should change our code or ignore the error and deal with it in a year when 3.14 comes out

@wch
Copy link
Author

wch commented Jun 6, 2024

I don't think the function is going to be removed. I believe it's that the onerror parameter is deprecated and will be removed.

@schloerke
Copy link

👍 Yes, agreed. rmtree isn't going away. Just onerror.

@erictraut
Copy link
Collaborator

Pyright is working correctly here, so I don't consider this a bug in pyright.

If you think that the shutil.pyi type stub is incorrect, please report the issue to the maintainers of typeshed.

@erictraut erictraut closed this as not planned Won't fix, can't repro, duplicate, stale Jun 6, 2024
@erictraut erictraut added the as designed Not a bug, working as intended label Jun 6, 2024
@wch
Copy link
Author

wch commented Jun 6, 2024

Thanks, I've filed it at python/typeshed#12103.

Note that I wasn't able to reproduce the issue with mypy, so it seems that pyright and mypy are handling overloads differently.

@erictraut
Copy link
Collaborator

Does mypy have support for PEP 702 (@deprecated)? I don't think it does, but I could be wrong. I'm not able to get it to produce any diagnostics related to the use of a @deprecated function even if I enable "--strict" mode. That could explain why mypy doesn't emit any error or warning in this case.

I'll note that pyright doesn't emit any error or warning by default when using a @deprecated function or overload. I presume that you've enabled the reportDeprecated check (either explicitly or implicitly through the use of strict mode). If you'd prefer not to see these errors, you can leave this check disabled.

Overload resolution behavior is not currently specified in the typing spec. But even if/when it is, there's still an ambiguity related to @deprecated. If multiple overloads are used to resolve a call and a subset of those overloads are marked as @deprecated, should the call be considered deprecated? Or should it be considered deprecated only if all applicable overloads are marked @deprecated? By my read of PEP 702 and the typing spec, I think the former is intended. Here's the wording:

Type checkers should produce a diagnostic whenever they encounter a usage of an object marked as deprecated. For deprecated overloads, this includes all calls that resolve to the deprecated overload.

But this wording doesn't seem to acknowledge that multiple overloads are sometimes used to resolve a single call.

@wch
Copy link
Author

wch commented Jun 8, 2024

Just following up:

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

No branches or pull requests

3 participants