pyre-check icon indicating copy to clipboard operation
pyre-check copied to clipboard

Invalid Undefined or invalid type [11] warning: Annotation is not defined as a type

Open jirkadanek opened this issue 5 years ago • 6 comments

pyre-check==0.0.41
mypy==0.761

I have a project (attached, in mypy_broken subdirectory) which can be checked by pyre, but not by mypy, due to bug https://github.com/python/mypy/issues/7203

In the project, there are modules named the same as the class they contain.

mypy_broken/dtests (git)-[master] % mypy .                                                                                                                                                                                                     dtestlib/clients/dtests/DebugClient.py:8: error: Module "dtestlib.clients.AbstractClient" is not valid as a type
dtestlib/clients/dtests/DebugClient.py:8: error: Invalid base class "AbstractClient"
Found 2 errors in 1 file (checked 7 source files)

I wanted to fix the project, so I replaced the imports in the manner of https://github.com/python/mypy/issues/7203#issuecomment-555504090

% diff -r mypy_broken/ pyre_broken/
diff -r mypy_broken/dtests/dtestlib/clients/dtests/DebugClient.py pyre_broken/dtests/dtestlib/clients/dtests/DebugClient.py
5c5
< from dtestlib.clients import AbstractClient
---
> from dtestlib.clients.AbstractClient import AbstractClient

Now I have project (attached in pyre_broken subdirectory) that passes through mypy, but pyre-check barfs at what I have

pyre_broken/dtests (git)-[master] % pyre --source-directory . --preserve-pythonpath check
Setting up a `.pyre_configuration` with `pyre init` may reduce overhead.
 ƛ Could not parse file at 45:37-45:37
      def addAsyncCleanup(self, func, /, *args, **kwargs):
                                      ^
 ƛ Could not parse file at 302:32-302:32
      def call(self, methodname, /, *args, **kwargs):
                                 ^
 ƛ Could not parse file at 824:32-824:32
      def update(self, other=(), /, **kwds):
                                 ^
 ƛ Could not parse file at 606:24-606:24
      def __call__(self, /, *args, **kwargs):
                         ^
 ƛ Could not parse file at 35:47-35:47
      def __new__(mcls, name, bases, namespace, /, **kwargs):
                                                ^
 ƛ Could not parse 5 files due to syntax errors!
 ƛ Found 1 type error!
dtestlib/clients/dtests/DebugClient.py:8:18 Undefined or invalid type [11]: Annotation `AbstractClient.AbstractClient` is not defined as a type.

When I use pyre-check==0.0.22, the fixed (for mypy) project passes. I tried to bisect and the flip from pass to fail happens between pyre 0.0.30 and 0.0.31.

Any ideas what I should do to have a project that passes with latest versions of either checker?

jirkadanek avatar Jan 28 '20 13:01 jirkadanek

Both projects, in directories mypy_broken and pyre_broken

pyre-check_232.zip

jirkadanek avatar Jan 28 '20 13:01 jirkadanek

Thanks for the report! I can confirm that this is indeed a Pyre bug. However, it might take quite a while before we can give you a fix, and in the meanwhile I don't have anything good suggestions for you at the moment that could help with mypy vs. pyre compatibility 😞

grievejia avatar Jan 29 '20 23:01 grievejia

Thanks for the triage.

I determined that files under dtestlib.clients are not changing much, they haven't been touched at all in last 3 months, so I'll just set pyre to ignore errors in them.

My .pyre_configuration

{
  "exclude": [
    ".*node_data.*"
  ],
  "ignore_all_errors": [
    "dtestlib/clients/"
  ]
}

My CI command

pyre --preserve-pythonpath --source-directory . --local-configuration .pyre_configuration check

jirkadanek avatar Jan 30 '20 08:01 jirkadanek

@grievejia, I tried pyre in a codebase as well and got quite a few of errors like the above.

If I understand correctly, the crux of the issue is here:

https://github.com/facebook/pyre-check/blob/b9f6f163c197aadf86437039d4d2e5554e347649/source/analysis/type.ml#L4293-L4297

In the OP example, we have:

# $ cat some_directory/ __init__.py

# We are only meant to import the class A here, rather than shadowing the whole of the module A
from .A import A
# $ cat some_directory/A.py

# This is a module that contains a function by the same name among other things
# Everything here gets shadowed by the import of the class in __init__.py

class A:
    pass

class B:
    pass

The import in the __init__.py file creates a TypeAlias.t from some_directory.A to some_directory.A.A

Given the fact that the pyre code I pasted above is eagerly resolving aliases, the example below fails:

# cat some_directory/example.py

# Will fail because Pyre tries to resolve class A to some_directory.A.A.A
from .A import A

# Will fail because Pyre tries to resolve class A to some_directory.A.A.B
from .A import B

# This actually works!
from . import A

I'm not sure what the best way to fix this would be. I'm thinking of recursively checking whether a type exists before creating the Type.type_t and if not have some sort of continuation stream of other potential fully-qualified names we could check. This may be duplicating effort in the type-check module though.

Long terms, we could possibly have a trie-like structure to represent the aliases. This could help encoding partial shadowing of the module A in the example above.

Any ideas?

vthemelis avatar Aug 08 '23 12:08 vthemelis

Btw, I'm happy to help with this but given that (afaict) it will require some refactoring first I would appreciate some guidance from the core team.

vthemelis avatar Aug 11 '23 14:08 vthemelis

@stroxler, is this likely to be prioritised? It seems to be fairly common in some of our code bases?

vthemelis avatar Sep 03 '23 12:09 vthemelis