* [PATCH 1/6] qapi/error.py: refactor error classes
2020-09-22 21:21 [PATCH 0/6] qapi: static typing conversion, pt4 John Snow
@ 2020-09-22 21:21 ` John Snow
2020-09-22 21:21 ` [PATCH 2/6] qapi/parser.py: remove parser context from QAPIDoc John Snow
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: John Snow @ 2020-09-22 21:21 UTC (permalink / raw)
To: Markus Armbruster
Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa
Create a ubiquitous, context-free base error class that all exceptions
in the qapi package should inherit from.
Move the QAPISourceInfo relevant fields up into QAPIErrorLocation;
making sure the order of arguments is consistent between
QAPIErrorLocation and QAPISemError.
---
The order of arguments for QAPIParseError is inconsistent, but handled
explicitly in the __init__ method; this will be addressed in forthcoming
patches, but it works correctly here.
Signed-off-by: John Snow <jsnow@redhat.com>
---
scripts/qapi/error.py | 35 +++++++++++++++++++++++------------
scripts/qapi/main.py | 2 +-
scripts/qapi/schema.py | 4 ++--
3 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/scripts/qapi/error.py b/scripts/qapi/error.py
index ae60d9e2fe..47f266f33d 100644
--- a/scripts/qapi/error.py
+++ b/scripts/qapi/error.py
@@ -11,23 +11,35 @@
# This work is licensed under the terms of the GNU GPL, version 2.
# See the COPYING file in the top-level directory.
+from typing import Optional
+
+from .source import QAPISourceInfo
+
class QAPIError(Exception):
- def __init__(self, info, col, msg):
- Exception.__init__(self)
+ """Base class for all exceptions from the QAPI module."""
+
+
+class QAPISourceError(QAPIError):
+ """Error class for all exceptions identifying a source location."""
+ def __init__(self,
+ info: QAPISourceInfo,
+ msg: str,
+ col: Optional[int] = None):
+ super().__init__()
self.info = info
- self.col = col
self.msg = msg
+ self.col = col
- def __str__(self):
+ def __str__(self) -> str:
loc = str(self.info)
if self.col is not None:
- assert self.info.line is not None
- loc += ':%s' % self.col
- return loc + ': ' + self.msg
+ loc += f":{self.col}"
+ return f"{loc}: {self.msg}"
-class QAPIParseError(QAPIError):
+class QAPIParseError(QAPISourceError):
+ """Error class for all QAPI schema parsing errors."""
def __init__(self, parser, msg):
col = 1
for ch in parser.src[parser.line_pos:parser.pos]:
@@ -35,9 +47,8 @@ def __init__(self, parser, msg):
col = (col + 7) % 8 + 1
else:
col += 1
- super().__init__(parser.info, col, msg)
+ super().__init__(parser.info, msg, col)
-class QAPISemError(QAPIError):
- def __init__(self, info, msg):
- super().__init__(info, None, msg)
+class QAPISemError(QAPISourceError):
+ """Error class for semantic QAPI errors."""
diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
index 3f8338ade8..302b9eee81 100644
--- a/scripts/qapi/main.py
+++ b/scripts/qapi/main.py
@@ -45,7 +45,7 @@ def generate(schema_file: str,
if match and match.end() != len(prefix):
msg = "funny character '{:s}' in prefix '{:s}'".format(
prefix[match.end()], prefix)
- raise QAPIError('', None, msg)
+ raise QAPIError(msg)
schema = QAPISchema(schema_file)
gen_types(schema, output_dir, prefix, builtins)
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 3023bab44b..121d8488d2 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -20,7 +20,7 @@
from typing import Optional
from .common import c_name, POINTER_SUFFIX
-from .error import QAPIError, QAPISemError
+from .error import QAPISourceError, QAPISemError
from .expr import check_exprs
from .parser import QAPISchemaParser
@@ -841,7 +841,7 @@ def _def_entity(self, ent):
other_ent = self._entity_dict.get(ent.name)
if other_ent:
if other_ent.info:
- where = QAPIError(other_ent.info, None, "previous definition")
+ where = QAPISourceError(other_ent.info, "previous definition")
raise QAPISemError(
ent.info,
"'%s' is already defined\n%s" % (ent.name, where))
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/6] qapi/parser.py: remove parser context from QAPIDoc
2020-09-22 21:21 [PATCH 0/6] qapi: static typing conversion, pt4 John Snow
2020-09-22 21:21 ` [PATCH 1/6] qapi/error.py: refactor error classes John Snow
@ 2020-09-22 21:21 ` John Snow
2020-09-22 21:21 ` [PATCH 3/6] qapi/error.py: move QAPIParseError to parser.py John Snow
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: John Snow @ 2020-09-22 21:21 UTC (permalink / raw)
To: Markus Armbruster
Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa
We only need the parser context for the purpose of raising
exceptions. What we can do instead is to raise a unique document parsing
error and affix additional context from the caller. This way, the
document parsing can be isolated.
Signed-off-by: John Snow <jsnow@redhat.com>
---
scripts/qapi/parser.py | 56 ++++++++++++++++++++++--------------------
1 file changed, 30 insertions(+), 26 deletions(-)
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 327cf05736..cb2595ce60 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -18,10 +18,14 @@
import re
from collections import OrderedDict
-from .error import QAPIParseError, QAPISemError
+from .error import QAPIError, QAPIParseError, QAPISemError
from .source import QAPISourceInfo
+class QAPIDocError(QAPIError):
+ """Documentation parsing error."""
+
+
class QAPISchemaParser:
def __init__(self, fname, previously_included=None, incl_info=None):
@@ -265,13 +269,13 @@ def get_expr(self, nested):
self, "expected '{', '[', string, boolean or 'null'")
return expr
- def get_doc(self, info):
+ def _get_doc(self, info):
if self.val != '##':
raise QAPIParseError(
self, "junk after '##' at start of documentation comment")
docs = []
- cur_doc = QAPIDoc(self, info)
+ cur_doc = QAPIDoc(info)
self.accept(False)
while self.tok == '#':
if self.val.startswith('##'):
@@ -292,12 +296,22 @@ def get_doc(self, info):
if cur_doc.body.text:
cur_doc.end_comment()
docs.append(cur_doc)
- cur_doc = QAPIDoc(self, info)
+ cur_doc = QAPIDoc(info)
cur_doc.append(self.val)
self.accept(False)
raise QAPIParseError(self, "documentation comment must end with '##'")
+ def get_doc(self, info):
+ try:
+ return self._get_doc(info)
+ except QAPIDocError as err:
+ # Tie the Doc parsing error to our parsing state. The
+ # resulting error position depends on the state of the
+ # parser. It happens to be the beginning of the comment.
+ # More or less servicable, but action at a distance.
+ raise QAPIParseError(self, str(err)) from err
+
class QAPIDoc:
"""
@@ -335,12 +349,7 @@ def __init__(self, name):
def connect(self, member):
self.member = member
- def __init__(self, parser, info):
- # self._parser is used to report errors with QAPIParseError. The
- # resulting error position depends on the state of the parser.
- # It happens to be the beginning of the comment. More or less
- # servicable, but action at a distance.
- self._parser = parser
+ def __init__(self, info):
self.info = info
self.symbol = None
self.body = QAPIDoc.Section()
@@ -377,7 +386,7 @@ def append(self, line):
return
if line[0] != ' ':
- raise QAPIParseError(self._parser, "missing space after #")
+ raise QAPIDocError("missing space after #")
line = line[1:]
self._append_line(line)
@@ -411,11 +420,11 @@ def _append_body_line(self, line):
# recognized, and get silently treated as ordinary text
if not self.symbol and not self.body.text and line.startswith('@'):
if not line.endswith(':'):
- raise QAPIParseError(self._parser, "line should end with ':'")
+ raise QAPIDocError("line should end with ':'")
self.symbol = line[1:-1]
# FIXME invalid names other than the empty string aren't flagged
if not self.symbol:
- raise QAPIParseError(self._parser, "invalid name")
+ raise QAPIDocError("invalid name")
elif self.symbol:
# This is a definition documentation block
if name.startswith('@') and name.endswith(':'):
@@ -498,9 +507,8 @@ def _append_various_line(self, line):
name = line.split(' ', 1)[0]
if name.startswith('@') and name.endswith(':'):
- raise QAPIParseError(self._parser,
- "'%s' can't follow '%s' section"
- % (name, self.sections[0].name))
+ raise QAPIDocError("'%s' can't follow '%s' section"
+ % (name, self.sections[0].name))
if self._is_section_tag(name):
line = line[len(name)+1:]
self._start_section(name[:-1])
@@ -514,10 +522,9 @@ def _append_various_line(self, line):
def _start_symbol_section(self, symbols_dict, name):
# FIXME invalid names other than the empty string aren't flagged
if not name:
- raise QAPIParseError(self._parser, "invalid parameter name")
+ raise QAPIDocError("invalid parameter name")
if name in symbols_dict:
- raise QAPIParseError(self._parser,
- "'%s' parameter name duplicated" % name)
+ raise QAPIDocError("'%s' parameter name duplicated" % name)
assert not self.sections
self._end_section()
self._section = QAPIDoc.ArgSection(name)
@@ -531,8 +538,7 @@ def _start_features_section(self, name):
def _start_section(self, name=None):
if name in ('Returns', 'Since') and self.has_section(name):
- raise QAPIParseError(self._parser,
- "duplicated '%s' section" % name)
+ raise QAPIDocError("duplicated '%s' section" % name)
self._end_section()
self._section = QAPIDoc.Section(name)
self.sections.append(self._section)
@@ -541,17 +547,15 @@ def _end_section(self):
if self._section:
text = self._section.text = self._section.text.strip()
if self._section.name and (not text or text.isspace()):
- raise QAPIParseError(
- self._parser,
+ raise QAPIDocError(
"empty doc section '%s'" % self._section.name)
self._section = None
def _append_freeform(self, line):
match = re.match(r'(@\S+:)', line)
if match:
- raise QAPIParseError(self._parser,
- "'%s' not allowed in free-form documentation"
- % match.group(1))
+ raise QAPIDocError("'%s' not allowed in free-form documentation"
+ % match.group(1))
self._section.append(line)
def connect_member(self, member):
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/6] qapi/error.py: move QAPIParseError to parser.py
2020-09-22 21:21 [PATCH 0/6] qapi: static typing conversion, pt4 John Snow
2020-09-22 21:21 ` [PATCH 1/6] qapi/error.py: refactor error classes John Snow
2020-09-22 21:21 ` [PATCH 2/6] qapi/parser.py: remove parser context from QAPIDoc John Snow
@ 2020-09-22 21:21 ` John Snow
2020-09-22 21:21 ` [PATCH 4/6] qapi/parser.py: refactor QAPIParseError John Snow
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: John Snow @ 2020-09-22 21:21 UTC (permalink / raw)
To: Markus Armbruster
Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa
Signed-off-by: John Snow <jsnow@redhat.com>
---
scripts/qapi/error.py | 12 ------------
scripts/qapi/parser.py | 14 +++++++++++++-
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/scripts/qapi/error.py b/scripts/qapi/error.py
index 47f266f33d..ab6a0f6271 100644
--- a/scripts/qapi/error.py
+++ b/scripts/qapi/error.py
@@ -38,17 +38,5 @@ def __str__(self) -> str:
return f"{loc}: {self.msg}"
-class QAPIParseError(QAPISourceError):
- """Error class for all QAPI schema parsing errors."""
- def __init__(self, parser, msg):
- col = 1
- for ch in parser.src[parser.line_pos:parser.pos]:
- if ch == '\t':
- col = (col + 7) % 8 + 1
- else:
- col += 1
- super().__init__(parser.info, msg, col)
-
-
class QAPISemError(QAPISourceError):
"""Error class for semantic QAPI errors."""
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index cb2595ce60..7e7eda9ed9 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -18,10 +18,22 @@
import re
from collections import OrderedDict
-from .error import QAPIError, QAPIParseError, QAPISemError
+from .error import QAPIError, QAPISourceError, QAPISemError
from .source import QAPISourceInfo
+class QAPIParseError(QAPISourceError):
+ """Error class for all QAPI schema parsing errors."""
+ def __init__(self, parser, msg):
+ col = 1
+ for ch in parser.src[parser.line_pos:parser.pos]:
+ if ch == '\t':
+ col = (col + 7) % 8 + 1
+ else:
+ col += 1
+ super().__init__(parser.info, msg, col)
+
+
class QAPIDocError(QAPIError):
"""Documentation parsing error."""
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 4/6] qapi/parser.py: refactor QAPIParseError
2020-09-22 21:21 [PATCH 0/6] qapi: static typing conversion, pt4 John Snow
` (2 preceding siblings ...)
2020-09-22 21:21 ` [PATCH 3/6] qapi/error.py: move QAPIParseError to parser.py John Snow
@ 2020-09-22 21:21 ` John Snow
2020-09-22 21:21 ` [PATCH 5/6] qapi/error.py: enable mypy checks John Snow
2020-09-22 21:21 ` [PATCH 6/6] qapi/error.py: enable pylint checks John Snow
5 siblings, 0 replies; 7+ messages in thread
From: John Snow @ 2020-09-22 21:21 UTC (permalink / raw)
To: Markus Armbruster
Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa
Instead of rewriting the constructor with arguments incompatible to the
base class we're extending, add a constructor-like class method that
accepts a QAPISchemaParser as an argument and builds the correct
Exception type.
Because this is a little extra typing, create a new helper in
QAPISchemaParser to build these exception objects.
Signed-off-by: John Snow <jsnow@redhat.com>
---
scripts/qapi/parser.py | 58 ++++++++++++++++++++++--------------------
1 file changed, 31 insertions(+), 27 deletions(-)
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 7e7eda9ed9..8be4570c31 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -17,6 +17,7 @@
import os
import re
from collections import OrderedDict
+from typing import Type, TypeVar
from .error import QAPIError, QAPISourceError, QAPISemError
from .source import QAPISourceInfo
@@ -24,14 +25,18 @@
class QAPIParseError(QAPISourceError):
"""Error class for all QAPI schema parsing errors."""
- def __init__(self, parser, msg):
+
+ T = TypeVar('T', bound='QAPIParseError')
+
+ @classmethod
+ def make(cls: Type[T], parser: 'QAPISchemaParser', msg: str) -> T:
col = 1
for ch in parser.src[parser.line_pos:parser.pos]:
if ch == '\t':
col = (col + 7) % 8 + 1
else:
col += 1
- super().__init__(parser.info, msg, col)
+ return cls(parser.info, msg, col)
class QAPIDocError(QAPIError):
@@ -112,6 +117,9 @@ def __init__(self, fname, previously_included=None, incl_info=None):
cur_doc = None
self.reject_expr_doc(cur_doc)
+ def _parse_error(self, msg: str) -> QAPIParseError:
+ return QAPIParseError.make(self, msg)
+
@staticmethod
def reject_expr_doc(doc):
if doc and doc.symbol:
@@ -183,13 +191,12 @@ def accept(self, skip_comment=True):
ch = self.src[self.cursor]
self.cursor += 1
if ch == '\n':
- raise QAPIParseError(self, "missing terminating \"'\"")
+ raise self._parse_error("missing terminating \"'\"")
if esc:
# Note: we recognize only \\ because we have
# no use for funny characters in strings
if ch != '\\':
- raise QAPIParseError(self,
- "unknown escape \\%s" % ch)
+ raise self._parse_error(f"unknown escape \\{ch}")
esc = False
elif ch == '\\':
esc = True
@@ -198,8 +205,7 @@ def accept(self, skip_comment=True):
self.val = string
return
if ord(ch) < 32 or ord(ch) >= 127:
- raise QAPIParseError(
- self, "funny character in string")
+ raise self._parse_error("funny character in string")
string += ch
elif self.src.startswith('true', self.pos):
self.val = True
@@ -220,7 +226,7 @@ def accept(self, skip_comment=True):
# character
match = re.match('[^[\\]{}:,\\s\'"]+',
self.src[self.cursor-1:])
- raise QAPIParseError(self, "stray '%s'" % match.group(0))
+ raise self._parse_error("stray '%s'" % match.group(0))
def get_members(self):
expr = OrderedDict()
@@ -228,24 +234,24 @@ def get_members(self):
self.accept()
return expr
if self.tok != "'":
- raise QAPIParseError(self, "expected string or '}'")
+ raise self._parse_error("expected string or '}'")
while True:
key = self.val
self.accept()
if self.tok != ':':
- raise QAPIParseError(self, "expected ':'")
+ raise self._parse_error("expected ':'")
self.accept()
if key in expr:
- raise QAPIParseError(self, "duplicate key '%s'" % key)
+ raise self._parse_error("duplicate key '%s'" % key)
expr[key] = self.get_expr(True)
if self.tok == '}':
self.accept()
return expr
if self.tok != ',':
- raise QAPIParseError(self, "expected ',' or '}'")
+ raise self._parse_error("expected ',' or '}'")
self.accept()
if self.tok != "'":
- raise QAPIParseError(self, "expected string")
+ raise self._parse_error("expected string")
def get_values(self):
expr = []
@@ -253,20 +259,20 @@ def get_values(self):
self.accept()
return expr
if self.tok not in "{['tfn":
- raise QAPIParseError(
- self, "expected '{', '[', ']', string, boolean or 'null'")
+ raise self._parse_error(
+ "expected '{', '[', ']', string, boolean or 'null'")
while True:
expr.append(self.get_expr(True))
if self.tok == ']':
self.accept()
return expr
if self.tok != ',':
- raise QAPIParseError(self, "expected ',' or ']'")
+ raise self._parse_error("expected ',' or ']'")
self.accept()
def get_expr(self, nested):
if self.tok != '{' and not nested:
- raise QAPIParseError(self, "expected '{'")
+ raise self._parse_error("expected '{'")
if self.tok == '{':
self.accept()
expr = self.get_members()
@@ -277,14 +283,14 @@ def get_expr(self, nested):
expr = self.val
self.accept()
else:
- raise QAPIParseError(
- self, "expected '{', '[', string, boolean or 'null'")
+ raise self._parse_error(
+ "expected '{', '[', string, boolean or 'null'")
return expr
def _get_doc(self, info):
if self.val != '##':
- raise QAPIParseError(
- self, "junk after '##' at start of documentation comment")
+ raise self._parse_error(
+ "junk after '##' at start of documentation comment")
docs = []
cur_doc = QAPIDoc(info)
@@ -293,8 +299,7 @@ def _get_doc(self, info):
if self.val.startswith('##'):
# End of doc comment
if self.val != '##':
- raise QAPIParseError(
- self,
+ raise self._parse_error(
"junk after '##' at end of documentation comment")
cur_doc.end_comment()
docs.append(cur_doc)
@@ -302,8 +307,7 @@ def _get_doc(self, info):
return docs
if self.val.startswith('# ='):
if cur_doc.symbol:
- raise QAPIParseError(
- self,
+ raise self._parse_error(
"unexpected '=' markup in definition documentation")
if cur_doc.body.text:
cur_doc.end_comment()
@@ -312,7 +316,7 @@ def _get_doc(self, info):
cur_doc.append(self.val)
self.accept(False)
- raise QAPIParseError(self, "documentation comment must end with '##'")
+ raise self._parse_error("documentation comment must end with '##'")
def get_doc(self, info):
try:
@@ -322,7 +326,7 @@ def get_doc(self, info):
# resulting error position depends on the state of the
# parser. It happens to be the beginning of the comment.
# More or less servicable, but action at a distance.
- raise QAPIParseError(self, str(err)) from err
+ raise self._parse_error(str(err)) from err
class QAPIDoc:
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 5/6] qapi/error.py: enable mypy checks
2020-09-22 21:21 [PATCH 0/6] qapi: static typing conversion, pt4 John Snow
` (3 preceding siblings ...)
2020-09-22 21:21 ` [PATCH 4/6] qapi/parser.py: refactor QAPIParseError John Snow
@ 2020-09-22 21:21 ` John Snow
2020-09-22 21:21 ` [PATCH 6/6] qapi/error.py: enable pylint checks John Snow
5 siblings, 0 replies; 7+ messages in thread
From: John Snow @ 2020-09-22 21:21 UTC (permalink / raw)
To: Markus Armbruster
Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa
Signed-off-by: John Snow <jsnow@redhat.com>
---
scripts/qapi/mypy.ini | 5 -----
1 file changed, 5 deletions(-)
diff --git a/scripts/qapi/mypy.ini b/scripts/qapi/mypy.ini
index df9b05e4ab..4d341c6b2d 100644
--- a/scripts/qapi/mypy.ini
+++ b/scripts/qapi/mypy.ini
@@ -4,11 +4,6 @@ strict_optional = False
disallow_untyped_calls = False
python_version = 3.6
-[mypy-qapi.error]
-disallow_untyped_defs = False
-disallow_incomplete_defs = False
-check_untyped_defs = False
-
[mypy-qapi.parser]
disallow_untyped_defs = False
disallow_incomplete_defs = False
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 6/6] qapi/error.py: enable pylint checks
2020-09-22 21:21 [PATCH 0/6] qapi: static typing conversion, pt4 John Snow
` (4 preceding siblings ...)
2020-09-22 21:21 ` [PATCH 5/6] qapi/error.py: enable mypy checks John Snow
@ 2020-09-22 21:21 ` John Snow
5 siblings, 0 replies; 7+ messages in thread
From: John Snow @ 2020-09-22 21:21 UTC (permalink / raw)
To: Markus Armbruster
Cc: Michael Roth, qemu-devel, John Snow, Eduardo Habkost, Cleber Rosa
Signed-off-by: John Snow <jsnow@redhat.com>
---
scripts/qapi/pylintrc | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/scripts/qapi/pylintrc b/scripts/qapi/pylintrc
index fb0386d529..88efbf71cb 100644
--- a/scripts/qapi/pylintrc
+++ b/scripts/qapi/pylintrc
@@ -2,8 +2,7 @@
# Add files or directories matching the regex patterns to the ignore list.
# The regex matches against base names, not paths.
-ignore-patterns=error.py,
- parser.py,
+ignore-patterns=parser.py,
schema.py,
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread