Enable Mypy "strict optional"

[danieleades]

Mypy has 'strict optional' checking disabled, as per https://github.com/sphinx-doc/sphinx/pull/4933

this is hiding a lot of type errors in the code base, and should be enabled.

Probably makes sense to do this incrementally, as it will require quite a few changes.

[danieleades]

the rebuild arg of add_config_value is consistently used with a value of None, whereas the type of the arg is bool | str. what should this value be (rather than None)?

def setup(app: Sphinx) -> Dict[str, Any]:
    app.add_builder(TexinfoBuilder)

    app.add_config_value('texinfo_documents', default_texinfo_documents, None)
    app.add_config_value('texinfo_appendices', [], None)
    app.add_config_value('texinfo_elements', {}, None)
    app.add_config_value('texinfo_domain_indices', True, None, [list])
    app.add_config_value('texinfo_show_urls', 'footnote', None)
    app.add_config_value('texinfo_no_detailmenu', False, None)
    app.add_config_value('texinfo_cross_references', True, None)

    return {
        'version': 'builtin',
        'parallel_read_safe': True,
        'parallel_write_safe': True,
    }

[AA-Turner]

There are very few recent issues that would have been solved by stricter None checking -- beyond type purity, I'm not sure what the goal is. I support the general effort, but I'm just not sure on prioritisation.

A

[AA-Turner]

A quick test for setting strict_optional = True (after fixing some Windows typing issues, which weren't really issues):

(sphinx) S:\Development\sphinx>mypy sphinx
<... 1399 lines of output elided ...>
Found 916 errors in 92 files (checked 175 source files)

A

[danieleades]

Id consider it more an aspirational goal, than a high priority. I think stricter type checking has more value when writing new code, rather than weeding out errors in battle-tested legacy code

[tk0miya]

+1: I agree and understand enabling "strict optional" is cool and worthy. But it's very tough work. So we need your help.

When I introduce annotations to Sphinx, I gave up to use "Optional" for our code and disable the strict check.

[danieleades]

+1: I agree and understand enabling "strict optional" is cool and worthy. But it's very tough work. So we need your help.

When I introduce annotations to Sphinx, I gave up to use "Optional" for our code and disable the strict check.

another option is to add a whitelist to disable strict optional checks for any files which currently fail, and then aim to incrementally shrink that whitelist in subsequent PRs. That's an approach I've used before for incremental mypy adoption (for example in poetry, poetry-core and sphinx-needs. The advantage is that you get immediate benefit for any new modules, without having to wait until the whole codebase is compliant.

see

• https://mypy.readthedocs.io/en/stable/config_file.html#per-module-and-global-options
• https://github.com/useblocks/sphinxcontrib-needs/blob/b9c5470c4f52f7866cbe5434bfda3e620d144b52/pyproject.toml#L82

[danieleades]

this is largely complete now, bar the C and C++ modules. Amazing work!