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

Intersphinx lookups: log warnings for ambiguous target definitions and resolutions. #12329

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
c65b877
Intersphinx lookups: log warning for ambiguous target resolution.
jayaddison Apr 25, 2024
fe3a647
Refactor: use existing shared Intersphinx logger.
jayaddison Apr 26, 2024
57099f1
Code review feedback: include type, subtype and location in warning.
jayaddison Apr 26, 2024
c992329
Cleanup: remove unused import from test module.
jayaddison Apr 26, 2024
d9417e0
Tests: refactor: extract INVENTORY_V2_AMBIGUOUS_TERMS fixture data.
jayaddison Apr 26, 2024
c07c799
Merge branch 'refs/heads/master' into pr-12033-follow/intersphinx-amb…
AA-Turner Apr 29, 2024
436297c
Allow translation
AA-Turner Apr 29, 2024
ea7349b
Cleanup: remove redundant domain.name from log message line.
jayaddison Apr 29, 2024
72b266d
Intersphinx lookups: add origin-hyperlink to log warnings about ambig…
jayaddison Apr 29, 2024
4935023
Revert "Intersphinx lookups: add origin-hyperlink to log warnings abo…
jayaddison Apr 29, 2024
ab78182
Intersphinx lookups: log inventory name at both load-time and ambiguo…
jayaddison Apr 29, 2024
88a338a
Intersphinx inventories: add loading-time warning when ambiguous name…
jayaddison Apr 29, 2024
8b51aaf
linting: resolve type-inconsistency warnings by returning an empty se…
jayaddison Apr 29, 2024
897e268
linting: cleanup: remove unused variable.
jayaddison Apr 29, 2024
7f0b375
Fixup: include type namespace when checking for ambiguities.
jayaddison Apr 29, 2024
4d61b14
Revert "linting: cleanup: remove unused variable."
jayaddison Apr 29, 2024
330c2eb
Merge branch 'master' into pr-12033-follow/intersphinx-ambiguous-look…
jayaddison Apr 29, 2024
15d2416
Add CHANGES.rst entry.
jayaddison May 4, 2024
f10c9ca
Merge branch 'master' into pr-12033-follow/intersphinx-ambiguous-look…
jayaddison May 9, 2024
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
5 changes: 5 additions & 0 deletions sphinx/ext/intersphinx/_resolve.py
Expand Up @@ -15,6 +15,7 @@
from sphinx.ext.intersphinx._shared import LOGGER, InventoryAdapter
from sphinx.locale import _, __
from sphinx.transforms.post_transforms import ReferencesResolver
from sphinx.util import logging
from sphinx.util.docutils import CustomReSTDispatcher, SphinxRole

if TYPE_CHECKING:
Expand All @@ -30,6 +31,8 @@
from sphinx.environment import BuildEnvironment
from sphinx.util.typing import Inventory, InventoryItem, RoleFunction

logger = logging.getLogger(__name__)


def _create_element_from_result(domain: Domain, inv_name: str | None,
data: InventoryItem,
Expand Down Expand Up @@ -80,6 +83,8 @@
target_lower = target.lower()
insensitive_matches = list(filter(lambda k: k.lower() == target_lower,
inventory[objtype].keys()))
if len(insensitive_matches) > 1:
logger.warning(f"{domain.name}: multiple matches found for {objtype}:{target}")

Check failure on line 87 in sphinx/ext/intersphinx/_resolve.py

View workflow job for this annotation

GitHub Actions / ruff

Ruff (G004)

sphinx/ext/intersphinx/_resolve.py:87:32: G004 Logging statement uses f-string
if insensitive_matches:
data = inventory[objtype][insensitive_matches[0]]
else:
Expand Down
19 changes: 19 additions & 0 deletions tests/test_extensions/test_ext_intersphinx.py
Expand Up @@ -2,6 +2,7 @@

import http.server
from unittest import mock
from warnings import catch_warnings

Check failure on line 5 in tests/test_extensions/test_ext_intersphinx.py

View workflow job for this annotation

GitHub Actions / ruff

Ruff (F401)

tests/test_extensions/test_ext_intersphinx.py:5:22: F401 `warnings.catch_warnings` imported but unused

import pytest
from docutils import nodes
Expand Down Expand Up @@ -247,6 +248,24 @@
assert rn.astext() == 'The Julia Domain'


def test_ambiguous_reference_warning(tmp_path, app, warning):
inv_file = tmp_path / 'inventory'
inv_file.write_bytes(INVENTORY_V2)
set_config(app, {
'cmd': ('https://docs.python.org/', str(inv_file)),
})

# load the inventory
normalize_intersphinx_mapping(app, app.config)
load_mappings(app)

# term reference (case insensitive)
node, contnode = fake_node('std', 'term', 'A TERM', 'A TERM')
missing_reference(app, app.env, node, contnode)

assert 'multiple matches found for std:term:A TERM' in warning.getvalue()


@pytest.mark.sphinx('html', testroot='ext-intersphinx-cppdomain')
def test_missing_reference_cppdomain(tmp_path, app, status, warning):
inv_file = tmp_path / 'inventory'
Expand Down
1 change: 1 addition & 0 deletions tests/test_util/intersphinx_data.py
Expand Up @@ -32,6 +32,7 @@
foons cpp:type 1 index.html#foons -
foons::bartype cpp:type 1 index.html#foons_bartype -
a term std:term -1 glossary.html#term-a-term -
A term std:term -1 glossary.html#term-a-term -
jayaddison marked this conversation as resolved.
Show resolved Hide resolved
ls.-l std:cmdoption 1 index.html#cmdoption-ls-l -
docname std:doc -1 docname.html -
foo js:module 1 index.html#foo -
Expand Down