| Message ID | 20201026213637.47087-4-jsnow@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | qapi: static typing conversion, pt3 | expand |
John Snow <jsnow@redhat.com> writes: > mypy does not know the types of values stored in Dicts that masquerade > as objects. Help the type checker out by constraining the type. I'm not sure I understand what you mean by "dict masquerading as object". > > Signed-off-by: John Snow <jsnow@redhat.com> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > Reviewed-by: Cleber Rosa <crosa@redhat.com> > --- > scripts/qapi/expr.py | 25 ++++++++++++++++++++++--- > 1 file changed, 22 insertions(+), 3 deletions(-) > > diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py > index 5694c501fa38..f7c7f91326ef 100644 > --- a/scripts/qapi/expr.py > +++ b/scripts/qapi/expr.py > @@ -15,9 +15,17 @@ > # See the COPYING file in the top-level directory. > > import re > +from typing import MutableMapping, Optional > > from .common import c_name > from .error import QAPISemError > +from .parser import QAPIDoc > +from .source import QAPISourceInfo > + > + > +# Expressions in their raw form are JSON-like structures with arbitrary forms. > +# Minimally, their top-level form must be a mapping of strings to values. PEP 8: "For flowing long blocks of text with fewer structural restrictions (docstrings or comments), the line length should be limited to 72 characters." > +Expression = MutableMapping[str, object] > > > # Names must be letters, numbers, -, and _. They must start with letter, > @@ -287,9 +295,20 @@ def check_event(expr, info): > > def check_exprs(exprs): > for expr_elem in exprs: > - expr = expr_elem['expr'] > - info = expr_elem['info'] > - doc = expr_elem.get('doc') dict is arguably a rather awkward choice here. I figure a named tuple would be neater. A class feels like overkill. Observation, not demand. > + # Expression > + assert isinstance(expr_elem['expr'], dict) > + expr: Expression = expr_elem['expr'] > + for key in expr.keys(): > + assert isinstance(key, str) > + > + # QAPISourceInfo > + assert isinstance(expr_elem['info'], QAPISourceInfo) > + info: QAPISourceInfo = expr_elem['info'] > + > + # Optional[QAPIDoc] > + tmp = expr_elem.get('doc') > + assert tmp is None or isinstance(tmp, QAPIDoc) > + doc: Optional[QAPIDoc] = tmp > > if 'include' in expr: > continue I hope further typing work will reduce the isinstance() assertions again.
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index 5694c501fa38..f7c7f91326ef 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -15,9 +15,17 @@ # See the COPYING file in the top-level directory. import re +from typing import MutableMapping, Optional from .common import c_name from .error import QAPISemError +from .parser import QAPIDoc +from .source import QAPISourceInfo + + +# Expressions in their raw form are JSON-like structures with arbitrary forms. +# Minimally, their top-level form must be a mapping of strings to values. +Expression = MutableMapping[str, object] # Names must be letters, numbers, -, and _. They must start with letter, @@ -287,9 +295,20 @@ def check_event(expr, info): def check_exprs(exprs): for expr_elem in exprs: - expr = expr_elem['expr'] - info = expr_elem['info'] - doc = expr_elem.get('doc') + # Expression + assert isinstance(expr_elem['expr'], dict) + expr: Expression = expr_elem['expr'] + for key in expr.keys(): + assert isinstance(key, str) + + # QAPISourceInfo + assert isinstance(expr_elem['info'], QAPISourceInfo) + info: QAPISourceInfo = expr_elem['info'] + + # Optional[QAPIDoc] + tmp = expr_elem.get('doc') + assert tmp is None or isinstance(tmp, QAPIDoc) + doc: Optional[QAPIDoc] = tmp if 'include' in expr: continue