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

Proposal: check if hardcoded URLs can be replaced with extlinks #9800

Merged
merged 4 commits into from Nov 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGES
Expand Up @@ -12,6 +12,9 @@ Deprecated

Features added
--------------

* #9800: extlinks: Emit warning if a hardcoded link is replaceable
by an extlink, suggesting a replacement.
hoefling marked this conversation as resolved.
Show resolved Hide resolved

* #9815: html theme: Wrap sidebar components in div to allow customizing their
layout via CSS
Expand Down
4 changes: 1 addition & 3 deletions doc/tutorial/narrative-documentation.rst
Expand Up @@ -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 <hyperlink-targets>`.

For example, to reference the "Installation" subsection, add a label right
before the heading, as follows:
Expand Down
44 changes: 44 additions & 0 deletions sphinx/ext/extlinks.py
Expand Up @@ -26,6 +26,7 @@
"""

import warnings
import re
from typing import Any, Dict, List, Tuple

from docutils import nodes, utils
Expand All @@ -35,9 +36,51 @@
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
Expand Down Expand Up @@ -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}
@@ -0,0 +1,5 @@
extensions = ['sphinx.ext.extlinks']
extlinks = {
'user': ('https://github.com/%s', '@%s'),
'repo': ('https://github.com/%s', 'project %s'),
}
@@ -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
2 changes: 2 additions & 0 deletions 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')}
28 changes: 28 additions & 0 deletions 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
36 changes: 36 additions & 0 deletions 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