Re: [PATCH 06/19] qapi/schema: adjust type narrowing for mypy's benefit

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On Wed, Nov 22, 2023, 7:00 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > On Tue, Nov 21, 2023, 9:09 AM Markus Armbruster <armbru@redhat.com> wrote:

[...]

>> >> Harmless enough.  I can't quite see the mypy problem, though.  Care to
>> >> elaborate a bit?
>> >>
>> >
>> > self.arg_type has a narrower type- or, it WILL at the end of this series -
>> > so we need to narrow a temporary variable first before assigning it to the
>> > object state.
>> >
>> > We already perform the necessary check/narrowing, so this is really just
>> > pointing out that it's a bad idea to assign the state before the type
>> > check. Now we type check before assigning state.
>>
>> After PATCH 16, .resolve_type() will return QAPISchemaType, and
>> self.arg_type will be Optional[QAPISchemaObjectType].  Correct?
>>
>
> Sounds right. Sometimes it's a little hard to see what the error is before
> the rest of the types go in, a hazard of needing all patches to bisect
> without regression.
>
> Do you want a more elaborate commit message?

Your commit messages of PATCH 3+4 show the error.  Helps.

Maybe

    qapi/schema: Adjust type narrowing for mypy's benefit

    We already take care to perform some type narrowing for arg_type and
    ret_type, but not in a way where mypy can utilize the result once we
    add type hints:

        error message goes here

    A simple change to use a temporary variable helps the medicine go
    down.