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.

Sign up for GitHub

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

Enum private attributes are not enum members #17182

Merged
merged 3 commits into from
Apr 27, 2024
Merged

Enum private attributes are not enum members #17182

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
08263d5
Enum private attributes are not enum members
hamdanal Apr 27, 2024
8c641d6
Add comments about name mangling in the tests
hamdanal Apr 27, 2024
84dc3bb
Merge remote-tracking branch 'upstream/master' into enum-private-attr
hamdanal Apr 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions mypy/checkmember.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1139,8 +1139,8 @@ def analyze_enum_class_attribute_access(
# Skip these since Enum will remove it
if name in ENUM_REMOVED_PROPS:
return report_missing_attribute(mx.original_type, itype, name, mx)
# For other names surrendered by underscores, we don't make them Enum members
if name.startswith("__") and name.endswith("__") and name.replace("_", "") != "":
# Dunders and private names are not Enum members
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I would call any name that starts with a single underscore a "private" (by convention) name, and any name that starts with two underscores a "mangled" name

Suggested change
# Dunders and private names are not Enum members
# Dunders and mangled names are not Enum members

(There's several other places in this PR where you also refer to them as "private" names; I'd change those too, personally)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used the same terminology used by the enum docs: https://docs.python.org/3/howto/enum.html#private-names

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough... though I sorta think the docs there are using the wrong terminology... but hey :P

if name.startswith("__") and name.replace("_", "") != "":
return None

enum_literal = LiteralType(name, fallback=itype)
Expand Down
7 changes: 6 additions & 1 deletion mypy/semanal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3979,7 +3979,12 @@ def analyze_name_lvalue(
existing = names.get(name)

outer = self.is_global_or_nonlocal(name)
if kind == MDEF and isinstance(self.type, TypeInfo) and self.type.is_enum:
if (
kind == MDEF
and isinstance(self.type, TypeInfo)
and self.type.is_enum
and not name.startswith("__")
):
# Special case: we need to be sure that `Enum` keys are unique.
if existing is not None and not isinstance(existing.node, PlaceholderNode):
self.fail(
Expand Down
7 changes: 6 additions & 1 deletion mypy/typeanal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -868,7 +868,12 @@ def analyze_unbound_type_without_type_info(
# If, in the distant future, we decide to permit things like
# `def foo(x: Color.RED) -> None: ...`, we can remove that
# check entirely.
if isinstance(sym.node, Var) and sym.node.info and sym.node.info.is_enum:
if (
isinstance(sym.node, Var)
and sym.node.info
and sym.node.info.is_enum
and not sym.node.name.startswith("__")
):
value = sym.node.name
base_enum_short_name = sym.node.info.name
if not defining_literal:
Expand Down
3 changes: 3 additions & 0 deletions mypy/typeops.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,9 @@ class Status(Enum):
# Skip these since Enum will remove it
if name in ENUM_REMOVED_PROPS:
continue
# Skip private attributes
if name.startswith("__"):
continue
new_items.append(LiteralType(name, typ))
return make_simplified_union(new_items, contract_literals=False)
elif typ.type.fullname == "builtins.bool":
Expand Down
32 changes: 32 additions & 0 deletions test-data/unit/check-enum.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1425,6 +1425,10 @@ from enum import Enum
class Correct(Enum):
x = 'y'
y = 'x'
class Correct2(Enum):
x = 'y'
__z = 'y'
__z = 'x'
class Foo(Enum):
A = 1
A = 'a' # E: Attempted to reuse member name "A" in Enum definition "Foo" \
Expand Down Expand Up @@ -2105,3 +2109,31 @@ class AllPartialList(Enum):

def check(self) -> None:
reveal_type(self.value) # N: Revealed type is "builtins.list[Any]"

[case testEnumPrivateAttributeNotMember]
from enum import Enum

class MyEnum(Enum):
A = 1
B = 2
__my_dict = {A: "ham", B: "spam"}

x: MyEnum = MyEnum.__my_dict # E: Incompatible types in assignment (expression has type "Dict[int, str]", variable has type "MyEnum")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm not sure this is correct? The name is mangled at runtime, so this attempted attribute access fails with an exception unless you use the mangled name:

>>> from enum import Enum
>>> class Foo(Enum):
...     __x = 'y'
...     y = 'z'
... 
>>> Foo.__x
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: type object 'Foo' has no attribute '__x'
>>> Foo._Foo__x
'y'

This PR definitely improves the handling of mangled names in enum class namespaces, but really mypy should be warning that MyEnum has no attribute __my_dict here (it has a _MyEnum__my_dict attribute instead). I think that's at least worth a comment, if it's not an easy fix.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm not sure this is correct?

It isn't :D, but mypy doesn't understand name mangling yet and it is out of scope here. You are right however, there should be a comment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oof, we don't understand name mangling in any context? 😬

You are right however, there should be a comment.

Thanks!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't: https://mypy-play.net/?mypy=latest&python=3.12&gist=13d2e1e6e60b8e3828129fc44b336381 but there is already an issue for that #8267

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh and a PR #16715


[case testEnumWithPrivateAttributeReachability]
# flags: --warn-unreachable
from enum import Enum

class MyEnum(Enum):
A = 1
B = 2
__my_dict = {A: "ham", B: "spam"}

e: MyEnum
if e == MyEnum.A:
reveal_type(e) # N: Revealed type is "Literal[__main__.MyEnum.A]"
elif e == MyEnum.B:
reveal_type(e) # N: Revealed type is "Literal[__main__.MyEnum.B]"
else:
reveal_type(e) # E: Statement is unreachable
[builtins fixtures/dict.pyi]
3 changes: 2 additions & 1 deletion test-data/unit/check-literal.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2503,7 +2503,7 @@ class Color(Enum):
RED = 1
GREEN = 2
BLUE = 3

__ROUGE = RED
def func(self) -> int: pass

r: Literal[Color.RED]
Expand All @@ -2512,6 +2512,7 @@ b: Literal[Color.BLUE]
bad1: Literal[Color] # E: Parameter 1 of Literal[...] is invalid
bad2: Literal[Color.func] # E: Parameter 1 of Literal[...] is invalid
bad3: Literal[Color.func()] # E: Invalid type: Literal[...] cannot contain arbitrary expressions
bad4: Literal[Color.__ROUGE] # E: Parameter 1 of Literal[...] is invalid

def expects_color(x: Color) -> None: pass
def expects_red(x: Literal[Color.RED]) -> None: pass
Expand Down
Loading