Merge remote-tracking branch 'origin/master' into feature/add_custom_use_and_derive

Author: alissapajer

.github/workflows/publish.yml

@@ -4,6 +4,7 @@ on:
 
 jobs:
   release_binary:
+    permissions: write-all
     runs-on: ${{ matrix.runs_on }}
     strategy:
       matrix:

crates/core/src/codegen/mod.rs

@@ -9,7 +9,7 @@ use crate::target::{
 use ast::{Ast, SchemaAst};
 use jtd::Schema;
 use namespace::Namespace;
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, BTreeSet};
 use std::fs::File;
 use std::io::Write;
 use std::path::Path;
@@ -36,6 +36,7 @@ struct CodeGenerator<'a, T> {
     out_dir: &'a Path,
     strategy: Strategy,
     definition_names: BTreeMap<String, String>,
+    recursive_definitions: BTreeSet<String>,
 }
 
 struct FileData<T> {
@@ -50,6 +51,7 @@ impl<'a, T: Target> CodeGenerator<'a, T> {
             out_dir,
             strategy: target.strategy(),
             definition_names: BTreeMap::new(),
+            recursive_definitions: BTreeSet::new(),
         }
     }
 
@@ -68,6 +70,18 @@ impl<'a, T: Target> CodeGenerator<'a, T> {
             self.definition_names.insert(name.clone(), ast_name);
         }
 
+        // Detect recursive definitions, as some target language need to handle
+        // them specifically (e.g. Rust).
+        // Note that a type is *not* considered to be recursive it it contains
+        // instances of itself only through "elements" or "values"
+        // (the underlying container is considered to break the recursion).
+        for (name, ast) in &schema_ast.definitions {
+            let mut visited = vec![];
+            if find_recursion(name, ast, &schema_ast.definitions, &mut visited) {
+                self.recursive_definitions.insert(name.clone());
+            }
+        }
+
         // If the target is using FilePerType partitioning, then this state
         // won't actually be used at all. If it's using SingleFile partitioning,
         // then this is the only file state that will be used.
@@ -120,8 +134,17 @@ impl<'a, T: Target> CodeGenerator<'a, T> {
             // Ref nodes are a special sort of "expr-like" node, where we
             // already know what the name of the expression is; it's the name of
             // the definition.
-            Ast::Ref { definition, .. } => self.definition_names[&definition].clone(),
-
+            // Note however that recursive definition may need some special
+            // treatment by the target.
+            Ast::Ref { metadata, definition } => {
+                let sub_expr = self.definition_names[&definition].clone();
+                if self.recursive_definitions.iter().any(|i| i == &definition) {
+                    self.target
+                        .expr(&mut file_data.state, metadata, Expr::RecursiveRef(sub_expr))
+                } else {
+                    sub_expr
+                }
+            }
             // The remaining "expr-like" node types just build up strings and
             // possibly alter the per-file state (usually in order to add
             // "imports" to the file).
@@ -479,3 +502,33 @@ impl<'a, T: Target> CodeGenerator<'a, T> {
         Ok(())
     }
 }
+
+fn find_recursion(name: &str, ast: &Ast, definitions: &BTreeMap<String, Ast>, visited: &mut Vec<String>) -> bool {
+    match ast {
+        Ast::Ref { definition, .. } => {
+            if definition == name {
+                true
+            } else if visited.iter().any(|i| i == &name) {
+                false
+            } else if let Some(ast2) = definitions.get(definition) {
+                visited.push(definition.clone());
+                find_recursion(name, &ast2, definitions, visited)
+            } else {
+                false
+            }
+        }
+        Ast::NullableOf { type_, .. } => {
+            find_recursion(name, type_, definitions, visited)
+        }
+        Ast::Struct { fields, .. } => {
+            fields.iter()
+                  .any(|f| find_recursion(name, &f.type_, definitions, visited))
+        }
+        Ast::Discriminator { variants, .. } => {
+            variants.iter()
+                    .flat_map(|v| v.fields.iter())
+                    .any(|f| find_recursion(name, &f.type_, definitions, visited))
+                }
+        _ => false,
+    }
+}

crates/core/src/target/mod.rs

@@ -87,6 +87,7 @@ pub enum Expr {
     ArrayOf(String),
     DictOf(String),
     NullableOf(String),
+    RecursiveRef(String),
 }
 
 #[derive(Debug)]

crates/target_csharp_system_text/src/lib.rs

@@ -125,6 +125,7 @@ impl jtd_codegen::target::Target for Target {
                 format!("IDictionary<string, {}>", sub_expr)
             }
             target::Expr::NullableOf(sub_expr) => format!("{}?", sub_expr),
+            target::Expr::RecursiveRef(sub_expr) => sub_expr,
         }
     }
 

crates/target_go/src/lib.rs

@@ -114,6 +114,7 @@ impl jtd_codegen::target::Target for Target {
             target::Expr::ArrayOf(sub_expr) => format!("[]{}", sub_expr),
             target::Expr::DictOf(sub_expr) => format!("map[string]{}", sub_expr),
             target::Expr::NullableOf(sub_expr) => format!("*{}", sub_expr),
+            target::Expr::RecursiveRef(sub_expr) => sub_expr,
         }
     }
 

