sphinx-doc / sphinx

The Sphinx documentation generator
https://www.sphinx-doc.org/
Other
6.46k stars 2.1k forks source link

annotate class variables using `typing.ClassVar` #12067

Open danieleades opened 6 months ago

danieleades commented 6 months ago

Explicitly annotating class variables using typing.ClassVar creates greater type safety. Furthermore, migrating to types-docutils will require this since the upstream types use explicit class variable annotation and mypy will return errors which sphinx subclasses overwrite these without explicit ClassVar annotations.

This is already causing downstream errors for users who are using Sphinx with types-docutils (see https://github.com/python/typeshed/pull/11550#issuecomment-1991703182)

danieleades commented 6 months ago

note that sphinx (or docutils?) uses some weird patterns that make this non-trivial. For example:

class SphinxI18nReader(SphinxBaseReader):
    """
    A document reader for i18n.

    This returns the source line number of original text as current source line number
    to let users know where the error happened.
    Because the translated texts are partial and they don't have correct line numbers.
    """

    def setup(self, app: Sphinx) -> None:
        super().setup(app)

        self.transforms = self.transforms + app.registry.get_transforms()
        unused = [PreserveTranslatableMessages, Locale, RemoveTranslatableInline,
                  AutoIndexUpgrader, SphinxDomains, DoctreeReadEvent,
                  UIDTransform]
        for transform in unused:
            if transform in self.transforms:
                self.transforms.remove(transform)

SphinxI18nReader subclasses SphinxBaseReader, which defines a class variable transforms

so in SphinxI18nReader we're overwriting a class variable with an instance variable. This is definitely an antipattern, and mypy will rightly complain about it.

So this won't just be a simple case of sprinkling ClassVar annotations around. Of course once this is done, the ClassVar annotations will prevent these antipatterns in future