Surround Invalid Expressions with single quotes instead of replacing them with an ellipses

[leanderbuerkin]

Hellau,

Thanks for this nice repository to create stubs!

I had an issue: Using '--print-invalid-expressions-as-is' I could not use ast.parse() to postprocess these invalid expression. If Not setting this flag replaces them with ellipses - so also not good.

My suggestion is, to surround the invalid expressions found in type annotations with single quotes.
This way they are represented as ast.Constant() by ast.parse().

I did test it on my own project creating stub files for casadi and it worked flawlessly.

I did not write any tests since I wanted your feedback first. E.g. if Printer.invalid_expr_as_ellipses should be removed, since there is a better option or if surrounding by quotes should even become the default from now on.

Greetings
Leander

[ax3l]

My feeling would be: if this is useful, then maybe make it the default of print_invalid_expressions_as_is and add no extra option for it?

[leanderbuerkin]

Thanks for the Review!

I also think removing --print-invalid-expressions-as-is and --surround-invalid-expressions-with-single-quotes would be the best solution.

I changed the files accordingly. What do you think?

[ax3l]

I think you got my review the wrong way: keep the option, just quote by default?

[leanderbuerkin]

Ah, sorry!

But what is the benefit of keeping the option?

Printing it without quotes is not valid python, so postprocessing is needed.

And if postprocessing is needed anyway, it's either beneficial having the quotes or very easy to remove them.

What do you think?

[ax3l]

I think, but I could be wrong, that this option is used for debugging or flagging in CI that the stubs are incomplete and erroring out...? We could search on GitHub for how people use it so far. I personally have not used it :)

In my use case, I rely sometimes on stubgen to produce partly invalid Python bindings when I messed up binding orders or forgot to instrument one of the arguments of a function. CI catches it quickly and complains. But I do not use the --print-invalid-expressions-as-is flag for this.
This is the workflow I would avoid breaking.

[leanderbuerkin]

Yep, that's right.

I did some research: Although ast.parse() accepts quoted invalid expressions, they are meant as a forward reference.
So surrounding the invalid expression is better than replacing it with an ellipses, but still not perfect for all.

Both solution are not correct, e.g. tuple[int, 'std::str'] should be a tuple with an int and a str, but it is turned into tuple[int, ...] which means an arbitrary amount of integers:
#191 (comment)

The more consistent method seems to be Annotated[Incomplete, CPPType]:
#191 (comment)

But this solution seems to be a bit harder to solve and is work in progress since 2023...

So I would suggest, implementing it this way, since it allows to parse the type afterwards with a simple parser (like in my case) or keep it as some information.

What do you think?

I also cleaned the commit history a bit and pulled again that's why the pull request got closed and reopened.