python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.45k stars 2.82k forks source link

mypy fails to report missing "private" __x attribute #8267

Open dckc opened 4 years ago

dckc commented 4 years ago

Mypy doesn't seem to grok that self.__x in a subclass is different from self.__x in a superclass. It also doesn't seem to understand order of initialization.

  • Are you reporting a bug, or opening a feature request?

bug

  • Please insert below the code you are checking with mypy
class A:
    def __init__(self, x: int) -> None:
        self.__x = x  # replace this with pass and mypy reports the error

class B(A):
    def __init__(self) -> None:
        A.__init__(self, self.__x)

B()
  • What is the actual behavior/output?
$ mypy --strict tytest.py 
Success: no issues found in 1 source file
$ python tytest.py 
Traceback (most recent call last):
  File "tytest.py", line 11, in <module>
    B()
  File "tytest.py", line 8, in __init__
    A.__init__(self, self.__x)
AttributeError: 'B' object has no attribute '_B__x
  • What is the behavior/output you expect?

mypy should report:

tytest.py:8: error: "B" has no attribute "__x

  • What are the versions of mypy and Python you are using?

mypy 0.700

Do you see the same issue after installing mypy from Git master?

yes (mypy-0.770+dev.01ca4e062e38a9db8f462b9464fbbd5dd945a8cf)

  • What are the mypy flags you are using? (For example --strict-optional) --strict
JukkaL commented 4 years ago

Yeah, mypy should understand private attributes like __x. I'm surprised that I couldn't find an existing issue.

mthuurne commented 4 years ago

523 is related to this. It was closed by PR #6790, which adds code to avoid reporting false positives when the same private name is used in a subclass and superclass. However, it doesn't implement name mangling nor does it add any checks for private names being accessed from outside the class. As a result, false negatives can occur, as in the test case above.

Here is another false negative test case:

class C:
    __private = 'foo'
print(C.__private)

mypy will not report any errors, but the code breaks at runtime.

There are good arguments for not implementing name mangling:

On the other hand, mypy is a type checker and not a style checker, so I don't know whether unclean code that does execute correctly should be rejected. The name mangling is documented in the official language reference, so it's not just a CPython detail.

In any case, to solve the false negatives, either name mangling should be implemented or additional checks should be done when a name is looked up, to report private name lookups from outside the class.

JukkaL commented 4 years ago

Mypy should clearly follow runtime Python semantics and implement name mangling. If the error messages would become confusing, we may have to add some extra logic or notes to error messages.

We'd be happy to review a PR if anybody would like to contribute one!

davidhalter commented 3 days ago

I personally feel like name mangling should not be supported and mangled names should stay private and filtered as such. People accessing symbols such as _Foobar should not be able to do that in Mypy. I also feel like the AttributeError `"B" has no attribute "x"is fine, even though it's technically wrong.'_B__x'` is probably simply confusing a lot of users.

JelleZijlstra commented 3 days ago

The first example in this issue is of code that fails with an AttributeError at runtime but not reported by mypy. It seems clear to me that we should fix that.

davidhalter commented 3 days ago

I completely agree. I was probably too tunnel-visioned on the potential fix in https://github.com/python/mypy/pull/16715. Sorry for that. I just feel like fully implementing name mangling on the parser level is not a good idea.

TeamSpen210 commented 2 days ago

Why shouldn't mypy fully support mangling? It's a perfectly well documented and valid part of the language, not deprecated or anything. There's no warnings at all about regular private attribute access, and plenty of things in typeshed include private/undocumented members. Mangled names aren't supposed to be any more private, there's perfectly valid reasons to want to access them from outside the class.

There is the import system exporting rules, but those have ways to specifically allow private names. Even if you do use them, mypy gives a specific error, but still sets the types appropriately so subsequent code is correct. If name mangling wasn't implemented, uses would just be produce Any. There is the slight issue where it could conflict with 3.7 positional-only arguments, but it's probably about time to drop support for those anyway?

JelleZijlstra commented 2 days ago

I agree in general that mypy should model the runtime correctly. It doesn't have to be mypy's job to warn users about access to private attributes, as that's easy enough to do in a general-purpose linter.

I don't think we can afford to deprecate double underscores for positional-only arguments yet, though; there must be a lot of existing code that relies on those. If we add better support for name mangling, we need to be careful not to break those.