• 
      

    Improve JSON typing by including Mapping/Sequence variants.

    Review Request #13125 — Created June 26, 2023 and submitted

    Information

    Djblets
    release-3.x

    Reviewers

    The new JSON typing introduced in Djblets 3.3 worked fine if exclusively
    working with JSONValue, JSONDict, JSONList, or simple primitives
    (like int or bool). However, trying to assign an
    otherwise-compatible Dict or List (such as a Dict[str, str] or
    List[int]) would result in typing errors.

    This was due to the mutability of the type. A Dict[str, str], for
    example, cannot be assigned as a JSONDict, because a JSONDict should
    be able to set any JSONValue as a value, and that would violate the
    type of a Dict[str, str]. Same with lists.

    The solution to this is to accept a Mapping and Sequence. These are
    both immutable types, and from a typing perspective, there's no risk to
    storing the wrong value there. In part, this is because we already can't
    deeply key into a nested JSONDict without casting at each level
    anyway, since a value could be any supported JSON type. So once we've
    stored it, it's up to the callers to validate types anyway (and at this
    complexity, we're wanting to represent structures as TypedDicts and/or
    sanity-check values).

    The new JSONDictImmutable and JSONListImmutable provide this typing
    option. They're part of the JSONValue union, allowing type checkers to
    check against them when assigning a Dict or a List.

    Going forward, we'll want to aim to use Mapping, Sequence,
    Iterable, and other more generic immutable types over Dict and
    List any time we're accepting a value we don't plan to modify, to
    avoid the sort of problems we've uncovered with our JSON types.

    Encountered this issue with in-progress code. Verified that the this
    addressed all issues.

    Played around a fair amount with creating and typing deeply-nested
    JSON data from dicts and lists containing compatible values, and
    verified that pyright and mypy were happy.

    Summary ID
    Improve JSON typing by including Mapping/Sequence variants.
    The new JSON typing introduced in Djblets 3.3 worked fine if exclusively working with `JSONValue`, `JSONDict`, `JSONList`, or simple primitives (like `int` or `bool`). However, trying to assign an otherwise-compatible `Dict` or `List` (such as a `Dict[str, str]` or `List[int]`) would result in typing errors. This was due to the mutability of the type. A `Dict[str, str]`, for example, cannot be assigned as a `JSONDict`, because a `JSONDict` should be able to set any `JSONValue` as a value, and that would violate the type of a `Dict[str, str]`. Same with lists. The solution to this is to accept a `Mapping` and `Sequence`. These are both immutable types, and from a typing perspective, there's no risk to storing the wrong value there. In part, this is because we already can't deeply key into a nested `JSONDict` without casting at each level anyway, since a value could be any supported JSON type. So once we've stored it, it's up to the callers to validate types anyway (and at this complexity, we're wanting to represent structures as `TypedDict`s and/or sanity-check values). The new `JSONDictImmutable` and `JSONListImmutable` provide this typing option. They're part of the `JSONValue` union, allowing type checkers to check against them when assigning a `Dict` or a `List`. Going forward, we'll want to aim to use `Mapping`, `Sequence`, `Iterable`, and other more generic immutable types over `Dict` and `List` any time we're accepting a value we don't plan to modify, to avoid the sort of problems we've uncovered with our `JSON` types.
    00b4c4f1ee8599627157a40c90a76bf6b96e7566
    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-3.x (53abc7a)