Hi all,
Well that was a fun rabbit hole....
Buildstream implements a custom `FastEnum`[1], so all over the code base
there are ugly `str` and `int` etc type hints in function and class
definitions [2], instead of the appropriate Enum classes like
`OverlapAction` or `_Scope` which are usually recorded separately in the
doc strings. This means we don't get proper static type checking on our
Enums :( . From what I can tell this is because mypy only supports
`enum.Enum` (and it's official variations) when it comes to doing static
type checks on enums [3], so just expects that raw types. We can't
subclass Enum in FastEnum to make mypy happy, because Enum doesn't allow
subclassing [4]. But we could have some fun with stub files [5][6] and
lie to mypy about what type the Enum classes are[7], so the type hints
around the codebase could be corrected [8]. Now we can have proper
static type checking on the custom Enums, Yay!
So
```
class OverlapAction(FastEnum):
ERROR: str
WARNING: str
IGNORE: str
```
gets stubbed as
```
from enum import Enum
class OverlapAction(Enum):
ERROR: str
WARNING: str
IGNORE: str
```
Thoughts on introducing these stubs, to provide Enum static type
checking on our custom `FastEnum`? I don't know what side effects there
might be, but all tests pass locally for me.
[1]
https://github.com/apache/buildstream/blob/master/src/buildstream/types.py#L32
[2] e.g.
https://github.com/apache/buildstream/blob/master/src/buildstream/_stream.py#L262
[3] https://mypy.readthedocs.io/en/stable/literal_types.html#enums
[4]
https://docs.python.org/3/howto/enum.html#restricted-enum-subclassing
[5] https://typing.python.org/en/latest/guides/writing_stubs.html
[6] https://mypy.readthedocs.io/en/stable/stubgen.html
[7] https://github.com/python/mypy/issues/3217
[8] I think I will include a sneaky patch in this PR with the stub file,
where I am currently fixing the linting and static type checking errors
that CI threw up, but I may also break it out into it's own PR:
https://github.com/apache/buildstream/pull/2147
P.S. I wonder if it's worth making this file a bit more obvious:
https://github.com/apache/buildstream/blob/master/doc/source/hacking/using_the_testsuite.rst
it took me a while to actually find this file so I could actually run
the tests, linting and other static checks.
thanks,
Nathan
