Argument-Clinic / cpython

The Python programming language
https://www.python.org/
Other
1 stars 0 forks source link

`DSLParser` should not store `function` as state #7

Open AlexWaygood opened 1 year ago

AlexWaygood commented 1 year ago

In most methods of the DSLParser, self.function can be safely assumed to be an instance of Function. However, it cannot be assumed to be an instance of Function in all methods, because it is set to None in self.reset() before starting to parse a block:

https://github.com/Argument-Clinic/cpython/blob/49f238e78c36532bcbca7f9cd172703eb4df319b/Tools/clinic/clinic.py#L4457-L4458

This is a pretty fragile design, and means we have to have a bunch of assertions scattered about the DSLParser class in order to keep mypy satisfied:

https://github.com/Argument-Clinic/cpython/blob/49f238e78c36532bcbca7f9cd172703eb4df319b/Tools/clinic/clinic.py#L4879

https://github.com/Argument-Clinic/cpython/blob/49f238e78c36532bcbca7f9cd172703eb4df319b/Tools/clinic/clinic.py#L4884

https://github.com/Argument-Clinic/cpython/blob/49f238e78c36532bcbca7f9cd172703eb4df319b/Tools/clinic/clinic.py#L4922

https://github.com/Argument-Clinic/cpython/blob/49f238e78c36532bcbca7f9cd172703eb4df319b/Tools/clinic/clinic.py#L5292

https://github.com/Argument-Clinic/cpython/blob/49f238e78c36532bcbca7f9cd172703eb4df319b/Tools/clinic/clinic.py#L5306

This is a pretty tell-tale sign that function shouldn't really be stored as state at all. Instead, it should be passed into these methods as parameters, and returned from these methods after having been modified. Unfortunately, I think getting there would be a pretty significant refactor...