element-hq / element-hq/synapse

We should make `JsonDict` default to stricter type checking, now that it's possible

Open
#20,103 0 comments 0 reactions 1 assignee Claimed by @reivilibre View on GitHub
A-Validation T-Task
Dominant language
Python
Stars
4.6k
Forks
600
Avg merge
5d 22h
Merged PRs (30d)
51

Description

In https://github.com/element-hq/synapse/commit/0a2456fef357b6ede022fc14308a7eb8db085115 I introduced `StrictJsonDict` (and a family of related 'strict' JSON types), after discovering that this has become possible. (In the past we had trouble getting Mypy to accept a recursive type definition.)

Our current codebase norm is to use `JsonDict` which is defined as `dict[str, Any]`.
I suspect there are more instances of under-validated JSON being propagated that are concealed by the `Any` in `JsonDict`.

I intend to repoint `JsonDict` to be strict, which will involve adding appropriate validation in some places.
I will introduce a `LaxJsonDict` type with the old definition, with the intention of using this in test code where we currently need it (as otherwise we will probably have to insert more verbose checks into our test code).

I will then attempt to fix up code that relies on `JsonDict = LaxJsonDict` and drive down the number of errors that occur from a changeover. I would like to avoid polluting git blame everywhere by doing a tree-wide rename, so I'll rely on doing the work upfront before a single changeover.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.