PyCQA / pydocstyle

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

Crash with simple f-string #640

Open kasium opened 1 year ago

kasium commented 1 year ago

Pydocstyle crashes with the below code:

Code: f"bar {123}" Traceback:

  File "venv/bin/pydocstyle", line 8, in <module>
    sys.exit(main())
  File "venv/lib/python3.7/site-packages/pydocstyle/cli.py", line 79, in main
    sys.exit(run_pydocstyle())
  File "venv/lib/python3.7/site-packages/pydocstyle/cli.py", line 54, in run_pydocstyle
    ignore_self_only_init=ignore_self_only_init,
  File "venv/lib/python3.7/site-packages/pydocstyle/checker.py", line 1171, in check
    ignore_self_only_init,
  File "venv/lib/python3.7/site-packages/pydocstyle/checker.py", line 159, in check_source
    self, definition, definition.docstring
  File "venv/lib/python3.7/site-packages/pydocstyle/checker.py", line 244, in check_docstring_empty
    if docstring and is_blank(ast.literal_eval(docstring)):
  File "/root/.pyenv/versions/3.7.2/lib/python3.7/ast.py", line 91, in literal_eval
    return _convert(node_or_string)
  File "/root/.pyenv/versions/3.7.2/lib/python3.7/ast.py", line 90, in _convert
    return _convert_signed_num(node)
  File "/root/.pyenv/versions/3.7.2/lib/python3.7/ast.py", line 63, in _convert_signed_num
    return _convert_num(node)
  File "/root/.pyenv/versions/3.7.2/lib/python3.7/ast.py", line 55, in _convert_num
    raise ValueError('malformed node or string: ' + repr(node))
ValueError: malformed node or string: <_ast.JoinedStr object at 0x7f296ff35a20>

Python 3.7.2 Pydocstyle: 6.3.0

sigmavirus24 commented 1 year ago

Is that the only code in the file? Pydocstyle works fine with f strings on all of my projects.

Further, the error is coming from the standard-library ast parsing library. That means it's not valid python and it wouldn't run anyway so this falls under the principle of

"Garbage in, garbage out"

kasium commented 1 year ago

@sigmavirus24 I drilled it down to that code. And python foo.py returns no error, so it's valid python. You are right, it's from the stdlib but it seems that pydocstyle passes something wrong to literal_eval

kasium commented 1 year ago

@sigmavirus24 the issue is in pydocstyle/checker.py check_docstring_empty. It passes the whole code to ast.literal_eval which is not valid, because literal_eval cannot handled f-strings. I'm happy to open a PR if you want

sigmavirus24 commented 1 year ago

@kasium you misunderstand me. f-strings cannot be documentation strings. Docstrings must be string literals. In other words, pydocstyle has every right - when it encounters something that appears to be a docstring - to attempt to use ast.literal_eval.

A file that has a string and only a string in it, would appear to have a docstring. That docstring being an f-string means that it's a garbage file since f-strings cannot be docstrings.

I think it makes sense that we attempt to catch this and report an error but I don't think the tool is doing anything other than exposing an exception to the user.

kasium commented 1 year ago

@sigmavirus24 I just found #381. Not sure about the process but I'm willing to pick it up since there is no progress since 2020. I guess a new PR makes sense, right?

sigmavirus24 commented 1 year ago

@kasium yes, I think that does make sense. There is some significant review there though about a better way to implement that, so don't feel too attached to what's there if it turns out to slow you down

kasium commented 1 year ago

Okay, let me read it and create a new PR to start with a fresh base. Thanks for your timely answers !