peterjc / flake8-rst-docstrings

flake8 plugin to validate Python docstrings as reStructuredText (RST)
MIT License
55 stars 8 forks source link

Consider adding rstcheck integration #22

Open sobolevn opened 4 years ago

sobolevn commented 4 years ago

Currently this code passes our CI:

def some():
    """
    ====
    Test
    ====

    .. code:: python

        print(

    End.
    """

But, the example is clearly invalid. It should not pass the CI in my opinion.

There's a tool for that: https://github.com/myint/rstcheck Maybe we can integrate this tool into flake8-rst-docstrings?

If it is not aligning nicely with the goal of this project, I will consider adding it directly into https://github.com/wemake-services/wemake-python-styleguide

peterjc commented 4 years ago

Interesting. I see rstcheck also wraps docutils, so this might would out...

peterjc commented 4 years ago

Looks like in theory we could call their rstcheck.check() function, it does not give column numbers which is a shame - but we didn't have that anyway. This could potentially be a big simplification to the current code base (which has large chunks of ported code from pydocstyle/parser.py etc).

peterjc commented 4 years ago

Proof of principle - doesn't save any code, in fact adds a bit to parse the rstcheck error messages as strings (which can be done more elegantly):

https://github.com/peterjc/flake8-rst-docstrings/tree/rstcheck

The RST303 test case fails with:

No directive entry for "req" in module "docutils.parsers.rst.languages.en".

The RST304 test case fails with:

No role entry for "need" in module "docutils.parsers.rst.languages.en".

May need to tweak https://github.com/peterjc/flake8-rst-docstrings/blob/master/tests/RST303/sphinx-directives.py and https://github.com/peterjc/flake8-rst-docstrings/blob/master/tests/RST304/sphinx-roles.py

Anyway, your example gives:

$ flake8 --select RST some_python.py 
some_python.py:8:1: RST999 Unexpected prefix: '(python) unexpected EOF while parsing'

Obviously would need to define a bunch of new flake8 codes - perhaps RST801 for python, RST802 for bash, etc (just one code per code block language, since otherwise would be very open ended):

https://github.com/myint/rstcheck#supported-languages-in-code-blocks

peterjc commented 4 years ago

It seems for some reason rstcheck returns the unknown directive and role messages with a different level (info not error) and different string, which is relatively easy to handle as a special case to keep the existing RST303 and RST304 behaviour.

@sobolevn I take it from your github 'reactions' that you like the way this is going in terms of additional functionality. Any thoughts on how best to assign new RST### codes? Is a single code for any python block fine?

peterjc commented 4 years ago

Might have to add rstcheck setting ignore-language to the plugin too?

peterjc commented 4 years ago

Reading the code, rstcheck unconditionally calls internal function ignore_sphinx to ignore Sphinx roles and directives - which would be a change in behaviour for this plugin. See #16.

Of course, most people using RST and linting it probably use Sphinx, but not everyone will.

I suppose we could ask rstcheck to make the Sphinx ignores a configurable option?

peterjc commented 4 years ago

Logged https://github.com/myint/rstcheck/issues/65