| Message ID | 20201005195158.2348217-28-jsnow@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | qapi: static typing conversion, pt1 | expand |
John Snow <jsnow@redhat.com> writes: > Annotations do not change runtime behavior. > This commit *only* adds annotations. > > Signed-off-by: John Snow <jsnow@redhat.com> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > Reviewed-by: Cleber Rosa <crosa@redhat.com> > --- > scripts/qapi/gen.py | 104 ++++++++++++++++++++++++-------------------- > 1 file changed, 57 insertions(+), 47 deletions(-) > > diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py > index 1bad37fc06b..d0391cd8718 100644 > --- a/scripts/qapi/gen.py > +++ b/scripts/qapi/gen.py > @@ -17,7 +17,13 @@ > import errno > import os > import re > -from typing import Optional > +from typing import ( > + Dict, > + Iterator, > + List, > + Optional, > + Tuple, > +) > > from .common import ( > c_fname, > @@ -29,31 +35,31 @@ > mcgen, > ) > from .schema import QAPISchemaObjectType, QAPISchemaVisitor > +from .source import QAPISourceInfo > > > class QAPIGen: > - > - def __init__(self, fname): > + def __init__(self, fname: Optional[str]): I'd expect fname: str. Can you point me to the spot that passes None? > self.fname = fname > self._preamble = '' > self._body = '' > > - def preamble_add(self, text): > + def preamble_add(self, text: str) -> None: > self._preamble += text > > - def add(self, text): > + def add(self, text: str) -> None: > self._body += text > > - def get_content(self): > + def get_content(self) -> str: > return self._top() + self._preamble + self._body + self._bottom() > > - def _top(self): > + def _top(self) -> str: > return '' > > - def _bottom(self): > + def _bottom(self) -> str: > return '' > > - def write(self, output_dir): > + def write(self, output_dir: str) -> None: > # Include paths starting with ../ are used to reuse modules of the main > # schema in specialised schemas. Don't overwrite the files that are > # already generated for the main schema. > @@ -78,7 +84,7 @@ def write(self, output_dir): > f.close() > > > -def _wrap_ifcond(ifcond, before, after): > +def _wrap_ifcond(ifcond: List[str], before: str, after: str) -> str: > if before == after: > return after # suppress empty #if ... #endif > > @@ -118,40 +124,38 @@ def build_params(arg_type: Optional[QAPISchemaObjectType], > > > class QAPIGenCCode(QAPIGen): > - > - def __init__(self, fname): > + def __init__(self, fname: Optional[str]): Likewise. > super().__init__(fname) > - self._start_if = None > + self._start_if: Optional[Tuple[List[str], str, str]] = None > > - def start_if(self, ifcond): > + def start_if(self, ifcond: List[str]) -> None: > assert self._start_if is None > self._start_if = (ifcond, self._body, self._preamble) > > - def end_if(self): > + def end_if(self) -> None: > assert self._start_if > self._wrap_ifcond() > self._start_if = None > > - def _wrap_ifcond(self): > + def _wrap_ifcond(self) -> None: > self._body = _wrap_ifcond(self._start_if[0], > self._start_if[1], self._body) > self._preamble = _wrap_ifcond(self._start_if[0], > self._start_if[2], self._preamble) > > - def get_content(self): > + def get_content(self) -> str: > assert self._start_if is None > return super().get_content() > > > class QAPIGenC(QAPIGenCCode): > - > - def __init__(self, fname, blurb, pydoc): > + def __init__(self, fname: str, blurb: str, pydoc: str): Here it's just str. > super().__init__(fname) > self._blurb = blurb > self._copyright = '\n * '.join(re.findall(r'^Copyright .*', pydoc, > re.MULTILINE)) > > - def _top(self): > + def _top(self) -> str: > return mcgen(''' > /* AUTOMATICALLY GENERATED, DO NOT MODIFY */ > > @@ -167,7 +171,7 @@ def _top(self): > ''', > blurb=self._blurb, copyright=self._copyright) > > - def _bottom(self): > + def _bottom(self) -> str: > return mcgen(''' > > /* Dummy declaration to prevent empty .o file */ > @@ -177,16 +181,15 @@ def _bottom(self): > > > class QAPIGenH(QAPIGenC): > - > - def _top(self): > + def _top(self) -> str: > return super()._top() + guardstart(self.fname) > > - def _bottom(self): > + def _bottom(self) -> str: > return guardend(self.fname) > > > @contextmanager > -def ifcontext(ifcond, *args): > +def ifcontext(ifcond: List[str], *args: QAPIGenCCode) -> Iterator[None]: Oh, the type hint for a *args is QAPIGenCCode, even though args is a tuple of excess positional arguments (which are all QAPIGenCCode). > """ > A with-statement context manager that wraps with `start_if()` / `end_if()`. > > @@ -214,8 +217,11 @@ def ifcontext(ifcond, *args): > > > class QAPISchemaMonolithicCVisitor(QAPISchemaVisitor): > - > - def __init__(self, prefix, what, blurb, pydoc): > + def __init__(self, > + prefix: str, > + what: str, > + blurb: str, > + pydoc: str): > self._prefix = prefix > self._what = what > self._genc = QAPIGenC(self._prefix + self._what + '.c', > @@ -223,38 +229,42 @@ def __init__(self, prefix, what, blurb, pydoc): > self._genh = QAPIGenH(self._prefix + self._what + '.h', > blurb, pydoc) > > - def write(self, output_dir): > + def write(self, output_dir: str) -> None: > self._genc.write(output_dir) > self._genh.write(output_dir) > > > class QAPISchemaModularCVisitor(QAPISchemaVisitor): > - > - def __init__(self, prefix, what, user_blurb, builtin_blurb, pydoc): > + def __init__(self, > + prefix: str, > + what: str, > + user_blurb: str, > + builtin_blurb: Optional[str], > + pydoc: str): > self._prefix = prefix > self._what = what > self._user_blurb = user_blurb > self._builtin_blurb = builtin_blurb > self._pydoc = pydoc > - self._genc = None > - self._genh = None > - self._module = {} > - self._main_module = None > + self._genc: Optional[QAPIGenC] = None > + self._genh: Optional[QAPIGenH] = None > + self._module: Dict[Optional[str], Tuple[QAPIGenC, QAPIGenH]] = {} > + self._main_module: Optional[str] = None > > @staticmethod > - def _is_user_module(name): > + def _is_user_module(name: Optional[str]) -> bool: > return bool(name and not name.startswith('./')) > > @staticmethod > - def _is_builtin_module(name): > + def _is_builtin_module(name: Optional[str]) -> bool: > return not name > > - def _module_dirname(self, what, name): > + def _module_dirname(self, what: str, name: Optional[str]) -> str: > if self._is_user_module(name): > return os.path.dirname(name) > return '' > > - def _module_basename(self, what, name): > + def _module_basename(self, what: str, name: Optional[str]) -> str: > ret = '' if self._is_builtin_module(name) else self._prefix > if self._is_user_module(name): > basename = os.path.basename(name) > @@ -266,27 +276,27 @@ def _module_basename(self, what, name): > ret += re.sub(r'-', '-' + name + '-', what) > return ret > > - def _module_filename(self, what, name): > + def _module_filename(self, what: str, name: Optional[str]) -> str: > return os.path.join(self._module_dirname(what, name), > self._module_basename(what, name)) > > - def _add_module(self, name, blurb): > + def _add_module(self, name: Optional[str], blurb: str) -> None: > basename = self._module_filename(self._what, name) > genc = QAPIGenC(basename + '.c', blurb, self._pydoc) > genh = QAPIGenH(basename + '.h', blurb, self._pydoc) > self._module[name] = (genc, genh) > self._genc, self._genh = self._module[name] > > - def _add_user_module(self, name, blurb): > + def _add_user_module(self, name: str, blurb: str) -> None: > assert self._is_user_module(name) > if self._main_module is None: > self._main_module = name > self._add_module(name, blurb) > > - def _add_system_module(self, name, blurb): > + def _add_system_module(self, name: Optional[str], blurb: str) -> None: > self._add_module(name and './' + name, blurb) > > - def write(self, output_dir, opt_builtins=False): > + def write(self, output_dir: str, opt_builtins: bool = False) -> None: > for name in self._module: > if self._is_builtin_module(name) and not opt_builtins: > continue > @@ -294,13 +304,13 @@ def write(self, output_dir, opt_builtins=False): > genc.write(output_dir) > genh.write(output_dir) > > - def _begin_system_module(self, name): > + def _begin_system_module(self, name: None) -> None: I figure the type hint None signals a simplifcation opportunity. No need to worry about it now. > pass > > - def _begin_user_module(self, name): > + def _begin_user_module(self, name: str) -> None: > pass > > - def visit_module(self, name): > + def visit_module(self, name: Optional[str]) -> None: > if name is None: > if self._builtin_blurb: > self._add_system_module(None, self._builtin_blurb) > @@ -314,7 +324,7 @@ def visit_module(self, name): > self._add_user_module(name, self._user_blurb) > self._begin_user_module(name) > > - def visit_include(self, name, info): > + def visit_include(self, name: str, info: QAPISourceInfo) -> None: > relname = os.path.relpath(self._module_filename(self._what, name), > os.path.dirname(self._genh.fname)) > self._genh.preamble_add(mcgen('''
John Snow <jsnow@redhat.com> writes: > Annotations do not change runtime behavior. > This commit *only* adds annotations. > > Signed-off-by: John Snow <jsnow@redhat.com> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > Reviewed-by: Cleber Rosa <crosa@redhat.com> > --- > scripts/qapi/gen.py | 104 ++++++++++++++++++++++++-------------------- > 1 file changed, 57 insertions(+), 47 deletions(-) > > diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py > index 1bad37fc06b..d0391cd8718 100644 > --- a/scripts/qapi/gen.py > +++ b/scripts/qapi/gen.py > @@ -17,7 +17,13 @@ > import errno > import os > import re > -from typing import Optional > +from typing import ( > + Dict, > + Iterator, > + List, > + Optional, > + Tuple, > +) > > from .common import ( > c_fname, > @@ -29,31 +35,31 @@ > mcgen, > ) > from .schema import QAPISchemaObjectType, QAPISchemaVisitor > +from .source import QAPISourceInfo PATCH 03 has a similar cleanup. Are there more? Perhaps a separate patch doing just this kind of cleanup would make sense. Up to you. [...]
On 10/7/20 8:21 AM, Markus Armbruster wrote: > John Snow <jsnow@redhat.com> writes: > >> Annotations do not change runtime behavior. >> This commit *only* adds annotations. >> >> Signed-off-by: John Snow <jsnow@redhat.com> >> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> >> Reviewed-by: Cleber Rosa <crosa@redhat.com> >> --- >> scripts/qapi/gen.py | 104 ++++++++++++++++++++++++-------------------- >> 1 file changed, 57 insertions(+), 47 deletions(-) >> >> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py >> index 1bad37fc06b..d0391cd8718 100644 >> --- a/scripts/qapi/gen.py >> +++ b/scripts/qapi/gen.py >> @@ -17,7 +17,13 @@ >> import errno >> import os >> import re >> -from typing import Optional >> +from typing import ( >> + Dict, >> + Iterator, >> + List, >> + Optional, >> + Tuple, >> +) >> >> from .common import ( >> c_fname, >> @@ -29,31 +35,31 @@ >> mcgen, >> ) >> from .schema import QAPISchemaObjectType, QAPISchemaVisitor >> +from .source import QAPISourceInfo >> >> >> class QAPIGen: >> - >> - def __init__(self, fname): >> + def __init__(self, fname: Optional[str]): > > I'd expect fname: str. Can you point me to the spot that passes None? > qapi/commands.py: self._regy = QAPIGenCCode(None) Good time to mention again: I am disabling strict none checks for now in this conversion. That means we can pass Optional[T] to functions expecting T and mypy will not complain. This should obviously be fixed long-term, but the cleanups involve things that are a higher class of finnicky. If this alarms you, good! We'll fix it in time, but it doesn't break anything any worse than it already was. If this check was disabled, you would be able to edit the Optional[str] to str and see what broke. But because I use this affordance for now, you would see no difference. >> self.fname = fname >> self._preamble = '' >> self._body = '' >> >> - def preamble_add(self, text): >> + def preamble_add(self, text: str) -> None: >> self._preamble += text >> >> - def add(self, text): >> + def add(self, text: str) -> None: >> self._body += text >> >> - def get_content(self): >> + def get_content(self) -> str: >> return self._top() + self._preamble + self._body + self._bottom() >> >> - def _top(self): >> + def _top(self) -> str: >> return '' >> >> - def _bottom(self): >> + def _bottom(self) -> str: >> return '' >> >> - def write(self, output_dir): >> + def write(self, output_dir: str) -> None: >> # Include paths starting with ../ are used to reuse modules of the main >> # schema in specialised schemas. Don't overwrite the files that are >> # already generated for the main schema. >> @@ -78,7 +84,7 @@ def write(self, output_dir): >> f.close() >> >> >> -def _wrap_ifcond(ifcond, before, after): >> +def _wrap_ifcond(ifcond: List[str], before: str, after: str) -> str: >> if before == after: >> return after # suppress empty #if ... #endif >> >> @@ -118,40 +124,38 @@ def build_params(arg_type: Optional[QAPISchemaObjectType], >> >> >> class QAPIGenCCode(QAPIGen): >> - >> - def __init__(self, fname): >> + def __init__(self, fname: Optional[str]): > > Likewise. > >> super().__init__(fname) >> - self._start_if = None >> + self._start_if: Optional[Tuple[List[str], str, str]] = None >> >> - def start_if(self, ifcond): >> + def start_if(self, ifcond: List[str]) -> None: >> assert self._start_if is None >> self._start_if = (ifcond, self._body, self._preamble) >> >> - def end_if(self): >> + def end_if(self) -> None: >> assert self._start_if >> self._wrap_ifcond() >> self._start_if = None >> >> - def _wrap_ifcond(self): >> + def _wrap_ifcond(self) -> None: >> self._body = _wrap_ifcond(self._start_if[0], >> self._start_if[1], self._body) >> self._preamble = _wrap_ifcond(self._start_if[0], >> self._start_if[2], self._preamble) >> >> - def get_content(self): >> + def get_content(self) -> str: >> assert self._start_if is None >> return super().get_content() >> >> >> class QAPIGenC(QAPIGenCCode): >> - >> - def __init__(self, fname, blurb, pydoc): >> + def __init__(self, fname: str, blurb: str, pydoc: str): > > Here it's just str. > That's right. >> super().__init__(fname) >> self._blurb = blurb >> self._copyright = '\n * '.join(re.findall(r'^Copyright .*', pydoc, >> re.MULTILINE)) >> >> - def _top(self): >> + def _top(self) -> str: >> return mcgen(''' >> /* AUTOMATICALLY GENERATED, DO NOT MODIFY */ >> >> @@ -167,7 +171,7 @@ def _top(self): >> ''', >> blurb=self._blurb, copyright=self._copyright) >> >> - def _bottom(self): >> + def _bottom(self) -> str: >> return mcgen(''' >> >> /* Dummy declaration to prevent empty .o file */ >> @@ -177,16 +181,15 @@ def _bottom(self): >> >> >> class QAPIGenH(QAPIGenC): >> - >> - def _top(self): >> + def _top(self) -> str: >> return super()._top() + guardstart(self.fname) >> >> - def _bottom(self): >> + def _bottom(self) -> str: >> return guardend(self.fname) >> >> >> @contextmanager >> -def ifcontext(ifcond, *args): >> +def ifcontext(ifcond: List[str], *args: QAPIGenCCode) -> Iterator[None]: > > Oh, the type hint for a *args is QAPIGenCCode, even though args is a > tuple of excess positional arguments (which are all QAPIGenCCode). > Yeah, it's the way type hints interact with the special '*' syntax. This is an Iterable of QAPIGenCCode. **kwargs works the same way: you annotate the values, and the keys are assumed to be str. >> """ >> A with-statement context manager that wraps with `start_if()` / `end_if()`. >> >> @@ -214,8 +217,11 @@ def ifcontext(ifcond, *args): >> >> >> class QAPISchemaMonolithicCVisitor(QAPISchemaVisitor): >> - >> - def __init__(self, prefix, what, blurb, pydoc): >> + def __init__(self, >> + prefix: str, >> + what: str, >> + blurb: str, >> + pydoc: str): >> self._prefix = prefix >> self._what = what >> self._genc = QAPIGenC(self._prefix + self._what + '.c', >> @@ -223,38 +229,42 @@ def __init__(self, prefix, what, blurb, pydoc): >> self._genh = QAPIGenH(self._prefix + self._what + '.h', >> blurb, pydoc) >> >> - def write(self, output_dir): >> + def write(self, output_dir: str) -> None: >> self._genc.write(output_dir) >> self._genh.write(output_dir) >> >> >> class QAPISchemaModularCVisitor(QAPISchemaVisitor): >> - >> - def __init__(self, prefix, what, user_blurb, builtin_blurb, pydoc): >> + def __init__(self, >> + prefix: str, >> + what: str, >> + user_blurb: str, >> + builtin_blurb: Optional[str], >> + pydoc: str): >> self._prefix = prefix >> self._what = what >> self._user_blurb = user_blurb >> self._builtin_blurb = builtin_blurb >> self._pydoc = pydoc >> - self._genc = None >> - self._genh = None >> - self._module = {} >> - self._main_module = None >> + self._genc: Optional[QAPIGenC] = None >> + self._genh: Optional[QAPIGenH] = None >> + self._module: Dict[Optional[str], Tuple[QAPIGenC, QAPIGenH]] = {} >> + self._main_module: Optional[str] = None >> >> @staticmethod >> - def _is_user_module(name): >> + def _is_user_module(name: Optional[str]) -> bool: >> return bool(name and not name.startswith('./')) >> >> @staticmethod >> - def _is_builtin_module(name): >> + def _is_builtin_module(name: Optional[str]) -> bool: >> return not name >> >> - def _module_dirname(self, what, name): >> + def _module_dirname(self, what: str, name: Optional[str]) -> str: >> if self._is_user_module(name): >> return os.path.dirname(name) >> return '' >> >> - def _module_basename(self, what, name): >> + def _module_basename(self, what: str, name: Optional[str]) -> str: >> ret = '' if self._is_builtin_module(name) else self._prefix >> if self._is_user_module(name): >> basename = os.path.basename(name) >> @@ -266,27 +276,27 @@ def _module_basename(self, what, name): >> ret += re.sub(r'-', '-' + name + '-', what) >> return ret >> >> - def _module_filename(self, what, name): >> + def _module_filename(self, what: str, name: Optional[str]) -> str: >> return os.path.join(self._module_dirname(what, name), >> self._module_basename(what, name)) >> >> - def _add_module(self, name, blurb): >> + def _add_module(self, name: Optional[str], blurb: str) -> None: >> basename = self._module_filename(self._what, name) >> genc = QAPIGenC(basename + '.c', blurb, self._pydoc) >> genh = QAPIGenH(basename + '.h', blurb, self._pydoc) >> self._module[name] = (genc, genh) >> self._genc, self._genh = self._module[name] >> >> - def _add_user_module(self, name, blurb): >> + def _add_user_module(self, name: str, blurb: str) -> None: >> assert self._is_user_module(name) >> if self._main_module is None: >> self._main_module = name >> self._add_module(name, blurb) >> >> - def _add_system_module(self, name, blurb): >> + def _add_system_module(self, name: Optional[str], blurb: str) -> None: >> self._add_module(name and './' + name, blurb) >> >> - def write(self, output_dir, opt_builtins=False): >> + def write(self, output_dir: str, opt_builtins: bool = False) -> None: >> for name in self._module: >> if self._is_builtin_module(name) and not opt_builtins: >> continue >> @@ -294,13 +304,13 @@ def write(self, output_dir, opt_builtins=False): >> genc.write(output_dir) >> genh.write(output_dir) >> >> - def _begin_system_module(self, name): >> + def _begin_system_module(self, name: None) -> None: > > I figure the type hint None signals a simplifcation opportunity. No > need to worry about it now. > Yes, the module name stuff in general has a smell to it. I didn't look deeper, but there's an opportunity for cleaning it up. I realize the type signature here has parity with _begin_user_module, but I noticed it was *never* called with anything that wasn't a literal None, so I added the stricter type. >> pass >> >> - def _begin_user_module(self, name): >> + def _begin_user_module(self, name: str) -> None: >> pass >> >> - def visit_module(self, name): >> + def visit_module(self, name: Optional[str]) -> None: >> if name is None: >> if self._builtin_blurb: >> self._add_system_module(None, self._builtin_blurb) >> @@ -314,7 +324,7 @@ def visit_module(self, name): >> self._add_user_module(name, self._user_blurb) >> self._begin_user_module(name) >> >> - def visit_include(self, name, info): >> + def visit_include(self, name: str, info: QAPISourceInfo) -> None: >> relname = os.path.relpath(self._module_filename(self._what, name), >> os.path.dirname(self._genh.fname)) >> self._genh.preamble_add(mcgen('''
On 10/7/20 9:20 AM, Markus Armbruster wrote: > John Snow <jsnow@redhat.com> writes: > >> Annotations do not change runtime behavior. >> This commit *only* adds annotations. >> >> Signed-off-by: John Snow <jsnow@redhat.com> >> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> >> Reviewed-by: Cleber Rosa <crosa@redhat.com> >> --- >> scripts/qapi/gen.py | 104 ++++++++++++++++++++++++-------------------- >> 1 file changed, 57 insertions(+), 47 deletions(-) >> >> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py >> index 1bad37fc06b..d0391cd8718 100644 >> --- a/scripts/qapi/gen.py >> +++ b/scripts/qapi/gen.py >> @@ -17,7 +17,13 @@ >> import errno >> import os >> import re >> -from typing import Optional >> +from typing import ( >> + Dict, >> + Iterator, >> + List, >> + Optional, >> + Tuple, >> +) >> >> from .common import ( >> c_fname, >> @@ -29,31 +35,31 @@ >> mcgen, >> ) >> from .schema import QAPISchemaObjectType, QAPISchemaVisitor >> +from .source import QAPISourceInfo > > PATCH 03 has a similar cleanup. Are there more? Perhaps a separate > patch doing just this kind of cleanup would make sense. Up to you. > > [...] > This isn't a cleanup, I am just importing QAPISourceInfo to use for an annotation. It's relevant and required for this patch, and doesn't make sense on its own. Patch 03 ... Oh, you mean identifying the correct location of QAPIError. Uh... nah? I think that was the only case of that one changing. Not worth pulling out or naming, I think.
John Snow <jsnow@redhat.com> writes: > On 10/7/20 9:20 AM, Markus Armbruster wrote: >> John Snow <jsnow@redhat.com> writes: >> >>> Annotations do not change runtime behavior. >>> This commit *only* adds annotations. >>> >>> Signed-off-by: John Snow <jsnow@redhat.com> >>> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> >>> Reviewed-by: Cleber Rosa <crosa@redhat.com> >>> --- >>> scripts/qapi/gen.py | 104 ++++++++++++++++++++++++-------------------- >>> 1 file changed, 57 insertions(+), 47 deletions(-) >>> >>> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py >>> index 1bad37fc06b..d0391cd8718 100644 >>> --- a/scripts/qapi/gen.py >>> +++ b/scripts/qapi/gen.py >>> @@ -17,7 +17,13 @@ >>> import errno >>> import os >>> import re >>> -from typing import Optional >>> +from typing import ( >>> + Dict, >>> + Iterator, >>> + List, >>> + Optional, >>> + Tuple, >>> +) >>> from .common import ( >>> c_fname, >>> @@ -29,31 +35,31 @@ >>> mcgen, >>> ) >>> from .schema import QAPISchemaObjectType, QAPISchemaVisitor >>> +from .source import QAPISourceInfo >> PATCH 03 has a similar cleanup. Are there more? Perhaps a separate >> patch doing just this kind of cleanup would make sense. Up to you. >> [...] >> > > This isn't a cleanup, I am just importing QAPISourceInfo to use for an > annotation. It's relevant and required for this patch, and doesn't > make sense on its own. I was mistaken. A case of patch-review-eye... > Patch 03 ... Oh, you mean identifying the correct location of > QAPIError. Uh... nah? I think that was the only case of that one > changing. Not worth pulling out or naming, I think. Agree.
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index 1bad37fc06b..d0391cd8718 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -17,7 +17,13 @@ import errno import os import re -from typing import Optional +from typing import ( + Dict, + Iterator, + List, + Optional, + Tuple, +) from .common import ( c_fname, @@ -29,31 +35,31 @@ mcgen, ) from .schema import QAPISchemaObjectType, QAPISchemaVisitor +from .source import QAPISourceInfo class QAPIGen: - - def __init__(self, fname): + def __init__(self, fname: Optional[str]): self.fname = fname self._preamble = '' self._body = '' - def preamble_add(self, text): + def preamble_add(self, text: str) -> None: self._preamble += text - def add(self, text): + def add(self, text: str) -> None: self._body += text - def get_content(self): + def get_content(self) -> str: return self._top() + self._preamble + self._body + self._bottom() - def _top(self): + def _top(self) -> str: return '' - def _bottom(self): + def _bottom(self) -> str: return '' - def write(self, output_dir): + def write(self, output_dir: str) -> None: # Include paths starting with ../ are used to reuse modules of the main # schema in specialised schemas. Don't overwrite the files that are # already generated for the main schema. @@ -78,7 +84,7 @@ def write(self, output_dir): f.close() -def _wrap_ifcond(ifcond, before, after): +def _wrap_ifcond(ifcond: List[str], before: str, after: str) -> str: if before == after: return after # suppress empty #if ... #endif @@ -118,40 +124,38 @@ def build_params(arg_type: Optional[QAPISchemaObjectType], class QAPIGenCCode(QAPIGen): - - def __init__(self, fname): + def __init__(self, fname: Optional[str]): super().__init__(fname) - self._start_if = None + self._start_if: Optional[Tuple[List[str], str, str]] = None - def start_if(self, ifcond): + def start_if(self, ifcond: List[str]) -> None: assert self._start_if is None self._start_if = (ifcond, self._body, self._preamble) - def end_if(self): + def end_if(self) -> None: assert self._start_if self._wrap_ifcond() self._start_if = None - def _wrap_ifcond(self): + def _wrap_ifcond(self) -> None: self._body = _wrap_ifcond(self._start_if[0], self._start_if[1], self._body) self._preamble = _wrap_ifcond(self._start_if[0], self._start_if[2], self._preamble) - def get_content(self): + def get_content(self) -> str: assert self._start_if is None return super().get_content() class QAPIGenC(QAPIGenCCode): - - def __init__(self, fname, blurb, pydoc): + def __init__(self, fname: str, blurb: str, pydoc: str): super().__init__(fname) self._blurb = blurb self._copyright = '\n * '.join(re.findall(r'^Copyright .*', pydoc, re.MULTILINE)) - def _top(self): + def _top(self) -> str: return mcgen(''' /* AUTOMATICALLY GENERATED, DO NOT MODIFY */ @@ -167,7 +171,7 @@ def _top(self): ''', blurb=self._blurb, copyright=self._copyright) - def _bottom(self): + def _bottom(self) -> str: return mcgen(''' /* Dummy declaration to prevent empty .o file */ @@ -177,16 +181,15 @@ def _bottom(self): class QAPIGenH(QAPIGenC): - - def _top(self): + def _top(self) -> str: return super()._top() + guardstart(self.fname) - def _bottom(self): + def _bottom(self) -> str: return guardend(self.fname) @contextmanager -def ifcontext(ifcond, *args): +def ifcontext(ifcond: List[str], *args: QAPIGenCCode) -> Iterator[None]: """ A with-statement context manager that wraps with `start_if()` / `end_if()`. @@ -214,8 +217,11 @@ def ifcontext(ifcond, *args): class QAPISchemaMonolithicCVisitor(QAPISchemaVisitor): - - def __init__(self, prefix, what, blurb, pydoc): + def __init__(self, + prefix: str, + what: str, + blurb: str, + pydoc: str): self._prefix = prefix self._what = what self._genc = QAPIGenC(self._prefix + self._what + '.c', @@ -223,38 +229,42 @@ def __init__(self, prefix, what, blurb, pydoc): self._genh = QAPIGenH(self._prefix + self._what + '.h', blurb, pydoc) - def write(self, output_dir): + def write(self, output_dir: str) -> None: self._genc.write(output_dir) self._genh.write(output_dir) class QAPISchemaModularCVisitor(QAPISchemaVisitor): - - def __init__(self, prefix, what, user_blurb, builtin_blurb, pydoc): + def __init__(self, + prefix: str, + what: str, + user_blurb: str, + builtin_blurb: Optional[str], + pydoc: str): self._prefix = prefix self._what = what self._user_blurb = user_blurb self._builtin_blurb = builtin_blurb self._pydoc = pydoc - self._genc = None - self._genh = None - self._module = {} - self._main_module = None + self._genc: Optional[QAPIGenC] = None + self._genh: Optional[QAPIGenH] = None + self._module: Dict[Optional[str], Tuple[QAPIGenC, QAPIGenH]] = {} + self._main_module: Optional[str] = None @staticmethod - def _is_user_module(name): + def _is_user_module(name: Optional[str]) -> bool: return bool(name and not name.startswith('./')) @staticmethod - def _is_builtin_module(name): + def _is_builtin_module(name: Optional[str]) -> bool: return not name - def _module_dirname(self, what, name): + def _module_dirname(self, what: str, name: Optional[str]) -> str: if self._is_user_module(name): return os.path.dirname(name) return '' - def _module_basename(self, what, name): + def _module_basename(self, what: str, name: Optional[str]) -> str: ret = '' if self._is_builtin_module(name) else self._prefix if self._is_user_module(name): basename = os.path.basename(name) @@ -266,27 +276,27 @@ def _module_basename(self, what, name): ret += re.sub(r'-', '-' + name + '-', what) return ret - def _module_filename(self, what, name): + def _module_filename(self, what: str, name: Optional[str]) -> str: return os.path.join(self._module_dirname(what, name), self._module_basename(what, name)) - def _add_module(self, name, blurb): + def _add_module(self, name: Optional[str], blurb: str) -> None: basename = self._module_filename(self._what, name) genc = QAPIGenC(basename + '.c', blurb, self._pydoc) genh = QAPIGenH(basename + '.h', blurb, self._pydoc) self._module[name] = (genc, genh) self._genc, self._genh = self._module[name] - def _add_user_module(self, name, blurb): + def _add_user_module(self, name: str, blurb: str) -> None: assert self._is_user_module(name) if self._main_module is None: self._main_module = name self._add_module(name, blurb) - def _add_system_module(self, name, blurb): + def _add_system_module(self, name: Optional[str], blurb: str) -> None: self._add_module(name and './' + name, blurb) - def write(self, output_dir, opt_builtins=False): + def write(self, output_dir: str, opt_builtins: bool = False) -> None: for name in self._module: if self._is_builtin_module(name) and not opt_builtins: continue @@ -294,13 +304,13 @@ def write(self, output_dir, opt_builtins=False): genc.write(output_dir) genh.write(output_dir) - def _begin_system_module(self, name): + def _begin_system_module(self, name: None) -> None: pass - def _begin_user_module(self, name): + def _begin_user_module(self, name: str) -> None: pass - def visit_module(self, name): + def visit_module(self, name: Optional[str]) -> None: if name is None: if self._builtin_blurb: self._add_system_module(None, self._builtin_blurb) @@ -314,7 +324,7 @@ def visit_module(self, name): self._add_user_module(name, self._user_blurb) self._begin_user_module(name) - def visit_include(self, name, info): + def visit_include(self, name: str, info: QAPISourceInfo) -> None: relname = os.path.relpath(self._module_filename(self._what, name), os.path.dirname(self._genh.fname)) self._genh.preamble_add(mcgen('''