Inferred optional fields for msggen schema evolution

contrib/msggen/msggen/__main__.py

@@ -7,6 +7,18 @@
 from msggen.gen.rust import RustGenerator
 from msggen.gen.generator import GeneratorChain
 from msggen.utils import load_jsonrpc_service
+import logging
+from msggen.patch import VersionAnnotationPatch, OptionalPatch
+from msggen.checks import VersioningCheck
+
+
+logging.basicConfig(
+    level=logging.DEBUG,
+    format="%(asctime)s [%(levelname)s] %(message)s",
+    handlers=[
+        logging.StreamHandler()
+    ]
+)
 
 
 def add_handler_gen_grpc(generator_chain: GeneratorChain, meta):
@@ -52,8 +64,18 @@ def write_msggen_meta(meta):
 
 def run(rootdir: Path):
     schemadir = rootdir / "doc" / "schemas"
-    service = load_jsonrpc_service(schema_dir=schemadir)
     meta = load_msggen_meta()
+    service = load_jsonrpc_service(
+        schema_dir=schemadir,
+    )
+
+    p = VersionAnnotationPatch(meta=meta)
+    p.apply(service)
+    OptionalPatch().apply(service)
+
+    # Run the checks here, we should eventually split that out to a
+    # separate subcommand
+    VersioningCheck().check(service)
     generator_chain = GeneratorChain()
 
     add_handler_gen_grpc(generator_chain, meta)

contrib/msggen/msggen/checks.py

@@ -0,0 +1,36 @@
+from abc import ABC
+from msggen import model
+
+
+class Check(ABC):
+    """A check is a visitor that throws exceptions on inconsistencies.
+
+    """
+    def visit(self, field: model.Field) -> None:
+        pass
+
+    def check(self, service: model.Service) -> None:
+        def recurse(f: model.Field):
+            # First recurse if we have further type definitions
+            if isinstance(f, model.ArrayField):
+                self.visit(f.itemtype)
+                recurse(f.itemtype)
+            elif isinstance(f, model.CompositeField):
+                for c in f.fields:
+                    self.visit(c)
+                    recurse(c)
+            # Now visit ourselves
+            self.visit(f)
+        for m in service.methods:
+            recurse(m.request)
+            recurse(m.response)
+
+
+class VersioningCheck(Check):
+    """Check that all schemas have the `added` and `deprecated` annotations.
+    """
+    def visit(self, f: model.Field) -> None:
+        if not hasattr(f, "added"):
+            raise ValueError(f"Field {f.path} is missing the `added` annotation")
+        if not hasattr(f, "deprecated"):
+            raise ValueError(f"Field {f.path} is missing the `deprecated` annotation")

contrib/msggen/msggen/gen/grpc.py

@@ -194,7 +194,7 @@ def generate_message(self, message: CompositeField):
             if overrides.get(f.path, "") is None:
                 continue
 
-            opt = "optional " if not f.required else ""
+            opt = "optional " if f.optional else ""
             if isinstance(f, ArrayField):
                 typename = typemap.get(f.itemtype.typename, f.itemtype.typename)
                 if f.path in overrides:
@@ -288,18 +288,18 @@ def generate_composite(self, prefix, field: CompositeField):
                     'secret': f'i.to_vec()',
                 }.get(typ, f'i.into()')
 
-                if f.required:
+                if not f.optional:
                     self.write(f"{name}: c.{name}.into_iter().map(|i| {mapping}).collect(), // Rule #3 for type {typ}\n", numindent=3)
                 else:
                     self.write(f"{name}: c.{name}.map(|arr| arr.into_iter().map(|i| {mapping}).collect()).unwrap_or(vec![]), // Rule #3\n", numindent=3)
             elif isinstance(f, EnumField):
-                if f.required:
+                if not f.optional:
                     self.write(f"{name}: c.{name} as i32,\n", numindent=3)
                 else:
                     self.write(f"{name}: c.{name}.map(|v| v as i32),\n", numindent=3)
 
             elif isinstance(f, PrimitiveField):
