False positive for "--enable default-role" in 0.6.2

Question

False positive for "--enable default-role" in 0.6.2

hugovk opened this issue 2 years ago · 17 comments

$ git clone https://github.com/python/devguide
$ cd devguide
$ python -m pip install sphinx-lint==0.6.1 -q
$ sphinx-lint getting-started/setup-building.rst --enable default-role
No problems found.
$ python -m pip install sphinx-lint==0.6.2 -q
$ sphinx-lint getting-started/setup-building.rst --enable default-role
getting-started/setup-building.rst:28: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:32: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:49: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:50: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:73: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:79: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:93: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:94: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:101: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:122: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:142: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:159: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:166: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:175: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:176: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:198: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:203: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:204: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:205: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:210: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:211: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:212: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:222: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:223: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:226: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:231: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:267: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:288: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:303: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:308: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:313: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:314: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:318: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:323: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:334: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:362: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:379: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:382: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:385: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:404: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:412: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:418: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:444: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:449: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:451: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:452: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:453: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:454: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:456: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:459: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:460: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:461: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:464: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:465: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:466: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:467: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:482: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:483: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:485: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:499: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:512: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:514: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:526: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:556: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:560: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:564: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:567: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:570: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:573: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:577: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:581: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:584: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:587: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:591: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:595: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:600: default role used (hint: for inline literals, use double backticks) (default-role)
getting-started/setup-building.rst:604: default role used (hint: for inline literals, use double backticks) (default-role)

The few of these checked look fine though:

Install ``git``
===============

https://github.com/python/devguide/blame/main/getting-started/setup-building.rst#L28

``Tools``
     Various tools that are (or have been) used to maintain Python.

https://github.com/python/devguide/blame/main/getting-started/setup-building.rst#L604

JulienPalard commented 2 years ago

Oops.

Answer 1 · 2022-10-06T18:38:23.000Z

Same thing happened in the CPython docs in python/cpython#97962 ; the new regex generated over 18 000 false positives and at least in my spot checks, appeared to match on most if not all double backtick inline string literals.

Answer 2 · 2022-10-06T19:09:16.000Z

I tested this on the "friends projects" and got:

$ # with an older version of sphinx-lint
$ sphinx-lint tests/fixtures/friends/ --enable=default-role | wc -l
598
$ # with the latest version
$ sphinx-lint tests/fixtures/friends/ --enable=default-role | wc -l
33604
$ sphinx-lint tests/fixtures/friends/cpython/ --enable=default-role | wc -l
18086
$ sphinx-lint tests/fixtures/friends/devguide/ --enable=default-role | wc -l
865
$ # no issues without --enable=default-role
$ sphinx-lint tests/fixtures/friends/
No problems found.

Answer 3 · 2022-10-06T19:28:50.000Z

I just published v0.6.3 with a fix for this:

$ sphinx-lint --enable default-role devguide/
devguide/testing/coverage.rst:262: default role used (hint: for inline literals, use double backticks) (default-role)
devguide/documentation/style-guide.rst:154: default role used (hint: for inline literals, use double backticks) (default-role)
devguide/internals/parser.rst:836: default role used (hint: for inline literals, use double backticks) (default-role)
devguide/internals/parser.rst:837: default role used (hint: for inline literals, use double backticks) (default-role)
devguide/internals/parser.rst:838: default role used (hint: for inline literals, use double backticks) (default-role)

Answer 4 · 2022-10-06T19:33:24.000Z

@CAM-Gerlach The 0.6.3 version finds 83 default-role (the few of them I checked were true positives) in cpython, I'll try to fix them later.

Answer 5 · 2022-10-06T19:37:40.000Z

Please keep this issue open until we find a proper way to check friend repositories with the right flags (I mean in download-more-tests.sh) to avoid this kind of surprises at release time.

