[pants_ng] An NG subsystem implementation

[benjyw]

Like OG subsystems, these are containers of options
read from flags, env vars and config files.

Unlike OG subsystems, these are not necessarily
singletons - you can have multiple instances of the
same subsystem active for different partitions of
sources that have different local config files.

Making OG subsystems non-singleton was not feasible,
for both design and performance reasons.

These NG subsystems are lighter weight and more
streamlined than their OG counterparts:

• They support a more limited set of value types.
• Option registration is via a new @option decorator
  instead of the large menagerie of field type classes.

AI disclosure: Claude found a way to create a mypy plugin
that prevents spurious empty body errors. I had to manually
tweak the plugin to make it less verbose and more streamlined.

[benjyw]

Reviewers: The one downside is that mypy doesn't like methods without bodies. So the # mypy: disable-error-code=empty-body you see in the test is necessary (of course we could move it to pyproject.toml so you don't have to repeat it everywhere).

Another solution could be to have the body return the default, instead of the default= option. Trouble is, we would have to execute the body to capture that default into the descriptor, or, more realistically, require that the body is just return <static constant>. And I fear that this will be misread by tooling and humans, as it's not obvious that we are overriding the body, and nothing indicates that this is just a default.

Thoughts?

[tdyas]

Reviewers: The one downside is that mypy doesn't like methods without bodies. So the # mypy: disable-error-code=empty-body you see in the test is necessary (of course we could move it to pyproject.toml so you don't have to repeat it everywhere).

Maybe have SubsystemNg inherit from typing.Protocol? Then the methods would be considered marked abstract and empty bodies are permitted. Given the implementation is supplied elsewhere, the subsystems really are just interfaces and not "concrete" classes and so typing.Protocol is appropriate.

[tdyas]

Another thought is to provide a Mypy plugin to handle typing of SubsystemNg subclasses. The plugin would mark the methods as abstract so the empty body error does not trigger. Moreover, a mypy plugin can enforce that the method bodies are in fact empty.

Regardless of the solution, I believe we should figure out how to configure mypy to not error since having to disable a type error to use PantsNG is a major anti-pattern in my view.

[benjyw]

subtyping Protocol does nothing - you have to mark the method as abstract, which it's not, really. And also if you do explicitly make each @option also @abstract then you get Cannot instantiate abstract class errors in tests at least.

So possibly a mypy plugin is the answer.

[benjyw]

Alas there does not appear to be a way to do this in a mypy plugin (possibly at all, and certainly not one that doesn't cause more problems than it solves). mypy plugins allow you to modify signatures and types, but they don't interact with this body detection feature.

TBH this check is pretty low value to begin with, so I'm comfortable with disabling it globally in the repo. But would like to hear more opinions, and I will continue to try and think some sort of solution.

[benjyw]

Hmm, Claude appears to have found a way to do this with a mypy plugin, by monkeying with the parse tree. I'll take it!

[benjyw]

OK, pushed a mypy plugin that does the trick.

[benjyw]

Now that we have an in-repo mypy plugin, we need mypy as a dev-time requirement.

[tdyas]

Maybe this should check the full-qualified name of option via dec.callee.fullname? Any use of just option could trigger this plugin, right?

[benjyw]

fullname doesn't work. It's an empty string.

[benjyw]

So, yes, if someone has an unrelated @option decorator and wants mypy to error if the function it decorates has an empty body, then we'll trip them up. What are the odds of that?

[benjyw]

And we know we don't have this case in our own repo, so it's just external users with plugins, and they also have the option of marking their options @abstract or of ignoring the error with a pragma.

[benjyw]

Thanks for the great review @tdyas. I will wait for more feedback, since the API here is high-touch for plugin developers.