crates/target_java_jackson/src/lib.rs

@@ -125,6 +125,7 @@ impl jtd_codegen::target::Target for Target {
                 format!("Map<String, {}>", sub_expr)
             }
             target::Expr::NullableOf(sub_expr) => sub_expr, // everything is already nullable
+            target::Expr::RecursiveRef(sub_expr) => sub_expr,
         }
     }
 

crates/target_python/src/lib.rs

@@ -135,6 +135,7 @@ impl jtd_codegen::target::Target for Target {
 
                 format!("Optional[{}]", sub_expr)
             }
+            target::Expr::RecursiveRef(sub_expr) => sub_expr,
         }
     }
 

crates/target_ruby/src/lib.rs

@@ -117,6 +117,7 @@ impl jtd_codegen::target::Target for Target {
                 format!("Hash[String, {}]", sub_expr)
             }
             target::Expr::NullableOf(sub_expr) => sub_expr,
+            target::Expr::RecursiveRef(sub_expr) => sub_expr,
         }
     }
 

crates/target_ruby_sig/src/lib.rs

@@ -116,6 +116,7 @@ impl jtd_codegen::target::Target for Target {
                 format!("Hash[String, {}]", sub_expr)
             }
             target::Expr::NullableOf(sub_expr) => format!("{}?", sub_expr),
+            target::Expr::RecursiveRef(sub_expr) => sub_expr,
         }
     }
 

crates/target_rust/output/custom_overrides/mod.rs

@@ -9,20 +9,20 @@ pub struct RootOverrideTypeDiscriminatorBaz {}
 #[derive(Serialize, Deserialize)]
 pub struct Root {
     #[serde(rename = "override_elements_container")]
-    pub overrideElementsContainer: Vec<String>,
+    pub override_elements_container: Vec<String>,
 
     #[serde(rename = "override_type_discriminator")]
-    pub overrideTypeDiscriminator: serde_json::Value,
+    pub override_type_discriminator: serde_json::Value,
 
     #[serde(rename = "override_type_enum")]
-    pub overrideTypeEnum: serde_json::Value,
+    pub override_type_enum: serde_json::Value,
 
     #[serde(rename = "override_type_expr")]
-    pub overrideTypeExpr: serde_json::Value,
+    pub override_type_expr: serde_json::Value,
 
     #[serde(rename = "override_type_properties")]
-    pub overrideTypeProperties: serde_json::Value,
+    pub override_type_properties: serde_json::Value,
 
     #[serde(rename = "override_values_container")]
-    pub overrideValuesContainer: HashMap<String, String>,
+    pub override_values_container: HashMap<String, String>,
 }

crates/target_rust/output/description/mod.rs

@@ -38,31 +38,31 @@ pub struct RootPropertiesWithDescription {}
 pub struct Root {
     /// A description for discriminator
     #[serde(rename = "discriminator_with_description")]
-    pub discriminatorWithDescription: RootDiscriminatorWithDescription,
+    pub discriminator_with_description: RootDiscriminatorWithDescription,
 
     /// A description for enum
     #[serde(rename = "enum_with_description")]
-    pub enumWithDescription: RootEnumWithDescription,
+    pub enum_with_description: RootEnumWithDescription,
 
     /// Whereas disregard and contempt for human rights have resulted in
     /// barbarous acts which have outraged the conscience of mankind, and the
     /// advent of a world in which human beings shall enjoy freedom of speech
     /// and belief and freedom from fear and want has been proclaimed as the
     /// highest aspiration of the common people,
     #[serde(rename = "long_description")]
-    pub longDescription: String,
+    pub long_description: String,
 
     /// A description for properties
     #[serde(rename = "properties_with_description")]
-    pub propertiesWithDescription: RootPropertiesWithDescription,
+    pub properties_with_description: RootPropertiesWithDescription,
 
     /// A description for ref
     #[serde(rename = "ref_with_description")]
-    pub refWithDescription: Baz,
+    pub ref_with_description: Baz,
 
     /// A description for string
     #[serde(rename = "string_with_description")]
-    pub stringWithDescription: String,
+    pub string_with_description: String,
 }
 
 /// A description for a definition

crates/target_rust/output/empty_and_nonascii_properties/mod.rs

