PyCQA / pydocstyle

docstring style checker
http://pydocstyle.org
MIT License
1.11k stars 189 forks source link

Fix crash when using f-strings #653

Open kasium opened 1 year ago

kasium commented 1 year ago

Closes #640

Thanks for submitting a PR!

Please make sure to check for the following items:

Please don't get discouraged as it may take a while to get a review.

sigmavirus24 commented 1 year ago

This prevents the crash but it misleads people and will lead to big reports of "but there's a docstring there" because it doesn't inform people that the f string is invalid and cannot be a docstring. I would not accept this PR as is

kasium commented 1 year ago

@sigmavirus24 Reason for this is that in https://github.com/PyCQA/pydocstyle/pull/381 the following was suggested

Mistakenly using an f-string for a docstring does not cause a syntax error, so we shouldn't raise a ParseError here. Instead, I suggest we rename this as eval_docstring and just return an empty string if we find an f-string.

I'm fine adding a new error code saying that an f-string is invalid, but this idea was dropped before

sigmavirus24 commented 1 year ago

The idea for adding a new error code was not dropped. The PR lost steam due to people being busy. The new error code is correct behavior. How we achieve that is up for debate