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

Support inference of Enum subclasses. #1121

Merged
merged 4 commits into from
Aug 12, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
7 changes: 7 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ Release date: TBA
* Import from ``astroid.node_classes`` and ``astroid.scoped_nodes`` has been deprecated in favor of
``astroid.nodes``. Only the imports from ``astroid.nodes`` will work in astroid 3.0.0.

* Add support for arbitrary Enum subclass hierachies

Closes PyCQA/pylint#533
Closes PyCQA/pylint#2224
Closes PyCQA/pylint#2626


What's New in astroid 2.6.7?
============================
Release date: TBA
Expand Down
53 changes: 42 additions & 11 deletions astroid/brain/brain_namedtuple_enum.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@
import keyword
from textwrap import dedent

import astroid
from astroid import arguments, inference_tip, nodes, util
from astroid.builder import AstroidBuilder, extract_node
from astroid.exceptions import (
AstroidTypeError,
AstroidValueError,
InferenceError,
MroError,
UseInferenceDefault,
)
from astroid.manager import AstroidManager
Expand Down Expand Up @@ -354,9 +356,7 @@ def __mul__(self, other):

def infer_enum_class(node):
"""Specific inference for enums."""
for basename in node.basenames:
# TODO: doesn't handle subclasses yet. This implementation
# is a hack to support enums.
for basename in (b for cls in node.mro() for b in cls.basenames):
if basename not in ENUM_BASE_NAMES:
continue
if node.root().name == "enum":
Expand Down Expand Up @@ -417,9 +417,9 @@ def name(self):
# should result in some nice symbolic execution
classdef += INT_FLAG_ADDITION_METHODS.format(name=target.name)

fake = AstroidBuilder(AstroidManager()).string_build(classdef)[
target.name
]
fake = AstroidBuilder(
AstroidManager(), apply_transforms=False
).string_build(classdef)[target.name]
fake.parent = target.parent
for method in node.mymethods():
fake.locals[method.name] = [method]
Expand Down Expand Up @@ -544,18 +544,49 @@ def infer_typing_namedtuple(node, context=None):
return infer_named_tuple(node, context)


def _is_enum_subclass(cls: astroid.ClassDef) -> bool:
"""Return whether cls is a subclass of an Enum.

Warning: For efficiency purposes, this function immediately returns False if enum hasn't
been imported in the module of the ClassDef. This means it fails to detect an Enum
subclass that is imported from a new module and subclassed, e.g.
Copy link
Member

Choose a reason for hiding this comment

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

I think we need this kind of compromise if we want to keep pylint reasonably fast. But on the other hand I think that what pylint bring is a lot of checks in a lot more time than what flake8 does for example. I guess if we try to make pylint's fast and compromise on correctness pylint will stay slow and only be a little less wrong. So I'm a little torn here. How slow would the check be is we check the ancestors of the class (Knowing that we're going to cache the ancestors in #1120) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, looking at this a bit more I think this is a case of premature optimization on my part. I ran pylint on this repository with astroid==2.6.6, then with this version, and then with this version but the early return commented out, and didn't see a noticeable difference, they were all in the ballpark for 19-20 seconds. In general it seems there are more important performance issues with pylint that your team is working on.

Especially with the ancestors being cached (very cool!), and the fact that mro is almost certainly going to be called at some point when using pylint since it's essential for E1101 (no-member), I don't think the extra check is worth having. And certainly not at the expense of weird false positives!

So if you agree, I'll update this PR to remove the check from the code.

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for taking the time to do the comparison. Let's use the ancestors then :) We do have major performance issue to tackle. I'm even thinking of a new checker freeze to be able to focus on performance, because it's hard to find time for that.


# a.py
import enum

class A(enum.Enum):
pass

# b.py
from a import A

class B(A): # is_enum_subclass returns False
attr = 1
"""
mod_locals = cls.root().locals
if "enum" not in mod_locals and all(
base not in mod_locals for base in ENUM_BASE_NAMES
):
return False

try:
return any(
klass.name in ENUM_BASE_NAMES
and getattr(klass.root(), "name", None) == "enum"
for klass in cls.mro()
)
except MroError:
return False


