On 9/23/20 3:42 PM, Eduardo Habkost wrote:
On Tue, Sep 22, 2020 at 05:13:00PM -0400, John Snow wrote:
mypy does not know the types of values stored in Dicts that masquerade
as objects. Help the type checker out by constraining the type.
Signed-off-by: John Snow <js...@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 1872a8a3cc..f6b55a87c1 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,
@@ -280,9 +288,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)
mypy doesn't seem to require the key type asserts, on my testing.
Strictly no. This code is removed somewhere in part 5 when I introduce a
typed structure to carry this data from the Parser to the Expression
checker.
(Sometimes, these asserts were for my own sake.)
+
+ # 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
Do you need a separate variable here? This seems to work too:
doc = expr_elem.get('doc')
assert doc is None or isinstance(doc, QAPIDoc)
after the assert, mypy will infer the type of doc to be
Optional[QAPIDoc].
In full honesty, I don't recall... but this code does get replaced by
the end of this marathon.
None of this should block a useful 120-patch cleanup series, so:
Reviewed-by: Eduardo Habkost <ehabk...@redhat.com>
Thanks!
