From 4c3363f9c597b6d392d00650afc49562a3dedfe1 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Sat, 23 Mar 2024 23:20:30 +0100 Subject: [PATCH 01/11] =?UTF-8?q?=F0=9F=91=8C=20Make=20`external`=20role?= =?UTF-8?q?=20warnings=20more=20helpful=20(revert=20object=20fallback)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- sphinx/ext/intersphinx.py | 180 ++++++++++++------ .../roots/test-ext-intersphinx-role/index.rst | 6 +- tests/test_extensions/test_ext_intersphinx.py | 8 +- 3 files changed, 130 insertions(+), 64 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index abc596f998b..cbf76d2e66f 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -34,7 +34,6 @@ import sphinx from sphinx.addnodes import pending_xref from sphinx.builders.html import INVENTORY_FILENAME -from sphinx.errors import ExtensionError from sphinx.locale import _, __ from sphinx.transforms.post_transforms import ReferencesResolver from sphinx.util import logging, requests @@ -533,17 +532,122 @@ def run(self) -> tuple[list[Node], list[system_message]]: assert self.name == self.orig_name.lower() inventory, name_suffix = self.get_inventory_and_name_suffix(self.orig_name) if inventory and not inventory_exists(self.env, inventory): - logger.warning(__('inventory for external cross-reference not found: %s'), - inventory, location=(self.env.docname, self.lineno)) + logger.warning( + __('inventory for external cross-reference not found: %s'), + inventory, + type='intersphinx', + subtype='external', + location=(self.env.docname, self.lineno), + ) return [], [] - role_name = self.get_role_name(name_suffix) + domain_name, role_name = self._get_domain_role(name_suffix) + if role_name is None: - logger.warning(__('role for external cross-reference not found: %s'), name_suffix, - location=(self.env.docname, self.lineno)) + logger.warning( + __('external cross-reference suffix not valid: %s'), + name_suffix, + type='intersphinx', + subtype='external', + location=(self.env.docname, self.lineno), + ) return [], [] - result, messages = self.invoke_role(role_name) + # attempt to find a matching role function + role_func: None | RoleFunction + + if domain_name is not None: + # the user specified a domain, so we only check that + if (domain := self.env.get_domain(domain_name)) is None: + logger.warning( + __('domain for external cross-reference not found: %s'), + domain_name, + type='intersphinx', + subtype='external', + location=(self.env.docname, self.lineno), + ) + return [], [] + if (role_func := domain.roles.get(role_name)) is None: + msg = 'role for external cross-reference not found in domain %s: %s' + if ( + object_types := domain.object_types.get(role_name) + ) is not None and object_types.roles: + logger.warning( + __(f'{msg} (perhaps you meant one of: %s)'), + domain_name, + role_name, + ','.join(sorted(object_types.roles)), + type='intersphinx', + subtype='external', + location=(self.env.docname, self.lineno), + ) + else: + logger.warning( + __(msg), + domain_name, + role_name, + type='intersphinx', + subtype='external', + location=(self.env.docname, self.lineno), + ) + return [], [] + + else: + # the user did not specify a domain, + # so we check first the default (if available) then standard domains + domains: list[Domain] = [] + if default_domain := self.env.temp_data.get('default_domain'): + domains.append(default_domain) + if ( + std_domain := self.env.get_domain('std') + ) is not None and std_domain not in domains: + domains.append(std_domain) + + role_func = None + for domain in domains: + if (role_func := domain.roles.get(role_name)) is not None: + domain_name = domain.name + break + + if role_func is None or domain_name is None: + domains_str = ','.join([d.name for d in domains]) + msg = 'role for external cross-reference not found in domains %s: %s' + possible_roles: set[str] = set() + for d in domains: + if o := d.object_types.get(role_name): + possible_roles.update(o.roles) + if possible_roles: + msg += ' (perhaps you meant one of: %s)' + logger.warning( + __(msg), + domains_str, + role_name, + ','.join(sorted(possible_roles)), + type='intersphinx', + subtype='external', + location=(self.env.docname, self.lineno), + ) + else: + logger.warning( + __(msg), + domains_str, + role_name, + type='intersphinx', + subtype='external', + location=(self.env.docname, self.lineno), + ) + return [], [] + + result, messages = role_func( + f"{domain_name}:{role_name}", + self.rawtext, + self.text, + self.lineno, + self.inliner, + self.options, + self.content, + ) + for node in result: if isinstance(node, pending_xref): node['intersphinx'] = True @@ -573,66 +677,20 @@ def get_inventory_and_name_suffix(self, name: str) -> tuple[str | None, str]: msg = f'Malformed :external: role name: {name}' raise ValueError(msg) - def get_role_name(self, name: str) -> tuple[str, str] | None: - """Find (if any) the corresponding ``(domain, role name)`` for *name*. + def _get_domain_role(self, name: str) -> tuple[str | None, str | None]: + """Convert the *name* string into a domain and a role name. - The *name* can be either a role name (e.g., ``py:function`` or ``function``) - given as ``domain:role`` or ``role``, or its corresponding object name - (in this case, ``py:func`` or ``func``) given as ``domain:objname`` or ``objname``. - - If no domain is given, or the object/role name is not found for the requested domain, - the 'std' domain is used. + - If *name* contains no ``:``, return ``(None, name)``. + - If *name* contains a single ``:``, the domain/role is split on this. + - If *name* contains multiple ``:``, return ``(None, None)``. """ names = name.split(':') if len(names) == 1: - default_domain = self.env.temp_data.get('default_domain') - domain = default_domain.name if default_domain else None - name = names[0] + return None, names[0] elif len(names) == 2: - domain = names[0] - name = names[1] - else: - return None - - if domain and (role := self.get_role_name_from_domain(domain, name)): - return (domain, role) - elif (role := self.get_role_name_from_domain('std', name)): - return ('std', role) - else: - return None - - def is_existent_role(self, domain_name: str, role_or_obj_name: str) -> bool: - """Check if the given role or object exists in the given domain.""" - return self.get_role_name_from_domain(domain_name, role_or_obj_name) is not None - - def get_role_name_from_domain(self, domain_name: str, role_or_obj_name: str) -> str | None: - """Check if the given role or object exists in the given domain, - and return the related role name if it exists, otherwise return None. - """ - try: - domain = self.env.get_domain(domain_name) - except ExtensionError: - return None - if role_or_obj_name in domain.roles: - return role_or_obj_name - if ( - (role_name := domain.role_for_objtype(role_or_obj_name)) - and role_name in domain.roles - ): - return role_name - return None - - def invoke_role(self, role: tuple[str, str]) -> tuple[list[Node], list[system_message]]: - """Invoke the role described by a ``(domain, role name)`` pair.""" - domain = self.env.get_domain(role[0]) - if domain: - role_func = domain.role(role[1]) - assert role_func is not None - - return role_func(':'.join(role), self.rawtext, self.text, self.lineno, - self.inliner, self.options, self.content) + return names[0], names[1] else: - return [], [] + return None, None class IntersphinxRoleResolver(ReferencesResolver): diff --git a/tests/roots/test-ext-intersphinx-role/index.rst b/tests/roots/test-ext-intersphinx-role/index.rst index 65865523496..63bccf081a6 100644 --- a/tests/roots/test-ext-intersphinx-role/index.rst +++ b/tests/roots/test-ext-intersphinx-role/index.rst @@ -35,10 +35,14 @@ - a function with explicit inventory: - :external+inv:c:func:`CFunc` or :external+inv:c:function:`CFunc` + :external+inv:c:func:`CFunc` - a class with explicit non-existing inventory, which also has upper-case in name: :external+invNope:cpp:class:`foo::Bar` +- An object type being mistakenly used instead of a role name: + + - :external+inv:c:function:`CFunc` + - :external+inv:function:`CFunc` - explicit title: :external:cpp:type:`FoonsTitle ` diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index ae64260fcb5..0551db23782 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -554,10 +554,14 @@ def test_intersphinx_role(app, warning): content = (app.outdir / 'index.html').read_text(encoding='utf8') warnings = strip_colors(warning.getvalue()).splitlines() index_path = app.srcdir / 'index.rst' + for w in warnings: + print(w) assert warnings == [ - f'{index_path}:21: WARNING: role for external cross-reference not found: py:nope', - f'{index_path}:28: WARNING: role for external cross-reference not found: nope', + f'{index_path}:21: WARNING: role for external cross-reference not found in domain py: nope', + f'{index_path}:28: WARNING: role for external cross-reference not found in domains cpp,std: nope', f'{index_path}:39: WARNING: inventory for external cross-reference not found: invNope', + f'{index_path}:44: WARNING: role for external cross-reference not found in domain c: function (perhaps you meant one of: func,identifier,type)', + f'{index_path}:45: WARNING: role for external cross-reference not found in domains cpp,std: function (perhaps you meant one of: func,identifier,type)', f'{index_path}:9: WARNING: external py:mod reference target not found: module3', f'{index_path}:14: WARNING: external py:mod reference target not found: module10', f'{index_path}:19: WARNING: external py:meth reference target not found: inv:Foo.bar', From 9f4197e1b092d46096ab95b099a49ad343650450 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Sat, 23 Mar 2024 23:46:25 +0100 Subject: [PATCH 02/11] minor improvement --- sphinx/ext/intersphinx.py | 2 +- tests/test_extensions/test_ext_intersphinx.py | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index cbf76d2e66f..c200f8fecf8 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -615,7 +615,7 @@ def run(self) -> tuple[list[Node], list[system_message]]: possible_roles: set[str] = set() for d in domains: if o := d.object_types.get(role_name): - possible_roles.update(o.roles) + possible_roles.update([f'{d.name}:{r}' for r in o.roles]) if possible_roles: msg += ' (perhaps you meant one of: %s)' logger.warning( diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index 0551db23782..792a6a753e9 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -554,14 +554,12 @@ def test_intersphinx_role(app, warning): content = (app.outdir / 'index.html').read_text(encoding='utf8') warnings = strip_colors(warning.getvalue()).splitlines() index_path = app.srcdir / 'index.rst' - for w in warnings: - print(w) assert warnings == [ f'{index_path}:21: WARNING: role for external cross-reference not found in domain py: nope', f'{index_path}:28: WARNING: role for external cross-reference not found in domains cpp,std: nope', f'{index_path}:39: WARNING: inventory for external cross-reference not found: invNope', f'{index_path}:44: WARNING: role for external cross-reference not found in domain c: function (perhaps you meant one of: func,identifier,type)', - f'{index_path}:45: WARNING: role for external cross-reference not found in domains cpp,std: function (perhaps you meant one of: func,identifier,type)', + f'{index_path}:45: WARNING: role for external cross-reference not found in domains cpp,std: function (perhaps you meant one of: cpp:func,cpp:identifier,cpp:type)', f'{index_path}:9: WARNING: external py:mod reference target not found: module3', f'{index_path}:14: WARNING: external py:mod reference target not found: module10', f'{index_path}:19: WARNING: external py:meth reference target not found: inv:Foo.bar', From ba17300a6e11bee2b40c526f6580292bc62b65d3 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Sun, 24 Mar 2024 00:26:21 +0100 Subject: [PATCH 03/11] Update CHANGES.rst --- CHANGES.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index f59fdd81f7c..72e3bfda908 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -27,8 +27,9 @@ Features added to annotate the return type of their ``setup`` function. Patch by Chris Sewell. -* #12133: Allow ``external`` roles to reference object types - (rather than role names). Patch by Chris Sewell. +* #12193: Improve ``external`` warnings for unknown roles. + In particular, suggest related role names if an object type is mistakenly used. + Patch by Chris Sewell. * #12131: Added :confval:`show_warning_types` configuration option. Patch by Chris Sewell. From 0ac2876e0b911d335544077b529f4b2273e2f2ee Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Sun, 24 Mar 2024 00:34:18 +0100 Subject: [PATCH 04/11] Update intersphinx.py --- sphinx/ext/intersphinx.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index c200f8fecf8..fc3989c6084 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -558,7 +558,7 @@ def run(self) -> tuple[list[Node], list[system_message]]: if domain_name is not None: # the user specified a domain, so we only check that - if (domain := self.env.get_domain(domain_name)) is None: + if (domain := self.env.domains.get(domain_name)) is None: logger.warning( __('domain for external cross-reference not found: %s'), domain_name, @@ -599,7 +599,7 @@ def run(self) -> tuple[list[Node], list[system_message]]: if default_domain := self.env.temp_data.get('default_domain'): domains.append(default_domain) if ( - std_domain := self.env.get_domain('std') + std_domain := self.env.domains.get('std') ) is not None and std_domain not in domains: domains.append(std_domain) From 4407fc2a2ae3737ba02051e952300234002167e2 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Sun, 24 Mar 2024 00:43:29 +0100 Subject: [PATCH 05/11] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> --- sphinx/ext/intersphinx.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index fc3989c6084..972196f28c6 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -610,12 +610,12 @@ def run(self) -> tuple[list[Node], list[system_message]]: break if role_func is None or domain_name is None: - domains_str = ','.join([d.name for d in domains]) + domains_str = ','.join(d.name for d in domains) msg = 'role for external cross-reference not found in domains %s: %s' possible_roles: set[str] = set() for d in domains: if o := d.object_types.get(role_name): - possible_roles.update([f'{d.name}:{r}' for r in o.roles]) + possible_roles.update(f'{d.name}:{r}' for r in o.roles) if possible_roles: msg += ' (perhaps you meant one of: %s)' logger.warning( From c1f09fa9ef8bd702b1650b6720d70d3f70052139 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Sun, 24 Mar 2024 00:52:45 +0100 Subject: [PATCH 06/11] Update intersphinx.py --- sphinx/ext/intersphinx.py | 63 +++++++++++++-------------------------- 1 file changed, 20 insertions(+), 43 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index 972196f28c6..57c0d1d165b 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -532,24 +532,16 @@ def run(self) -> tuple[list[Node], list[system_message]]: assert self.name == self.orig_name.lower() inventory, name_suffix = self.get_inventory_and_name_suffix(self.orig_name) if inventory and not inventory_exists(self.env, inventory): - logger.warning( - __('inventory for external cross-reference not found: %s'), - inventory, - type='intersphinx', - subtype='external', - location=(self.env.docname, self.lineno), + self._emit_warning( + __('inventory for external cross-reference not found: %s'), inventory ) return [], [] domain_name, role_name = self._get_domain_role(name_suffix) if role_name is None: - logger.warning( - __('external cross-reference suffix not valid: %s'), - name_suffix, - type='intersphinx', - subtype='external', - location=(self.env.docname, self.lineno), + self._emit_warning( + __('external cross-reference suffix not valid: %s'), name_suffix ) return [], [] @@ -559,12 +551,8 @@ def run(self) -> tuple[list[Node], list[system_message]]: if domain_name is not None: # the user specified a domain, so we only check that if (domain := self.env.domains.get(domain_name)) is None: - logger.warning( - __('domain for external cross-reference not found: %s'), - domain_name, - type='intersphinx', - subtype='external', - location=(self.env.docname, self.lineno), + self._emit_warning( + __('domain for external cross-reference not found: %s'), domain_name ) return [], [] if (role_func := domain.roles.get(role_name)) is None: @@ -572,24 +560,14 @@ def run(self) -> tuple[list[Node], list[system_message]]: if ( object_types := domain.object_types.get(role_name) ) is not None and object_types.roles: - logger.warning( + self._emit_warning( __(f'{msg} (perhaps you meant one of: %s)'), domain_name, role_name, ','.join(sorted(object_types.roles)), - type='intersphinx', - subtype='external', - location=(self.env.docname, self.lineno), ) else: - logger.warning( - __(msg), - domain_name, - role_name, - type='intersphinx', - subtype='external', - location=(self.env.docname, self.lineno), - ) + self._emit_warning(__(msg), domain_name, role_name) return [], [] else: @@ -615,27 +593,17 @@ def run(self) -> tuple[list[Node], list[system_message]]: possible_roles: set[str] = set() for d in domains: if o := d.object_types.get(role_name): - possible_roles.update(f'{d.name}:{r}' for r in o.roles) + possible_roles.update(f"{d.name}:{r}" for r in o.roles) if possible_roles: msg += ' (perhaps you meant one of: %s)' - logger.warning( + self._emit_warning( __(msg), domains_str, role_name, ','.join(sorted(possible_roles)), - type='intersphinx', - subtype='external', - location=(self.env.docname, self.lineno), ) else: - logger.warning( - __(msg), - domains_str, - role_name, - type='intersphinx', - subtype='external', - location=(self.env.docname, self.lineno), - ) + self._emit_warning(__(msg), domains_str, role_name) return [], [] result, messages = role_func( @@ -692,6 +660,15 @@ def _get_domain_role(self, name: str) -> tuple[str | None, str | None]: else: return None, None + def _emit_warning(self, msg: str, *args: str) -> None: + logger.warning( + msg, + *args, + type='intersphinx', + subtype='external', + location=(self.env.docname, self.lineno), + ) + class IntersphinxRoleResolver(ReferencesResolver): """pending_xref node resolver for intersphinx role. From 7b2823236526845bcf20747cdbd297189a5c4162 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Sun, 24 Mar 2024 01:00:48 +0100 Subject: [PATCH 07/11] re-add deprecated methods --- sphinx/ext/intersphinx.py | 53 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index 57c0d1d165b..953462dad59 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -34,6 +34,8 @@ import sphinx from sphinx.addnodes import pending_xref from sphinx.builders.html import INVENTORY_FILENAME +from sphinx.deprecation import _deprecation_warning +from sphinx.errors import ExtensionError from sphinx.locale import _, __ from sphinx.transforms.post_transforms import ReferencesResolver from sphinx.util import logging, requests @@ -669,6 +671,57 @@ def _emit_warning(self, msg: str, *args: str) -> None: location=(self.env.docname, self.lineno), ) + # deprecated methods + + def get_role_name(self, name: str) -> tuple[str, str] | None: + _deprecation_warning( + __name__, f'{self.__class__.__name__}.get_role_name', '', remove=(9, 0) + ) + names = name.split(':') + if len(names) == 1: + # role + default_domain = self.env.temp_data.get('default_domain') + domain = default_domain.name if default_domain else None + role = names[0] + elif len(names) == 2: + # domain:role: + domain = names[0] + role = names[1] + else: + return None + + if domain and self.is_existent_role(domain, role): + return (domain, role) + elif self.is_existent_role('std', role): + return ('std', role) + else: + return None + + def is_existent_role(self, domain_name: str, role_name: str) -> bool: + _deprecation_warning( + __name__, f'{self.__class__.__name__}.is_existent_role', '', remove=(9, 0) + ) + try: + domain = self.env.get_domain(domain_name) + return role_name in domain.roles + except ExtensionError: + return False + + def invoke_role(self, role: tuple[str, str]) -> tuple[list[Node], list[system_message]]: + """Invoke the role described by a ``(domain, role name)`` pair.""" + _deprecation_warning( + __name__, f'{self.__class__.__name__}.invoke_role', '', remove=(9, 0) + ) + domain = self.env.get_domain(role[0]) + if domain: + role_func = domain.role(role[1]) + assert role_func is not None + + return role_func(':'.join(role), self.rawtext, self.text, self.lineno, + self.inliner, self.options, self.content) + else: + return [], [] + class IntersphinxRoleResolver(ReferencesResolver): """pending_xref node resolver for intersphinx role. From e6d1b21ecdfcc2471ee368f8cbf4ec2f8513d229 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Sun, 24 Mar 2024 01:27:19 +0100 Subject: [PATCH 08/11] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> --- sphinx/ext/intersphinx.py | 16 ++++++++-------- tests/test_extensions/test_ext_intersphinx.py | 6 +++--- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index 953462dad59..d7e17d6025d 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -543,7 +543,7 @@ def run(self) -> tuple[list[Node], list[system_message]]: if role_name is None: self._emit_warning( - __('external cross-reference suffix not valid: %s'), name_suffix + __('invalid external cross-reference suffix: %s'), name_suffix ) return [], [] @@ -566,7 +566,7 @@ def run(self) -> tuple[list[Node], list[system_message]]: __(f'{msg} (perhaps you meant one of: %s)'), domain_name, role_name, - ','.join(sorted(object_types.roles)), + ', '.join(sorted(object_types.roles)), ) else: self._emit_warning(__(msg), domain_name, role_name) @@ -590,26 +590,26 @@ def run(self) -> tuple[list[Node], list[system_message]]: break if role_func is None or domain_name is None: - domains_str = ','.join(d.name for d in domains) + domains_str = ', '.join(d.name for d in domains) msg = 'role for external cross-reference not found in domains %s: %s' possible_roles: set[str] = set() for d in domains: if o := d.object_types.get(role_name): - possible_roles.update(f"{d.name}:{r}" for r in o.roles) + possible_roles.update(f'{d.name}:{r}' for r in o.roles) if possible_roles: - msg += ' (perhaps you meant one of: %s)' + msg = f'{msg} (perhaps you meant one of: %s)' self._emit_warning( __(msg), domains_str, role_name, - ','.join(sorted(possible_roles)), + ', '.join(sorted(possible_roles)), ) else: self._emit_warning(__(msg), domains_str, role_name) return [], [] result, messages = role_func( - f"{domain_name}:{role_name}", + f'{domain_name}:{role_name}', self.rawtext, self.text, self.lineno, @@ -662,7 +662,7 @@ def _get_domain_role(self, name: str) -> tuple[str | None, str | None]: else: return None, None - def _emit_warning(self, msg: str, *args: str) -> None: + def _emit_warning(self, msg: str, /, *args: Any) -> None: logger.warning( msg, *args, diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index 792a6a753e9..04146bed2be 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -556,10 +556,10 @@ def test_intersphinx_role(app, warning): index_path = app.srcdir / 'index.rst' assert warnings == [ f'{index_path}:21: WARNING: role for external cross-reference not found in domain py: nope', - f'{index_path}:28: WARNING: role for external cross-reference not found in domains cpp,std: nope', + f'{index_path}:28: WARNING: role for external cross-reference not found in domains cpp, std: nope', f'{index_path}:39: WARNING: inventory for external cross-reference not found: invNope', - f'{index_path}:44: WARNING: role for external cross-reference not found in domain c: function (perhaps you meant one of: func,identifier,type)', - f'{index_path}:45: WARNING: role for external cross-reference not found in domains cpp,std: function (perhaps you meant one of: cpp:func,cpp:identifier,cpp:type)', + f'{index_path}:44: WARNING: role for external cross-reference not found in domain c: function (perhaps you meant one of: func, identifier, type)', + f'{index_path}:45: WARNING: role for external cross-reference not found in domains cpp, std: function (perhaps you meant one of: cpp:func, cpp:identifier, cpp:type)', f'{index_path}:9: WARNING: external py:mod reference target not found: module3', f'{index_path}:14: WARNING: external py:mod reference target not found: module10', f'{index_path}:19: WARNING: external py:meth reference target not found: inv:Foo.bar', From f54fb11f9888c42ed06d339be8312a948226f867 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Sun, 24 Mar 2024 02:20:20 +0100 Subject: [PATCH 09/11] Update sphinx/ext/intersphinx.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> --- sphinx/ext/intersphinx.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index d7e17d6025d..e5f2dcd5d9d 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -548,7 +548,7 @@ def run(self) -> tuple[list[Node], list[system_message]]: return [], [] # attempt to find a matching role function - role_func: None | RoleFunction + role_func: RoleFunction | None if domain_name is not None: # the user specified a domain, so we only check that From 19e496badc92bff90909ada8f9484b979386b4b7 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Sun, 24 Mar 2024 02:31:43 +0100 Subject: [PATCH 10/11] change all names to be quoted --- sphinx/ext/intersphinx.py | 19 +++++++++++-------- tests/test_extensions/test_ext_intersphinx.py | 10 +++++----- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index e5f2dcd5d9d..459148bffdd 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -535,7 +535,7 @@ def run(self) -> tuple[list[Node], list[system_message]]: inventory, name_suffix = self.get_inventory_and_name_suffix(self.orig_name) if inventory and not inventory_exists(self.env, inventory): self._emit_warning( - __('inventory for external cross-reference not found: %s'), inventory + __('inventory for external cross-reference not found: %r'), inventory ) return [], [] @@ -543,7 +543,7 @@ def run(self) -> tuple[list[Node], list[system_message]]: if role_name is None: self._emit_warning( - __('invalid external cross-reference suffix: %s'), name_suffix + __('invalid external cross-reference suffix: %r'), name_suffix ) return [], [] @@ -554,11 +554,11 @@ def run(self) -> tuple[list[Node], list[system_message]]: # the user specified a domain, so we only check that if (domain := self.env.domains.get(domain_name)) is None: self._emit_warning( - __('domain for external cross-reference not found: %s'), domain_name + __('domain for external cross-reference not found: %r'), domain_name ) return [], [] if (role_func := domain.roles.get(role_name)) is None: - msg = 'role for external cross-reference not found in domain %s: %s' + msg = 'role for external cross-reference not found in domain %r: %r' if ( object_types := domain.object_types.get(role_name) ) is not None and object_types.roles: @@ -566,7 +566,7 @@ def run(self) -> tuple[list[Node], list[system_message]]: __(f'{msg} (perhaps you meant one of: %s)'), domain_name, role_name, - ', '.join(sorted(object_types.roles)), + self._concat_strings(object_types.roles), ) else: self._emit_warning(__(msg), domain_name, role_name) @@ -590,8 +590,8 @@ def run(self) -> tuple[list[Node], list[system_message]]: break if role_func is None or domain_name is None: - domains_str = ', '.join(d.name for d in domains) - msg = 'role for external cross-reference not found in domains %s: %s' + domains_str = self._concat_strings((d.name for d in domains)) + msg = 'role for external cross-reference not found in domains %s: %r' possible_roles: set[str] = set() for d in domains: if o := d.object_types.get(role_name): @@ -602,7 +602,7 @@ def run(self) -> tuple[list[Node], list[system_message]]: __(msg), domains_str, role_name, - ', '.join(sorted(possible_roles)), + self._concat_strings(possible_roles), ) else: self._emit_warning(__(msg), domains_str, role_name) @@ -671,6 +671,9 @@ def _emit_warning(self, msg: str, /, *args: Any) -> None: location=(self.env.docname, self.lineno), ) + def _concat_strings(self, strings: Iterable[str]) -> str: + return ', '.join(f'{s!r}' for s in sorted(strings)) + # deprecated methods def get_role_name(self, name: str) -> tuple[str, str] | None: diff --git a/tests/test_extensions/test_ext_intersphinx.py b/tests/test_extensions/test_ext_intersphinx.py index 04146bed2be..de776103f21 100644 --- a/tests/test_extensions/test_ext_intersphinx.py +++ b/tests/test_extensions/test_ext_intersphinx.py @@ -555,11 +555,11 @@ def test_intersphinx_role(app, warning): warnings = strip_colors(warning.getvalue()).splitlines() index_path = app.srcdir / 'index.rst' assert warnings == [ - f'{index_path}:21: WARNING: role for external cross-reference not found in domain py: nope', - f'{index_path}:28: WARNING: role for external cross-reference not found in domains cpp, std: nope', - f'{index_path}:39: WARNING: inventory for external cross-reference not found: invNope', - f'{index_path}:44: WARNING: role for external cross-reference not found in domain c: function (perhaps you meant one of: func, identifier, type)', - f'{index_path}:45: WARNING: role for external cross-reference not found in domains cpp, std: function (perhaps you meant one of: cpp:func, cpp:identifier, cpp:type)', + f"{index_path}:21: WARNING: role for external cross-reference not found in domain 'py': 'nope'", + f"{index_path}:28: WARNING: role for external cross-reference not found in domains 'cpp', 'std': 'nope'", + f"{index_path}:39: WARNING: inventory for external cross-reference not found: 'invNope'", + f"{index_path}:44: WARNING: role for external cross-reference not found in domain 'c': 'function' (perhaps you meant one of: 'func', 'identifier', 'type')", + f"{index_path}:45: WARNING: role for external cross-reference not found in domains 'cpp', 'std': 'function' (perhaps you meant one of: 'cpp:func', 'cpp:identifier', 'cpp:type')", f'{index_path}:9: WARNING: external py:mod reference target not found: module3', f'{index_path}:14: WARNING: external py:mod reference target not found: module10', f'{index_path}:19: WARNING: external py:meth reference target not found: inv:Foo.bar', From dd093600d65987b526f90fd6c325ba0a40df4346 Mon Sep 17 00:00:00 2001 From: Chris Sewell Date: Sun, 24 Mar 2024 17:58:52 +0100 Subject: [PATCH 11/11] Update sphinx/ext/intersphinx.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> --- sphinx/ext/intersphinx.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sphinx/ext/intersphinx.py b/sphinx/ext/intersphinx.py index 459148bffdd..a8a2cf13161 100644 --- a/sphinx/ext/intersphinx.py +++ b/sphinx/ext/intersphinx.py @@ -590,7 +590,7 @@ def run(self) -> tuple[list[Node], list[system_message]]: break if role_func is None or domain_name is None: - domains_str = self._concat_strings((d.name for d in domains)) + domains_str = self._concat_strings(d.name for d in domains) msg = 'role for external cross-reference not found in domains %s: %r' possible_roles: set[str] = set() for d in domains: