Enable strict_optional in mypy config
- Dominant language
- Rust
- Stars
- 31.2k
- Forks
- 3.2k
- Avg merge
- 2d 8h
- Merged PRs (30d)
- 72
Description
Our mypy configuration uses `no_strict_optional = true`. This tells mypy to assume nullable variables will not be none, so code like the following will pass a check, and fail at runtime instead:
```
foo: int | None = None
foo + 5
```
If we adjust the mypy config, we can get it to throw errors instead:
```diff
diff --git a/.mypy.ini b/.mypy.ini
index ef32ed7a2..1207b6aa3 100644
--- a/.mypy.ini
+++ b/.mypy.ini
@@ -22,6 +22,7 @@ exclude = (qt/bundle/PyOxidizer|pylib/anki/_vendor)
[mypy-anki.*]
disallow_untyped_defs = True
+no_strict_optional = false
[mypy-anki.importing.*]
disallow_untyped_defs = False
[mypy-anki.exporting]
```
There's quite a few in pylib that need correcting, e.g.
```
pylib/anki/importing/anki2.py:316: error: Item "None" of "Optional[DBProxy]" has no attribute "execute" [union-attr]
pylib/anki/importing/csvfile.py:114: error: Value of type "Union[str, list[str], None]" is not indexable [index]
pylib/anki/importing/csvfile.py:116: error: Value of type "Union[str, list[str], None]" is not indexable [index]
pylib/anki/importing/csvfile.py:118: error: Value of type "Union[str, list[str], None]" is not indexable [index]
pylib/anki/importing/csvfile.py:122: error: Argument 1 to "reader" has incompatible type "Union[str, list[str], None]"; expected "Iterable[str]" [arg-type]
pylib/anki/importing/apkg.py:62: error: Item "None" of "Optional[ZipFile]" has no attribute "read" [union-attr]
Found 181 errors in 24 files (checked 336 source files)
```
The first error is caused by doing 'col.db.execute()' when col.db is DBProxy | None. Adding an `assert col.db` will tell mypy that we are confident db will not be null in that case, and the error will go away. In other cases where we're not 100% sure if the value will be valid, the routine should check for null / exit early / etc.
The work on this can be done incrementally, as you can enable no_strict_optional for individual submodules at a time (e.g. pylib.importing).
Many of the errors are in legacy code (old stats, old importing code) that will eventually be removed. In such cases, sprinkling asserts above the problem lines is probably all that is required. Our goal here is not really to change the way the errors are handled at run time. The goal is to get our code working with the new setting, so we can benefit from its increased protection in other parts of the codebase.
Another upside of doing this is that pyright (the type checker used by VS Code) does not support no_strict_optional. That causes our current codebase to show a lot errors when checked with pyright.
I suspect sorting out pylib first will bring the most value. aqt would benefit from being tackled too in the future, but there are 1000+ errors at the moment. Sometimes a single assert at the top of a function will fix a bunch of errors at once, as all the subsequent code can assume some variable is non-null. So the total issues to fix are likely smaller than that. But better to start with the low-hanging fruit.
Contributor guide
Research direction
Start with .mypy.ini and the reported errors in pylib/anki/importing/anki2.py, csvfile.py, and apkg.py. Enable strict optional handling for a small pylib module or submodule, run mypy, and resolve its nullable-value errors; done means the selected scope passes with the stricter setting without changing runtime behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- developer-experience, tooling
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100