refactor(api): Make sure command implementations return something compatible with the command's result type

[SyntaxColoring]

Overview

This is a step towards EXEC-427.

Every command declares a result type, e.g Aspirate has AspirateResult. But it turns out that nothing enforces that the command's implementation returns that type—AspirateImplementation could return a DispenseResult. This has been okay in practice so far because each implementation only returns one thing, and it's not easy for them to fall out of sync. But you can imagine how it will get messy when we start introducing more structured error types. My rough plan is for execute() -> AspirateResult to become execute() -> AspirateResult | AspirateErrorA | AspirateErrorB | ..., and we will want to enforce that AspirateErrorA | AspirateErrorB matches the type of the Aspirate.error field.

Changelog

See my inline notes.

Test plan

• Make sure CI keeps passing.
• Make sure the "new field" doesn't show up in robot-server's HTTP API documentation.

Review requests

Does this make sense?

Risk assessment

Very low.

[SyntaxColoring]

By convention, each command links itself to its implementation via an _ImplementationCls class attribute.

Here, I'm declaring that attribute on the base class. This enforces that each command's ._ImplementationCls matches its .params and .result. In the near future, this will also make sure it matches its .error.

[SyntaxColoring]

In English:

• An implementation that accepts a broader parameter type can be placed in a variable where it's annotated to accept a narrower parameter type. It's contravariant in its parameters.
• An implementation that returns a narrower result type can be placed in a variable where it's annotated to return a broader parameter type. It's covariant in its result.

This is the usual behavior for callables, which is basically what a command implementation is. I think we need to spell it out manually because we're going through a wrapper class; the type-checker would understand this automatically if, e.g., the AspirateImplementation.execute() method were just a free-standing execute() function.

[SyntaxColoring]

If you're using VS Code or another editor with Pyright integration, you'll get a red squiggly on every command's override of _ImplementationCls. (Mypy does not complain.)

I think it has to do with the fact that _ImplementationCls looks writable, so it's unsound for a subclass to have a more restrictive type than the base class. It looks like commandType has this problem, too. I don't have a solution right now. Maybe it'll get easier with Pydantic v2.

[jbleon95]

Changes make sense to me.