From e86c5ebec311b92348d7901f82973aa52efb0099 Mon Sep 17 00:00:00 2001 From: James Addison Date: Mon, 15 Jul 2024 14:17:23 +0100 Subject: [PATCH 1/7] intersphinx ambiguities: add illustrative test coverage to handle bugreport/false-positive case #12585 --- tests/test_util/intersphinx_data.py | 2 ++ tests/test_util/test_util_inventory.py | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_util/intersphinx_data.py b/tests/test_util/intersphinx_data.py index 889645903dd..95cf80a9b39 100644 --- a/tests/test_util/intersphinx_data.py +++ b/tests/test_util/intersphinx_data.py @@ -59,4 +59,6 @@ ''' + zlib.compress(b'''\ a term std:term -1 glossary.html#term-a-term - A term std:term -1 glossary.html#term-a-term - +b term std:term -1 document.html#id5 - +B term std:term -1 document.html#B - ''') diff --git a/tests/test_util/test_util_inventory.py b/tests/test_util/test_util_inventory.py index 0bdef9f67d9..d01785fda24 100644 --- a/tests/test_util/test_util_inventory.py +++ b/tests/test_util/test_util_inventory.py @@ -53,7 +53,8 @@ def test_ambiguous_definition_warning(warning): f = BytesIO(INVENTORY_V2_AMBIGUOUS_TERMS) InventoryFile.load(f, '/util', posixpath.join) - assert 'contains multiple definitions for std:term:a' in warning.getvalue().lower() + assert 'contains multiple definitions for std:term:a' not in warning.getvalue().lower() + assert 'contains multiple definitions for std:term:b' in warning.getvalue().lower() def _write_appconfig(dir, language, prefix=None): From 96c57978fdcd2183abd26a519fbfb8fc6664c278 Mon Sep 17 00:00:00 2001 From: James Addison Date: Mon, 15 Jul 2024 14:25:18 +0100 Subject: [PATCH 2/7] intersphinx ambiguities: don't warn about pure-duplicate ambiguous definitions in `objects.inv` entries. --- sphinx/util/inventory.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/sphinx/util/inventory.py b/sphinx/util/inventory.py index 55d7efd8396..3e34bdda659 100644 --- a/sphinx/util/inventory.py +++ b/sphinx/util/inventory.py @@ -126,7 +126,7 @@ def load_v2( invdata: Inventory = {} projname = stream.readline().rstrip()[11:] version = stream.readline().rstrip()[11:] - potential_ambiguities = set() + potential_ambiguities = {} actual_ambiguities = set() line = stream.readline() if 'zlib' not in line: @@ -154,11 +154,16 @@ def load_v2( # Some types require case insensitive matches: # * 'term': https://github.com/sphinx-doc/sphinx/issues/9291 # * 'label': https://github.com/sphinx-doc/sphinx/issues/12008 - definition = f"{type}:{name}" - if definition.lower() in potential_ambiguities: - actual_ambiguities.add(definition) + definition, content = f"{type}:{name}", {prio, location, dispname} + lowercase_definition = definition.lower() + if lowercase_definition in potential_ambiguities: + if potential_ambiguities[lowercase_definition] != content: + actual_ambiguities.add(definition) + else: + logger.debug(__("inventory <%s> contains duplicate definitions of %s"), + uri, definition, type='intersphinx', subtype='external') else: - potential_ambiguities.add(definition.lower()) + potential_ambiguities[lowercase_definition] = content if location.endswith('$'): location = location[:-1] + name location = join(uri, location) From b89d09a7d3b4fe10a976c57e51f1892fd8076d9c Mon Sep 17 00:00:00 2001 From: James Addison Date: Mon, 15 Jul 2024 14:46:24 +0100 Subject: [PATCH 3/7] intersphinx ambiguities: resolution: add test coverage. --- tests/test_extensions/test_ext_intersphinx.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index d475c60f9e7..78aefbc9481 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -251,7 +251,11 @@ def test_missing_reference_stddomain(tmp_path, app, status, warning): assert rn.astext() == 'The Julia Domain' -def test_ambiguous_reference_warning(tmp_path, app, warning): +@pytest.mark.parametrize(('term', 'is_ambiguous'), [ + ('A TERM', False), + ('B TERM', True), +]) +def test_ambiguous_reference_handling(term, is_ambiguous, tmp_path, app, warning): inv_file = tmp_path / 'inventory' inv_file.write_bytes(INVENTORY_V2_AMBIGUOUS_TERMS) set_config(app, { @@ -263,10 +267,11 @@ def test_ambiguous_reference_warning(tmp_path, app, warning): load_mappings(app) # term reference (case insensitive) - node, contnode = fake_node('std', 'term', 'A TERM', 'A TERM') + node, contnode = fake_node('std', 'term', term, term) missing_reference(app, app.env, node, contnode) - assert 'multiple matches found for std:term:A TERM' in warning.getvalue() + reported_ambiguous = f'multiple matches found for std:term:{term}' in warning.getvalue() + assert is_ambiguous is reported_ambiguous @pytest.mark.sphinx('html', testroot='ext-intersphinx-cppdomain') From dd86851f967e00457b2150c488d0d4e09553f9c6 Mon Sep 17 00:00:00 2001 From: James Addison Date: Mon, 15 Jul 2024 14:56:33 +0100 Subject: [PATCH 4/7] intersphinx ambiguities: resolution: reduce log message severity for ambiguous-but-duplicate intersphinx resolutions. --- sphinx/ext/intersphinx/_resolve.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/sphinx/ext/intersphinx/_resolve.py b/sphinx/ext/intersphinx/_resolve.py index 0a3cc8949d2..84af20e9017 100644 --- a/sphinx/ext/intersphinx/_resolve.py +++ b/sphinx/ext/intersphinx/_resolve.py @@ -81,10 +81,16 @@ def _resolve_reference_in_domain_by_target( insensitive_matches = list(filter(lambda k: k.lower() == target_lower, inventory[objtype].keys())) if len(insensitive_matches) > 1: + data_items = {inventory[objtype][match] for match in insensitive_matches} inv_descriptor = inv_name or 'main_inventory' - LOGGER.warning(__("inventory '%s': multiple matches found for %s:%s"), - inv_descriptor, objtype, target, - type='intersphinx', subtype='external', location=node) + if len(data_items) == 1: # these are duplicates; relatively innocuous + LOGGER.debug(__("inventory '%s': duplicate matches found for %s:%s"), + inv_descriptor, objtype, target, + type='intersphinx', subtype='external', location=node) + else: + LOGGER.warning(__("inventory '%s': multiple matches found for %s:%s"), + inv_descriptor, objtype, target, + type='intersphinx', subtype='external', location=node) if insensitive_matches: data = inventory[objtype][insensitive_matches[0]] else: From 82934dcbc5426056fabaddd678906cdf76c77f69 Mon Sep 17 00:00:00 2001 From: James Addison Date: Mon, 15 Jul 2024 15:11:11 +0100 Subject: [PATCH 5/7] Simplify: use direct tuple comparison of entries for duplicate-detection (and add corresponding type-hint). --- sphinx/util/inventory.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sphinx/util/inventory.py b/sphinx/util/inventory.py index 3e34bdda659..9a4c45870ad 100644 --- a/sphinx/util/inventory.py +++ b/sphinx/util/inventory.py @@ -126,7 +126,7 @@ def load_v2( invdata: Inventory = {} projname = stream.readline().rstrip()[11:] version = stream.readline().rstrip()[11:] - potential_ambiguities = {} + potential_ambiguities: dict[str, tuple[str, str, str]] = {} actual_ambiguities = set() line = stream.readline() if 'zlib' not in line: @@ -154,7 +154,7 @@ def load_v2( # Some types require case insensitive matches: # * 'term': https://github.com/sphinx-doc/sphinx/issues/9291 # * 'label': https://github.com/sphinx-doc/sphinx/issues/12008 - definition, content = f"{type}:{name}", {prio, location, dispname} + definition, content = f"{type}:{name}", (prio, location, dispname) lowercase_definition = definition.lower() if lowercase_definition in potential_ambiguities: if potential_ambiguities[lowercase_definition] != content: From 929a251c60acfedea23f04213280cc0a9a46fc3f Mon Sep 17 00:00:00 2001 From: James Addison Date: Thu, 18 Jul 2024 12:46:56 +0100 Subject: [PATCH 6/7] Add CHANGES.rst entry. --- CHANGES.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index a77bb761d79..da9dfed4422 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -7,6 +7,10 @@ Bugs fixed * #12859, #9743, #12609: autosummary: Do not add the package prefix when generating autosummary directives for modules within a package. Patch by Adam Turner. +* #12587: Do not warn when potential ambiguity detected during Intersphinx + resolution occurs due to duplicate targets that differ case-insensitively. + Patch by James Addison. + Release 7.4.5 (released Jul 16, 2024) ===================================== From 36f5313e777ddfb5a0137242130d768809bb512b Mon Sep 17 00:00:00 2001 From: James Addison Date: Sun, 11 Aug 2024 15:52:49 +0100 Subject: [PATCH 7/7] Nitpick: refactor variable naming to allow single-line assertion --- tests/test_extensions/test_ext_intersphinx.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index b107db36a70..e4b2c0d2ffe 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -314,13 +314,13 @@ def test_missing_reference_stddomain(tmp_path, app): @pytest.mark.parametrize( - ('term', 'is_ambiguous'), + ('term', 'expected_ambiguity'), [ ('A TERM', False), ('B TERM', True), ], ) -def test_ambiguous_reference_handling(term, is_ambiguous, tmp_path, app, warning): +def test_ambiguous_reference_handling(term, expected_ambiguity, tmp_path, app, warning): inv_file = tmp_path / 'inventory' inv_file.write_bytes(INVENTORY_V2_AMBIGUOUS_TERMS) set_config( @@ -338,10 +338,8 @@ def test_ambiguous_reference_handling(term, is_ambiguous, tmp_path, app, warning node, contnode = fake_node('std', 'term', term, term) missing_reference(app, app.env, node, contnode) - reported_ambiguous = ( - f'multiple matches found for std:term:{term}' in warning.getvalue() - ) - assert is_ambiguous is reported_ambiguous + ambiguity = f'multiple matches found for std:term:{term}' in warning.getvalue() + assert ambiguity is expected_ambiguity @pytest.mark.sphinx('html', testroot='ext-intersphinx-cppdomain')