From 8356260554fee9ebd26a2c11cdf039af36cd951e Mon Sep 17 00:00:00 2001 From: "oleg.hoefling" Date: Sat, 30 Oct 2021 17:57:17 +0200 Subject: [PATCH 1/4] extlinks: replacement suggestions for hardcoded links Signed-off-by: oleg.hoefling --- sphinx/ext/extlinks.py | 45 +++++++++++++++++++ .../conf.py | 5 +++ .../index.rst | 22 +++++++++ .../test-ext-extlinks-hardcoded-urls/conf.py | 2 + .../index.rst | 28 ++++++++++++ tests/test_ext_extlinks.py | 36 +++++++++++++++ 6 files changed, 138 insertions(+) create mode 100644 tests/roots/test-ext-extlinks-hardcoded-urls-multiple-replacements/conf.py create mode 100644 tests/roots/test-ext-extlinks-hardcoded-urls-multiple-replacements/index.rst create mode 100644 tests/roots/test-ext-extlinks-hardcoded-urls/conf.py create mode 100644 tests/roots/test-ext-extlinks-hardcoded-urls/index.rst create mode 100644 tests/test_ext_extlinks.py diff --git a/sphinx/ext/extlinks.py b/sphinx/ext/extlinks.py index 0af335686c4..4791a68ed38 100644 --- a/sphinx/ext/extlinks.py +++ b/sphinx/ext/extlinks.py @@ -26,6 +26,7 @@ """ import warnings +import re from typing import Any, Dict, List, Tuple from docutils import nodes, utils @@ -35,10 +36,53 @@ import sphinx from sphinx.application import Sphinx from sphinx.deprecation import RemovedInSphinx60Warning +from sphinx.locale import __ +from sphinx.transforms.post_transforms import SphinxPostTransform +from sphinx.util import logging from sphinx.util.nodes import split_explicit_title from sphinx.util.typing import RoleFunction +class ExternalLinksChecker(SphinxPostTransform): + """ + For each external link, check if it can be replaced by an extlink. + + We treat each ``reference`` node without ``internal`` attribute as an external link. + """ + + default_priority = 900 + + def run(self, **kwargs: Any) -> None: + for refnode in self.document.traverse(nodes.reference): + self.check_uri(refnode) + + def check_uri(self, refnode: nodes.reference) -> None: + """ + If the URI in ``refnode`` has a replacement in ``extlinks``, + emit a warning with a replacement suggestion. + """ + if 'internal' in refnode or 'refuri' not in refnode: + return + + uri = refnode['refuri'] + lineno = sphinx.util.nodes.get_node_line(refnode) + extlinks_config = getattr(self.app.config, 'extlinks', dict()) + + for alias, (base_uri, caption) in extlinks_config.items(): + uri_pattern = re.compile(base_uri.replace('%s', '(?P.+)')) + match = uri_pattern.match(uri) + if match and match.groupdict().get('value'): + # build a replacement suggestion + replacement = f":{alias}:`{match.groupdict().get('value')}`" + location = (self.env.docname, lineno) + logger.warning( + 'hardcoded link %r could be replaced by an extlink (try using %r instead)', + uri, + replacement, + location=location, + ) + + def make_link_role(name: str, base_url: str, caption: str) -> RoleFunction: # Check whether we have base_url and caption strings have an '%s' for # expansion. If not, fall back the the old behaviour and use the string as @@ -85,4 +129,5 @@ def setup_link_roles(app: Sphinx) -> None: def setup(app: Sphinx) -> Dict[str, Any]: app.add_config_value('extlinks', {}, 'env') app.connect('builder-inited', setup_link_roles) + app.add_post_transform(ExternalLinksChecker) return {'version': sphinx.__display_version__, 'parallel_read_safe': True} diff --git a/tests/roots/test-ext-extlinks-hardcoded-urls-multiple-replacements/conf.py b/tests/roots/test-ext-extlinks-hardcoded-urls-multiple-replacements/conf.py new file mode 100644 index 00000000000..f97077300ad --- /dev/null +++ b/tests/roots/test-ext-extlinks-hardcoded-urls-multiple-replacements/conf.py @@ -0,0 +1,5 @@ +extensions = ['sphinx.ext.extlinks'] +extlinks = { + 'user': ('https://github.com/%s', '@%s'), + 'repo': ('https://github.com/%s', 'project %s'), +} diff --git a/tests/roots/test-ext-extlinks-hardcoded-urls-multiple-replacements/index.rst b/tests/roots/test-ext-extlinks-hardcoded-urls-multiple-replacements/index.rst new file mode 100644 index 00000000000..c8b008ea2b6 --- /dev/null +++ b/tests/roots/test-ext-extlinks-hardcoded-urls-multiple-replacements/index.rst @@ -0,0 +1,22 @@ +test-ext-extlinks-hardcoded-urls +================================ + +.. Links generated by extlinks extension should not raise any warnings. +.. Only hardcoded URLs are affected. + +:user:`octocat` + +:repo:`sphinx-doc/sphinx` + +.. hardcoded replaceable link which can be replaced as +.. :repo:`octocat` or :user:`octocat` + +https://github.com/octocat + +`inline replaceable link `_ + +`replaceable link`_ + +.. hyperlinks + +.. _replaceable link: https://github.com/octocat diff --git a/tests/roots/test-ext-extlinks-hardcoded-urls/conf.py b/tests/roots/test-ext-extlinks-hardcoded-urls/conf.py new file mode 100644 index 00000000000..0fa9f8c76f1 --- /dev/null +++ b/tests/roots/test-ext-extlinks-hardcoded-urls/conf.py @@ -0,0 +1,2 @@ +extensions = ['sphinx.ext.extlinks'] +extlinks = {'issue': ('https://github.com/sphinx-doc/sphinx/issues/%s', 'issue %s')} diff --git a/tests/roots/test-ext-extlinks-hardcoded-urls/index.rst b/tests/roots/test-ext-extlinks-hardcoded-urls/index.rst new file mode 100644 index 00000000000..ada6f07a6d8 --- /dev/null +++ b/tests/roots/test-ext-extlinks-hardcoded-urls/index.rst @@ -0,0 +1,28 @@ +test-ext-extlinks-hardcoded-urls +================================ + +.. Links generated by extlinks extension should not raise any warnings. +.. Only hardcoded URLs are affected. + +:issue:`1` + +.. hardcoded replaceable link + +https://github.com/sphinx-doc/sphinx/issues/1 + +`inline replaceable link `_ + +`replaceable link`_ + +.. hardcoded non-replaceable link + +https://github.com/sphinx-doc/sphinx/pulls/1 + +`inline non-replaceable link `_ + +`non-replaceable link`_ + +.. hyperlinks + +.. _replaceable link: https://github.com/sphinx-doc/sphinx/issues/1 +.. _non-replaceable link: https://github.com/sphinx-doc/sphinx/pulls/1 diff --git a/tests/test_ext_extlinks.py b/tests/test_ext_extlinks.py new file mode 100644 index 00000000000..2be9789f068 --- /dev/null +++ b/tests/test_ext_extlinks.py @@ -0,0 +1,36 @@ +import pytest + + +@pytest.mark.sphinx('html', testroot='ext-extlinks-hardcoded-urls') +def test_replaceable_uris_emit_extlinks_warnings(app, warning): + app.build() + warning_output = warning.getvalue() + # there should be exactly three warnings for replaceable URLs + message = ( + "WARNING: hardcoded link 'https://github.com/sphinx-doc/sphinx/issues/1' " + "could be replaced by an extlink (try using ':issue:`1`' instead)" + ) + assert f"index.rst:11: {message}" in warning_output + assert f"index.rst:13: {message}" in warning_output + assert f"index.rst:15: {message}" in warning_output + + +@pytest.mark.sphinx('html', testroot='ext-extlinks-hardcoded-urls-multiple-replacements') +def test_all_replacements_suggested_if_multiple_replacements_possible(app, warning): + app.build() + warning_output = warning.getvalue() + # there should be six warnings for replaceable URLs, three pairs per link + message = ( + "WARNING: hardcoded link 'https://github.com/octocat' " + "could be replaced by an extlink (try using ':user:`octocat`' instead)" + ) + assert f"index.rst:14: {message}" in warning_output + assert f"index.rst:16: {message}" in warning_output + assert f"index.rst:18: {message}" in warning_output + message = ( + "WARNING: hardcoded link 'https://github.com/octocat' " + "could be replaced by an extlink (try using ':repo:`octocat`' instead)" + ) + assert f"index.rst:14: {message}" in warning_output + assert f"index.rst:16: {message}" in warning_output + assert f"index.rst:18: {message}" in warning_output From 629145a0e8fe2e08e40ad627bb4ee70fb4e1c5f8 Mon Sep 17 00:00:00 2001 From: "oleg.hoefling" Date: Sat, 30 Oct 2021 18:07:17 +0200 Subject: [PATCH 2/4] replace hardcoded refs in docs with extlinks Signed-off-by: oleg.hoefling --- doc/tutorial/narrative-documentation.rst | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/doc/tutorial/narrative-documentation.rst b/doc/tutorial/narrative-documentation.rst index b1f23b0ffd5..a81204d4ca6 100644 --- a/doc/tutorial/narrative-documentation.rst +++ b/doc/tutorial/narrative-documentation.rst @@ -91,9 +91,7 @@ you created earlier. Alternatively, you can also add a cross-reference to an arbitrary part of the project. For that, you need to use the :rst:role:`ref` role, and add an -explicit *label* that acts as `a target`__. - -__ https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#hyperlink-targets +explicit *label* that acts as :duref:`a target `. For example, to reference the "Installation" subsection, add a label right before the heading, as follows: From bc5dd0e6f379c4bc9f442a87225257a03d25d991 Mon Sep 17 00:00:00 2001 From: "oleg.hoefling" Date: Sat, 30 Oct 2021 18:09:03 +0200 Subject: [PATCH 3/4] add changelog entry Signed-off-by: oleg.hoefling --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index b35f126d788..c62d16c1f53 100644 --- a/CHANGES +++ b/CHANGES @@ -12,6 +12,9 @@ Deprecated Features added -------------- + +* #9800: extlinks: Emit warning if a hardcoded link is replaceable + by an extlink, suggesting a replacement. * #9815: html theme: Wrap sidebar components in div to allow customizing their layout via CSS From 7b318d8acb4d982e4d5cdccb69395c6a8519f556 Mon Sep 17 00:00:00 2001 From: Oleg Hoefling Date: Fri, 12 Nov 2021 09:56:16 +0100 Subject: [PATCH 4/4] apply review suggestions Signed-off-by: Oleg Hoefling --- sphinx/ext/extlinks.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/sphinx/ext/extlinks.py b/sphinx/ext/extlinks.py index 4791a68ed38..59c5d9309bd 100644 --- a/sphinx/ext/extlinks.py +++ b/sphinx/ext/extlinks.py @@ -42,6 +42,8 @@ from sphinx.util.nodes import split_explicit_title from sphinx.util.typing import RoleFunction +logger = logging.getLogger(__name__) + class ExternalLinksChecker(SphinxPostTransform): """ @@ -50,7 +52,7 @@ class ExternalLinksChecker(SphinxPostTransform): We treat each ``reference`` node without ``internal`` attribute as an external link. """ - default_priority = 900 + default_priority = 100 def run(self, **kwargs: Any) -> None: for refnode in self.document.traverse(nodes.reference): @@ -65,21 +67,18 @@ def check_uri(self, refnode: nodes.reference) -> None: return uri = refnode['refuri'] - lineno = sphinx.util.nodes.get_node_line(refnode) - extlinks_config = getattr(self.app.config, 'extlinks', dict()) - for alias, (base_uri, caption) in extlinks_config.items(): + for alias, (base_uri, caption) in self.app.config.extlinks.items(): uri_pattern = re.compile(base_uri.replace('%s', '(?P.+)')) match = uri_pattern.match(uri) if match and match.groupdict().get('value'): # build a replacement suggestion replacement = f":{alias}:`{match.groupdict().get('value')}`" - location = (self.env.docname, lineno) logger.warning( - 'hardcoded link %r could be replaced by an extlink (try using %r instead)', + __('hardcoded link %r could be replaced by an extlink (try using %r instead)'), uri, replacement, - location=location, + location=refnode, )