extlinks: replacement suggestions for hardcoded links

Signed-off-by: oleg.hoefling <oleg.hoefling@gmail.com>

sphinx/ext/extlinks.py

@@ -25,6 +25,7 @@
     :license: BSD, see LICENSE for details.
 """
 
+import re
 from typing import Any, Dict, List, Tuple
 
 from docutils import nodes, utils
@@ -34,13 +35,54 @@
 import sphinx
 from sphinx.application import Sphinx
 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 = 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<value>.+)'))
+            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
@@ -87,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}

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