Fix NG subsystem mypy plugin

[benjyw]

The previous implementation failed if the subsystem
extended an intermediate SubsystemNg subclass
instead of SubsystemNg directly.

This change switches to get_base_class_hook, so that
setting abstract_status = IS_ABSTRACT on the
underlying FuncDef sticks when the type checker runs.

[benjyw]

AI Disclosure: Claude discovered the cause, suggested a fix (which I simplified a bit) and generated a test (which I simplified a lot).

[tdyas]

Can these be discovered by the plugin from semantic analysis in a performant manner?

[benjyw]

I don't think that can be done reliably, let alone performantly?

[tdyas]

I asked Codex and it suggests looking at ctx.cls.info.mro to access the MRO for the class:

def _is_subsystem_ng(ctx: ClassDefContext) -> bool:
      return any(info.fullname == _SUBSYSTEM_ROOT for info in ctx.cls.info.mro)

Open question whether that is performant enough.

[tdyas]

Yes, but not with the current get_base_class_hook(fullname) shape alone.

get_base_class_hook() only receives the direct base class fullname being analyzed, so for:

class FooSubsystem(GoalSubsystemNg):
...

mypy asks the plugin about pants.ng.goal.GoalSubsystemNg, not about its transitive base
SubsystemNg. That is why _SUBSYSTEM_BASE_CLASSES currently has to include intermediate
classes like GoalSubsystemNg, UniversalSubsystem, and ContextualSubsystem.

A dynamic version is possible by making the hook run more broadly, then filtering inside the
callback using the class MRO:

  _SUBSYSTEM_ROOT = "pants.ng.subsystem.SubsystemNg"

  def _is_subsystem_ng(ctx: ClassDefContext) -> bool:
      return any(info.fullname == _SUBSYSTEM_ROOT for info in ctx.cls.info.mro)

  def _process_subclass_hook(ctx: ClassDefContext) -> None:
      if _is_subsystem_ng(ctx):
          _process_defns(ctx.cls.defs.body)

  class SubsystemPlugin(Plugin):
      def get_base_class_hook(self, fullname: str):
          return _process_subclass_hook

That would infer intermediate subclasses dynamically from mypy’s semantic model, avoiding
the manually maintained _SUBSYSTEM_BASE_CLASSES.

Tradeoff: returning a hook for every base class means the callback runs for many non-
subsystem classes, and possibly more than once for classes with multiple bases. The callback
is cheap and idempotent enough here, but I’d add a guard such as metadata on ctx.cls.info if
duplicate processing becomes a concern.

[tdyas]

Maybe add a conformance check in the plugin for all direct subclasses of pants.ng.subsystem.SubsystemNg that they are listed in _SUBSYSTEM_BASE_CLASSES?

Basically, it would be better if the plugin either discovers the contents of _SUBSYSTEM_BASE_CLASSES or otherwise flags an error when items are missing from it.