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

[Markus Armbruster]

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?
>> 
>
> Nope.
>
>> In case it matters:
>>      $ mypy --version
>>      mypy 0.761
>> 
>
> I am using mypy 0.782.
>
> I will investigate to see if there is an *easy* win to allow older
> versions to work.
>
> In the meantime, please consider trying this:
>
> pip install --user mypy==0.782
> ~/.local/bin/mypy --config-file=qapi/mypy.ini qapi/

I'll consider dragging my feet until upgrading Fedora gives it to me for
free.

The less I interact with package managers, the happier I am.

>>> Signed-off-by: John Snow <jsnow@redhat.com>
>>> ---
>>>   scripts/qapi/doc.py    |  3 +-
>>>   scripts/qapi/mypy.ini  | 65 ++++++++++++++++++++++++++++++++++++++++++
>>>   scripts/qapi/schema.py |  3 +-
>>>   3 files changed, 69 insertions(+), 2 deletions(-)
>>>   create mode 100644 scripts/qapi/mypy.ini
>>>
>>> diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
>>> index cbf7076ed9..70f7cdfaa6 100644
>>> --- a/scripts/qapi/doc.py
>>> +++ b/scripts/qapi/doc.py
>>> @@ -5,7 +5,8 @@
>>>   """This script produces the documentation of a qapi schema in texinfo 
>>> format"""
>>>     import re
>>> -from .gen import QAPIGenDoc, QAPISchemaVisitor
>>> +from .gen import QAPIGenDoc
>>> +from .schema import QAPISchemaVisitor
>> Your mypy doesn't like such lazy imports?  Mine seems not to care.
>> 
>
> Yeah, it specifically complained that no such definition existed in
> that file.

I sense a certain wobbliness in mypy.  Perhaps to be expected from a
tool with major version zero.  There's a risk that developers' local
"make check" and our gating CI differ too much.  We'll see.

>>>     
>>>   MSG_FMT = """
>>> diff --git a/scripts/qapi/mypy.ini b/scripts/qapi/mypy.ini
>>> new file mode 100644
>>> index 0000000000..a0f2365a53
>>> --- /dev/null
>>> +++ b/scripts/qapi/mypy.ini
>>> @@ -0,0 +1,65 @@
>>> +[mypy]
>>> +strict = True
>>> +strict_optional = False
>>> +disallow_untyped_calls = False
>>> +python_version = 3.6
>>> +
>>> +[mypy-qapi.commands]
>>> +disallow_untyped_defs = False
>>> +disallow_incomplete_defs = False
>>> +check_untyped_defs = False
>>> +
>>> +[mypy-qapi.doc]
>>> +disallow_subclassing_any = False
>>> +disallow_untyped_defs = False
>>> +disallow_incomplete_defs = False
>>> +
>>> +[mypy-qapi.error]
>>> +disallow_untyped_defs = False
>>> +disallow_incomplete_defs = False
>>> +check_untyped_defs = False
>>> +
>>> +[mypy-qapi.events]
>>> +disallow_untyped_defs = False
>>> +disallow_incomplete_defs = False
>>> +check_untyped_defs = False
>>> +
>>> +[mypy-qapi.expr]
>>> +disallow_untyped_defs = False
>>> +disallow_incomplete_defs = False
>>> +check_untyped_defs = False
>>> +
>>> +[mypy-qapi.gen]
>>> +disallow_untyped_defs = False
>>> +disallow_incomplete_defs = False
>>> +check_untyped_defs = False
>>> +
>>> +[mypy-qapi.introspect]
>>> +disallow_untyped_defs = False
>>> +disallow_incomplete_defs = False
>>> +check_untyped_defs = False
>>> +
>>> +[mypy-qapi.parser]
>>> +disallow_untyped_defs = False
>>> +disallow_incomplete_defs = False
>>> +check_untyped_defs = False
>>> +
>>> +[mypy-qapi.schema]
>>> +disallow_untyped_defs = False
>>> +disallow_incomplete_defs = False
>>> +check_untyped_defs = False
>>> +
>>> +[mypy-qapi.source]
>>> +disallow_untyped_defs = False
>>> +disallow_incomplete_defs = False
>>> +check_untyped_defs = False
>>> +
>>> +[mypy-qapi.types]
>>> +disallow_untyped_defs = False
>>> +disallow_incomplete_defs = False
>>> +check_untyped_defs = False
>>> +
>>> +[mypy-qapi.visit]
>>> +disallow_untyped_defs = False
>>> +disallow_incomplete_defs = False
>>> +check_untyped_defs = False
>> Greek to me.  I'll learn in due time.
>> 
>
> I am using these options:
>
> --strict, which is effectively -Wall.
>
> --no-strict-optional, which disables type checking errors on conflict
>   between Optional[T] and T. Namely, when you initialize a class field
>  to None and set that variable after initialization, callers must be 
> prepared to see if that field was None or not. We do that effectively
> nowhere.
>
> As Python will surely explode in a noticeable way if we got an
> unexpected 'None', I am just suppressing these warnings "for now".

Okay.

We may want to rethink how we define and initialize these variables.  We
initialize mostly to keep pylint happy.  We initialize to None precisely
to make use before the real initialization explode.

> --allow-untyped-calls silences errors in files that have calls to
>   functions in files I still have not typed. By the end of the series, 
> this option goes away, because there's nothing untyped left.
>
>
> For each untyped file, we are actually starting with all of the above
> options and then layering these options on top. Any egregious typing 
> errors present in these "ignored" files will be spotted.
>
> To get the bad files to pass, we only need three options:
>
> allow untyped defs -- Simply permits us to have functions without
> annotations.
>
> allow incomplete defs -- allows functions that are only partially typed.
>
> check untyped defs = False -- Don't try to type check untyped definitions.

Thanks!

[...]