John Snow <js...@redhat.com> writes:
> Annotations do not change runtime behavior.
> This commit *only* adds annotations.
>
> (Annotations for QAPIDoc are in a later commit.)
>
> Signed-off-by: John Snow <js...@redhat.com>
> ---
> scripts/qapi/parser.py | 61 ++++++++++++++++++++++++++++--------------
> 1 file changed, 41 insertions(+), 20 deletions(-)
>
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index d02a134aae9..f2b57d5642a 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -17,16 +17,29 @@
> from collections import OrderedDict
> import os
> import re
> -from typing import List
> +from typing import (
> + Dict,
> + List,
> + Optional,
> + Set,
> + Union,
> +)
>
> from .common import match_nofail
> from .error import QAPISemError, QAPISourceError
> from .source import QAPISourceInfo
>
>
> +#: Represents a parsed JSON object; semantically: one QAPI schema expression.
> +Expression = Dict[str, object]
I believe you use this for what qapi-code-gen.txt calls a top-level
expression. TopLevelExpression is rather long, but it's used just once,
and once more in RFC PATCH 13. What do you think?
> +
> +# Return value alias for get_expr().
> +_ExprValue = Union[List[object], Dict[str, object], str, bool]
This is essentially a node in our pidgin-JSON parser's abstract syntax
tree. Tree roots use the Dict branch of this Union.
See also my review of PATCH 06.
> +
> +
> class QAPIParseError(QAPISourceError):
> """Error class for all QAPI schema parsing errors."""
> - def __init__(self, parser, msg):
> + def __init__(self, parser: 'QAPISchemaParser', msg: str):
Forward reference needs quotes. Can't be helped.
> col = 1
> for ch in parser.src[parser.line_pos:parser.pos]:
> if ch == '\t':
> @@ -38,7 +51,10 @@ def __init__(self, parser, msg):
>
> class QAPISchemaParser:
>
> - def __init__(self, fname, previously_included=None, incl_info=None):
> + def __init__(self,
> + fname: str,
> + previously_included: Optional[Set[str]] = None,
This needs to be Optional[] because using the empty set as default
parameter value would be a dangerous trap. Python's choice to evaluate
the default parameter value just once has always been iffy. Stirring
static typing into the language makes it iffier. Can't be helped.
> + incl_info: Optional[QAPISourceInfo] = None):
> self._fname = fname
> self._included = previously_included or set()
> self._included.add(os.path.abspath(self._fname))
> @@ -46,20 +62,20 @@ def __init__(self, fname, previously_included=None,
> incl_info=None):
>
> # Lexer state (see `accept` for details):
> self.info = QAPISourceInfo(self._fname, incl_info)
> - self.tok = None
> + self.tok: Optional[str] = None
Would
self.tok: str
work?
> self.pos = 0
> self.cursor = 0
> - self.val = None
> + self.val: Optional[Union[bool, str]] = None
> self.line_pos = 0
>
> # Parser output:
> - self.exprs = []
> - self.docs = []
> + self.exprs: List[Expression] = []
> + self.docs: List[QAPIDoc] = []
>
> # Showtime!
> self._parse()
>
> - def _parse(self):
> + def _parse(self) -> None:
> cur_doc = None
>
> with open(self._fname, 'r', encoding='utf-8') as fp:
> @@ -122,7 +138,7 @@ def _parse(self):
> self.reject_expr_doc(cur_doc)
>
> @staticmethod
> - def reject_expr_doc(doc):
> + def reject_expr_doc(doc: Optional['QAPIDoc']) -> None:
> if doc and doc.symbol:
> raise QAPISemError(
> doc.info,
> @@ -130,10 +146,14 @@ def reject_expr_doc(doc):
> % doc.symbol)
>
> @staticmethod
> - def _include(include, info, incl_fname, previously_included):
> + def _include(include: str,
> + info: QAPISourceInfo,
> + incl_fname: str,
> + previously_included: Set[str]
> + ) -> Optional['QAPISchemaParser']:
> incl_abs_fname = os.path.abspath(incl_fname)
> # catch inclusion cycle
> - inf = info
> + inf: Optional[QAPISourceInfo] = info
> while inf:
> if incl_abs_fname == os.path.abspath(inf.fname):
> raise QAPISemError(info, "inclusion loop for %s" % include)
> @@ -152,9 +172,9 @@ def _include(include, info, incl_fname,
> previously_included):
> ) from err
>
> @staticmethod
> - def _pragma(name, value, info):
> + def _pragma(name: str, value: object, info: QAPISourceInfo) -> None:
value: object isn't wrong, but why not _ExprValue?
>
> - def _check(name, value) -> List[str]:
> + def _check(name: str, value: object) -> List[str]:
> if (not isinstance(value, list) or
> any([not isinstance(elt, str) for elt in value])):
> raise QAPISemError(
> @@ -176,7 +196,7 @@ def _check(name, value) -> List[str]:
> else:
> raise QAPISemError(info, "unknown pragma '%s'" % name)
>
> - def accept(self, skip_comment=True):
> + def accept(self, skip_comment: bool = True) -> None:
> while True:
> self.tok = self.src[self.cursor]
> self.pos = self.cursor
> @@ -240,8 +260,8 @@ def accept(self, skip_comment=True):
> self.src[self.cursor-1:])
> raise QAPIParseError(self, "stray '%s'" % match.group(0))
>
> - def get_members(self):
> - expr = OrderedDict()
> + def get_members(self) -> 'OrderedDict[str, object]':
> + expr: 'OrderedDict[str, object]' = OrderedDict()
> if self.tok == '}':
> self.accept()
> return expr
> @@ -267,8 +287,8 @@ def get_members(self):
> if self.tok != "'":
> raise QAPIParseError(self, "expected string")
>
> - def get_values(self):
> - expr = []
> + def get_values(self) -> List[object]:
> + expr: List[object] = []
> if self.tok == ']':
> self.accept()
> return expr
> @@ -284,8 +304,9 @@ def get_values(self):
> raise QAPIParseError(self, "expected ',' or ']'")
> self.accept()
>
> - def get_expr(self, nested):
> + def get_expr(self, nested: bool = False) -> _ExprValue:
> # TODO: Teach mypy that nested=False means the retval is a Dict.
> + expr: _ExprValue
> if self.tok != '{' and not nested:
> raise QAPIParseError(self, "expected '{'")
> if self.tok == '{':
> @@ -303,7 +324,7 @@ def get_expr(self, nested):
> self, "expected '{', '[', string, or boolean")
> return expr
>
> - def get_doc(self, info):
> + def get_doc(self, info: QAPISourceInfo) -> List['QAPIDoc']:
> if self.val != '##':
> raise QAPIParseError(
> self, "junk after '##' at start of documentation comment")