-                typ = f.typename + ("?" if not f.required else "")
+                typ = f.typename + ("?" if f.optional else "")
                 # We may need to reduce or increase the size of some
                 # types, or have some conversion such as
                 # hex-decoding. Also includes the `Some()` that grpc
@@ -344,7 +344,7 @@ def generate_composite(self, prefix, field: CompositeField):
 
             elif isinstance(f, CompositeField):
                 rhs = ""
-                if f.required:
+                if not f.optional:
                     rhs = f'Some(c.{name}.into())'
                 else:
                     rhs = f'c.{name}.map(|v| v.into())'
@@ -446,21 +446,21 @@ def generate_composite(self, prefix, field: CompositeField) -> None:
                     self.write(f" state_changes: None,")
                     continue
 
-                if f.required:
+                if not f.optional:
                     self.write(f"{name}: c.{name}.into_iter().map(|s| {mapping}).collect(), // Rule #4\n", numindent=3)
                 else:
                     self.write(f"{name}: Some(c.{name}.into_iter().map(|s| {mapping}).collect()), // Rule #4\n", numindent=3)
 
             elif isinstance(f, EnumField):
                 if f.path == 'ListPeers.peers[].channels[].htlcs[].state':
                     continue
-                if f.required:
+                if not f.optional:
                     self.write(f"{name}: c.{name}.try_into().unwrap(),\n", numindent=3)
                 else:
                     self.write(f"{name}: c.{name}.map(|v| v.try_into().unwrap()),\n", numindent=3)
                 pass
             elif isinstance(f, PrimitiveField):
-                typ = f.typename + ("?" if not f.required else "")
+                typ = f.typename + ("?" if f.optional else "")
                 # We may need to reduce or increase the size of some
                 # types, or have some conversion such as
                 # hex-decoding. Also includes the `Some()` that grpc
@@ -503,7 +503,7 @@ def generate_composite(self, prefix, field: CompositeField) -> None:
                 self.write(f"{name}: {rhs}, // Rule #1 for type {typ}\n", numindent=3)
             elif isinstance(f, CompositeField):
                 rhs = ""
-                if f.required:
+                if not f.optional:
                     rhs = f'c.{name}.unwrap().into()'
                 else:
                     rhs = f'c.{name}.map(|v| v.into())'

contrib/msggen/msggen/gen/rust.py

@@ -132,7 +132,7 @@ def gen_enum(e):
         decl = ""  # No declaration if we have an override
         typename = overrides[e.path]
 
-    if e.required:
+    if not e.optional:
         defi = f"    // Path `{e.path}`\n"
         defi += rename_if_necessary(str(e.name), e.name.normalized())
         defi += f"    pub {e.name.normalized()}: {typename},\n"
@@ -152,7 +152,7 @@ def gen_primitive(p):
     if p.deprecated:
         defi += "    #[deprecated]\n"
     defi += rename_if_necessary(org, p.name.name)
-    if p.required:
+    if not p.optional:
         defi += f"    pub {p.name}: {typename},\n"
     else:
         defi += f"    #[serde(skip_serializing_if = \"Option::is_none\")]\n    pub {p.name}: Option<{typename}>,\n"
@@ -191,7 +191,7 @@ def gen_array(a):
     if a.deprecated:
         defi += "    #[deprecated]\n"
     defi += rename_if_necessary(alias, name)
-    if a.required:
+    if not a.optional:
         defi += f"    pub {name}: {'Vec<'*a.dims}{itemtype}{'>'*a.dims},\n"
     else:
         defi += f"    #[serde(skip_serializing_if = \"crate::is_none_or_empty\")]\n    pub {name}: Option<{'Vec<'*a.dims}{itemtype}{'>'*a.dims}>,\n"
@@ -216,7 +216,7 @@ def gen_composite(c) -> Tuple[str, str]:
     defi = ""
     if c.deprecated:
         defi += "    #[deprecated]\n"
-    if c.required:
+    if not c.optional:
         defi += f"    pub {c.name}: {c.typename},\n"
     else:
         defi += f"    #[serde(skip_serializing_if = \"Option::is_none\")]\n    pub {c.name}: Option<{c.typename}>,\n"