Re: [PATCH 16/37] qapi: establish mypy type-checking baseline

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> On 9/18/20 7:55 AM, Markus Armbruster wrote:
>> Ignorant question: why does this come after PATCH 13 "qapi/common.py:
>> add notational type hints", but before all the other patches adding type
>> hints?
>> John Snow <jsnow@redhat.com> writes:
>> 
>>> Fix two very minor issues, and then establish a mypy type-checking
>>> baseline.
>>>
>>> Like pylint, this should be run from the folder above:
>>>
>>>   > mypy --config-file=qapi/mypy.ini qapi/
>> I get:
>>      $ mypy --config-file qapi/mypy.ini qapi
>>      qapi/mypy.ini: [mypy]: Strict mode is not supported in configuration files: specify individual flags instead (see 'mypy -h' for the list of flags enabled in strict mode)
>>      qapi/types.py:29: error: Need type annotation for 'objects_seen' (hint: "objects_seen: Set[<type>] = ...")
>>      Found 1 error in 1 file (checked 18 source files)
>> Is this expected?
>> In case it matters:
>>      $ mypy --version
>>      mypy 0.761
>> 
>
> (Warning; FiSH and stgit ahead)
>
> cd ~/src/qemu/scripts
> pipenv --python 3.6
> pipenv shell
> pip install pylint flake8
>
> ### Testing mypy 0.770
>
> pip install mypy==0.770
> stg goto qapi-establish-mypy-type
>
> while true; and flake8 qapi/; and pylint --rcfile=qapi/pylintrc qapi/;
> and mypy --config-file=qapi/mypy.ini qapi/; and stg push; end
>
> pip uninstall mypy
>
> ###
>
>
>
> 0.782 - OK
> 0.770 - OK
> 0.760 - FAIL, Fixable*
> 0.750 - OK*
> 0.740 - OK*
> 0.730 - OK*
> 0.720 - OK*
> 0.710 - OK** (Does not recognize 'no_implicit_reexport' option)
> 0.700 - OK*** (Not compatible with bleeding edge pylint/flake8)
> 0.670 - OK***
> 0.660 - OK***
> 0.650 - OK***
> 0.641 - OK***
> 0.630 - Fails again.
>
>
>
> 0.760 doesn't support strict in the config file (It needs component
> options), and it wants a few extra annotations where its inference
> power is weaker. Well, easy enough to fix up.
>
> 0.630 fails again and insists that __init__ should have a return type
> annotation of None. Modern mypy is smart enough to know that's what
> that type is supposed to be. Arbitrarily, this is my cutoff for what
> seems reasonable to even want to support.
>
> I still find the lack of "strict=true" in the config file irritating
> and might ask to target 0.770 or newer. There should be no reason we
> can't install that in a venv for CI to chew on.
>
> Humbly ask I take the lazy way out and document that we support mypy
> >= 0.770.

Our "supported build platforms" policy puts hard limits on the minimum
versions for tools the build requires.

Mypy is not such a tool.  I hope we get to the point where we can have
"make check" run it, but skipping the test when we don't have a
sufficiently modern mypy feels okay to me, as long as our gating CI
still guards the master branch.