@@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize};
 #[derive(Serialize, Deserialize)]
 pub struct Root {
     #[serde(rename = "")]
-    pub defaultName: String,
+    pub default_name: String,
 
     #[serde(rename = "$foo")]
     pub foo: String,
@@ -17,14 +17,14 @@ pub struct Root {
     pub foo1: String,
 
     #[serde(rename = "foo\nbar")]
-    pub fooBar: String,
+    pub foo_bar: String,
 
     #[serde(rename = "foo bar")]
-    pub fooBar0: String,
+    pub foo_bar0: String,
 
     #[serde(rename = "foo0bar")]
     pub foo0bar: String,
 
     #[serde(rename = "foo﷽bar")]
-    pub fooBar1: String,
+    pub foo_bar1: String,
 }

crates/target_rust/output/enum_collisions/mod.rs

@@ -32,5 +32,5 @@ pub struct Root {
     pub foo: RootFoo,
 
     #[serde(rename = "foo_bar")]
-    pub fooBar: RootFooBar0,
+    pub foo_bar: RootFooBar0,
 }

crates/target_rust/output/initialisms/mod.rs

@@ -20,14 +20,14 @@ pub struct Root {
     pub id: String,
 
     #[serde(rename = "nested_id_initialism")]
-    pub nestedIdInitialism: RootNestedIdInitialism,
+    pub nested_id_initialism: RootNestedIdInitialism,
 
     #[serde(rename = "utf8")]
     pub utf8: String,
 
     #[serde(rename = "word_with_embedded_id_initialism")]
-    pub wordWithEmbeddedIdInitialism: String,
+    pub word_with_embedded_id_initialism: String,
 
     #[serde(rename = "word_with_trailing_initialism_id")]
-    pub wordWithTrailingInitialismId: String,
+    pub word_with_trailing_initialism_id: String,
 }

crates/target_rust/output/nullable_references/mod.rs

@@ -5,22 +5,22 @@ use serde::{Deserialize, Serialize};
 #[derive(Serialize, Deserialize)]
 pub struct Root {
     #[serde(rename = "notnull_ref_notnull_string")]
-    pub notnullRefNotnullString: NotnullRefNotnullString,
+    pub notnull_ref_notnull_string: NotnullRefNotnullString,
 
     #[serde(rename = "notnull_ref_null_string")]
-    pub notnullRefNullString: NotnullRefNullString,
+    pub notnull_ref_null_string: NotnullRefNullString,
 
     #[serde(rename = "notnull_string")]
-    pub notnullString: NotnullString,
+    pub notnull_string: NotnullString,
 
     #[serde(rename = "null_ref_notnull_string")]
-    pub nullRefNotnullString: NullRefNotnullString,
+    pub null_ref_notnull_string: NullRefNotnullString,
 
     #[serde(rename = "null_ref_null_string")]
-    pub nullRefNullString: NullRefNullString,
+    pub null_ref_null_string: NullRefNullString,
 
     #[serde(rename = "null_string")]
-    pub nullString: NullString,
+    pub null_string: NullString,
 }
 
 pub type NotnullRefNotnullString = NotnullString;

crates/target_rust/output/type_collisions/mod.rs

@@ -26,5 +26,5 @@ pub struct Root {
     pub foo: RootFoo,
 
     #[serde(rename = "foo_bar")]
-    pub fooBar: RootFooBar0,
+    pub foo_bar: RootFooBar0,
 }

crates/target_rust/src/lib.rs

@@ -19,7 +19,7 @@ lazy_static! {
     static ref FIELD_NAMING_CONVENTION: Box<dyn inflect::Inflector + Send + Sync> =
         Box::new(inflect::KeywordAvoidingInflector::new(
             KEYWORDS.clone(),
-            inflect::TailInflector::new(inflect::Case::camel_case())
+            inflect::TailInflector::new(inflect::Case::snake_case())
         ));
     static ref ENUM_MEMBER_NAMING_CONVENTION: Box<dyn inflect::Inflector + Send + Sync> =
         Box::new(inflect::KeywordAvoidingInflector::new(
@@ -122,14 +122,16 @@ impl jtd_codegen::target::Target for Target {
                 format!("HashMap<String, {}>", sub_expr)
             }
 
-            // TODO: A Box here is necessary because otherwise a recursive data
-            // structure may fail to compile, such as in the geojson test case.
+            target::Expr::NullableOf(sub_expr) => format!("Option<{}>", sub_expr),
+            // A Box here is usually necessary for recursive data structures,
+            // such as in the geojson test case.
             //
-            // A more "proper" fix to this problem would be to have a cyclical
-            // reference detector, and insert Box<T> only if it's necessary to
-            // break a cyclic dependency. It's unclear how much of a problem
-            // this is in the real world.
-            target::Expr::NullableOf(sub_expr) => format!("Option<Box<{}>>", sub_expr),
+            // Note that this strategy is slighyly over-defensive;
+            // in a cycle of mutually recursive types,
+            // only one of the types needs to be boxed to break the cycle.
+            // In such cases, the user may want to optimize the code,
+            // overriding some of the boxings with metadata.rustType.
+            target::Expr::RecursiveRef(sub_expr) => format!("Box<{}>", sub_expr),
         }
     }
 

crates/target_typescript/src/lib.rs

@@ -92,6 +92,7 @@ impl jtd_codegen::target::Target for Target {
             target::Expr::ArrayOf(sub_expr) => format!("{}[]", sub_expr),
             target::Expr::DictOf(sub_expr) => format!("{{ [key: string]: {} }}", sub_expr),
             target::Expr::NullableOf(sub_expr) => format!("({} | null)", sub_expr),
+            target::Expr::RecursiveRef(sub_expr) => sub_expr,
         }
     }