facebook / pyre-check

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

Question: Undefined import [21] on typed dependency #167

Open adrianbn opened 5 years ago

adrianbn commented 5 years ago

Hi,

I'm using Pipenv and pyre 0.0.26 on OSX. I have two in-house libraries, A and B, both with type hints. B imports A and makes use of some of its classes like this:

from A import Api
from A.errors import AError

Those imports generate a pyre error:

B/__main__.py:5:0 Undefined import [21]: Could not find a module corresponding to import `A`.
B/__main__.py:6:0 Undefined import [21]: Could not find a module corresponding to import `A.errors`.

I'm sure there's something I'm not understanding correctly about Pyre, since I expected it to be able to go over B's code and find the type hints. I saw issue #64, but that is about an untyped 3rd party library whereas I control the dependency and is typed.

Additionally, if I tried to run pyre with --preserve-pythonpath, I run into a bunch of parsing errors from third party dependencies:

ƛ Could not parse file at pipenv/patched/piptools/sync.py:99:60-99:60
      requirements_lut = {r.link or key_from_req(r.req): r for r in compiled_requirements}
                                                             ^
 ƛ Could not parse file at test/test_unicode_identifiers.py:7:12-7:12
              ä = 1
             ^
 ƛ Could not parse file at test/badsyntax_3131.py:2:0-2:0
  € = 2
  ^
 ƛ Could not parse file at test/test_grammar.py:654:28-654:28
          l6 = lambda x, y, *, k=20: x+y+k
                             ^
 ƛ Could not parse file at test/test_print.py:206:20-206:20
              print << sys.stderr
                     ^
 ƛ Could not parse file at test/test_xmlrpc.py:814:32-814:32
              self.assertEqual(p.têšt(42), 42)
                                 ^
 ƛ Could not parse file at test/test_keywordonlyarg.py:156:17-156:17
          lambda *, k1=unittest: None
                  ^
 ƛ Could not parse file at lib2to3/tests/data/py2_test_grammar.py:61:14-61:14
          x = 0l
               ^
 ƛ Could not parse file at lib2to3/tests/data/py3_test_grammar.py:371:28-371:28
          l6 = lambda x, y, *, k=20: x+y+k
                             ^
 ƛ Could not parse 9 external files due to syntax errors!
 ƛ Client exited with error code 1

Do I need some special annotation on A's __init__.py? Something on the constructors? Are stubs the only way forward even though A is typed?

Thanks for the help and sorry if this is not the right place to ask these questions.

adrianbn commented 5 years ago

I forgot to mention that I've noticed that pyre --search-path "$(pipenv --venv)/lib/python3.7/site-packages/" does not find those problems, but this creates two other problems for us:

  1. It requires a python version in the call to pyre, which makes it hard to run on multiple people's laptops and build environments.
  2. It comes up with some other errors in one of our dependencies (GitPython):
B/utils.py:32:8 Undefined attribute [16]: Module `git` has no attribute `Repo`.
B/utils.py:33:15 Undefined attribute [16]: Module `git` has no attribute `Repo`.

The code on those lines looks like this:

Repo.clone_from(url, repo_dir, branch=branch, depth=depth)
repo = Repo(repo_dir)

It is imported like this:

from git import Repo
grievejia commented 5 years ago
adrianbn commented 5 years ago

Hi @grievejia,

thank you for the support. How do you suggest I specify the source_directory or search_paths in the configuration so that everyone in the team and the CI machines can find the right path? Right now the dependencies (even the internal ones we control) are in the virtual environment. I can programmatically get the path to it, but I don't think .pyre_configuration supports scripting inside?

Regarding the parsing errors, they do stop pyre from running, but I don't know why. See the debug output when using --preserve-pythonpath:

2019-07-21 18:26:55,143 INFO Parsing 5993 stubs and sources...
2019-07-21 18:26:55,362 ERROR Could not parse file at test/badsyntax_3131.py:2:0-2:0
  € = 2
  ^
2019-07-21 18:26:55,759 ERROR Could not parse file at test/test_grammar.py:654:28-654:28
          l6 = lambda x, y, *, k=20: x+y+k
                             ^
2019-07-21 18:26:56,370 ERROR Could not parse file at test/test_unicode_identifiers.py:7:12-7:12
              ä = 1
             ^
2019-07-21 18:26:56,488 ERROR Could not parse file at test/test_xmlrpc.py:814:32-814:32
              self.assertEqual(p.têšt(42), 42)
                                 ^
2019-07-21 18:26:57,089 ERROR Could not parse file at test/test_print.py:206:20-206:20
              print << sys.stderr
                     ^
2019-07-21 18:26:57,882 ERROR Could not parse file at lib2to3/tests/data/py2_test_grammar.py:61:14-61:14
          x = 0l
               ^
