diff mbox series

[17/26] qapi/pragma.py: Move pragma parsing out of parser.py

Message ID 20200922223525.4085762-18-jsnow@redhat.com
State New
Headers show
Series qapi: static typing conversion, pt5 | expand

Commit Message

John Snow Sept. 22, 2020, 10:35 p.m. UTC
parser.py is a JSON parser at heart and shouldn't necessarily understand
what it is parsing on a semantic level. Move pragma parsing to pragma.py,
and leave the parser a little more happily ignorant.

Note: the type annotation in error.py now creates a cyclic import, because
error -> source -> pragma -> error. Use the magical mypy constant TYPE_CHECKING
to avoid this cycle at runtime. pylint dislikes this cycle still, but it can
be safely ignored.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/error.py  |  8 +++---
 scripts/qapi/parser.py | 41 ++++---------------------------
 scripts/qapi/pragma.py | 55 +++++++++++++++++++++++++++++++++++++-----
 3 files changed, 59 insertions(+), 45 deletions(-)
diff mbox series

Patch

diff --git a/scripts/qapi/error.py b/scripts/qapi/error.py
index ab6a0f6271..be5fd24218 100644
--- a/scripts/qapi/error.py
+++ b/scripts/qapi/error.py
@@ -11,9 +11,11 @@ 
 # 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 typing import Optional, TYPE_CHECKING
 
-from .source import QAPISourceInfo
+if TYPE_CHECKING:
+    # pylint: disable=cyclic-import
+    from .source import QAPISourceInfo
 
 
 class QAPIError(Exception):
@@ -23,7 +25,7 @@  class QAPIError(Exception):
 class QAPISourceError(QAPIError):
     """Error class for all exceptions identifying a source location."""
     def __init__(self,
-                 info: QAPISourceInfo,
+                 info: 'QAPISourceInfo',
                  msg: str,
                  col: Optional[int] = None):
         super().__init__()
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index f65afa4eb2..5b3a9b5da8 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -26,10 +26,10 @@ 
     Type,
     TypeVar,
     Union,
-    cast,
 )
 
 from .error import QAPIError, QAPISourceError, QAPISemError
+from .pragma import PragmaError
 from .source import QAPISourceInfo
 
 
@@ -151,14 +151,10 @@  def _parse(self) -> None:
                     self.docs.extend(exprs_include.docs)
             elif "pragma" in expr:
                 self.reject_expr_doc(cur_doc)
-                if len(expr) != 1:
-                    raise QAPISemError(info, "invalid 'pragma' directive")
-                pragma = expr['pragma']
-                if not isinstance(pragma, dict):
-                    raise QAPISemError(
-                        info, "value of 'pragma' must be an object")
-                for name, value in pragma.items():
-                    self._pragma(name, value, info)
+                try:
+                    info.pragma.parse(expr)
+                except PragmaError as err:
+                    raise QAPISemError(info, str(err)) from err
             else:
                 if cur_doc and not cur_doc.symbol:
                     raise QAPISemError(
@@ -204,33 +200,6 @@  def _include(cls,
 
         return QAPISchemaParser(incl_fname, previously_included, info)
 
-    @classmethod
-    def _pragma(cls,
-                name: str,
-                value: object,
-                info: QAPISourceInfo) -> None:
-        if name == 'doc-required':
-            if not isinstance(value, bool):
-                raise QAPISemError(info,
-                                   "pragma 'doc-required' must be boolean")
-            info.pragma.doc_required = value
-        elif name == 'returns-whitelist':
-            if (not isinstance(value, list)
-                    or any([not isinstance(elt, str) for elt in value])):
-                raise QAPISemError(
-                    info,
-                    "pragma returns-whitelist must be a list of strings")
-            info.pragma.returns_whitelist = cast(List[str], value)
-        elif name == 'name-case-whitelist':
-            if (not isinstance(value, list)
-                    or any([not isinstance(elt, str) for elt in value])):
-                raise QAPISemError(
-                    info,
-                    "pragma name-case-whitelist must be a list of strings")
-            info.pragma.name_case_whitelist = cast(List[str], value)
-        else:
-            raise QAPISemError(info, "unknown pragma '%s'" % name)
-
     def accept(self, skip_comment: bool = True) -> None:
         """
         Read the next lexeme.
diff --git a/scripts/qapi/pragma.py b/scripts/qapi/pragma.py
index 7f3db9ab87..03ba3cac90 100644
--- a/scripts/qapi/pragma.py
+++ b/scripts/qapi/pragma.py
@@ -9,17 +9,60 @@ 
 # 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 List
+from typing import Mapping, Sequence
+
+from .error import QAPIError
+
+
+class PragmaError(QAPIError):
+    """For errors relating to Pragma validation."""
 
 
 class QAPISchemaPragma:
-    # Replace with @dataclass in Python 3.7+
-    # pylint: disable=too-few-public-methods
-
     def __init__(self) -> None:
         # Are documentation comments required?
         self.doc_required = False
         # Whitelist of commands allowed to return a non-dictionary
-        self.returns_whitelist: List[str] = []
+        self.returns_whitelist: Sequence[str] = tuple()
         # Whitelist of entities allowed to violate case conventions
-        self.name_case_whitelist: List[str] = []
+        self.name_case_whitelist: Sequence[str] = tuple()
+
+    def _add_doc_required(self, value: object) -> None:
+        if not isinstance(value, bool):
+            raise PragmaError("pragma 'doc-required' must be boolean")
+        self.doc_required = value
+
+    def _add_returns_whitelist(self, value: object) -> None:
+        if (not isinstance(value, list)
+                or any([not isinstance(elt, str) for elt in value])):
+            raise PragmaError(
+                "pragma returns-whitelist must be a list of strings")
+        self.returns_whitelist = tuple(value)
+
+    def _add_name_case_whitelist(self, value: object) -> None:
+        if (not isinstance(value, list)
+                or any([not isinstance(elt, str) for elt in value])):
+            raise PragmaError(
+                "pragma name-case-whitelist must be a list of strings")
+        self.name_case_whitelist = tuple(value)
+
+    def add(self, name: str, value: object) -> None:
+        if name == 'doc-required':
+            self._add_doc_required(value)
+        elif name == 'returns-whitelist':
+            self._add_returns_whitelist(value)
+        elif name == 'name-case-whitelist':
+            self._add_name_case_whitelist(value)
+        else:
+            raise PragmaError(f"unknown pragma '{name}'")
+
+    def parse(self, expression: Mapping[str, object]) -> None:
+        if expression.keys() != {'pragma'}:
+            raise PragmaError("invalid 'pragma' directive")
+
+        body = expression['pragma']
+        if not isinstance(body, dict):
+            raise PragmaError("value of 'pragma' must be an object")
+
+        for name, value in body.items():
+            self.add(name, value)