(download-more-tests.sh has been written just so we don't have this kind of surprises at release time...)

Answer 6 · 2022-10-06T19:50:30.000Z

This is a false positive:

$ echo '`issue tracker`_ and submit a :ref:`pull request <pullrequest>`' > buggy.rst
$ sphinx-lint --enable=default-role buggy.rst 
buggy.rst:1: default role used (hint: for inline literals, use double backticks) (default-role)
$ echo '`tracker`_ and submit a :ref:`pull request <pullrequest>`' > buggy.rst
$ sphinx-lint --enable=default-role buggy.rst 
buggy.rst:1: default role used (hint: for inline literals, use double backticks) (default-role)
$ echo '`tracker`_ and submit a :ref:`pull`' > buggy.rst
$ sphinx-lint --enable=default-role buggy.rst 
buggy.rst:1: default role used (hint: for inline literals, use double backticks) (default-role)

$ echo '`issue tracker`_ and submit a' > buggy.rst
$ sphinx-lint --enable=default-role buggy.rst 
No problems found.
$ echo 'and submit a :ref:`pull request <pullrequest>`' > buggy.rst
$ sphinx-lint --enable=default-role buggy.rst 
No problems found.

It fails when a `link`_ is followed by a :role:`text`.

Answer 7 · 2022-10-06T20:08:13.000Z

Another one:

$ cat buggy.rst 
Instead, these security concerns should be gathered into a dedicated
"Security Considerations" section within the module's documentation, and
cross-referenced from the documentation of affected interfaces with a note
similar to ``"Please refer to the :ref:`security-considerations` section
for important information on how to avoid common mistakes."``.

Similarly, if there is a common error that affects many interfaces in a
$ sphinx-lint --enable=default-role buggy.rst 
buggy.rst:4: default role used (hint: for inline literals, use double backticks) (default-role)
$ echo 'A ref in a literal looks like: ``foo :ref:`bar` baz``.' > buggy.rst 
$ sphinx-lint --enable=default-role buggy.rst 
No problems found.

The issue seem to be related to the multiline literal that contains a pair of `...` -- it works fine when it's on a single line.

Answer 8 · 2022-10-06T21:19:05.000Z

A false positive multiline with ellipsis:

:class:`subprocess.Popen` destructor now emits a :exc:`ResourceWarning` warning
if the child process is still running. Use the context manager protocol (``with
proc: ...``) or explicitly call the :meth:`~subprocess.Popen.wait` method to
read the exit status of the child process. (Contributed by Victor Stinner in
:issue:`26741`.)

1.rst:3: default role used (hint: for inline literals, use double backticks) (default-role)

And multiline without ellipsis:

A new :data:`~html.entities.html5` dictionary that maps HTML5 named character
references to the equivalent Unicode character(s) (e.g. ``html5['gt;'] ==
'>'``) has been added to the :mod:`html.entities` module.  The dictionary is
now also used by :class:`~html.parser.HTMLParser`.  (Contributed by Ezio
Melotti in :issue:`11113` and :issue:`15156`.)

1.rst:3: default role used (hint: for inline literals, use double backticks) (default-role)

Answer 9 · 2022-10-06T21:35:41.000Z

It's not recognising the :envvar: role, due to the other opening and closing double backticks being on another line:

   * ``-X importtime`` to show how long each import takes. It shows module
     name, cumulative time (including nested imports) and self time (excluding
     nested imports).  Note that its output may be broken in multi-threaded
     application.  Typical usage is ``python3 -X importtime -c 'import
     asyncio'``.  See also :envvar:`PYTHONPROFILEIMPORTTIME`.

1.rst:5: default role used (hint: for inline literals, use double backticks) (default-role)

For example, no complaints about this:

   * ``-X importtime`` to show how long each import takes. It shows module
     name, cumulative time (including nested imports) and self time (excluding
     nested imports).  Note that its output may be broken in multi-threaded
     application.  Typical usage is python3 -X importtime -c 'import
     asyncio'.  See also :envvar:`PYTHONPROFILEIMPORTTIME`.

(As it happens, PyCharm has a similar bug)

Answer 10 · 2022-10-06T21:41:00.000Z

Wow thanks all for your examples!

I just released v0.6.4 that should fix all of them, can you try it?

Answer 11 · 2022-10-06T22:11:18.000Z

A clean run on the PEP repo comes back green ✔️

FWIW, maybe a little too late now, but the pre-commit pygrep check uses ^(?! ).*(^| )`[^`]+`([^_]|$), and while I don't know about false negatives, I've very rarely seen false positives with it.

Answer 12 · 2022-10-06T23:50:30.000Z

All good with 0.6.4 and 0.6.5! Thanks for the quick fixes and releases!

Answer 13 · 2022-10-06T23:50:32.000Z

v0.6.5 was released too, and it fixes this:

$ cat buggy.rst
.. this is a comment
   this is not a `default role`
$ sphinx-lint --enable=default-role buggy.rst 
buggy.rst:2: default role used (hint: for inline literals, use double backticks) (default-role)

Answer 14 · 2022-10-07T00:33:06.000Z

Thanks for your hard work at short notice, @JulienPalard @ezio-melotti and @hugovk .

Answer 15 · 2022-10-07T00:55:57.000Z

Just to note, I can confirm that #34 (false positive when last character inside a verbatim is `:`` is still present and reproducible on the PEPs repo with Sphinx-Lint 0.6.5, so we still can't enable this flag for now, unfortunately.

Answer 16 · 2022-10-07T07:52:33.000Z

I'm closing this issue as I think all reported false positives are now fixed, don't hesitate to report new ones as new issues.

We were having a script to fetch "friend repos" (cpython, sphinx, ...) to test them before relasing to avoid this exact issue, but it was just testing with the default flags, not with --enable default-role. Now each repo is tested with its own set of flags.