Skip to content

Commit

Permalink
Refactor how we analyse classes (#460)
Browse files Browse the repository at this point in the history
Currently we analyse `ClassDef` nodes in many different places across
the tree in a somewhat scattershot and ad-hoc way. This means that
there's a fair amount of duplication currently, and that it can be
somewhat awkward to query information about the enclosing class
definition when we're visiting a method.

This PR thoroughly refactors the way we analyse classes:
- It's now all done in one place, the `EnclosingClassContext` class. As
well as reducing duplication, this also reduces potential for bugs: e.g.
we currently don't detect a protocol as being unused if the protocol is
generic (because that's an `ast.Subscript` node in the class's bases
tuple, rather than an `ast.Name` or `ast.Attribute` node).
- Get rid of the `_get_collections_abc_obj_id` function. It's not really
necessary; the name is really unclear; it's an over-abstraction that
obfuscates rather than clarifies.
- Get rid of the `in_class` nesting counter on the `PyiVisitor` class.
This works well to keep track of other contexts when visiting subnodes,
but doesn't work well for class definitions: it's no more code to just
check whether `self.enclosing_class_ctx is None` or not.
  • Loading branch information
AlexWaygood authored Jan 5, 2024
1 parent 4711624 commit 8fba6ed
Show file tree
Hide file tree
Showing 3 changed files with 168 additions and 130 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ Bugfixes:
[PEP 570 syntax](https://peps.python.org/pep-0570/) and the first
positional-or-keyword parameter following the positional-only parameters used
a custom TypeVar (see #455).
* Y046: Fix false negative where an unused protocol would not be detected if
the protocol was generic.

## 23.11.0

Expand Down
Loading

0 comments on commit 8fba6ed

Please sign in to comment.