gh-104050: Annotate more Argument Clinic DSLParser state methods - #106376
gh-104050: Annotate more Argument Clinic DSLParser state methods#106376erlend-aasland merged 3 commits into
Conversation
Sorry, something went wrong.
|
@AlexWaygood, can you help me with the last bits on this PR? |
Sorry, something went wrong.
Taking a look now :) |
Sorry, something went wrong.
|
This diff fixes the first two mypy errors: --- a/Tools/clinic/clinic.py
+++ b/Tools/clinic/clinic.py
@@ -4846,7 +4846,9 @@ def state_parameter(self, line: str | None) -> None:
if not module:
fail("Function " + self.function.name + " has an invalid parameter declaration:\n\t" + line)
- function_args = module.body[0].args
+ function = module.body[0]
+ assert isinstance(function, ast.FunctionDef)
+ function_args = function.args
if len(function_args.args) > 1:
fail("Function " + self.function.name + " has an invalid parameter declaration (comma?):\n\t" + line)
@@ -4931,7 +4933,9 @@ def bad_node(self, node):
if bad:
fail("Unsupported expression as default value: " + repr(default))
- expr = module.body[0].value
+ assignment = module.body[0]
+ assert isinstance(assignment, ast.Assign)
+ expr = assignment.value
# mild hack: explicitly support NULL as a default value
c_default: str | None
if isinstance(expr, ast.Name) and expr.id == 'NULL':In both cases, mypy is flagging that, in principle, an ast.Module node can have any kind of ast.stmt nodes in its .body. Not all ast.stmt instances have a .args attribute or a .value attribute (only instances of certain subclasses of ast.stmt do), so we need to do some kind of type narrowing here to help mypy out. In both cases, we can be confident that these assertions are safe. For the first one, the source code that we're parsing is generated here: cpython/Tools/clinic/clinic.py Lines 4849 to 4850 in 71b4044 For the second one, the source code that we're parsing is generated here: cpython/Tools/clinic/clinic.py Lines 4902 to 4904 in 71b4044 So we can be 100% confident that in the first case, module.body[0] will always be an instance of ast.FunctionDef, and in the second case, module.body[0] will always be an instance of ast.Assign. This diff fixes the last mypy error: --- a/Tools/clinic/clinic.py
+++ b/Tools/clinic/clinic.py
@@ -4970,7 +4974,7 @@ def bad_node(self, node):
else:
value = ast.literal_eval(expr)
py_default = repr(value)
- if isinstance(value, (bool, None.__class__)):
+ if isinstance(value, (bool, NoneType)):
c_default = "Py_" + py_default
elif isinstance(value, str):
c_default = c_repr(value)Type checkers do a lot of special casing around None due to its status as a singleton; they understand isinstance() checks against types.NoneType, but get bamboozled by more unusual idioms like isinstance(x, None.__class__) :) |
Sorry, something went wrong.
|
Of course, the assert isinstance trick 📝 |
Sorry, something went wrong.
|
Thanks! |
Sorry, something went wrong.
|
No problem at all! 🚀 |
Sorry, something went wrong.
AlexWaygood
left a comment
There was a problem hiding this comment.
LGTM.
Off-topic: this function is way too large and complicated, and desperately in need of modernisation as well. I had a go a while back (main...AlexWaygood:cpython:clinic-asts), but some tests were failing... that branch is also quite out of date now :-) And test coverage was poor, IIRC.
But maybe I'll get back to it at some point...
Sorry, something went wrong.
* main: pythongh-106368: Increase Argument Clinic test coverage (python#106389) pythongh-106320: Fix _PyImport_GetModuleAttr() declaration (python#106386) pythongh-106368: Harden Argument Clinic parser tests (python#106384) pythongh-106320: Remove private _PyImport C API functions (python#106383) pythongh-86085: Remove _PyCodec_Forget() declaration (python#106377) pythongh-106320: Remove more private _PyUnicode C API functions (python#106382) pythongh-104050: Annotate more Argument Clinic DSLParser state methods (python#106376) pythongh-106368: Clean up Argument Clinic tests (python#106373)
Annotate the following methods: