Merge pull request #9800 from hoefling/extlinks/replacements-check

Proposal: check if hardcoded URLs can be replaced with extlinks

CHANGES

@@ -13,6 +13,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
 * #9831: Autosummary now documents only the members specified in a module's

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`__.
+explicit *label* that acts as :duref:`a target <hyperlink-targets>`.
-
-__ https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#hyperlink-targets
 
 For example, to reference the "Installation" subsection, add a label right
 before the heading, as follows:

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,9 +36,51 @@ from docutils.parsers.rst.states import Inliner
 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
 
+logger = logging.getLogger(__name__)
+
+
+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 = 100
+
+    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']
+
+        for alias, (base_uri, caption) in self.app.config.extlinks.items():
+            uri_pattern = re.compile(base_uri.replace('%s', '(?P<value>.+)'))
+            match = uri_pattern.match(uri)
+            if match and match.groupdict().get('value'):
+                # build a replacement suggestion
+                replacement = f":{alias}:`{match.groupdict().get('value')}`"
+                logger.warning(
+                    __('hardcoded link %r could be replaced by an extlink (try using %r instead)'),
+                    uri,
+                    replacement,
+                    location=refnode,
+                )
+
 
 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
@@ -85,4 +128,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}

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'),
+}

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 <https://github.com/octocat>`_
+
+`replaceable link`_
+
+.. hyperlinks
+
+.. _replaceable link: https://github.com/octocat

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')}

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 <https://github.com/sphinx-doc/sphinx/issues/1>`_
+
+`replaceable link`_
+
+.. hardcoded non-replaceable link
+
+https://github.com/sphinx-doc/sphinx/pulls/1
+
+`inline non-replaceable link <https://github.com/sphinx-doc/sphinx/pulls/1>`_
+
+`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

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