👌 Improve footnote def/ref warnings and translations (#931)

Footnotes are now parsed similar to the corresponding restructuredtext, in that resolution (between definitions and references) and ordering is now deferred to transforms on the doctree.

This allows for the proper interaction with other docutils/sphinx transforms, including those that perform translations.

In addition, an upstream improvement to unreferenced footnote definitions is also added here: sphinx-doc/sphinx#12730, so that unreferenced and duplicate definitions are correctly warned about, e.g.:

```
source/index.md:1: WARNING: Footnote [1] is not referenced. [ref.footnote]
source/index.md:4: WARNING: Duplicate footnote definition found for label: 'a' [ref.footnote]
```

It is of note that warnings for references with no corresponding definitions are deferred to docutils to handle, e.g. for `[^a]` with no definition:

```
source/index.md:1: ERROR: Too many autonumbered footnote references: only 0 corresponding footnotes available. [docutils]
source/index.md:1: ERROR: Unknown target name: "a". [docutils]
```

These warning messages are a little obscure, and it would be ideal that one clear warning was emitted for the issue.
However, it is non-trivial in this extension; to both suppress the existing warnings, and then replace them with a better one,
so for now we don't do it here, and ideally this would be improved upstream in docutils.

docs/configuration.md

@@ -195,13 +195,13 @@ tasklist
 (myst-warnings)=
 ## Build Warnings
 
-Below lists the MyST specific warnings that may be emitted during the build process. These will be prepended to the end of the warning message, e.g.
+Below lists the MyST specific warnings that may be emitted during the build process. These will be prepended to the end of the warning message (see also <inv:sphinx#show_warning_types>), e.g.
 
 ```
 WARNING: Non-consecutive header level increase; H1 to H3 [myst.header]
 ```
 
-**In general, if your build logs any warnings, you should either fix them or [raise an Issue](https://github.com/executablebooks/MyST-Parser/issues/new/choose) if you think the warning is erroneous.**
+In general, if your build logs any warnings, you should either fix them or [raise an Issue](https://github.com/executablebooks/MyST-Parser/issues/new/choose) if you think the warning is erroneous.
 
 However, in some circumstances if you wish to suppress the warning you can use the <inv:sphinx#suppress_warnings> configuration option, e.g.
 

docs/syntax/typography.md

@@ -295,13 +295,16 @@ that are not separated by a blank line
 This is not part of the footnote.
 :::
 
-````{important}
-Although footnote references can be used just fine within directives, e.g.[^myref],
-it is recommended that footnote definitions are not set within directives,
-unless they will only be referenced within that same directive:
+By default, the footnotes will be collected, sorted and moved to the end of the document,
+with a transition line placed before any footnotes (that has a `footnotes` class).
 
-This is because, they may not be available to reference in text outside that particular directive.
-````
+This behaviour can be modified using the [configuration options](#sphinx/config-options):
 
-By default, a transition line (with a `footnotes` class) will be placed before any footnotes.
-This can be turned off by adding `myst_footnote_transition = False` to the config file.
+```python
+myst_footnote_sort = False
+myst_footnote_transition = False
+```
+
+```{versionadded} 4.0.0
+``myst_footnote_sort`` configuration option
+```

myst_parser/config/main.py

@@ -319,11 +319,19 @@ def __repr__(self) -> str:
         },
     )
 
+    footnote_sort: bool = dc.field(
+        default=True,
+        metadata={
+            "validator": instance_of(bool),
+            "help": "Move all footnotes to the end of the document, and sort by reference order",
+        },
+    )
+
     footnote_transition: bool = dc.field(
         default=True,
         metadata={
             "validator": instance_of(bool),
-            "help": "Place a transition before any footnotes",
+            "help": "Place a transition before sorted footnotes",
         },
     )
 

myst_parser/mdit_to_docutils/base.py

@@ -7,7 +7,6 @@
 import os
 import posixpath
 import re
-from collections import OrderedDict
 from collections.abc import Callable, Iterable, Iterator, MutableMapping, Sequence
 from contextlib import contextmanager, suppress
 from datetime import date, datetime
@@ -159,8 +158,9 @@ def sphinx_env(self) -> BuildEnvironment | None:
     def create_warning(
         self,
         message: str,
-        subtype: MystWarnings,
+        subtype: MystWarnings | str,
         *,
+        wtype: str | None = None,
         line: int | None = None,
         append_to: nodes.Element | None = None,
     ) -> nodes.system_message | None:
@@ -173,6 +173,7 @@ def create_warning(
             self.document,
             message,
             subtype,
+            wtype=wtype,
             line=line,
             append_to=append_to,
         )
@@ -190,20 +191,6 @@ def _render_tokens(self, tokens: list[Token]) -> None:
 
         # nest tokens
         node_tree = SyntaxTreeNode(tokens)
-
-        # move footnote definitions to env
-        self.md_env.setdefault("foot_refs", {})
-        for node in node_tree.walk(include_self=True):
-            new_children = []
-            for child in node.children:
-                if child.type == "footnote_reference":
-                    label = child.meta["label"]
-                    self.md_env["foot_refs"].setdefault(label, []).append(child)
-                else:
-                    new_children.append(child)
-
-            node.children = new_children
-
         # render
         for child in node_tree.children:
             # skip hidden?
@@ -254,6 +241,12 @@ def _render_finalise(self) -> None:
                 self._heading_slugs
             )
 
+        # ensure these settings are set for later footnote transforms
+        self.document.settings.myst_footnote_transition = (
+            self.md_config.footnote_transition
+        )
+        self.document.settings.myst_footnote_sort = self.md_config.footnote_sort
+
         # log warnings for duplicate reference definitions
         # "duplicate_refs": [{"href": "ijk", "label": "B", "map": [4, 5], "title": ""}],
         for dup_ref in self.md_env.get("duplicate_refs", []):
@@ -264,35 +257,6 @@ def _render_finalise(self) -> None:
                 append_to=self.document,
             )
 
-        # we don't use the foot_references stored in the env
-        # since references within directives/roles will have been added after
-        # those from the initial markdown parse
-        # instead we gather them from a walk of the created document
-        foot_refs = OrderedDict()
-        for refnode in findall(self.document)(nodes.footnote_reference):
-            if refnode["refname"] not in foot_refs:
-                foot_refs[refnode["refname"]] = True
-
-        if foot_refs and self.md_config.footnote_transition:
-            self.current_node.append(nodes.transition(classes=["footnotes"]))
-        for footref in foot_refs:
-            foot_ref_tokens = self.md_env["foot_refs"].get(footref, [])
-            if len(foot_ref_tokens) > 1:
-                self.create_warning(
-                    f"Multiple footnote definitions found for label: '{footref}'",
-                    MystWarnings.MD_FOOTNOTE_DUPE,
-                    append_to=self.current_node,
-                )
-
-            if len(foot_ref_tokens) < 1:
-                self.create_warning(
-                    f"No footnote definitions found for label: '{footref}'",
-                    MystWarnings.MD_FOOTNOTE_MISSING,
-                    append_to=self.current_node,
-                )
-            else:
-                self.render_footnote_reference(foot_ref_tokens[0])
-
         # Add the wordcount, generated by the ``mdit_py_plugins.wordcount_plugin``.
         wordcount_metadata = self.md_env.get("wordcount", {})
         if wordcount_metadata:
@@ -1469,29 +1433,50 @@ def render_footnote_ref(self, token: SyntaxTreeNode) -> None:
 
         refnode = nodes.footnote_reference(f"[^{target}]")
         self.add_line_and_source_path(refnode, token)
-        if not target.isdigit():
+        if target.isdigit():
+            # a manually numbered footnote, similar to rST ``[1]_``
+            refnode += nodes.Text(target)
+        else:
+            # an auto-numbered footnote, similar to rST ``[#label]_``
             refnode["auto"] = 1
             self.document.note_autofootnote_ref(refnode)
-        else:
-            refnode += nodes.Text(target)
 
         refnode["refname"] = target
         self.document.note_footnote_ref(refnode)
 
         self.current_node.append(refnode)
 
     def render_footnote_reference(self, token: SyntaxTreeNode) -> None:
+        """Despite the name, this is actually a footnote definition, e.g. `[^a]: ...`"""
         target = token.meta["label"]
 
+        if target in self.document.nameids:
+            # note we chose to directly omit these footnotes in the parser,
+            # rather than let docutils/sphinx handle them, since otherwise you end up with a confusing warning:
+            # WARNING: Duplicate explicit target name: "x". [docutils]
+            # we use [ref.footnote] as the type/subtype, rather than a myst specific warning,
+            # to make it more aligned with sphinx warnings for unreferenced footnotes
+            self.create_warning(
+                f"Duplicate footnote definition found for label: '{target}'",
+                "footnote",
+                wtype="ref",
+                line=token_line(token),
+                append_to=self.current_node,
+            )
+            return
+
         footnote = nodes.footnote()
         self.add_line_and_source_path(footnote, token)
         footnote["names"].append(target)
-        if not target.isdigit():
-            footnote["auto"] = 1
-            self.document.note_autofootnote(footnote)
-        else:
+        if target.isdigit():
+            # a manually numbered footnote, similar to rST ``.. [1]``
             footnote += nodes.label("", target)
             self.document.note_footnote(footnote)
+        else:
+            # an auto-numbered footnote, similar to rST ``.. [#label]``
+            footnote["auto"] = 1
+            self.document.note_autofootnote(footnote)
+
         self.document.note_explicit_target(footnote, footnote)
         with self.current_node_context(footnote, append=True):
             self.render_children(token)

myst_parser/mdit_to_docutils/transforms.py

@@ -6,15 +6,140 @@
 
 from docutils import nodes
 from docutils.transforms import Transform
+from docutils.transforms.references import Footnotes
 from markdown_it.common.normalize_url import normalizeLink
 
 from myst_parser._compat import findall
 from myst_parser.mdit_to_docutils.base import clean_astext
 from myst_parser.warnings_ import MystWarnings, create_warning
 
 
+class UnreferencedFootnotesDetector(Transform):
+    """Detect unreferenced footnotes and emit warnings.
+
+    Replicates https://github.com/sphinx-doc/sphinx/pull/12730,
+    but also allows for use in docutils (without sphinx).
+    """
+
+    default_priority = Footnotes.default_priority + 2
+
+    # document: nodes.document
+
+    def apply(self, **kwargs: t.Any) -> None:
+        """Apply the transform."""
+
+        for node in self.document.footnotes:
+            # note we do not warn on duplicate footnotes here
+            # (i.e. where the name has been moved to dupnames)
+            # since this is already reported by docutils
+            if not node["backrefs"] and node["names"]:
+                create_warning(
+                    self.document,
+                    "Footnote [{}] is not referenced.".format(node["names"][0])
+                    if node["names"]
+                    else node["dupnames"][0],
+                    wtype="ref",
+                    subtype="footnote",
+                    node=node,
+                )
+        for node in self.document.symbol_footnotes:
+            if not node["backrefs"]:
+                create_warning(
+                    self.document,
+                    "Footnote [*] is not referenced.",
+                    wtype="ref",
+                    subtype="footnote",
+                    node=node,
+                )
+        for node in self.document.autofootnotes:
+            # note we do not warn on duplicate footnotes here
+            # (i.e. where the name has been moved to dupnames)
+            # since this is already reported by docutils
+            if not node["backrefs"] and node["names"]:
+                create_warning(
+                    self.document,
+                    "Footnote [#] is not referenced.",
+                    wtype="ref",
+                    subtype="footnote",
+                    node=node,
+                )
+
+
+class SortFootnotes(Transform):
+    """Sort auto-numbered, labelled footnotes by the order they are referenced.
+
+    This is run before the docutils ``Footnote`` transform, where numbered labels are assigned.
+    """
+
+    default_priority = Footnotes.default_priority - 2
+
+    # document: nodes.document
+
+    def apply(self, **kwargs: t.Any) -> None:
+        """Apply the transform."""
+        if not self.document.settings.myst_footnote_sort:
+            return
+
+        ref_order: list[str] = [
+            node["refname"]
+            for node in self.document.autofootnote_refs
+            if "refname" in node
+        ]
+
+        def _sort_key(node: nodes.footnote) -> int:
+            if node["names"] and node["names"][0] in ref_order:
+                return ref_order.index(node["names"][0])
+            return 999
+
+        self.document.autofootnotes.sort(key=_sort_key)
+
+
+class CollectFootnotes(Transform):
+    """Transform to move footnotes to the end of the document, and sort by label."""
+
+    default_priority = Footnotes.default_priority + 3
+
+    # document: nodes.document
+
+    def apply(self, **kwargs: t.Any) -> None:
+        """Apply the transform."""
+        if not self.document.settings.myst_footnote_sort:
+            return
+
+        footnotes: list[tuple[str, nodes.footnote]] = []
+        for footnote in (
+            self.document.symbol_footnotes
+            + self.document.footnotes
+            + self.document.autofootnotes
+        ):
+            label = footnote.children[0]
+            footnotes.append((label.astext(), footnote))
+
+        if (
+            footnotes
+            and self.document.settings.myst_footnote_transition
+            # avoid warning: Document or section may not begin with a transition
+            and not all(isinstance(c, nodes.footnote) for c in self.document.children)
+        ):
+            transition = nodes.transition(classes=["footnotes"])
+            transition.source = self.document.source
+            self.document += transition
+
+        def _sort_key(footnote: tuple[str, nodes.footnote]) -> int | str:
+            label, _ = footnote
+            try:
+                # ensure e.g 10 comes after 2
+                return int(label)
+            except ValueError:
+                return label
+
+        for _, footnote in sorted(footnotes, key=_sort_key):
+            footnote.parent.remove(footnote)
+            self.document += footnote
+
+
 class ResolveAnchorIds(Transform):
-    """Directive for resolving `[name](#id)` type links."""
+    """Transform for resolving `[name](#id)` type links."""
 
     default_priority = 879  # this is the same as Sphinx's StandardDomain.process_doc
 

myst_parser/parsers/docutils_.py

@@ -23,7 +23,12 @@
     read_topmatter,
 )
 from myst_parser.mdit_to_docutils.base import DocutilsRenderer
