Skip to content

Commit 7d6f3f2

Browse files
authored
Improve handling of aggregations for comprehensions in Pyreverse (#10344)
* Enhance AggregationsHandler to also handle list comprehensions * Fix order of the if statements ==> now it should work * Flatten the nested if statements by using early returns * Add a test case for comprehensions * Remove guard * Use aggregation instead of composition arrows in test file * Remove try except block * Remove print statements * Update test files with new containers and rename * rename containers to comprehensions for better clarity * Simplify detection logic * Simplify test case * Fix comprehension mmd file * Update inspector to handle all types of comprehensions * Adding type annotations for now, because astroid does not support type inference in comprehensions * Improve codecov by using the utility funciton safe_infer * Add newsfragment * Rename newsfragment
1 parent d1b1802 commit 7d6f3f2

File tree

4 files changed

+62
-5
lines changed

4 files changed

+62
-5
lines changed

doc/whatsnew/fragments/10236.feature

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Enhanced pyreverse to properly detect aggregations for comprehensions (list, dict, set, generator).
2+
3+
Closes #10236

pylint/pyreverse/inspector.py

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
from astroid import nodes
2121

2222
from pylint import constants
23+
from pylint.checkers.utils import safe_infer
2324
from pylint.pyreverse import utils
2425

2526
_WrapperFuncT = Callable[
@@ -328,15 +329,37 @@ def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None:
328329

329330
class AggregationsHandler(AbstractAssociationHandler):
330331
def handle(self, node: nodes.AssignAttr, parent: nodes.ClassDef) -> None:
331-
if isinstance(node.parent, (nodes.AnnAssign, nodes.Assign)) and isinstance(
332-
node.parent.value, astroid.node_classes.Name
333-
):
332+
# Check if we're not in an assignment context
333+
if not isinstance(node.parent, (nodes.AnnAssign, nodes.Assign)):
334+
super().handle(node, parent)
335+
return
336+
337+
value = node.parent.value
338+
339+
# Handle direct name assignments
340+
if isinstance(value, astroid.node_classes.Name):
334341
current = set(parent.aggregations_type[node.attrname])
335342
parent.aggregations_type[node.attrname] = list(
336343
current | utils.infer_node(node)
337344
)
338-
else:
339-
super().handle(node, parent)
345+
return
346+
347+
# Handle comprehensions
348+
if isinstance(
349+
value, (nodes.ListComp, nodes.DictComp, nodes.SetComp, nodes.GeneratorExp)
350+
):
351+
# Determine the type of the element in the comprehension
352+
if isinstance(value, nodes.DictComp):
353+
element_type = safe_infer(value.value)
354+
else:
355+
element_type = safe_infer(value.elt)
356+
if element_type:
357+
current = set(parent.aggregations_type[node.attrname])
358+
parent.aggregations_type[node.attrname] = list(current | {element_type})
359+
return
360+
361+
# Fallback to parent handler
362+
super().handle(node, parent)
340363

341364

342365
class OtherAssociationsHandler(AbstractAssociationHandler):
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
classDiagram
2+
class Component {
3+
name : str
4+
}
5+
class Container {
6+
component_dict : dict[int, Component]
7+
components : list[Component]
8+
components_set : set[Component]
9+
lazy_components : Generator[Component]
10+
}
11+
Component --o Container : components
12+
Component --o Container : component_dict
13+
Component --o Container : components_set
14+
Component --o Container : lazy_components
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
# Test for https://github.com/pylint-dev/pylint/issues/10236
2+
from collections.abc import Generator
3+
4+
5+
class Component:
6+
"""A component class."""
7+
def __init__(self, name: str):
8+
self.name = name
9+
10+
11+
class Container:
12+
"""A container class that uses comprehension to create components."""
13+
def __init__(self):
14+
self.components: list[Component] = [Component(f"component_{i}") for i in range(3)] # list
15+
self.component_dict: dict[int, Component] = {i: Component(f"dict_component_{i}") for i in range(2)} # dict
16+
self.components_set: set[Component] = {Component(f"set_component_{i}") for i in range(2)} # set
17+
self.lazy_components: Generator[Component] = (Component(f"lazy_{i}") for i in range(2)) # generator

0 commit comments

Comments
 (0)