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

[John Snow]

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/

> > 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.

> >   
> >   
> >   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".

--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.

> > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> > index b4921b46c9..bb0cd717f1 100644
> > --- a/scripts/qapi/schema.py
> > +++ b/scripts/qapi/schema.py
> > @@ -17,6 +17,7 @@
> >   import os
> >   import re
> >   from collections import OrderedDict
> > +from typing import Optional
> >   
> >   from .common import c_name, POINTER_SUFFIX
> >   from .error import QAPIError, QAPISemError
> > @@ -25,7 +26,7 @@
> >   
> >   
> >   class QAPISchemaEntity:
> > -    meta = None
> > +    meta: Optional[str] = None
> >   
> >       def __init__(self, name, info, doc, ifcond=None, features=None):
> >           assert name is None or isinstance(name, str)