DocTestFinder.find fails if module contains class objects with special characters

[gertjanvanzwieten]

Bug report

Checklist

• I am confident this is a bug in CPython, not a bug in a third-party project
• I have searched the CPython issue tracker, and am confident this bug has not been reported before

A clear and concise description of the bug

Doctests initialization may fail with re.error in case a module contains (dynamically generated) class objects with special characters in their name. This is caused by the regular expression compiled in doctest.DocTestFinder._find_lineno failing to escape the class name before embedding it. If compilation does succeed, then invalid matches may result from interpreting the class name as a search pattern.

Your environment

• CPython versions tested on: 3.9.2
• Operating system and architecture: Debian linux amd64

Linked PRs

• gh-107715: Escape class name in regular expression #107716
• [3.12] gh-107715: Escape class name in regular expression (GH-107716) #107726
• [3.11] gh-107715: Escape class name in regular expression (GH-107716) #107727

[serhiy-storchaka]

Did this happen in the real world?

Your PR looks good to me, but there may be a better solution of this problem. If the class was dynamically generated, the class defining statement cannot be found in the source code. It is better to not even try to search.

BTW, re.escape() will fail if __name__ is not a string, so there is small chance of regression.

[gertjanvanzwieten]

I ran into this for our physical units framework, in which dimensions are represented by classes and quantities by instances thereof, and operations on the quantities result in dynamically created classes representing the resulting dimension. The special characters in the automatically generated names represent the operation: e.g. L*M/T2 for force. While we could use any other naming scheme to avoid triggering this issue, we prefer to keep the current one for readability during live inspection.

It is true that the line number will not be found. This is fine, we just need to get rid of the exception. Avoiding the search altogether would be equally fine if there is a way to identify dynamically created classes. However, we though that embedding an unescaped string in a regular expression is considered bad practice in general, which would suggest that the proposed fix is an improvement regardless. I see the issue with __name__ potentially being not a string, but I am not aware of a mechanism for creating this situation while still testing true for inspect.isclass.

[serhiy-storchaka]

I agree that embedding an unescaped and not validated string in a regular expression is not a good practice. It could be validated with str.isidentifier(), but more complex code would need tests. Also the surrounding code cries for refactoring. I cannot require it from you.

Thank you for your contribution @gertjanvanzwieten.

[gertjanvanzwieten]

Thanks for merging!

[terryjreedy]

The danger of closing before the backports are merged is that the backports may fail for weird reasons. The CLA bot failed again on both backports. Gertjan, try reclicking as you did before. I added a note about the flakey bot to the coredev discord.

[gertjanvanzwieten]

Done!