Refactor how we analyse classes

[AlexWaygood]

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.

[AlexWaygood]

Deleted a comment I made that I don't think made any sense — I don't think this is urgent :)

[srittau]

Good refactoring. I just have two questions below.

[srittau]

Can this happen with the current ast? Wouldn't it be safer to raise an exception here, or use an assertion?

[AlexWaygood]

It can definitely happen; the .value of an Attribute node can be any number of things:

>>> class EvenStranger:
...     def __getitem__(self, key):
...         return self
...     def __getattr__(self, attr):
...         return int
...
>>> class Foo(EvenStranger()[1][2].foo): ...
>>> tree = ast.parse("EvenStranger()[1][2].foo")
>>> print(ast.dump(tree, indent=2))
Module(
  body=[
    Expr(
      value=Attribute(
        value=Subscript(
          value=Subscript(
            value=Call(
              func=Name(id='EvenStranger', ctx=Load()),
              args=[],
              keywords=[]),
            slice=Constant(value=1),
            ctx=Load()),
          slice=Constant(value=2),
          ctx=Load()),
        attr='foo',
        ctx=Load()))],
  type_ignores=[])

This is obviously pretty unlikely to happen in a stub, but I don't think we should be raising an exception if we come across something like this

[AlexWaygood]

I'll add some tests for these edge cases; I think you're correct that the handling isn't quite right for them at present.

[srittau]

Same question here. (A subscript can't be a Subscript's value, I think.)

[AlexWaygood]

Well, it is possible...

(I'll admit I had to try quite hard here!)

Python 3.12.1 (tags/v3.12.1:2305ca5, Dec  7 2023, 22:03:25) [MSC v.1937 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> class Strange:
...     def __class_getitem__(cls, item):
...         return Strange
...
>>> class Foo(Strange[Strange][Strange]): ...
...
>>> import ast
>>> tree = ast.parse("class Foo(Strange[Strange][Strange]): ...")
>>> tree.body[0].bases[0]
<ast.Subscript object at 0x000001E44FC43FD0>
>>> _.value
<ast.Subscript object at 0x000001E44FC43F90>

[github-actions]

⚠ Flake8 diff showing the effect of this PR on typeshed:

> ./stubs/openpyxl/openpyxl/xml/_functions_overloads.pyi:32:1: Y046 Protocol "_HasTagAndGet" is not used
> ./stubs/protobuf/google/protobuf/internal/type_checkers.pyi:5:1: Y046 Protocol "_ValueChecker" is not used

[AlexWaygood]

Thanks for the quick review!