From 5707c0a5b9cd5f14110e730e8a2d54633481f3bd Mon Sep 17 00:00:00 2001
From: Christian Kellner <christian@kellner.me>
Date: Sun, 6 Jun 2021 17:49:56 +0200
Subject: [PATCH] meta: proper error reporting for schema parsing

When parsing the module file, parse the JSON directly from the AST
node, because the AST node contains the line number of the schema
in the  module and thus we can resolve the correct line number for
errors  within the JSON. Convert the `JSONDecodeError` to a
`SyntaxError` which results in an overall better exception message:

Before:
Traceback (most recent call last):
  File "/workspaces/osbuild/osbuild/meta.py", line 331, in get_schema
    opts = self._make_options(version)
  [...]
  File "/usr/lib64/python3.9/json/decoder.py", line 353, in raw_decode
    obj, end = self.scan_once(s, idx)
json.decoder.JSONDecodeError: Expecting property name enclosed in
                              double quotes: line 2 column 1 (char 14)

After:
Traceback (most recent call last):
  File "/usr/lib64/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  [...]
    raise SyntaxError(msg, detail) from None
  File "stages/org.osbuild.ostree.init-fs", line 31
    additionalProperties: False
    ^
SyntaxError: Invalid schema: Expecting property name enclosed in ...
---
 osbuild/meta.py | 47 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 14 deletions(-)

diff --git a/osbuild/meta.py b/osbuild/meta.py
index f30b5150..322f515a 100644
--- a/osbuild/meta.py
+++ b/osbuild/meta.py
@@ -301,8 +301,7 @@ class ModuleInfo:
             raw = self.opts[fallback]
         if not raw:
             raise ValueError(f"Unsupported version: {version}")
-
-        return json.loads("{" + raw + "}")
+        return raw
 
     def _make_options(self, version):
         if version == "2":
@@ -310,11 +309,11 @@ class ModuleInfo:
             if not raw:
                 return self._make_options("1")
         elif version == "1":
-            raw = '"options": {' + self.opts["1"] + '}'
+            raw = {"options": self.opts["1"]}
         else:
             raise ValueError(f"Unsupported version: {version}")
 
-        return json.loads("{" + raw + "}")
+        return raw
 
     def get_schema(self, version="1"):
         schema = {
@@ -346,16 +345,29 @@ class ModuleInfo:
 
         return schema
 
+    @classmethod
+    def _parse_schema(cls, klass, name, node):
+        if not node:
+            return {}
+
+        value = node.value
+        if not isinstance(value, ast.Str):
+            return {}
+
+        try:
+            return json.loads("{" + value.s + "}")
+        except json.decoder.JSONDecodeError as e:
+            msg = "Invalid schema: " + e.msg
+            line = e.doc.splitlines()[e.lineno - 1]
+            fullname = cls.MODULES[klass] + "/" + name
+            lineno = e.lineno + node.lineno - 1
+            detail = fullname, lineno, e.colno, line
+            raise SyntaxError(msg, detail) from None
+
     @classmethod
     def load(cls, root, klass, name) -> Optional["ModuleInfo"]:
         names = ["SCHEMA", "SCHEMA_2"]
 
-        def value(a):
-            v = a.value
-            if isinstance(v, ast.Str):
-                return v.s
-            return ""
-
         def filter_type(lst, target):
             return [x for x in lst if isinstance(x, target)]
 
@@ -379,13 +391,20 @@ class ModuleInfo:
         doclist = docstring.split("\n")
 
         assigns = filter_type(tree.body, ast.Assign)
-        targets = [(t, a) for a in assigns for t in targets(a)]
-        values = {k: value(v) for k, v in targets if k in names}
+        values = {
+            t: a
+            for a in assigns
+            for t in targets(a)
+            if t in names
+        }
+
+        def parse_schema(node):
+            return cls._parse_schema(klass, name, node)
 
         info = {
             'schema': {
-                "1": values.get("SCHEMA", ""),
-                "2": values.get("SCHEMA_2")
+                "1": parse_schema(values.get("SCHEMA")),
+                "2": parse_schema(values.get("SCHEMA_2")),
             },
             'desc': doclist[0],
             'info': "\n".join(doclist[1:])