chore!: Checks/validation on t.either (#868)

Emit a warning or an error when a variant is a subtype of another one. #### Migration notes **BREAKING CHANGE**: Previously valid typegraph might fail validation. You will need to fix your types to add some consistency in `t.either`/`t.union` types. - [x] The change comes with new or modified tests - [ ] Hard-to-understand functions have explanatory comments - [ ] End-user documentation is updated to reflect the change
metatypedev · Oct 17, 2024 · defae4c · defae4c
1 parent 1203968
commit defae4c
Show file tree

Hide file tree

Showing 12 changed files with 247 additions and 124 deletions.
diff --git a/src/common/src/typegraph/validator/mod.rs b/src/common/src/typegraph/validator/mod.rs
@@ -15,6 +15,8 @@ use super::visitor::{
     TypeVisitorContext, VisitLayer, VisitResult, VisitorResult,
 };
 
+use self::types::{EnsureSubtypeOf, ErrorCollector, ExtendedTypeNode};
+
 #[allow(dead_code)]
 fn assert_unique_titles(types: &[TypeNode]) -> Vec<ValidatorError> {
     let mut duplicates = vec![];
@@ -46,6 +48,7 @@ pub fn validate_typegraph(tg: &Typegraph) -> Vec<ValidatorError> {
     // errors.extend(assert_unique_titles(&tg.types));
     let context = ValidatorContext { typegraph: tg };
     let validator = Validator::default();
+
     errors.extend(tg.traverse_types(validator, &context, Layer).unwrap());
     errors
 }
@@ -122,9 +125,66 @@ impl<'a> TypeVisitor<'a> for Validator {
     ) -> VisitResult<Self::Return> {
         let type_node = current_node.type_node;
 
+        let tg = context.get_typegraph();
+
+        let get_type_name = |idx: u32| tg.types.get(idx as usize).unwrap().type_name();
+
         if let TypeNode::Function { .. } = type_node {
             // validate materializer??
             // TODO deno static
+        } else if let TypeNode::Either { data, .. } = type_node {
+            let variants = data.one_of.clone();
+            for i in 0..variants.len() {
+                for j in (i + 1)..variants.len() {
+                    let type1 = ExtendedTypeNode::new(tg, variants[i]);
+                    let type2 = ExtendedTypeNode::new(tg, variants[j]);
+
+                    let mut subtype_errors = ErrorCollector::default();
+                    type1.ensure_subtype_of(&type2, tg, &mut subtype_errors);
+
+                    if subtype_errors.errors.is_empty() {
+                        self.push_error(
+                            current_node.path,
+                            format!(
+                                "Invalid either type: variant #{i} ('{}') is a subtype of variant #{j} ('{}')",
+                                get_type_name(variants[i]),
+                                get_type_name(variants[j]),
+                            ),
+                        );
+                    }
+
+                    let mut subtype_errors = ErrorCollector::default();
+                    type2.ensure_subtype_of(&type1, tg, &mut subtype_errors);
+
+                    if subtype_errors.errors.is_empty() {
+                        self.push_error(
+                            current_node.path,
+                            format!(
+                                "Invalid either type: variant #{j} ('{}') is a subtype of variant #{i} ('{}')",
+                                get_type_name(variants[j]),
+                                get_type_name(variants[i]),
+                            ),
+                        );
+                    }
+                }
+            }
+        } else if let TypeNode::Union { data, .. } = type_node {
+            let variants = data.any_of.clone();
+
+            for i in 0..variants.len() {
+                for j in (i + 1)..variants.len() {
+                    if variants[i] == variants[j] {
+                        self.push_error(
+                            current_node.path,
+                            format!(
+                                "Invalid union type: variant #{i} ('{}') is the same type as variant #{j} ('{}')",
+                                get_type_name(variants[i]),
+                                get_type_name(variants[j]),
+                            ),
+                        );
+                    }
+                }
+            }
         }
 
         if let Some(enumeration) = &type_node.base().enumeration {

diff --git a/src/meta-cli/src/main.rs b/src/meta-cli/src/main.rs
@@ -82,7 +82,7 @@ fn main() -> Result<()> {
 
     if args.verbose.is_present() {
         let filter = args.verbose.log_level_filter().to_string();
-        std::env::set_var("RUST_LOG", format!("warn,meta={filter}"));
+        unsafe { std::env::set_var("RUST_LOG", format!("warn,meta={filter}")) };
     }
     logger::init();
     if args.version {

diff --git a/src/meta-cli/src/utils/mod.rs b/src/meta-cli/src/utils/mod.rs
@@ -38,8 +38,8 @@ pub fn ensure_venv<P: AsRef<Path>>(dir: P) -> Result<()> {
             venv_bin = venv_dir.as_path().join("bin").to_str().unwrap()
         );
 
-        set_var("VIRTUAL_ENV", venv_dir.to_str().unwrap());
-        set_var("PATH", path);
+        unsafe { set_var("VIRTUAL_ENV", venv_dir.to_str().unwrap()) };
+        unsafe { set_var("PATH", path) };
         Ok(())
     } else {
         bail!("Python venv required")

diff --git a/src/mt_deno/src/lib.rs b/src/mt_deno/src/lib.rs
@@ -305,7 +305,9 @@ pub fn new_thread_builder() -> std::thread::Builder {
         // FIXME: find a better location for this as tihs won't work
         // if a second thread has already launched by this point
         if std::env::var("RUST_MIN_STACK").is_err() {
-            std::env::set_var("RUST_MIN_STACK", "8388608");
+            unsafe {
+                std::env::set_var("RUST_MIN_STACK", "8388608");
+            }
         }
         // deno & swc need 8 MiB with dev profile (release is ok)
         // https://github.com/swc-project/swc/blob/main/CONTRIBUTING.md

diff --git a/src/typegraph/core/src/validation/types.rs b/src/typegraph/core/src/validation/types.rs
@@ -69,6 +69,7 @@ pub fn validate_value(value: &serde_json::Value, type_id: TypeId, path: String)
 
         TypeDef::Either(inner) => {
             let mut match_count = 0;
+
             for type_id in inner.data.variants.iter() {
                 match validate_value(value, type_id.into(), path.clone()) {
                     Ok(()) => match_count += 1,

diff --git a/tests/e2e/typegraph/__snapshots__/validator_test.ts.snap b/tests/e2e/typegraph/__snapshots__/validator_test.ts.snap
@@ -19,6 +19,16 @@ snapshot[`typegraph validation 1`] = `
 [31mERROR[0m [2mmeta::deploy::actors::console[0m[2m:[0m - at validator:/testFromParent/[out]/nested/[in]/b: from_parent injection: 'minimum_length' is required on the subtype if it is defined on the supertype
 [31mERROR[0m [2mmeta::deploy::actors::console[0m[2m:[0m - at validator:/testFromParent/[out]/nested/[in]/b: from_parent injection: 'maximum_length' cannot be higher on the subtype: 20 > 16
 [31mERROR[0m [2mmeta::deploy::actors::console[0m[2m:[0m - at validator:/testFromParent/[out]/nested/[in]/c: from_parent injection: property b is not allowed: it is not defined in the supertype
+[31mERROR[0m [2mmeta::deploy::actors::console[0m[2m:[0m - at validator:/testEither/[out]/a: Invalid either type: variant #0 ('integer') is a subtype of variant #1 ('float')
+[31mERROR[0m [2mmeta::deploy::actors::console[0m[2m:[0m - at validator:/testEither/[out]/b: Invalid either type: variant #0 ('string') is a subtype of variant #1 ('string')
+[31mERROR[0m [2mmeta::deploy::actors::console[0m[2m:[0m - at validator:/testEither/[out]/d: Invalid either type: variant #0 ('list') is a subtype of variant #1 ('list')
+[31mERROR[0m [2mmeta::deploy::actors::console[0m[2m:[0m - at validator:/testEither/[out]/f: Invalid either type: variant #0 ('string') is a subtype of variant #1 ('string')
+[31mERROR[0m [2mmeta::deploy::actors::console[0m[2m:[0m - at validator:/testEither/[out]/g: Invalid either type: variant #1 ('object') is a subtype of variant #0 ('object')
+[31mERROR[0m [2mmeta::deploy::actors::console[0m[2m:[0m - at validator:/testEither/[out]/h: Invalid either type: variant #0 ('list') is a subtype of variant #1 ('list')
+[31mERROR[0m [2mmeta::deploy::actors::console[0m[2m:[0m - at validator:/testUnion/[in]/a: Value '25' did not match any of the variants of the union
+[31mERROR[0m [2mmeta::deploy::actors::console[0m[2m:[0m - at validator:/testUnion/[in]/c: Value '{"x":1,"y":"test","z":"not a boolean"}' did not match any of the variants of the union
+[31mERROR[0m [2mmeta::deploy::actors::console[0m[2m:[0m - at validator:/testUnion/[in]/d: Value '[1,"2",3]' did not match any of the variants of the union
+[31mERROR[0m [2mmeta::deploy::actors::console[0m[2m:[0m - at validator:/testUnion/[out]/b: Invalid union type: variant #0 ('string') is the same type as variant #1 ('string')
 [31mERROR[0m [2mmeta::deploy::actors::console[0m[2m:[0m - Typegraph validator failed validation
 Error: 
    0: [91mfailed[0m

diff --git a/tests/e2e/typegraph/validator.py b/tests/e2e/typegraph/validator.py
@@ -38,6 +38,53 @@ def validator(g: Graph):
         }
     )
 
+    either = t.struct(
+        {
+            "a": t.either([t.integer(), t.float()]),
+            "b": t.either([t.string(max=10), t.string()]),
+            "c": t.either(
+                [
+                    t.struct({"x": t.integer(), "y": t.string()}),
+                    t.struct({"x": t.integer(), "y": t.string(), "z": t.boolean()}),
+                ]
+            ),
+            "d": t.either([t.list(t.integer()), t.list(t.float())]),
+            "e": t.either([t.integer(min=0, max=10), t.integer(min=5, max=15)]),
+            "f": t.either(
+                [t.string(enum=["a", "b", "c"]), t.string(enum=["a", "b", "c", "d"])]
+            ),
+            "g": t.either(
+                [t.struct({"x": t.integer().optional()}), t.struct({"x": t.integer()})]
+            ),
+            "h": t.either(
+                [
+                    t.list(t.either([t.integer(), t.string()])),
+                    t.list(t.either([t.float(), t.string()])),
+                ]
+            ),
+        }
+    )
+
+    union = t.struct(
+        {
+            "a": t.union(
+                [
+                    t.integer(min=0, max=10),
+                    t.integer(min=5, max=15),
+                    t.integer(min=10, max=20),
+                ]
+            ).set(25),
+            "b": t.union([t.string(), t.string()]),
+            "c": t.union(
+                [
+                    t.struct({"x": t.integer(), "y": t.string()}),
+                    t.struct({"x": t.integer(), "y": t.string(), "z": t.boolean()}),
+                ]
+            ).set({"x": 1, "y": "test", "z": "not a boolean"}),
+            "d": t.union([t.list(t.integer()), t.list(t.string())]).set([1, "2", 3]),
+        }
+    )
+
     g.expose(
         test=deno.identity(injection),
         testEnums=deno.identity(enums),
@@ -59,4 +106,6 @@ def validator(g: Graph):
                 ),
             }
         ),
+        testEither=deno.identity(either),
+        testUnion=deno.identity(union),
     )
diff --git a/tests/metagen/typegraphs/python.py b/tests/metagen/typegraphs/python.py
@@ -14,7 +14,7 @@ def example(g: Graph):
             "integer": t.integer(),
             "email": t.email().optional(),
             "list_integer": t.list(t.integer()),
-            "opt_union_flat": t.union([t.integer(), t.integer(), t.float()]).optional(),
+            "opt_union_flat": t.union([t.integer(), t.float()]).optional(),
             "reference": t.list(g.ref("Example")).optional(),
             "nested_ref": t.struct(
                 {"either": t.either([g.ref("Example"), references])}

diff --git a/tests/metagen/typegraphs/sample/py/client.py b/tests/metagen/typegraphs/sample/py/client.py
@@ -609,17 +609,6 @@ class NodeDescs:
     def scalar():
         return NodeMeta()
 
-    @staticmethod
-    def RootScalarUnionFn():
-        return_node = NodeDescs.scalar()
-        return NodeMeta(
-            sub_nodes=return_node.sub_nodes,
-            variants=return_node.variants,
-            arg_types={
-                "id": "RootScalarArgsFnOutput",
-            },
-        )
-
     @staticmethod
     def Post():
         return NodeMeta(
@@ -630,6 +619,14 @@ def Post():
             },
         )
 
+    @staticmethod
+    def RootCompositeNoArgsFn():
+        return_node = NodeDescs.Post()
+        return NodeMeta(
+            sub_nodes=return_node.sub_nodes,
+            variants=return_node.variants,
+        )
+
     @staticmethod
     def User():
         return NodeMeta(
@@ -649,38 +646,35 @@ def RootGetUserFn():
         )
 
     @staticmethod
-    def RootScalarNoArgsFn():
-        return_node = NodeDescs.scalar()
+    def RootGetPostsFn():
+        return_node = NodeDescs.Post()
         return NodeMeta(
             sub_nodes=return_node.sub_nodes,
             variants=return_node.variants,
         )
 
     @staticmethod
-    def RootScalarArgsFn():
+    def RootScalarNoArgsFn():
         return_node = NodeDescs.scalar()
         return NodeMeta(
             sub_nodes=return_node.sub_nodes,
             variants=return_node.variants,
-            arg_types={
-                "id": "UserIdStringUuid",
-                "slug": "PostSlugString",
-                "title": "PostSlugString",
-            },
         )
 
     @staticmethod
-    def RootMixedUnionFnOutput():
+    def RootCompositeArgsFn():
+        return_node = NodeDescs.Post()
         return NodeMeta(
-            variants={
-                "post": NodeDescs.Post,
-                "user": NodeDescs.User,
+            sub_nodes=return_node.sub_nodes,
+            variants=return_node.variants,
+            arg_types={
+                "id": "RootScalarArgsFnOutput",
             },
         )
 
     @staticmethod
-    def RootMixedUnionFn():
-        return_node = NodeDescs.RootMixedUnionFnOutput()
+    def RootScalarUnionFn():
+        return_node = NodeDescs.scalar()
         return NodeMeta(
             sub_nodes=return_node.sub_nodes,
             variants=return_node.variants,
@@ -690,16 +684,17 @@ def RootMixedUnionFn():
         )
 
     @staticmethod
-    def RootGetPostsFn():
-        return_node = NodeDescs.Post()
+    def RootMixedUnionFnOutput():
         return NodeMeta(
-            sub_nodes=return_node.sub_nodes,
-            variants=return_node.variants,
+            variants={
+                "post": NodeDescs.Post,
+                "user": NodeDescs.User,
+            },
         )
 
     @staticmethod
-    def RootCompositeArgsFn():
-        return_node = NodeDescs.Post()
+    def RootMixedUnionFn():
+        return_node = NodeDescs.RootMixedUnionFnOutput()
         return NodeMeta(
             sub_nodes=return_node.sub_nodes,
             variants=return_node.variants,
@@ -708,14 +703,6 @@ def RootCompositeArgsFn():
             },
         )
 
-    @staticmethod
-    def RootCompositeNoArgsFn():
-        return_node = NodeDescs.Post()
-        return NodeMeta(
-            sub_nodes=return_node.sub_nodes,
-            variants=return_node.variants,
-        )
-
     @staticmethod
     def RootCompositeUnionFnOutput():
         return NodeMeta(
@@ -736,6 +723,29 @@ def RootCompositeUnionFn():
             },
         )
 
+    @staticmethod
+    def RootScalarArgsFn():
+        return_node = NodeDescs.scalar()
+        return NodeMeta(
+            sub_nodes=return_node.sub_nodes,
+            variants=return_node.variants,
+            arg_types={
+                "id": "UserIdStringUuid",
+                "slug": "PostSlugString",
+                "title": "PostSlugString",
+            },
+        )
+
+
+RootScalarArgsFnOutput = str
+
+RootCompositeArgsFnInput = typing.TypedDict(
+    "RootCompositeArgsFnInput",
+    {
+        "id": RootScalarArgsFnOutput,
+    },
+    total=False,
+)
 
 UserIdStringUuid = str
 
@@ -751,16 +761,6 @@ def RootCompositeUnionFn():
     total=False,
 )
 
-RootScalarArgsFnOutput = str
-
-RootCompositeArgsFnInput = typing.TypedDict(
-    "RootCompositeArgsFnInput",
-    {
-        "id": RootScalarArgsFnOutput,
-    },
-    total=False,
-)
-
 UserEmailStringEmail = str
 
 UserPostsPostList = typing.List[Post]
@@ -847,8 +847,8 @@ def __init__(self):
                 "UserIdStringUuid": "String!",
                 "PostSlugString": "String!",
                 "RootScalarArgsFnOutput": "String!",
-                "post": "post!",
                 "user": "user!",
+                "post": "post!",
             }
         )