handle redundant types with $ref; handle untagged enum full name conf…

…licts (oxidecomputer#200)

typify-impl/src/convert.rs

@@ -6,7 +6,7 @@ use crate::type_entry::{
     EnumTagType, TypeEntry, TypeEntryDetails, TypeEntryEnum, TypeEntryNewtype, TypeEntryStruct,
     Variant, VariantDetails,
 };
-use crate::util::{all_mutually_exclusive, none_or_single, recase, Case, StringValidator};
+use crate::util::{all_mutually_exclusive, none_or_single, recase, ref_key, Case, StringValidator};
 use log::info;
 use schemars::schema::{
     ArrayValidation, InstanceType, Metadata, ObjectValidation, Schema, SchemaObject, SingleOrVec,
@@ -330,6 +330,32 @@ impl TypeSpace {
                 extensions: _,
             } => self.convert_reference(metadata, reference),
 
+            // Accept references that... for some reason... include the type.
+            // TODO this could be generalized to validate any redundant
+            // validation here or could be used to compute a new, more
+            // constrained type.
+            SchemaObject {
+                metadata,
+                instance_type,
+                format: None,
+                enum_values: None,
+                const_value: None,
+                subschemas: None,
+                number: None,
+                string: None,
+                array: None,
+                object: None,
+                reference: Some(reference),
+                extensions: _,
+            } => {
+                let ref_schema = self.definitions.get(ref_key(reference)).unwrap();
+                assert!(matches!(ref_schema, Schema::Object(SchemaObject {
+                        instance_type: it, ..
+                    }) if instance_type == it));
+
+                self.convert_reference(metadata, reference)
+            }
+
             // Enum of a single, known, non-String type (strings above).
             SchemaObject {
                 instance_type: Some(SingleOrVec::Single(_)),
@@ -910,10 +936,7 @@ impl TypeSpace {
         metadata: &'a Option<Box<Metadata>>,
         ref_name: &str,
     ) -> Result<(TypeEntry, &'a Option<Box<Metadata>>)> {
-        let key = match ref_name.rfind('/') {
-            Some(idx) => &ref_name[idx + 1..],
-            None => ref_name,
-        };
+        let key = ref_key(ref_name);
         let type_id = self
             .ref_to_id
             .get(key)

typify-impl/src/enums.rs

@@ -743,6 +743,11 @@ impl TypeSpace {
                     }
                     (Some(name), Some(prefix)) => {
                         common_prefix = Some(get_common_prefix(name, prefix));
+                        // If the common prefix is the whole name, we can't use
+                        // these names.
+                        if common_prefix.as_ref() == Some(name) {
+                            names_from_variants = false;
+                        }
                     }
                 }
 
@@ -764,6 +769,7 @@ impl TypeSpace {
                 } else {
                     format!("Variant{}", idx)
                 };
+                assert!(!name.is_empty());
                 Variant {
                     name,
                     rename: None,

typify-impl/src/lib.rs

@@ -384,12 +384,7 @@ impl TypeSpace {
         // previous step because each type may create additional types. This
         // effectively is doing the work of `add_type_with_name` but for a
         // batch of types.
-        for (index, (ref_name, schema)) in definitions.into_iter().enumerate() {
-            let type_name = match ref_name.rfind('/') {
-                Some(idx) => &ref_name[idx..],
-                None => &ref_name,
-            };
-
+        for (index, (type_name, schema)) in definitions.into_iter().enumerate() {
             info!(
                 "converting type: {} with schema {}",
                 type_name,
@@ -399,9 +394,9 @@ impl TypeSpace {
             // Check for manually replaced types. Proceed with type conversion
             // if there is none; use the specified type if there is.
             let type_id = TypeId(base_id + index as u64);
-            let check_name = sanitize(type_name, Case::Pascal);
+            let check_name = sanitize(&type_name, Case::Pascal);
             match self.settings.replace.get(&check_name) {
-                None => self.convert_ref_type(type_name, schema, type_id)?,
+                None => self.convert_ref_type(&type_name, schema, type_id)?,
 
                 Some(replace_type) => {
                     let type_entry = TypeEntry::new_native(

typify-impl/src/util.rs

@@ -483,7 +483,7 @@ pub(crate) fn constant_string_value(schema: &Schema) -> Option<&str> {
     }
 }
 
-pub(crate) fn ref_key(ref_name: &String) -> &str {
+pub(crate) fn ref_key(ref_name: &str) -> &str {
     match ref_name.rfind('/') {
         Some(idx) => &ref_name[idx + 1..],
         None => ref_name,

typify/tests/schemas/id-or-name.json

@@ -1,32 +1,46 @@
 {
   "$schema": "http://json-schema.org/draft-07/schema#",
-  "title": "IdOrName",
-  "oneOf": [
-    {
-      "title": "Id",
-      "allOf": [
+  "definitions": {
+    "IdOrName": {
+      "oneOf": [
         {
-          "type": "string",
-          "format": "uuid"
-        }
-      ]
-    },
-    {
-      "title": "Name",
-      "allOf": [
+          "title": "Id",
+          "allOf": [
+            {
+              "type": "string",
+              "format": "uuid"
+            }
+          ]
+        },
         {
-          "$ref": "#/definitions/Name"
+          "title": "Name",
+          "allOf": [
+            {
+              "$ref": "#/definitions/Name"
+            }
+          ]
         }
       ]
-    }
-  ],
-  "definitions": {
+    },
     "Name": {
       "title": "A name unique within the parent collection",
       "description": "Names must begin with a lower case ASCII letter, be composed exclusively of lowercase ASCII, uppercase ASCII, numbers, and '-', and may not end with a '-'. Names cannot be a UUID though they may contain a UUID.",
       "type": "string",
       "pattern": "^(?![0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$)^[a-z][a-z0-9-]*[a-zA-Z0-9]$",
       "maxLength": 63
+    },
+    "IdOrNameRedundant": {
+      "$comment": "tests references that include a redundant type field",
+      "oneOf": [
+        {
+          "type": "string",
+          "format": "uuid"
+        },
+        {
+          "type": "string",
+          "$ref": "#/definitions/Name"
+        }
+      ]
     }
   }
 }

typify/tests/schemas/id-or-name.rs

@@ -49,6 +49,55 @@ impl ToString for IdOrName {
         }
     }
 }
+#[derive(Clone, Debug, Deserialize, Serialize)]
+#[serde(untagged)]
+pub enum IdOrNameRedundant {
+    Variant0(uuid::Uuid),
+    Variant1(Name),
+}
+impl From<&IdOrNameRedundant> for IdOrNameRedundant {
+    fn from(value: &IdOrNameRedundant) -> Self {
+        value.clone()
+    }
+}
+impl std::str::FromStr for IdOrNameRedundant {
+    type Err = &'static str;
+    fn from_str(value: &str) -> Result<Self, &'static str> {
+        if let Ok(v) = value.parse() {
+            Ok(Self::Variant0(v))
+        } else if let Ok(v) = value.parse() {
+            Ok(Self::Variant1(v))
+        } else {
+            Err("string conversion failed for all variants")
+        }
+    }
+}
+impl std::convert::TryFrom<&str> for IdOrNameRedundant {
+    type Error = &'static str;
+    fn try_from(value: &str) -> Result<Self, &'static str> {
+        value.parse()
+    }
+}
+impl std::convert::TryFrom<&String> for IdOrNameRedundant {
+    type Error = &'static str;
+    fn try_from(value: &String) -> Result<Self, &'static str> {
+        value.parse()
+    }
+}
+impl std::convert::TryFrom<String> for IdOrNameRedundant {
+    type Error = &'static str;
+    fn try_from(value: String) -> Result<Self, &'static str> {
+        value.parse()
+    }
+}
+impl ToString for IdOrNameRedundant {
+    fn to_string(&self) -> String {
+        match self {
+            Self::Variant0(x) => x.to_string(),
+            Self::Variant1(x) => x.to_string(),
+        }
+    }
+}
 #[doc = "Names must begin with a lower case ASCII letter, be composed exclusively of lowercase ASCII, uppercase ASCII, numbers, and '-', and may not end with a '-'. Names cannot be a UUID though they may contain a UUID."]
 #[derive(Clone, Debug, Serialize)]
 pub struct Name(String);

typify/tests/schemas/various-enums.json

@@ -94,6 +94,23 @@
         "state",
         "alternate"
       ]
+    },
+    "JankNames": {
+      "anyOf": [
+        {
+          "title": "Animation Specification",
+          "type": "string"
+        },
+        {
+          "title": "Animation Specification",
+          "type": "object",
+          "maxProperties": 1,
+          "minProperties": 1,
+          "additionalProperties": {
+            "type": "string"
+          }
+        }
+      ]
     }
   }
 }

typify/tests/schemas/various-enums.rs

@@ -55,6 +55,17 @@ impl Default for AlternativeEnum {
     }
 }
 #[derive(Clone, Debug, Deserialize, Serialize)]
+#[serde(deny_unknown_fields)]
+pub struct AnimationSpecification {
+    #[serde(flatten)]
+    pub extra: std::collections::HashMap<String, String>,
+}
+impl From<&AnimationSpecification> for AnimationSpecification {
+    fn from(value: &AnimationSpecification) -> Self {
+        value.clone()
+    }
+}
+#[derive(Clone, Debug, Deserialize, Serialize)]
 pub struct DiskAttachment {
     pub alternate: AlternativeEnum,
     pub state: DiskAttachmentState,
@@ -236,6 +247,17 @@ impl ToString for Ipv6Net {
     }
 }
 #[derive(Clone, Debug, Deserialize, Serialize)]
+#[serde(untagged)]
+pub enum JankNames {
+    Variant0(String),
+    Variant1(AnimationSpecification),
+}
+impl From<&JankNames> for JankNames {
+    fn from(value: &JankNames) -> Self {
+        value.clone()
+    }
+}
+#[derive(Clone, Debug, Deserialize, Serialize)]
 pub struct NullStringEnumWithUnknownFormat(pub Option<NullStringEnumWithUnknownFormatInner>);
 impl std::ops::Deref for NullStringEnumWithUnknownFormat {
     type Target = Option<NullStringEnumWithUnknownFormatInner>;