Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider adding rstcheck integration #22

Open
sobolevn opened this issue Mar 8, 2020 · 7 comments
Open

Consider adding rstcheck integration #22

sobolevn opened this issue Mar 8, 2020 · 7 comments

Comments

@sobolevn
Copy link

sobolevn commented Mar 8, 2020

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
Copy link
Owner

peterjc commented Mar 8, 2020

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

@peterjc
Copy link
Owner

peterjc commented Mar 8, 2020

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 added a commit that referenced this issue Mar 8, 2020
Idea here is rstcheck will also validate python
blocks etc, as well as the reStructuredText via
docutils. That would need new error codes:

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

See GitHub issue #22, using the example there:

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

    .. code:: python

        print(

    End.
    """
    pass
@peterjc
Copy link
Owner

peterjc commented Mar 8, 2020

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
Copy link
Owner

peterjc commented Mar 9, 2020

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
Copy link
Owner

peterjc commented Mar 9, 2020

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

@peterjc
Copy link
Owner

peterjc commented Mar 9, 2020

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
Copy link
Owner

peterjc commented Apr 2, 2020

Logged rstcheck/rstcheck#65

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants