From e5a41cbb97aa2d156df1806ce4a3c0184e0b7604 Mon Sep 17 00:00:00 2001 From: junkmd Date: Sat, 20 Dec 2025 11:45:18 +0900 Subject: [PATCH 1/2] fix: Correct signature generation for positional-only arguments. The `DispMethodAnnotator` was generating syntactically invalid method signatures for dispmethods in two specific scenarios, leading to `SyntaxError` when the generated type hint files were used. First, when a method signature required `**kwargs` due to optional arguments being followed by non-optional ones, the generated hint was `def method(..., **kwargs: hints.Any, /)`. This is invalid syntax. Second, when a method had an argument name that is a Python keyword, the annotator would fall back to `*args: hints.Any, **kwargs: hints.Any`, but would still incorrectly append a final `/`. This commit corrects the logic in `DispMethodAnnotator.getvalue` to properly handle the placement of the positional-only marker (`/`): - The marker is now placed before `**kwargs` when present. - The marker is omitted entirely when `*args` is in the signature, as a trailing slash is not valid in that context. The test case in `test_typeannotator.py` has been updated to reflect and verify these corrections. --- comtypes/test/test_typeannotator.py | 6 ++--- comtypes/tools/codegenerator/typeannotator.py | 22 ++++++++++++++++++- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/comtypes/test/test_typeannotator.py b/comtypes/test/test_typeannotator.py index 1417aa37..33ec7b4a 100644 --- a/comtypes/test/test_typeannotator.py +++ b/comtypes/test/test_typeannotator.py @@ -70,13 +70,13 @@ def test_disp_interface(self): " def ham(self) -> hints.Incomplete: ...\n" " pass # @property # dispprop\n" " pass # avoid using a keyword for def except(self) -> hints.Incomplete: ...\n" # noqa - " def bacon(self, *args: hints.Any, **kwargs: hints.Any, /) -> hints.Incomplete: ...\n" # noqa + " def bacon(self, *args: hints.Any, **kwargs: hints.Any) -> hints.Incomplete: ...\n" # noqa " def _get_spam(self, arg1: hints.Incomplete = ..., /) -> hints.Incomplete: ...\n" # noqa - " def _set_spam(self, arg1: hints.Incomplete = ..., **kwargs: hints.Any, /) -> hints.Incomplete: ...\n" # noqa + " def _set_spam(self, arg1: hints.Incomplete = ..., /, **kwargs: hints.Any) -> hints.Incomplete: ...\n" # noqa " spam = hints.named_property('spam', _get_spam, _set_spam)\n" " pass # avoid using a keyword for def raise(self, foo: hints.Incomplete, bar: hints.Incomplete = ..., /) -> hints.Incomplete: ...\n" # noqa " def _get_def(self, arg1: hints.Incomplete = ..., /) -> hints.Incomplete: ...\n" # noqa - " def _set_def(self, arg1: hints.Incomplete = ..., **kwargs: hints.Any, /) -> hints.Incomplete: ...\n" # noqa + " def _set_def(self, arg1: hints.Incomplete = ..., /, **kwargs: hints.Any) -> hints.Incomplete: ...\n" # noqa " pass # avoid using a keyword for def = hints.named_property('def', _get_def, _set_def)\n" # noqa " def egg(self) -> hints.Incomplete: ..." # noqa ) diff --git a/comtypes/tools/codegenerator/typeannotator.py b/comtypes/tools/codegenerator/typeannotator.py index c0dd6938..87783672 100644 --- a/comtypes/tools/codegenerator/typeannotator.py +++ b/comtypes/tools/codegenerator/typeannotator.py @@ -300,7 +300,27 @@ def getvalue(self, name: str) -> str: # TODO: After named parameters are supported, the positional-only parameter # markers will be removed. if inargs: - content = f"def {name}(self, {', '.join(inargs)}, /) -> {out}: ..." + has_star_args = "*args: hints.Any" in inargs + if has_star_args: + # Cannot have positional-only marker with *args in this way. + # The signature is already a fallback. + content = f"def {name}(self, {', '.join(inargs)}) -> {out}: ..." + else: + # Here, we only need to deal with **kwargs + kwargs_arg = "**kwargs: hints.Any" + if inargs[-1] == kwargs_arg: + kwargs = inargs.pop() + params = ", ".join(inargs) + if params: + content = ( + f"def {name}(self, {params}, /, {kwargs}) -> {out}: ..." + ) + else: + # It is unlikely that there are no parameters before `**kwargs`, + # but we are writing this for the sake of completeness. + content = f"def {name}(self, /, {kwargs}) -> {out}: ..." + else: + content = f"def {name}(self, {', '.join(inargs)}, /) -> {out}: ..." else: content = f"def {name}(self) -> {out}: ..." if keyword.iskeyword(name): From a6533e2f867d3e9cf6f1426be63db5f8823e90b4 Mon Sep 17 00:00:00 2001 From: junkmd Date: Sat, 20 Dec 2025 11:45:18 +0900 Subject: [PATCH 2/2] refactor: Simplify signature generation in `DispMethodAnnotator`. The logic in `DispMethodAnnotator.getvalue` for placing the positional-only argument marker (`/`) was complex. It involved several conditional checks after the main argument list had been constructed. This commit refactors the method to integrate the marker placement directly into the `inargs` construction loop. --- comtypes/tools/codegenerator/typeannotator.py | 43 +++++++------------ 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/comtypes/tools/codegenerator/typeannotator.py b/comtypes/tools/codegenerator/typeannotator.py index 87783672..9dc54f7a 100644 --- a/comtypes/tools/codegenerator/typeannotator.py +++ b/comtypes/tools/codegenerator/typeannotator.py @@ -277,13 +277,23 @@ class DispMethodAnnotator(_MethodAnnotator[typedesc.DispMethod]): def getvalue(self, name: str) -> str: inargs = [] has_optional = False + # NOTE: Since named parameters are not yet implemented, all arguments + # for the dispmethod (called via `Invoke`) are marked as + # positional-only parameters, introduced in PEP570. + # See also `automation.IDispatch.Invoke`. + # See https://github.com/enthought/comtypes/issues/371 for _, argname, default in self.inarg_specs: if keyword.iskeyword(argname): inargs = ["*args: hints.Any", "**kwargs: hints.Any"] break if default is None: if has_optional: + # Required parameter follows an optional one. # probably propput or propputref + # TODO: After named parameters are supported, + # the positional-only parameter markers + # will be removed. + inargs.append("/") # HACK: Something that goes into this conditional branch # should be a special callback. inargs.append("**kwargs: hints.Any") @@ -292,35 +302,14 @@ def getvalue(self, name: str) -> str: else: inargs.append(f"{argname}: hints.Incomplete = ...") has_optional = True + else: + # TODO: After named parameters are supported, the positional-only + # parameter markers will be removed. + if inargs: + inargs.append("/") out = _to_outtype(self.method.returns) - # NOTE: Since named parameters are not yet implemented, all arguments - # for the dispmethod (called via `Invoke`) are marked as positional-only - # parameters, introduced in PEP570. See also `automation.IDispatch.Invoke`. - # See https://github.com/enthought/comtypes/issues/371 - # TODO: After named parameters are supported, the positional-only parameter - # markers will be removed. if inargs: - has_star_args = "*args: hints.Any" in inargs - if has_star_args: - # Cannot have positional-only marker with *args in this way. - # The signature is already a fallback. - content = f"def {name}(self, {', '.join(inargs)}) -> {out}: ..." - else: - # Here, we only need to deal with **kwargs - kwargs_arg = "**kwargs: hints.Any" - if inargs[-1] == kwargs_arg: - kwargs = inargs.pop() - params = ", ".join(inargs) - if params: - content = ( - f"def {name}(self, {params}, /, {kwargs}) -> {out}: ..." - ) - else: - # It is unlikely that there are no parameters before `**kwargs`, - # but we are writing this for the sake of completeness. - content = f"def {name}(self, /, {kwargs}) -> {out}: ..." - else: - content = f"def {name}(self, {', '.join(inargs)}, /) -> {out}: ..." + content = f"def {name}(self, {', '.join(inargs)}) -> {out}: ..." else: content = f"def {name}(self) -> {out}: ..." if keyword.iskeyword(name):