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

bpo-42238: Check Misc/NEWS.d/next/ for reStructuredText issues. #23802

Merged
merged 3 commits into from
Dec 18, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Doc/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ dist:

check:
$(PYTHON) tools/rstlint.py -i tools -i $(VENVDIR) -i README.rst
$(PYTHON) tools/rstlint.py ../Misc/NEWS.d/next/

serve:
$(PYTHON) ../Tools/scripts/serve.py build/html
Expand Down
61 changes: 61 additions & 0 deletions Doc/tools/rstlint.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import re
import sys
import getopt
from string import ascii_letters
from os.path import join, splitext, abspath, exists
from collections import defaultdict

Expand Down Expand Up @@ -128,6 +129,66 @@ def check_leaked_markup(fn, lines):
yield lno+1, 'possibly leaked markup: %r' % line


def hide_literal_blocks(lines):
"""Tool to remove literal blocks from given lines.

It yields empty lines in place of blocks, so line numbers are
still meaningfull.
JulienPalard marked this conversation as resolved.
Show resolved Hide resolved
"""
in_block = False
for line in lines:
if line.endswith("::\n"):
in_block = True
elif in_block:
if line == "\n" or line.startswith(" "):
line = "\n"
else:
in_block = False
yield line

def hide_comments(lines):
"""Tool to remove comments from given lines.

It yields empty lines in place of comments, so line numbers are
still meaningfull.
"""
in_multiline_comment = False
for line in lines:
if line == "..\n":
in_multiline_comment = True
elif in_multiline_comment:
if line == "\n" or line.startswith(" "):
line = "\n"
else:
in_multiline_comment = False
if line.startswith(".. "):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry if I'm misinterpreting something here, but wouldn't this remove directives like .. describes:: or .. class:: too? Or is that intentional?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're understanding it correctly, it removes a bit too much. But the documentation tells comments are Every explicit markup block which isn’t a valid markup construct.

an explicit markup is:

An explicit markup block begins with a line starting with .. followed by whitespace and is terminated by the next paragraph at the same level of indentation.

According to the ref, explicite markup are

used for footnotes, citations, hyperlink targets, directives, substitution definitions, and comments

in directives there's potentially a lot, as it's possible to add new directives, like versionchanged in our doc.

So we have like three choices:

  • Keep it simple and introduce some false negatives (current version)
  • Do it right with no false positives and no false negative, it's probably doable as we have the exhaustive list of directives already hardcoded in rstlint.py
  • Keep it simple, the other way around, and introduce some false positives.

I'm really not in favor of false positives: it's a burden for contributors. False negatives are OK though: if an rst error is introduced it'll still fixable later.

I'll try myself at the "do it right version" today to see if it's worth it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was not that hard, probably worth it, I pushed it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your patience and explaining it to me :).

line = "\n"
yield line



@checker(".rst", severity=2)
def check_missing_surrogate_space_on_plural(fn, lines):
r"""Check for missing 'backslash-space' between a code sample a letter.

Good: ``Point``\ s
Bad: ``Point``s
"""
in_code_sample = False
check_next_one = False
for lno, line in enumerate(hide_comments(hide_literal_blocks(lines))):
tokens = line.split("``")
for token_no, token in enumerate(tokens):
if check_next_one:
if token[0] in ascii_letters:
yield lno + 1, f"Missing backslash-space between code sample and {token!r}."
check_next_one = False
if token_no == len(tokens) - 1:
continue
if in_code_sample:
check_next_one = True
in_code_sample = not in_code_sample

def main(argv):
usage = '''\
Usage: %s [-v] [-f] [-s sev] [-i path]* [path]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
The ``__args__`` of the parameterized generics for :data:`typing.Callable`
and :class:`collections.abc.Callable` are now consistent. The ``__args__``
for :class:`collections.abc.Callable` are now flattened while
:data:`typing.Callable`'s have not changed. To allow this change,
:class:`types.GenericAlias` can now be subclassed and
and :class:`collections.abc.Callable` are now consistent. The ``__args__``
for :class:`collections.abc.Callable` are now flattened while
:data:`typing.Callable`'s have not changed. To allow this change,
:class:`types.GenericAlias` can now be subclassed and
``collections.abc.Callable``'s ``__class_getitem__`` will now return a subclass
of ``types.GenericAlias``. Tests for typing were also updated to not subclass
of ``types.GenericAlias``. Tests for typing were also updated to not subclass
things like ``Callable[..., T]`` as that is not a valid base class. Finally,
both ``Callable``\ s no longer validate their ``argtypes``, in
``Callable[[argtypes], resulttype]`` to prepare for :pep:`612`. Patch by Ken Jin.

both ``Callable``\ s no longer validate their ``argtypes``, in
``Callable[[argtypes], resulttype]`` to prepare for :pep:`612`. Patch by Ken Jin.
Original file line number Diff line number Diff line change
@@ -1 +1 @@
fix `format()` behavior for `IntFlag`
fix ``format()`` behavior for ``IntFlag``
Original file line number Diff line number Diff line change
@@ -1 +1 @@
`Enum`: call `__init_subclass__` after members have been added
``Enum``: call ``__init_subclass__`` after members have been added
Original file line number Diff line number Diff line change
@@ -1 +1 @@
[tarfile] update nested exception raising to use `from None` or `from e`
[tarfile] update nested exception raising to use ``from None`` or ``from e``
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
`logging.disable` will now validate the types and value of its parameter. It
also now accepts strings representing the levels (as does `loging.setLevel`)
``logging.disable`` will now validate the types and value of its parameter. It
also now accepts strings representing the levels (as does ``loging.setLevel``)
instead of only the numerical values.