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

Pylance ignore a final attribute overrides when base class is Exception #1495

Closed
pasunx opened this issue Jun 24, 2021 · 4 comments
Closed
Labels
fixed in next version (main) A fix has been implemented and will appear in an upcoming version type checking

Comments

@pasunx
Copy link

pasunx commented Jun 24, 2021

Environment data

  • Language Server version: 2021.6.3
  • OS and version: win32 x64
  • Python version (and distribution if applicable, e.g. Anaconda):
  • python.analysis.indexing: undefined
  • python.analysis.typeCheckingMode: strict

Code Snippet / Additional information

from typing import final

class Foo:

    @final
    def __init__(self) -> None:
        print("Foo")

class Bar(Exception): ...

class Baz(Bar, Foo):

    def __init__(self) -> None:
        print("BAZ")

print(Baz())
$ mypy final.py
final.py:13: error: Cannot override final attribute "__init__" (previously declared in base class "Foo")
Found 1 error in 1 file (checked 1 source file)
@erictraut
Copy link
Contributor

The current behavior is by design. We could consider changing the design, but it is working as I would expect currently.

When pylance performs its @Final override checks for methods, it takes into account the method resolution ordering (MRO) rules used in Python. MRO rules become especially important when multiple inheritance is used, like in your example above.

In your example, the __init__ method in Baz is not overriding the __init__ method in Foo. Instead, it is overriding the __init__ method provided by Exception. You can demonstrate this by calling super().__init__() from within Baz.__init__.

class Baz(Bar, Foo):
    def __init__(self) -> None:
        super().__init__()
        print("BAZ")

The output of this program is simply BAZ rather than Foo \ BAZ. That's because Foo.__init__ is not accessible via MRO rules.

If you swap the order of the parameters within the Baz class declaration, pylance will emit the error you're looking for because Baz.__init__ is now attempting to override Foo.__init__, which is marked @final.

class Baz(Foo, Bar): # Foo is first, then Bar
    def __init__(self) -> None:
        super().__init__()
        print("BAZ")

If you run this program, you'll now see the output Foo \ BAZ which demonstrates that Foo.__init__ is being overridden by Baz.__init__.

PEP 591 (which defines the behavior of @final) doesn't clarify what is meant by "override", so there's some ambiguity here. We'll need to discuss this further and decide whether a change in the design is in order.

@Azureblade3808
Copy link

Maybe Pylance could forbid class Baz(Foo, Bar) with a reason that Foo.__init__ overrides Bar.__init__.

Assuming there is class Baz(Foo, Bar), even if there weren't a declared Baz.__init__, the actual Baz.__init__ would still be different from Bar.__init__, which would still violate the "final" restriction on Bar.__init__, while Baz is indeed a subclass of Bar.

So I think the main conflict is not Baz.__init__ overriding Bar.__init__, but Foo.__init__ overriding Bar.__init__, though this course is shadowed when there is a declared Baz.__init__.

@erictraut
Copy link
Contributor

I've updated the code to check all base classes rather than simply using the MRO rules. This change affects all method and attribute override incompatibility checks plus @Final and constant checks.

@erictraut erictraut added fixed in next version (main) A fix has been implemented and will appear in an upcoming version and removed needs decision Do we want this enhancement? labels Aug 9, 2021
@heejaechang heejaechang removed the fixed in next version (main) A fix has been implemented and will appear in an upcoming version label Aug 11, 2021
@heejaechang
Copy link
Contributor

fixed in 2021.8.1 release.

@jakebailey jakebailey added the fixed in next version (main) A fix has been implemented and will appear in an upcoming version label Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed in next version (main) A fix has been implemented and will appear in an upcoming version type checking
Projects
None yet
Development

No branches or pull requests

6 participants