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

From: John Snow <jsnow@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Cleber Rosa" <crosa@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	qemu-devel@nongnu.org, "Eduardo Habkost" <ehabkost@redhat.com>
Subject: Re: [PATCH 16/37] qapi: establish mypy type-checking baseline
Date: Fri, 18 Sep 2020 15:03:41 -0400	[thread overview]
Message-ID: <d8faa68c-6900-5666-943c-98815348fc5b@redhat.com> (raw)
In-Reply-To: <874knvguzb.fsf@dusky.pond.sub.org>

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.

--js