Fix AST safety check false negative#4270
Conversation
Fixes psf#4268 Previously we would allow whitespace changes in all strings, now only in docstrings.
for more information, see https://pre-commit.ci
|
Seems like we got unlucky and someone added 3.12+ syntax to a diff-shades project just two hours ago (koaning/scikit-lego@64485a9#diff-db41f509a7991f7b7579a72fe7bdba315b5ab82bdde2f237c6a53d03396c0081R97 line 97). |
hauntsaninja
left a comment
There was a problem hiding this comment.
Thanks, two small things
Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
Fixes psf#4268 Previously we would allow whitespace changes in all strings, now only in docstrings. Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
| return code | ||
|
|
||
|
|
||
| class ASTSafetyError(Exception): |
There was a problem hiding this comment.
This should probably inherit from AssertionError since you are replacing raise AssertionError with this new exception.
There was a problem hiding this comment.
We don't provide compatibility guarantees for this function. I don't think inheriting from AssertionError makes semantic sense.
There was a problem hiding this comment.
Thanks for the response. That makes sense considering that many people probably don't use this functionality directly and I maybe agree inheriting from AssertionError is odd from a semantic/purity standpoint, even if it's less practical for compatibility (which as you mention is a non-goal here).
…no longer raises an AssertionError
…no longer raises an AssertionError
Fixes #4268
Previously we would allow whitespace changes in all strings, now
only in docstrings.