Repository: oxidecomputer/typify
Pull request #236 (2023-03-23 05:30:28)

add TypeSpace::add_root_schema() and general cleanup


Patch:
diff --git a/cargo-typify/src/lib.rs b/cargo-typify/src/lib.rs
index 1007e94..56f23bc 100644
--- a/cargo-typify/src/lib.rs
+++ b/cargo-typify/src/lib.rs
@@ -1,8 +1,9 @@
+// Copyright 2023 Oxide Computer Company
+
 use std::path::PathBuf;
 
 use clap::{ArgGroup, Args};
 use color_eyre::eyre::{Context, Result};
-use schemars::schema::Schema;
 use typify::{TypeSpace, TypeSpaceSettings};
 
 /// A CLI for the `typify` crate that converts JSON Schema files to Rust code.
@@ -75,21 +76,8 @@ pub fn convert(args: &CliArgs) -> Result<String> {
 
     let mut type_space = TypeSpace::new(settings);
     type_space
-        .add_ref_types(schema.definitions)
-        .wrap_err("Could not add ref types from the 'definitions' field in the JSON Schema")?;
-
-    let base_type = &schema.schema;
-
-    // Only convert the top-level type if it has a name
-    if let Some(base_title) = &(|| base_type.metadata.as_ref()?.title.as_ref())() {
-        let base_title = base_title.to_string();
-
-        type_space
-            .add_type(&Schema::Object(schema.schema))
-            .wrap_err_with(|| {
-                format!("Could not add the top level type `{base_title}` to the type space")
-            })?;
-    }
+        .add_root_schema(schema)
+        .wrap_err("Schema conversion failed")?;
 
     let intro = "#![allow(clippy::redundant_closure_call)]
 #![allow(clippy::needless_lifetimes)]
@@ -99,7 +87,7 @@ pub fn convert(args: &CliArgs) -> Result<String> {
 use serde::{Deserialize, Serialize};
 ";
 
-    let contents = format!("{intro}\n{}", type_space.to_stream().to_string());
+    let contents = format!("{intro}\n{}", type_space.to_stream());
 
     let contents = rustfmt_wrapper::rustfmt(contents).wrap_err("Failed to format Rust code")?;
 
diff --git a/example-build/build.rs b/example-build/build.rs
index 71af1b9..2145297 100644
--- a/example-build/build.rs
+++ b/example-build/build.rs
@@ -1,6 +1,7 @@
+// Copyright 2023 Oxide Computer Company
+
 use std::{env, fs, path::Path};
 
-use schemars::schema::Schema;
 use typify::{TypeSpace, TypeSpaceSettings};
 
 fn main() {
@@ -8,12 +9,7 @@ fn main() {
     let schema = serde_json::from_str::<schemars::schema::RootSchema>(&content).unwrap();
 
     let mut type_space = TypeSpace::new(TypeSpaceSettings::default().with_struct_builder(true));
-    type_space.add_ref_types(schema.definitions).unwrap();
-    let base_type = &schema.schema;
-    // Only convert the top-level type if it has a name
-    if (|| base_type.metadata.as_ref()?.title.as_ref())().is_some() {
-        let _ = type_space.add_type(&Schema::Object(schema.schema)).unwrap();
-    }
+    type_space.add_root_schema(schema).unwrap();
 
     let contents = format!(
         "{}\n{}",
diff --git a/typify-impl/src/convert.rs b/typify-impl/src/convert.rs
index 32bbbbc..9315201 100644
--- a/typify-impl/src/convert.rs
+++ b/typify-impl/src/convert.rs
@@ -1425,11 +1425,11 @@ impl TypeSpace {
         }
     }
 
-    pub(crate) fn convert_option<'a, 'b>(
+    pub(crate) fn convert_option<'a>(
         &mut self,
         type_name: Name,
         metadata: &'a Option<Box<Metadata>>,
-        schema: &'b Schema,
+        schema: &'_ Schema,
     ) -> Result<(TypeEntry, &'a Option<Box<Metadata>>)> {
         let (ty, _) = self.convert_schema(type_name, schema)?;
         let ty = self.type_to_option(ty);
diff --git a/typify-impl/src/defaults.rs b/typify-impl/src/defaults.rs
index 332ab9f..4b16213 100644
--- a/typify-impl/src/defaults.rs
+++ b/typify-impl/src/defaults.rs
@@ -341,7 +341,9 @@ pub(crate) fn validate_default_for_external_enum(
         Some(DefaultKind::Specific)
     } else {
         let map = default.as_object()?;
-        (map.len() == 1).then(|| ())?;
+        if map.len() != 1 {
+            return None;
+        }
 
         let (name, value) = map.iter().next()?;
 
@@ -462,7 +464,9 @@ fn validate_default_tuple(
     default: &serde_json::Value,
 ) -> Option<DefaultKind> {
     let arr = default.as_array()?;
-    (arr.len() == types.len()).then_some(())?;
+    if arr.len() != types.len() {
+        return None;
+    }
 
     types
         .iter()
diff --git a/typify-impl/src/lib.rs b/typify-impl/src/lib.rs
index 2b86f21..7ba7aaa 100644
--- a/typify-impl/src/lib.rs
+++ b/typify-impl/src/lib.rs
@@ -6,8 +6,8 @@ use conversions::SchemaCache;
 use log::info;
 use output::OutputSpace;
 use proc_macro2::TokenStream;
-use quote::{quote, ToTokens};
-use schemars::schema::{Metadata, Schema};
+use quote::ToTokens;
+use schemars::schema::{Metadata, RootSchema, Schema};
 use thiserror::Error;
 use type_entry::{
     StructPropertyState, TypeEntry, TypeEntryDetails, TypeEntryNative, TypeEntryNewtype,
@@ -644,6 +644,20 @@ impl TypeSpace {
         Ok(type_id)
     }
 
+    /// Add all the types contained within a RootSchema including any
+    /// referenced types and the top-level type (if there is one and it has a
+    /// title).
+    pub fn add_root_schema(&mut self, schema: RootSchema) -> Result<Option<TypeId>> {
+        self.add_ref_types(schema.definitions)?;
+
+        // Only convert the top-level type if it has a name
+        if (|| schema.schema.metadata.as_ref()?.title.as_ref())().is_some() {
+            self.add_type(&Schema::Object(schema.schema)).map(Some)
+        } else {
+            Ok(None)
+        }
+    }
+
     /// Get a type given its ID.
     pub fn get_type(&self, type_id: &TypeId) -> Result<Type> {
         let type_entry = self.id_to_entry.get(type_id).ok_or(Error::InvalidTypeId)?;
@@ -678,19 +692,6 @@ impl TypeSpace {
         })
     }
 
-    /// Common code, shared by types.
-    pub fn common_code(&self) -> TokenStream {
-        if self.defaults.is_empty() {
-            quote! {}
-        } else {
-            let fns = self.defaults.iter().map(TokenStream::from);
-            quote! {
-                mod defaults {
-                    #(#fns)*
-                }
-            }
-        }
-    }
     /// All code for processed types.
     pub fn to_stream(&self) -> TokenStream {
         let mut output = OutputSpace::default();
diff --git a/typify-impl/src/structs.rs b/typify-impl/src/structs.rs
index 4c5b795..f7c1690 100644
--- a/typify-impl/src/structs.rs
+++ b/typify-impl/src/structs.rs
@@ -352,9 +352,9 @@ impl TypeSpace {
 
     /// This handles the case where an allOf is used to denote constraints.
     /// Currently we just look for a referenced type, which may be "closed"
-    /// (i.e. no unknown types permitted) and an explicit type. The latter must
-    /// be a subset of the former and compatible from the perspective of JSON
-    /// Schema validation (that is to say, it must be "open" or must fully
+    /// (i.e. no unknown fields permitted) and an explicit type. The latter
+    /// must be a subset of the former and compatible from the perspective of
+    /// JSON Schema validation (that is to say, it must be "open" or must fully
     /// cover the named type).
     ///
     /// ```text
@@ -391,24 +391,21 @@ impl TypeSpace {
 
         // We required exactly one named subschema and at least one unnamed
         // subschema.
-        if unnamed.len() != 1 && !named.is_empty() {
+        if named.len() != 1 || unnamed.is_empty() {
             return None;
         }
 
         let (named_schema, _) = named.first()?;
-
         let (_, _, validation) = get_object(Name::Unknown, named_schema, &self.definitions)?;
 
-        if validation.additional_properties.as_ref().map(Box::as_ref) != Some(&Schema::Bool(false))
+        // We need all unnamed schemas to be a subset of the named schema.
+        if !unnamed
+            .into_iter()
+            .all(|constraint_schema| is_obj_subset(validation, constraint_schema))
         {
             return None;
         }
 
-        unnamed
-            .into_iter()
-            .all(|constraint_schema| is_obj_subset(validation, constraint_schema))
-            .then_some(())?;
-
         let (type_entry, _) = self.convert_schema(type_name, named_schema).ok()?;
         Some(type_entry)
     }
diff --git a/typify-impl/src/value.rs b/typify-impl/src/value.rs
index d0ee141..5148321 100644
--- a/typify-impl/src/value.rs
+++ b/typify-impl/src/value.rs
@@ -138,7 +138,9 @@ impl TypeEntry {
                 quote! { #v }
             }
             TypeEntryDetails::Integer(type_name) | TypeEntryDetails::Float(type_name) => {
-                value.is_number().then(|| ())?;
+                if !value.is_number() {
+                    return None;
+                }
                 let val = match proc_macro2::Literal::from_str(&format!("{}_{}", value, type_name))
                 {
                     Ok(v) => v,
@@ -174,7 +176,9 @@ fn value_for_external_enum(
         Some(quote! { #scope #type_ident::#var_ident })
     } else {
         let map = value.as_object()?;
-        (map.len() == 1).then(|| ())?;
+        if map.len() != 1 {
+            return None;
+        }
 
         let (name, var_value) = map.iter().next()?;
 
@@ -328,7 +332,9 @@ fn value_for_tuple(
     scope: &TokenStream,
 ) -> Option<Vec<TokenStream>> {
     let arr = value.as_array()?;
-    (arr.len() == types.len()).then(|| ())?;
+    if arr.len() != types.len() {
+        return None;
+    }
     types
         .iter()
         .zip(arr)
diff --git a/typify-macro/src/lib.rs b/typify-macro/src/lib.rs
index e552539..d36c060 100644
--- a/typify-macro/src/lib.rs
+++ b/typify-macro/src/lib.rs
@@ -4,7 +4,6 @@ use std::{collections::HashMap, path::Path};
 
 use proc_macro::TokenStream;
 use quote::{quote, ToTokens};
-use schemars::schema::Schema;
 use serde::Deserialize;
 use serde_tokenstream::ParseWrapper;
 use syn::LitStr;
@@ -143,20 +142,12 @@ fn do_import_types(item: TokenStream) -> Result<TokenStream, syn::Error> {
 
     let mut type_space = TypeSpace::new(&settings);
     type_space
-        .add_ref_types(root_schema.definitions)
+        .add_root_schema(root_schema)
         .map_err(|e| into_syn_err(e, schema.span()))?;
-    let base_type = &root_schema.schema;
-    // Only convert the top-level type if it has a name
-    if (|| base_type.metadata.as_ref()?.title.as_ref())().is_some() {
-        let _ = type_space
-            .add_type(&Schema::Object(root_schema.schema))
-            .map_err(|e| into_syn_err(e, schema.span()))?;
-    }
 
-    let types = type_space.to_stream();
     let path_str = path.to_string_lossy();
     let output = quote! {
-        #types
+        #type_space
 
         // Force a rebuild when the given file is modified.
         const _: &str = include_str!(#path_str);


Test patch:
diff --git a/typify-impl/tests/test_github.rs b/typify-impl/tests/test_github.rs
index fda3e36..528f0b2 100644
--- a/typify-impl/tests/test_github.rs
+++ b/typify-impl/tests/test_github.rs
@@ -1,8 +1,8 @@
-// Copyright 2022 Oxide Computer Company
+// Copyright 2023 Oxide Computer Company
 
 use std::{fs::File, io::BufReader, path::Path};
 
-use schemars::schema::{RootSchema, Schema};
+use schemars::schema::RootSchema;
 use typify_impl::{TypeSpace, TypeSpaceImpl, TypeSpaceSettings};
 
 #[test]
@@ -16,8 +16,7 @@ fn test_github() {
     let mut schema: RootSchema = serde_json::from_reader(reader).unwrap();
     schema.schema.metadata().title = Some("Everything".to_string());
 
-    type_space.add_ref_types(schema.definitions).unwrap();
-    type_space.add_type(&Schema::Object(schema.schema)).unwrap();
+    type_space.add_root_schema(schema).unwrap();
 
     let file = type_space.to_stream();
 
@@ -27,7 +26,7 @@ fn test_github() {
 }
 
 #[test]
-fn test_other() {
+fn test_vega() {
     let mut settings = TypeSpaceSettings::default();
     let raw_schema = serde_json::json! {
         {
@@ -70,8 +69,7 @@ fn test_other() {
     let mut schema: RootSchema = serde_json::from_reader(reader).unwrap();
     schema.schema.metadata().title = Some("Everything".to_string());
 
-    type_space.add_ref_types(schema.definitions).unwrap();
-    type_space.add_type(&Schema::Object(schema.schema)).unwrap();
+    type_space.add_root_schema(schema).unwrap();
 
     let file = type_space.to_stream();
 
diff --git a/typify-test/build.rs b/typify-test/build.rs
index 1933770..88ef00d 100644
--- a/typify-test/build.rs
+++ b/typify-test/build.rs
@@ -127,17 +127,6 @@ where
 {
     fn add(type_space: &mut TypeSpace) {
         let schema = schemars::schema_for!(T);
-        type_space.add_ref_types(schema.definitions).unwrap();
-
-        let base_type = &schema.schema;
-        // Only convert the top-level type if it has a name
-        if base_type
-            .metadata
-            .as_ref()
-            .and_then(|m| m.title.as_ref())
-            .is_some()
-        {
-            let _ = type_space.add_type(&Schema::Object(schema.schema)).unwrap();
-        }
+        let _ = type_space.add_root_schema(schema).unwrap();
     }
 }
diff --git a/typify/tests/schemas.rs b/typify/tests/schemas.rs
index a93a6f1..57c0be3 100644
--- a/typify/tests/schemas.rs
+++ b/typify/tests/schemas.rs
@@ -5,7 +5,7 @@ use std::{error::Error, fs::File, io::BufReader};
 use expectorate::assert_contents;
 use glob::glob;
 use quote::quote;
-use schemars::schema::{RootSchema, Schema};
+use schemars::schema::RootSchema;
 use serde_json::json;
 use typify::{TypeSpace, TypeSpacePatch, TypeSpaceSettings};
 use typify_impl::TypeSpaceImpl;
@@ -25,7 +25,7 @@ fn validate_schema(path: std::path::PathBuf) -> Result<(), Box<dyn Error>> {
     let mut out_path = path.clone();
     out_path.set_extension("rs");
 
-    let file = File::open(path.clone())?;
+    let file = File::open(path)?;
     let reader = BufReader::new(file);
 
     // Read the JSON contents of the file as an instance of `User`.
@@ -58,21 +58,7 @@ fn validate_schema(path: std::path::PathBuf) -> Result<(), Box<dyn Error>> {
                 [TypeSpaceImpl::Display].into_iter(),
             ),
     );
-    type_space.add_ref_types(root_schema.definitions)?;
-
-    let base_type = &root_schema.schema;
-
-    // Only convert the top-level type if it has a name
-    if base_type
-        .metadata
-        .as_ref()
-        .and_then(|m| m.title.as_ref())
-        .is_some()
-    {
-        let _ = type_space
-            .add_type(&Schema::Object(root_schema.schema))
-            .unwrap();
-    }
+    type_space.add_root_schema(root_schema)?;
 
     // Make a file with the generated code.
     let code = quote! {
@@ -84,7 +70,7 @@ fn validate_schema(path: std::path::PathBuf) -> Result<(), Box<dyn Error>> {
 
         fn main() {}
     };
-    let text = rustfmt_wrapper::rustfmt(code).unwrap();
+    let text = rustfmt_wrapper::rustfmt(code)?;
     assert_contents(out_path, &text);
 
     Ok(())