Add `visit_` and `depart_` methods to `docutils` visitor classes #13928

bzoracler · May 3, 2025

Adds the visit_* and depart_* methods for each node class to docutils.nodes.NodeVisitor. Node classes known to docutils are kept in the list docutils.nodes.node_class_names.

The motivation and implementation is similar to #3796, which does this for the standard library's ast.NodeVisitor. The difference is that docutils.nodes.NodeVisitor itself does not have a fallback if a method is not defined for a node, and will fail at runtime. This is documented in the runtime class, but I added a short comment to the stubs about this as well.

The subclasses SparseNodeVisitor and the more common GenericNodeVisitor do actually have these methods at runtime (they are added dynamically to the class), but to aid discoverability of these methods to docutils.nodes.NodeVisitor (and to avoid repeating the same set of methods 2 more times) I only added them to the base NodeVisitor class.

I did not make the node argument to the visit methods positional-only, even though the node dispatch methods only use a positional-only API. This is because, after a quick browse through visitor method overrides in docutils and sphinx, I didn't find anyone else implementing positional-only overrides, so having positional-only signatures may cause a lot of issues downstream.

bzoracler · May 3, 2025

All of these are true positives.

Most of these are due to the visit_* method annotating the node argument with an imprecise Element annotation instead of the precise subclass.

A few items to note:

```
+ sphinx/writers/manpage.py: note: In member "visit_seealso" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:268:9: error: Too many arguments for "visit_admonition" of "NodeVisitor"  [call-arg]
```
Calls an untyped but overridden visit_admonition with one more argument (see implementation). The method being untyped causes the type-checker to fall back to the stub implementation (that is, without the extra argument) on NodeVisitor.

+ sphinx/writers/manpage.py: note: In member "visit_reference" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:304:25: error: Argument 1 to "visit_Text" of "NodeVisitor" has incompatible type "Element"; expected "Text"  [arg-type]

Text is not a subclass of Element. The implementation relies on the method astext() existing on both Text and Element nodes to avoid some computation.

bzoracler · May 4, 2025

New commits:

Adds a visit_admonition signature override, which removes the extra arguments error from before.
Adds a visit_comment override which introduces a new true positive. With reference to the runtime implementation:
```
+ sphinx/writers/manpage.py: note: In member "visit_comment" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:248:5: error: Signature of "visit_comment" incompatible with supertype "Translator"  [override]
+ sphinx/writers/manpage.py:248:5: note:      Superclass:
+ sphinx/writers/manpage.py:248:5: note:          def visit_comment(self, node: comment, sub: _RegexPatternSub = ...) -> Never
+ sphinx/writers/manpage.py:248:5: note:      Subclass:
+ sphinx/writers/manpage.py:248:5: note:          def visit_comment(self, node: Element) -> None
```
- The parent method has an extra argument, so sphinx needs to reflect this or suppress the signature mismatch error
- The docutils.writers.Manpage implementation and child sphinx.writers.Manpage implementation both are guaranteed to raise docutils.nodes.SkipNode, which is used to control node traversal. I think it's appropriate for the parent method to indicate -> Never rather than None to publicly indicate that node traversal is skipped here, so sphinx needs to reflect this in the return signature.

srittau

Thanks! I think this is fine, despite the fact that sphinx needs to update a lot of their annotations.

github-actions · May 22, 2025

Diff from mypy_primer, showing the effect of this PR on open source code:

