gh-104050: Argument clinic: annotate post_parsing() and cleanup()

[AlexWaygood]

The docstring for CConverter.post_parsing() is here:

cpython/Tools/clinic/clinic.py

Lines 3021 to 3026 in f443b54

	defpost_parsing(self) ->str:
	"""
	The C statements required to do some operations after the end of parsing but before cleaning up.
	Return a string containing this code indented at column 0.
	If no operation is necessary, return an empty string.
	"""

The docstring for CConverter.cleanup() is here:

cpython/Tools/clinic/clinic.py

Lines 3029 to 3034 in f443b54

	defcleanup(self) ->str:
	"""
	The C statements required to clean up after this variable.
	Returns a string containing this code indented at column 0.
	If no cleanup is necessary, returns an empty string.
	"""

Both docstrings state that these methods should always return a str, so it's arguably a micro-bug that str_converter.post_parsing()Py_UNICODE_converter.cleanup() can both sometimes return None currently, since str_converter and Py_UNICODE_converter are both subclasses of CConverter. (The micro-bug doesn't result in any erroneous behaviour currently, however.)

As such, I've slightly changed the implementations of these methods so that they always return a str, like the docstring promises.

• Issue: Add type annotations to clinic.py #104050

[AlexWaygood]

The alternative way of fixing this problem would be to do this (diff is relative to main):

Alternative fix

diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 8c959ed4d0..e8e2710a18 100755 --- a/Tools/clinic/clinic.py+++ b/Tools/clinic/clinic.py@@ -3018,19 +3018,19 @@ def modify(self) -> str: """ return "" - def post_parsing(self) -> str:+ def post_parsing(self) -> str | None: """ The C statements required to do some operations after the end of parsing but before cleaning up. Return a string containing this code indented at column 0. - If no operation is necessary, return an empty string.+ If no operation is necessary, return an empty string or None. """ return "" - def cleanup(self) -> str:+ def cleanup(self) -> str | None: """ The C statements required to clean up after this variable. Returns a string containing this code indented at column 0. - If no cleanup is necessary, returns an empty string.+ If no cleanup is necessary, returns an empty string or None. """ return "" @@ -3660,10 +3660,11 @@ def converter_init( if NoneType in accept and self.c_default == "Py_None": self.c_default = "NULL" - def post_parsing(self):+ def post_parsing(self) -> str | None: if self.encoding: name = self.name return f"PyMem_FREE({name});\n" + return None def parse_arg(self, argname: str, displayname: str) -> str: if self.format_unit == 's': @@ -3841,11 +3842,12 @@ def converter_init( fail("Py_UNICODE_converter: illegal 'accept' argument " + repr(accept)) self.c_default = "NULL" - def cleanup(self):+ def cleanup(self) -> str | None: if not self.length: return """\ PyMem_Free((void *){name}); """.format(name=self.name) + return None def parse_arg(self, argname: str, argnum: str) -> str: if not self.length:

[erlend-aasland]

LGTM! Thanks

[erlend-aasland]

FWIW, I agree that it is better to fix these microbugs instead of taking the str | None approach.