AstroidManager().register_transform(
nodes.Call, inference_tip(infer_named_tuple), _looks_like_namedtuple
)
AstroidManager().register_transform(
nodes.Call, inference_tip(infer_enum), _looks_like_enum
)
AstroidManager().register_transform(
nodes.ClassDef,
infer_enum_class,
predicate=lambda cls: any(
basename for basename in cls.basenames if basename in ENUM_BASE_NAMES
),
nodes.ClassDef, infer_enum_class, predicate=_is_enum_subclass
)
AstroidManager().register_transform(
nodes.ClassDef, inference_tip(infer_typing_namedtuple_class), _has_namedtuple_base
Expand Down
9 changes: 7 additions & 2 deletions astroid/nodes/scoped_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
from astroid.interpreter.dunder_lookup import lookup
from astroid.interpreter.objectmodel import ClassModel, FunctionModel, ModuleModel
from astroid.manager import AstroidManager
from astroid.nodes import node_classes
from astroid.nodes import Const, node_classes

ITER_METHODS = ("__iter__", "__getitem__")
EXCEPTION_BASE_CLASSES = frozenset({"Exception", "BaseException"})
Expand Down Expand Up @@ -2962,7 +2962,12 @@ def _inferred_bases(self, context=None):

for stmt in self.bases:
try:
baseobj = next(stmt.infer(context=context.clone()))
# Find the first non-None inferred base value
baseobj = next(
b
for b in stmt.infer(context=context.clone())
if not (isinstance(b, Const) and b.value is None)
)
except (InferenceError, StopIteration):
continue
if isinstance(baseobj, bases.Instance):
Expand Down
77 changes: 77 additions & 0 deletions tests/unittest_brain.py
Original file line number Diff line number Diff line change
Expand Up @@ -791,6 +791,24 @@ def __init__(self, name, enum_list):
test = next(enumeration.igetattr("test"))
self.assertEqual(test.value, 42)

def test_user_enum_false_positive(self):
# Test that a user-defined class named Enum is not considered a builtin enum.
ast_node = astroid.extract_node(
"""
class Enum:
pass

class Color(Enum):
red = 1

Color.red #@
"""
)
inferred = ast_node.inferred()
self.assertEqual(len(inferred), 1)
self.assertIsInstance(inferred[0], astroid.Const)
self.assertEqual(inferred[0].value, 1)

def test_ignores_with_nodes_from_body_of_enum(self):
code = """
import enum
Expand Down Expand Up @@ -1051,6 +1069,65 @@ def func(self):
assert isinstance(inferred, bases.Instance)
assert inferred.pytype() == ".TrickyEnum.value"

def test_enum_subclass_member_name(self):
ast_node = astroid.extract_node(
"""
from enum import Enum

class EnumSubclass(Enum):
pass

class Color(EnumSubclass):
red = 1

Color.red.name #@
"""
)
inferred = ast_node.inferred()
self.assertEqual(len(inferred), 1)
self.assertIsInstance(inferred[0], astroid.Const)
self.assertEqual(inferred[0].value, "red")

def test_enum_subclass_member_value(self):
ast_node = astroid.extract_node(
"""
from enum import Enum

class EnumSubclass(Enum):
pass

class Color(EnumSubclass):
red = 1

Color.red.value #@
"""
)
inferred = ast_node.inferred()
self.assertEqual(len(inferred), 1)
self.assertIsInstance(inferred[0], astroid.Const)
self.assertEqual(inferred[0].value, 1)

def test_enum_subclass_member_method(self):
# See Pylint issue #2626
ast_node = astroid.extract_node(
"""
from enum import Enum

class EnumSubclass(Enum):
def hello_pylint(self) -> str:
return self.name

class Color(EnumSubclass):
red = 1

Color.red.hello_pylint() #@
"""
)
inferred = ast_node.inferred()
self.assertEqual(len(inferred), 1)
self.assertIsInstance(inferred[0], astroid.Const)
self.assertEqual(inferred[0].value, "red")


@unittest.skipUnless(HAS_DATEUTIL, "This test requires the dateutil library.")
class DateutilBrainTest(unittest.TestCase):
Expand Down