-from myst_parser.mdit_to_docutils.transforms import ResolveAnchorIds
+from myst_parser.mdit_to_docutils.transforms import (
+    CollectFootnotes,
+    ResolveAnchorIds,
+    SortFootnotes,
+    UnreferencedFootnotesDetector,
+)
 from myst_parser.parsers.mdit import create_md_parser
 from myst_parser.warnings_ import MystWarnings, create_warning
 
@@ -246,7 +251,12 @@ class Parser(RstParser):
     translate_section_name = None
 
     def get_transforms(self):
-        return super().get_transforms() + [ResolveAnchorIds]
+        return super().get_transforms() + [
+            UnreferencedFootnotesDetector,
+            SortFootnotes,
+            CollectFootnotes,
+            ResolveAnchorIds,
+        ]
 
     def parse(self, inputstring: str, document: nodes.document) -> None:
         """Parse source text.

myst_parser/parsers/mdit.py

@@ -61,11 +61,8 @@ def create_md_parser(
         .use(front_matter_plugin)
         .use(myst_block_plugin)
         .use(myst_role_plugin)
-        .use(footnote_plugin)
+        .use(footnote_plugin, inline=False, move_to_end=False, always_match_refs=True)
         .use(wordcount_plugin, per_minute=config.words_per_minute)
-        .disable("footnote_inline")
-        # disable this for now, because it need a new implementation in the renderer
-        .disable("footnote_tail")
     )
 
     typographer = False