sphinx (https://github.com/sphinx-doc/sphinx)
+ sphinx/writers/html5.py: note: In member "depart_number_reference" of class "HTML5Translator":
+ sphinx/writers/html5.py:361:31: error: Argument 1 to "depart_reference" of "NodeVisitor" has incompatible type "Element"; expected "reference"  [arg-type]
+ sphinx/writers/html5.py: note: In member "visit_bullet_list" of class "HTML5Translator":
+ sphinx/writers/html5.py:458:35: error: Argument 1 to "visit_bullet_list" of "NodeVisitor" has incompatible type "Element"; expected "bullet_list"  [arg-type]
+ sphinx/writers/html5.py: note: In member "visit_title" of class "HTML5Translator":
+ sphinx/writers/html5.py:521:33: error: Argument 1 to "visit_title" of "NodeVisitor" has incompatible type "Element"; expected "title"  [arg-type]
+ sphinx/writers/html5.py: note: In member "depart_title" of class "HTML5Translator":
+ sphinx/writers/html5.py:562:30: error: Argument 1 to "depart_title" of "NodeVisitor" has incompatible type "Element"; expected "title"  [arg-type]
+ sphinx/writers/html5.py: note: In member "visit_literal_block" of class "HTML5Translator":
+ sphinx/writers/html5.py:592:48: error: Argument 1 to "visit_literal_block" of "NodeVisitor" has incompatible type "Element"; expected "literal_block"  [arg-type]
+ sphinx/writers/html5.py: note: In member "visit_caption" of class "HTML5Translator":
+ sphinx/writers/html5.py:624:35: error: Argument 1 to "visit_caption" of "NodeVisitor" has incompatible type "Element"; expected "caption"  [arg-type]
+ sphinx/writers/html5.py: note: In member "depart_caption" of class "HTML5Translator":
+ sphinx/writers/html5.py:648:36: error: Argument 1 to "depart_caption" of "NodeVisitor" has incompatible type "Element"; expected "caption"  [arg-type]
+ sphinx/writers/html5.py: note: In member "visit_figure" of class "HTML5Translator":
+ sphinx/writers/html5.py:747:37: error: Argument 1 to "visit_figure" of "NodeVisitor" has incompatible type "Element"; expected "figure"  [arg-type]
+ sphinx/writers/html5.py: note: In member "visit_image" of class "HTML5Translator":
+ sphinx/writers/html5.py:775:29: error: Argument 1 to "visit_image" of "NodeVisitor" has incompatible type "Element"; expected "image"  [arg-type]
+ sphinx/writers/html5.py: note: In member "depart_image" of class "HTML5Translator":
+ sphinx/writers/html5.py:782:34: error: Argument 1 to "depart_image" of "NodeVisitor" has incompatible type "Element"; expected "image"  [arg-type]
+ sphinx/writers/html5.py: note: In member "visit_literal_emphasis" of class "HTML5Translator":
+ sphinx/writers/html5.py:896:36: error: Argument 1 to "visit_emphasis" of "NodeVisitor" has incompatible type "Element"; expected "emphasis"  [arg-type]
+ sphinx/writers/html5.py: note: In member "depart_literal_emphasis" of class "HTML5Translator":
+ sphinx/writers/html5.py:899:37: error: Argument 1 to "depart_emphasis" of "NodeVisitor" has incompatible type "Element"; expected "emphasis"  [arg-type]
+ sphinx/writers/html5.py: note: In member "visit_literal_strong" of class "HTML5Translator":
+ sphinx/writers/html5.py:902:34: error: Argument 1 to "visit_strong" of "NodeVisitor" has incompatible type "Element"; expected "strong"  [arg-type]
+ sphinx/writers/html5.py: note: In member "depart_literal_strong" of class "HTML5Translator":
+ sphinx/writers/html5.py:905:35: error: Argument 1 to "depart_strong" of "NodeVisitor" has incompatible type "Element"; expected "strong"  [arg-type]
+ sphinx/writers/html5.py: note: In member "depart_table" of class "HTML5Translator":
+ sphinx/writers/html5.py:941:30: error: Argument 1 to "depart_table" of "NodeVisitor" has incompatible type "Element"; expected "table"  [arg-type]
+ sphinx/writers/html5.py: note: In member "visit_field_list" of class "HTML5Translator":
+ sphinx/writers/html5.py:954:41: error: Argument 1 to "visit_field_list" of "NodeVisitor" has incompatible type "Element"; expected "field_list"  [arg-type]
+ sphinx/writers/html5.py: note: In member "depart_field_list" of class "HTML5Translator":
+ sphinx/writers/html5.py:958:42: error: Argument 1 to "depart_field_list" of "NodeVisitor" has incompatible type "Element"; expected "field_list"  [arg-type]
+ sphinx/writers/manpage.py:74: error: Unused "type: ignore" comment  [unused-ignore]
+ sphinx/writers/manpage.py: note: In member "visit_desc" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:134:36: error: Argument 1 to "visit_definition_list" of "NodeVisitor" has incompatible type "Element"; expected "definition_list"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "depart_desc" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:137:37: error: Argument 1 to "depart_definition_list" of "NodeVisitor" has incompatible type "Element"; expected "definition_list"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "visit_desc_signature" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:140:41: error: Argument 1 to "visit_definition_list_item" of "NodeVisitor" has incompatible type "Element"; expected "definition_list_item"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "depart_desc_signature" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:144:26: error: Argument 1 to "depart_term" of "NodeVisitor" has incompatible type "Element"; expected "term"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "visit_desc_content" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:153:31: error: Argument 1 to "visit_definition" of "NodeVisitor" has incompatible type "Element"; expected "definition"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "depart_desc_content" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:156:32: error: Argument 1 to "depart_definition" of "NodeVisitor" has incompatible type "Element"; expected "definition"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "visit_versionmodified" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:235:30: error: Argument 1 to "visit_paragraph" of "NodeVisitor" has incompatible type "Element"; expected "paragraph"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "depart_versionmodified" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:238:31: error: Argument 1 to "depart_paragraph" of "NodeVisitor" has incompatible type "Element"; expected "paragraph"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "visit_term" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:245:32: error: Argument 1 to "visit_term" of "NodeVisitor" has incompatible type "Element"; expected "term"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "visit_comment" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:248:5: error: Signature of "visit_comment" incompatible with supertype "Translator"  [override]
+ sphinx/writers/manpage.py:248:5: note:      Superclass:
+ sphinx/writers/manpage.py:248:5: note:          def visit_comment(self, node: comment, sub: _RegexPatternSub = ...) -> Never
+ sphinx/writers/manpage.py:248:5: note:      Subclass:
+ sphinx/writers/manpage.py:248:5: note:          def visit_comment(self, node: Element) -> None
+ sphinx/writers/manpage.py: note: In member "visit_footnote" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:254:32: error: Argument 1 to "visit_footnote" of "NodeVisitor" has incompatible type "Element"; expected "footnote"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "visit_seealso" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:268:31: error: Argument 1 to "visit_admonition" of "Translator" has incompatible type "Element"; expected "admonition"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "depart_seealso" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:271:32: error: Argument 1 to "depart_admonition" of "NodeVisitor" has incompatible type "Element"; expected "admonition"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "visit_reference" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:304:25: error: Argument 1 to "visit_Text" of "NodeVisitor" has incompatible type "Element"; expected "Text"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "visit_hlist" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:373:32: error: Argument 1 to "visit_bullet_list" of "NodeVisitor" has incompatible type "Element"; expected "bullet_list"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "depart_hlist" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:376:33: error: Argument 1 to "depart_bullet_list" of "NodeVisitor" has incompatible type "Element"; expected "bullet_list"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "visit_literal_emphasis" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:385:36: error: Argument 1 to "visit_emphasis" of "NodeVisitor" has incompatible type "Element"; expected "emphasis"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "depart_literal_emphasis" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:388:37: error: Argument 1 to "depart_emphasis" of "NodeVisitor" has incompatible type "Element"; expected "emphasis"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "visit_literal_strong" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:391:34: error: Argument 1 to "visit_strong" of "NodeVisitor" has incompatible type "Element"; expected "strong"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "depart_literal_strong" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:394:35: error: Argument 1 to "depart_strong" of "NodeVisitor" has incompatible type "Element"; expected "strong"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "visit_manpage" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:403:34: error: Argument 1 to "visit_strong" of "NodeVisitor" has incompatible type "Element"; expected "strong"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "depart_manpage" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:406:35: error: Argument 1 to "depart_strong" of "NodeVisitor" has incompatible type "Element"; expected "strong"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "visit_caption" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:416:35: error: Argument 1 to "visit_caption" of "NodeVisitor" has incompatible type "Element"; expected "caption"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "depart_caption" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:425:36: error: Argument 1 to "depart_caption" of "NodeVisitor" has incompatible type "Element"; expected "caption"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "visit_title" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:439:36: error: Argument 1 to "visit_title" of "NodeVisitor" has incompatible type "Element"; expected "title"  [arg-type]
+ sphinx/writers/manpage.py: note: In member "depart_title" of class "ManualPageTranslator":
+ sphinx/writers/manpage.py:445:37: error: Argument 1 to "depart_title" of "NodeVisitor" has incompatible type "Element"; expected "title"  [arg-type]

bzoracler and others added 4 commits May 3, 2025 19:28

feat: add visit_* and depart_* methods

c4405af

improvement: dynamic attribute getter no longer needed

b9e1e86

docs: add usage notes

e8b0605

[pre-commit.ci] auto fixes from pre-commit.com hooks

f0fdd05

bzoracler marked this pull request as draft May 3, 2025 08:11

bzoracler added 3 commits May 3, 2025 20:16

fix: Avoid usage of shadowed document class member

25b4101

fix: line length

bda672f

fix: Missing depart_* methods in allowlist

fc1a140

This comment has been minimized.

Sign in to view

bzoracler marked this pull request as ready for review May 3, 2025 08:37

bzoracler and others added 3 commits May 4, 2025 12:36

improvement: add basic signature override for sphinx usage

a66570c

[pre-commit.ci] auto fixes from pre-commit.com hooks

48f604b

fix: Add signature override for visit_comment

9bd9535

This comment has been minimized.

Sign in to view

improvement: add default visit/departure methods

5c44ffe

This comment has been minimized.

Sign in to view

Merge branch 'main' into main

2365268

srittau approved these changes May 22, 2025

View reviewed changes

srittau merged commit 341fab4 into python:main May 22, 2025
49 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Add `visit_` and `depart_` methods to `docutils` visitor classes #13928

Add `visit_` and `depart_` methods to `docutils` visitor classes #13928

Uh oh!

bzoracler commented May 3, 2025 •

edited

Loading

Uh oh!

This comment has been minimized.

bzoracler commented May 3, 2025 •

edited

Loading

Uh oh!

This comment has been minimized.

bzoracler commented May 4, 2025

Uh oh!

This comment has been minimized.

srittau left a comment

Uh oh!

github-actions bot commented May 22, 2025

Uh oh!

Uh oh!

Uh oh!

Search code, repositories, users, issues, pull requests...

Uh oh!

Add visit_* and depart_* methods to docutils visitor classes #13928

Add visit_* and depart_* methods to docutils visitor classes #13928

Uh oh!

Conversation

bzoracler commented May 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

bzoracler commented May 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

bzoracler commented May 4, 2025

Uh oh!

This comment has been minimized.

srittau left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented May 22, 2025

Uh oh!

Uh oh!

Uh oh!

Add `visit_` and `depart_` methods to `docutils` visitor classes #13928

Add `visit_` and `depart_` methods to `docutils` visitor classes #13928

bzoracler commented May 3, 2025 •

edited

Loading

bzoracler commented May 3, 2025 •

edited

Loading