Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Incorrect type reported by pyreverse for unions and generics #8888

Closed
loicleyendecker opened this issue Jul 26, 2023 · 1 comment · Fixed by #9012 or #9093
Closed

Incorrect type reported by pyreverse for unions and generics #8888

loicleyendecker opened this issue Jul 26, 2023 · 1 comment · Fixed by #9012 or #9093
Labels
Bug 🪲 pyreverse Related to pyreverse component
Milestone

Comments

@loicleyendecker
Copy link

Bug description

Consider the following class:

class Foo:
    def __init__(self) -> None:
        self.val: str | int = "1"

    def bar(self) -> None:
        self.val = "2"
        pass

pyreverse will infer the type of Foo.val to be "str | int, str", which is incorrect as the second assignment does not actually modify the type and "str" is a more specific type than the union "str | int".

Additionally, if the class is modified to look like this:

class Foo:
    def __init__(self) -> None:
        self.val: list[str] = []

    def bar(self) -> None:
        self.val = []
        pass

pyrverse will infer the type to be "list[str] | list", which should also be incorrect because assigning an empty list here doesn't mean that suddenly the type is list[Any].

I think fixing this issue should look at 3 different steps:

  • type hints should have precedence over inferred types: it should not be the responsibility of pyreverse to type check the code, so if type hints are provided, it should be assumed those are correct. This also allows users to get a consistent behaviour by just fixing the type hints in their code
  • if a type hint is read from the init, it should have precedence over other calls, since again, type checkers will consider other calls to be an error
  • failing the two points above, types should be aggregated and consistent: "str | int, str" should be just "str | int" and the order should be consistent across runs (in my testing, I sometimes got "str | int, str", and other times "str, str | int".

Configuration

No response

Command used

pyreverse repro.py

Pylint output

parsing repro.py


### Expected behavior

As explained in the description, the type inferred for the `val` attribute should be `str | int` in the first case and `list[str]` in the second case.

### Pylint version

```shell
pyreverse is included in pylint:
pylint 2.17.4
astroid 2.15.6
Python 3.11.4 (main, Jun  7 2023, 12:45:49) [GCC 9.4.0]

OS / Environment

No response

Additional dependencies

No response

@loicleyendecker loicleyendecker added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jul 26, 2023
@Pierre-Sassoulas Pierre-Sassoulas added the pyreverse Related to pyreverse component label Jul 26, 2023
@nickdrozd
Copy link
Contributor

There are a variety of open Pyreverse issues dealing with duplicate attributes. For example, #8189. Probably they can all be fixed in one fell swoop.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Bug 🪲 pyreverse Related to pyreverse component
Projects
None yet
3 participants