pydocstyle icon indicating copy to clipboard operation
pydocstyle copied to clipboard

Fix crash when using f-strings

Open kasium opened this issue 1 year ago • 3 comments

Closes #640

Thanks for submitting a PR!

Please make sure to check for the following items:

  • [X] Add unit tests and integration tests where applicable.
    If you've added an error code or changed an error code behavior, you should probably add or change a test case file under tests/test_cases/ and add it to the list under tests/test_definitions.py.
    If you've added or changed a command line option, you should probably add or change a test in tests/test_integration.py.
  • [x] Add a line to the release notes (docs/release_notes.rst) under "Current Development Version".
    Make sure to include the PR number after you open and get one.

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

kasium avatar Sep 13 '23 20:09 kasium

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

sigmavirus24 avatar Sep 13 '23 20:09 sigmavirus24

@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

kasium avatar Sep 13 '23 20:09 kasium

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

sigmavirus24 avatar Sep 13 '23 21:09 sigmavirus24