facebook / pyre-check

Performant type-checking for python.
https://pyre-check.org/
MIT License
6.79k stars 432 forks source link

Pyre not always using the correct type stubs #447

Open abadger opened 3 years ago

abadger commented 3 years ago

Pyre Feature Request

Is your feature request related to a problem? Please describe. I've started using argparse.BooleanOptionalActions which is present in Python3.9+. When my code runs on older versions of Python, I use a compatibility shim that vendors the feature. However, pyre (run on Python-3.8) errors out because python-3.8 does not have BooleanOptionalAction and it doesn't realize that code is unreachable on python-3.8:

In compat.py:

if sys.version_info < (3, 9):
    #: In Python3.9+, argparse.BooleanOptionalAction gives us a simple way to add
    #: --feature/--no-feature command line switches.  In earluer versions, we use
    #: code that we've copied from the upstream code.
    from .vendored._argparse_booleanoptionalaction import BooleanOptionalAction
elif sys.version_info >= (3, 9):
    from argparse import BooleanOptionalAction

In cli/antsibull_docs.py:

from ..compat import BooleanOptionalAction
[...]
    whole_site_parser.add_argument('--indexes',
                                   dest='indexes', action=BooleanOptionalAction,
                                   default=argparse.SUPPRESS,
                                   help='Test')

Pyre output:

antsibull/cli/antsibull_docs.py:175:58 Undefined attribute [16]: Module argparse has no attribute BooleanOptionalAction.

Describe the solution you'd like mypy has a feature which seems to look for sys.version_info in a conditional and will only check the branch which is relevant to the python version it is running on. That way it doesn't get stuck trying to type check what will be unreachable code on that python version:

https://mypy.readthedocs.io/en/stable/common_issues.html#python-version-and-system-platform-checks

I think that solution would work well.

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Additional context Add any other context or screenshots about the feature request here.

kbansal commented 3 years ago

Pyre does take into account sys.version_info -- were you able to resolve this issue (I see you have a merged pull request now)? If not, can you provide a small python file reproducing the issue together with the .pyre_configuration -- that would by very helpful in debugging the issue. Thanks!

abadger commented 3 years ago

I was able to workaround the issue but I wasn't able to resolve it. It looks like the problem is that in some cases, pyre isn't using the proper type stub for argparse. (my workaround was to copy a recent version of argparse.pyi to my local stubs/ directory).

I was able to create a simple reproducer:

# Install an old version of pyre with old type stubs into the user's home dir
python3.8 -m pip install --user pyre-check==0.0.46
mkdir pyre-test 
cd pyre-test
git clone http://github.com/abadger/test-repo test
python3.8 -m venv venv 
. venv/bin/activate
which python
# Install a new version of pyre into the venv
python -m pip install pyre-check
cd test
which pyre
pyre --source-directory lib

This results in:

[pts/18@camelback /var/tmp/pyre-test/test]$ pyre --source-directory lib             [main]  (16:15:25)
No watchman binary found. 
To enable pyre incremental, you can install watchman: https://facebook.github.io/watchman/docs/install.html
Defaulting to non-incremental check.
Setting up a `.pyre_configuration` with `pyre init` may reduce overhead.
 ƛ Found 1 type error!
lib/test.py:6:4 Undefined import [21]: Could not find a name `BooleanOptionalAction` defined in module `argparse`.

Note that this is probably not the exact same case as when I am running pyre in my CI. In that case, pyre is probably not installed outside of the virtualenv but it's still not finding its own argparse.pyi file. Maybe because in that case I passed in several search paths?

This is how that one runs:

poetry run pyre --source-directory antsibull --source-directory sphinx_antsibull_ext --search-path $(poetry run python -c 'from distutils.sysconfig import get_python_lib;print(get_python_lib())') --search-path stubs/ --search-path . "$@"
kbansal commented 3 years ago

Thanks for taking the time to create a minimized repro.

It looks like the problem is that in some cases, pyre isn't using the proper type stub for argparse

Ah, I see. @shannonzhu: you were looking into some type stub issue -- is this related / had any recommendation here?