From 423e7ab2ca42123584504ef03e0a3c07b9369275 Mon Sep 17 00:00:00 2001 From: Takeshi KOMIYA Date: Mon, 21 Dec 2020 01:43:36 +0900 Subject: [PATCH 1/6] Fix #8559: AttributeError is raised when using ForwardRef The restify() helper crashes when ForwardRef is passed. --- CHANGES | 3 +++ sphinx/util/typing.py | 2 ++ tests/test_util_typing.py | 6 ++++++ 3 files changed, 11 insertions(+) diff --git a/CHANGES b/CHANGES index 226f7da6d..bcf7cf234 100644 --- a/CHANGES +++ b/CHANGES @@ -16,6 +16,9 @@ Features added Bugs fixed ---------- +* #8559: autodoc: AttributeError is raised when using forward-reference type + annotations + Testing -------- diff --git a/sphinx/util/typing.py b/sphinx/util/typing.py index 222e2edf2..2d4f67bba 100644 --- a/sphinx/util/typing.py +++ b/sphinx/util/typing.py @@ -153,6 +153,8 @@ def _restify_py37(cls: Optional["Type"]) -> str: return ':obj:`%s`' % cls._name else: return ':obj:`%s.%s`' % (cls.__module__, cls._name) + elif isinstance(cls, ForwardRef): + return ':class:`%s`' % cls.__forward_arg__ else: # not a class (ex. TypeVar) return ':obj:`%s.%s`' % (cls.__module__, cls.__name__) diff --git a/tests/test_util_typing.py b/tests/test_util_typing.py index 882c652cc..a2565f1e1 100644 --- a/tests/test_util_typing.py +++ b/tests/test_util_typing.py @@ -109,6 +109,12 @@ def test_restify_type_hints_alias(): assert restify(MyTuple) == ":class:`Tuple`\\ [:class:`str`, :class:`str`]" # type: ignore +@pytest.mark.skipif(sys.version_info < (3, 7), reason='python 3.7+ is required.') +def test_restify_type_ForwardRef(): + from typing import ForwardRef # type: ignore + assert restify(ForwardRef("myint")) == ":class:`myint`" + + def test_restify_broken_type_hints(): assert restify(BrokenType) == ':class:`tests.test_util_typing.BrokenType`' From 4431f11876e05ff2e8f85c087b3df749d3b28a6f Mon Sep 17 00:00:00 2001 From: Rafal Wojdyla Date: Mon, 21 Dec 2020 14:23:31 +0100 Subject: [PATCH 2/6] Fixes #8568. Ignore TypeError from getslots in isslotsattribute --- sphinx/ext/autodoc/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sphinx/ext/autodoc/__init__.py b/sphinx/ext/autodoc/__init__.py index a23547a61..4bf97cc85 100644 --- a/sphinx/ext/autodoc/__init__.py +++ b/sphinx/ext/autodoc/__init__.py @@ -2114,7 +2114,7 @@ class SlotsMixin(DataDocumenterMixinBase): return True else: return False - except (AttributeError, ValueError): + except (AttributeError, ValueError, TypeError): return False def import_object(self, raiseerror: bool = False) -> bool: From 22080940e252fe47cb1dccf3fd9029b7c9d21de5 Mon Sep 17 00:00:00 2001 From: Takeshi KOMIYA Date: Tue, 22 Dec 2020 00:53:27 +0900 Subject: [PATCH 3/6] doc: Update docstring of getslots() --- sphinx/util/inspect.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sphinx/util/inspect.py b/sphinx/util/inspect.py index a26c818c0..767ef319c 100644 --- a/sphinx/util/inspect.py +++ b/sphinx/util/inspect.py @@ -187,6 +187,7 @@ def getslots(obj: Any) -> Optional[Dict]: Return None if gienv *obj* does not have __slots__. Raises AttributeError if given *obj* raises an error on accessing __slots__. + Raises TypeError if given *obj* is not a class. Raises ValueError if given *obj* have invalid __slots__. """ if not inspect.isclass(obj): From b3292b55b39ca075595cfe01a4fe4bcc7b7d9a57 Mon Sep 17 00:00:00 2001 From: Takeshi KOMIYA Date: Tue, 22 Dec 2020 01:14:04 +0900 Subject: [PATCH 4/6] Update CHANGES for PR #8569 --- CHANGES | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES b/CHANGES index bcf7cf234..6125b83a1 100644 --- a/CHANGES +++ b/CHANGES @@ -18,6 +18,7 @@ Bugs fixed * #8559: autodoc: AttributeError is raised when using forward-reference type annotations +* #8568: autodoc: TypeError is raised on checking slots attribute Testing -------- From 70bb2262d6677e5bcf2c85786b5f410a3a0f68aa Mon Sep 17 00:00:00 2001 From: Takeshi KOMIYA Date: Tue, 22 Dec 2020 02:21:27 +0900 Subject: [PATCH 5/6] Fix #8567: autodoc: Instance attributes are incorrectly added to Parent class The instance attributes on subclasses are shown on the document of parent class unexpectedly because of autodoc modifies `__annotations__` in place. This fix creates a copy of `__annotations__` attribute and attach it to the subclass. --- CHANGES | 1 + sphinx/ext/autodoc/__init__.py | 13 +++++++++---- tests/test_ext_autodoc.py | 1 + 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/CHANGES b/CHANGES index 6125b83a1..7241391ae 100644 --- a/CHANGES +++ b/CHANGES @@ -19,6 +19,7 @@ Bugs fixed * #8559: autodoc: AttributeError is raised when using forward-reference type annotations * #8568: autodoc: TypeError is raised on checking slots attribute +* #8567: autodoc: Instance attributes are incorrectly added to Parent class Testing -------- diff --git a/sphinx/ext/autodoc/__init__.py b/sphinx/ext/autodoc/__init__.py index 4bf97cc85..05696448d 100644 --- a/sphinx/ext/autodoc/__init__.py +++ b/sphinx/ext/autodoc/__init__.py @@ -1856,13 +1856,14 @@ class DataDocumenter(GenericAliasMixin, NewTypeMixin, TypeVarMixin, def update_annotations(self, parent: Any) -> None: """Update __annotations__ to support type_comment and so on.""" try: - annotations = inspect.getannotations(parent) + annotations = dict(inspect.getannotations(parent)) + parent.__annotations__ = annotations analyzer = ModuleAnalyzer.for_module(self.modname) analyzer.analyze() for (classname, attrname), annotation in analyzer.annotations.items(): if classname == '' and attrname not in annotations: - annotations[attrname] = annotation # type: ignore + annotations[attrname] = annotation except AttributeError: pass @@ -2292,7 +2293,8 @@ class AttributeDocumenter(GenericAliasMixin, NewTypeMixin, SlotsMixin, # type: def update_annotations(self, parent: Any) -> None: """Update __annotations__ to support type_comment and so on.""" try: - annotations = inspect.getannotations(parent) + annotations = dict(inspect.getannotations(parent)) + parent.__annotations__ = annotations for cls in inspect.getmro(parent): try: @@ -2303,11 +2305,14 @@ class AttributeDocumenter(GenericAliasMixin, NewTypeMixin, SlotsMixin, # type: analyzer.analyze() for (classname, attrname), annotation in analyzer.annotations.items(): if classname == qualname and attrname not in annotations: - annotations[attrname] = annotation # type: ignore + annotations[attrname] = annotation except (AttributeError, PycodeError): pass except AttributeError: pass + except TypeError: + # Failed to set __annotations__ (built-in, extensions, etc.) + pass def import_object(self, raiseerror: bool = False) -> bool: try: diff --git a/tests/test_ext_autodoc.py b/tests/test_ext_autodoc.py index 392ad1a68..1fa0c1d7d 100644 --- a/tests/test_ext_autodoc.py +++ b/tests/test_ext_autodoc.py @@ -690,6 +690,7 @@ def test_autodoc_special_members(app): actual = do_autodoc(app, 'class', 'target.Class', options) assert list(filter(lambda l: '::' in l, actual)) == [ '.. py:class:: Class(arg)', + ' .. py:attribute:: Class.__annotations__', ' .. py:attribute:: Class.__dict__', ' .. py:method:: Class.__init__(arg)', ' .. py:attribute:: Class.__module__', From a1b8b1febb6ce0b7423bb78df30b0fc424c12325 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Freitag?= Date: Tue, 22 Dec 2020 19:49:53 +0100 Subject: [PATCH 6/6] Ensure linkcheck items are comparable Linkcheck organizes the URLs to checks in a PriorityQueue. The items are tuples (priority, url, docname, lineno). Tuples where the lineno is `None` are not comparable with tuples that have an integer lineno, and PriorityQueue items must be comparable (see https://bugs.python.org/issue31145). Fixes an issue when a document contains two links to the same URL, one with an int line number and the other without line number metadata (such as an image :target: attribute). Using 0 instead of None to represent no line number should not lead to observable changes, the result logger only logs the line number when it is truthy. Close #8565 --- CHANGES | 2 + sphinx/builders/linkcheck.py | 14 +++++-- .../conf.py | 1 + .../index.rst | 6 +++ tests/test_build_linkcheck.py | 37 +++++++++++++++++++ 5 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 tests/roots/test-linkcheck-localserver-two-links/conf.py create mode 100644 tests/roots/test-linkcheck-localserver-two-links/index.rst diff --git a/CHANGES b/CHANGES index 7241391ae..90633e980 100644 --- a/CHANGES +++ b/CHANGES @@ -20,6 +20,8 @@ Bugs fixed annotations * #8568: autodoc: TypeError is raised on checking slots attribute * #8567: autodoc: Instance attributes are incorrectly added to Parent class +* #8565: linkcheck: Fix PriorityQueue crash when link tuples are not + comparable Testing -------- diff --git a/sphinx/builders/linkcheck.py b/sphinx/builders/linkcheck.py index 06a6293d2..3be887902 100644 --- a/sphinx/builders/linkcheck.py +++ b/sphinx/builders/linkcheck.py @@ -22,7 +22,7 @@ from typing import Any, Dict, List, NamedTuple, Optional, Set, Tuple from urllib.parse import unquote, urlparse from docutils import nodes -from docutils.nodes import Node +from docutils.nodes import Element, Node from requests import Response from requests.exceptions import HTTPError, TooManyRedirects @@ -47,6 +47,14 @@ QUEUE_POLL_SECS = 1 DEFAULT_DELAY = 60.0 +def node_line_or_0(node: Element) -> int: + """ + PriorityQueue items must be comparable. The line number is part of the + tuple used by the PriorityQueue, keep an homogeneous type for comparison. + """ + return get_node_line(node) or 0 + + class AnchorCheckParser(HTMLParser): """Specialized HTML parser that looks for a specific anchor.""" @@ -406,7 +414,7 @@ class CheckExternalLinksBuilder(Builder): if 'refuri' not in refnode: continue uri = refnode['refuri'] - lineno = get_node_line(refnode) + lineno = node_line_or_0(refnode) uri_info = (CHECK_IMMEDIATELY, uri, docname, lineno) self.wqueue.put(uri_info, False) n += 1 @@ -415,7 +423,7 @@ class CheckExternalLinksBuilder(Builder): for imgnode in doctree.traverse(nodes.image): uri = imgnode['candidates'].get('?') if uri and '://' in uri: - lineno = get_node_line(imgnode) + lineno = node_line_or_0(imgnode) uri_info = (CHECK_IMMEDIATELY, uri, docname, lineno) self.wqueue.put(uri_info, False) n += 1 diff --git a/tests/roots/test-linkcheck-localserver-two-links/conf.py b/tests/roots/test-linkcheck-localserver-two-links/conf.py new file mode 100644 index 000000000..a45d22e28 --- /dev/null +++ b/tests/roots/test-linkcheck-localserver-two-links/conf.py @@ -0,0 +1 @@ +exclude_patterns = ['_build'] diff --git a/tests/roots/test-linkcheck-localserver-two-links/index.rst b/tests/roots/test-linkcheck-localserver-two-links/index.rst new file mode 100644 index 000000000..4c1bcfd6a --- /dev/null +++ b/tests/roots/test-linkcheck-localserver-two-links/index.rst @@ -0,0 +1,6 @@ +.. image:: http://localhost:7777/ + :target: http://localhost:7777/ + +`weblate.org`_ + +.. _weblate.org: http://localhost:7777/ diff --git a/tests/test_build_linkcheck.py b/tests/test_build_linkcheck.py index e62276931..cfb3a032c 100644 --- a/tests/test_build_linkcheck.py +++ b/tests/test_build_linkcheck.py @@ -573,3 +573,40 @@ def test_limit_rate_bails_out_after_waiting_max_time(app): checker.rate_limits = {"localhost": RateLimit(90.0, 0.0)} next_check = checker.limit_rate(FakeResponse()) assert next_check is None + + +@pytest.mark.sphinx( + 'linkcheck', testroot='linkcheck-localserver-two-links', freshenv=True, +) +def test_priorityqueue_items_are_comparable(app): + with http_server(OKHandler): + app.builder.build_all() + content = (app.outdir / 'output.json').read_text() + rows = [json.loads(x) for x in sorted(content.splitlines())] + assert rows == [ + { + 'filename': 'index.rst', + # Should not be None. + 'lineno': 0, + 'status': 'working', + 'code': 0, + 'uri': 'http://localhost:7777/', + 'info': '', + }, + { + 'filename': 'index.rst', + 'lineno': 0, + 'status': 'working', + 'code': 0, + 'uri': 'http://localhost:7777/', + 'info': '', + }, + { + 'filename': 'index.rst', + 'lineno': 4, + 'status': 'working', + 'code': 0, + 'uri': 'http://localhost:7777/', + 'info': '', + } + ]