2019-07-21 18:26:58,181 ERROR Could not parse file at test/test_keywordonlyarg.py:156:17-156:17
          lambda *, k1=unittest: None
                  ^
2019-07-21 18:26:58,832 ERROR Could not parse file at site-packages/flake8/utils.py:343:54-343:54
  def _default_predicate(*args):  # type: (*str) -> bool
                                                       ^
2019-07-21 18:26:59,117 ERROR Could not parse file at lib2to3/tests/data/py3_test_grammar.py:371:28-371:28
          l6 = lambda x, y, *, k=20: x+y+k
                             ^
2019-07-21 18:27:00,092 WARNING Could not parse 9 files due to syntax errors!
2019-07-21 18:27:00,193 PERFORMANCE Sources parsed: 5.053000s
2019-07-21 18:27:06,858 PERFORMANCE Registered ignores: 6.763000s
2019-07-21 18:27:06,859 INFO Adding environment information to shared memory...
2019-07-21 18:27:06,859 PERFORMANCE Added environment to shared memory: 0.000000s
2019-07-21 18:27:06,960 INFO Building type environment...
2019-07-21 18:27:27,904 ERROR Client exited with error code 1

Nothing else happens after that, ant the $? error code is 2.

grievejia commented 5 years ago

Thanks for the feedback! You are right that we don't have a good story to tell when it comes to specifying those configurations in a platform-specific way, which is not good. But I could definitely make it better. Is it reasonable to say that the search paths you want to programmatically get are all related to the site package directory of the virtual environment? If this is the case, I could, for example, allow some special placeholders like ${SITE_PACKAGE_DIRECTORY} to appear in paths strings to improve the portability?

As to --preserve-pythonpath, if you look at the log closely you can see that parsing errors didn't stop pyre from running. The error came out when we were building the environment. After some debugging I found the issue is that --preserve-pythonpath tried to include the entire Python standard library as well as all the corresponding unit tests, and some of those source files currently crash pyre. On the one hand, we definitely need to do some work to stop pyre from crashing on those files. On the other hand, I also think it's of questionable value to include the standard library source files since typeshed is more lightweight and is good enough for a large number of use cases. Furthermore, for virtual environment integration the --preserve-pythonpath option would also be of questionable value if we can do ${SITE_PACKAGE_DIRECTORY} in search path. So I'm more inclined to just address the portability issue with configurations rather than --preserve-pythonpath.

Please let me know if you have more thoughts on this issue.

adrianbn commented 5 years ago

Hi @grievejia,

you are correct. The paths are all related to the virtual environment site-packages directory. Having a way to interpolate that in the config file will help finding the code for the dependencies that are typed.

Thanks!

grievejia commented 5 years ago

Hi @adrianbn ,

Here's the approach I ended up arriving at: In addition to absolute path, the search_path configuration now accepts { "site-package": "XXX" } as it element in the latest release. It pretty much does what you'd expect: adding venv/lib/python3.7/site-packages/XXX to the pyre search path, without all the jankiness of the ${SITE_PACKAGE_DIRECTORY} approach mentioned earlier.

Let me know if it works for you.

adrianbn commented 5 years ago

Hi @grievejia,

was away for a while. I'll test it this week and let you know, but it should do what we need. Thanks!

hellasecure commented 5 years ago

Hi @grievejia,

I've been creating the updates to our project implementing the new "site-package" element to our .pyre_configuration. Happy to report this fixes our issue with our in-house library, however we are seeing an issue when attempting to run pyre-check within our pipenv. When interacting with the project within our pipenv shell, we are now seeing the following error:

ƛ module 'site' has no attribute 'getsitepackages'
ƛ Traceback (most recent call last):
File "B/lib/python3.6/site-packages/pyre_check/pyre.py", line 499, in main excludes=arguments.exclude,
File "B/lib/python3.6/site-packages/pyre_check/configuration.py", line 207, in __init__self._read(CONFIGURATION_FILE)
File "B/lib/python3.6/site-packages/pyre_check/configuration.py", line 444, in _read for path in additional_search_path
File "B/lib/python3.6/site-packages/pyre_check/configuration.py", line 445, in <listcomp> for expanded_path in expand_search_path(path)
File "B/lib/python3.6/site-packages/pyre_check/configuration.py", line 68, in expand_search_path site_root = site.getsitepackages()
AttributeError: module 'site' has no attribute 'getsitepackages'

I did some research, and appears to be an issue with virtualenv not having access to the function (https://github.com/pypa/virtualenv/issues/737). Any ideas on a workaround or solution is greatly appreciated, thanks!

hellasecure commented 5 years ago

In addition, we are seeing new errors when using site-package for certain libraries, for example, GitPython and jsonobject:

B/utils.py:29:12 Undefined attribute [16]: Module `git` has no attribute `Repo`.
B/utils.py:30:19 Undefined attribute [16]: Module `git` has no attribute `Repo`.
B/utils.py:36:19 Undefined attribute [16]: Module `git` has no attribute `Repo`.

And in cases where we import from jsonobject like:

from jsonobject.properties import IntegerProperty, StringProperty

We get the following:

ƛ Found 10 type errors!
B/request.py:2:0 Undefined import [21]: Could not find a module corresponding to import `jsonobject.properties`.
B/error.py:2:0 Undefined import [21]: Could not find a module corresponding to import `jsonobject.properties`.
B/set.py:2:0 Undefined import [21]: Could not find a module corresponding to import `jsonobject.properties`.
B/issue.py:2:0 Undefined import [21]: Could not find a module corresponding to import `jsonobject.properties`.
B/template.py:2:0 Undefined import [21]: Could not find a module corresponding to import `jsonobject.properties`.
B/group.py:2:0 Undefined import [21]: Could not find a module corresponding to import `jsonobject.properties`.
B/server.py:2:0 Undefined import [21]: Could not find a module corresponding to import `jsonobject.properties`.
B/scan.py:2:0 Undefined import [21]: Could not find a module corresponding to import `jsonobject.properties`.
B/file.py:2:0 Undefined import [21]: Could not find a module corresponding to import `jsonobject.properties`.
B/config.py:2:0 Undefined import [21]: Could not find a module corresponding to import `jsonobject.properties`.

It appears to be having issues finding some submodules.

adrianbn commented 4 years ago

Hey @grievejia,

is there any suggestion you have to work around this issue?

Thank you!

adrianbn commented 4 years ago

Hi @grievejia , @MaggieMoss ,

this issue has been around for a while but it is still a problem in our infrastructure. We use Pipenv and have multiple Python projects that we've typed, but pyre still fails to find those and their types, generating a lot of error [21].

The fix @grievejia proposed goes in the right direction, but when using it we come across the error @hellasecure described above.

This is our configuration:

{
  "source_directories": [
    "."
  ],
  "ignore_all_errors": [
    "setup.py",
    "test/",
    "build"
  ],
  "search_path": [
    {"site-package": "our-other-package"}
  ]
}

Running pyre with the above generates the following error:

Traceback (most recent call last):
  File "/Users/adrian/.local/share/virtualenvs/XXXXXXXXX-tMlZ554P/lib/python3.7/site-packages/pyre_check/client/pyre.py", line 92, in main
    command: CommandParser = arguments.command(arguments, original_directory)
  File "/Users/adrian/.local/share/virtualenvs/XXXXXXXXX-tMlZ554P/lib/python3.7/site-packages/pyre_check/client/commands/check.py", line 31, in __init__
    arguments, original_directory, configuration, analysis_directory
  File "/Users/adrian/.local/share/virtualenvs/XXXXXXXXX-tMlZ554P/lib/python3.7/site-packages/pyre_check/client/commands/reporting.py", line 36, in __init__
    arguments, original_directory, configuration, analysis_directory
  File "/Users/adrian/.local/share/virtualenvs/XXXXXXXXX-tMlZ554P/lib/python3.7/site-packages/pyre_check/client/commands/command.py", line 453, in __init__
    configuration or self.generate_configuration()
  File "/Users/adrian/.local/share/virtualenvs/XXXXXXXXX-tMlZ554P/lib/python3.7/site-packages/pyre_check/client/commands/command.py", line 487, in generate_configuration
    log_directory=self._log_directory,
  File "/Users/adrian/.local/share/virtualenvs/XXXXXXXXX-tMlZ554P/lib/python3.7/site-packages/pyre_check/client/configuration.py", line 239, in __init__
    self._read(CONFIGURATION_FILE)
  File "/Users/adrian/.local/share/virtualenvs/XXXXXXXXX-tMlZ554P/lib/python3.7/site-packages/pyre_check/client/configuration.py", line 514, in _read
    [expand_search_path(path) for path in additional_search_path]
  File "/Users/adrian/.local/share/virtualenvs/XXXXXXXXX-tMlZ554P/lib/python3.7/site-packages/pyre_check/client/configuration.py", line 514, in <listcomp>
    [expand_search_path(path) for path in additional_search_path]
  File "/Users/adrian/.local/share/virtualenvs/XXXXXXXXX-tMlZ554P/lib/python3.7/site-packages/pyre_check/client/configuration.py", line 73, in expand_search_path
    site_root = site.getsitepackages()
AttributeError: module 'site' has no attribute 'getsitepackages'

We'd really appreciate some advice with this. Thank you!