Skip to content

Commit

Permalink
Cache result
Browse files Browse the repository at this point in the history
When searching recursively from the top, we can cache the result. If we have a deeply nested struct

A -> B -> C -> D -> Enum

and start by checking A, the other structs B, C and D can cache the result of the recursive search.

This also adds more tests.

Signed-off-by: Aaron Dewes <[email protected]>
  • Loading branch information
AaronDewes committed May 18, 2024
1 parent 936dd56 commit 991292e
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use heck::ToUpperCamelCase;
use k8s_openapi::apiextensions_apiserver::pkg::apis::apiextensions::v1::{
JSONSchemaProps, JSONSchemaPropsOrArray, JSONSchemaPropsOrBool, JSON,
};
use std::collections::{BTreeMap, HashMap};
use std::{cell::OnceCell, collections::{BTreeMap, HashMap}};

const IGNORED_KEYS: [&str; 3] = ["metadata", "apiVersion", "kind"];

Expand Down Expand Up @@ -225,6 +225,7 @@ fn analyze_enum_properties(
level,
docs: schema.description.clone(),
is_enum: true,
supports_derive_default: OnceCell::new(),
})
}

Expand Down Expand Up @@ -333,6 +334,7 @@ fn extract_container(
level,
docs: schema.description.clone(),
is_enum: false,
supports_derive_default: OnceCell::new(),
})
}

Expand Down
64 changes: 64 additions & 0 deletions src/output.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::cell::OnceCell;

use heck::{ToPascalCase, ToSnakeCase};

/// All found containers
Expand All @@ -16,6 +18,7 @@ pub struct Container {
pub docs: Option<String>,
/// Whether this container is an enum
pub is_enum: bool,
pub supports_derive_default: OnceCell<bool>,
}

/// Output member belonging to an Container
Expand Down Expand Up @@ -89,6 +92,10 @@ impl Container {
// or we can insert enum defaults
return false;
}

if let Some(can_derive) = self.supports_derive_default.get() {
return *can_derive;
}

for m in &self.members {
// If the type contains a <, it's a container type. All container types kopium uses right now (Map, Vec, Option) have a default implementation.
Expand All @@ -105,11 +112,14 @@ impl Container {
.find(|c| c.name == m.type_)
.is_some_and(|c| !c.can_derive_default(containers))
{
self.supports_derive_default.set(false).unwrap();
return false;
}
}
}

// We can't fail here, because if it's set, we always return, and set only fails it the Cell is already set
self.supports_derive_default.set(true).unwrap();
true
}
}
Expand Down Expand Up @@ -243,6 +253,8 @@ impl MapType {
// unit tests
#[cfg(test)]
mod test {
use std::cell::OnceCell;

use super::{Container, Member};
fn name_only_enum_member(name: &str) -> Member {
Member {
Expand Down Expand Up @@ -281,6 +293,7 @@ mod test {
],
docs: None,
is_enum: true,
supports_derive_default: OnceCell::new(),
};

c.rename();
Expand All @@ -306,6 +319,7 @@ mod test {
],
docs: None,
is_enum: false,
supports_derive_default: OnceCell::new(),
};
cs.rename();
assert_eq!(&cs.members[0].name, "jwks_uri");
Expand All @@ -323,13 +337,15 @@ mod test {
members: vec![],
docs: None,
is_enum: false,
supports_derive_default: OnceCell::new(),
},
Container {
name: "Enum".to_string(),
level: 1,
members: vec![],
docs: None,
is_enum: true,
supports_derive_default: OnceCell::new(),
},
Container {
name: "Nested".to_string(),
Expand All @@ -343,6 +359,7 @@ mod test {
}],
docs: None,
is_enum: false,
supports_derive_default: OnceCell::new(),
},
Container {
name: "ReferencesEnum".to_string(),
Expand All @@ -356,6 +373,7 @@ mod test {
}],
docs: None,
is_enum: false,
supports_derive_default: OnceCell::new(),
},
Container {
name: "ReferencesEnumNested".to_string(),
Expand All @@ -369,12 +387,58 @@ mod test {
}],
docs: None,
is_enum: false,
supports_derive_default: OnceCell::new(),
},
Container {
name: "ReferencesEnumOption".to_string(),
level: 1,
members: vec![Member {
name: "maybe_enum".to_string(),
type_: "Option<Enum>".to_string(),
serde_annot: vec![],
extra_annot: vec![],
docs: None,
}],
docs: None,
is_enum: false,
supports_derive_default: OnceCell::new(),
},
Container {
name: "ReferencesEnumVec".to_string(),
level: 1,
members: vec![Member {
name: "enum_list".to_string(),
type_: "Vec<Enum>".to_string(),
serde_annot: vec![],
extra_annot: vec![],
docs: None,
}],
docs: None,
is_enum: false,
supports_derive_default: OnceCell::new(),
},
Container {
name: "ReferencesEnumNestedOption".to_string(),
level: 1,
members: vec![Member {
name: "maybe_references_enum".to_string(),
type_: "Option<ReferencesEnum>".to_string(),
serde_annot: vec![],
extra_annot: vec![],
docs: None,
}],
docs: None,
is_enum: false,
supports_derive_default: OnceCell::new(),
},
];
assert!(containers[0].can_derive_default(&containers)); // Simple
assert!(!containers[1].can_derive_default(&containers)); // Enum
assert!(containers[2].can_derive_default(&containers)); // Nested
assert!(!containers[3].can_derive_default(&containers)); // ReferencesEnum
assert!(!containers[4].can_derive_default(&containers)); // ReferencesEnumNested
assert!(containers[5].can_derive_default(&containers)); // ReferencesEnumOption
assert!(containers[6].can_derive_default(&containers)); // ReferencesEnumVec
assert!(containers[7].can_derive_default(&containers)); // ReferencesEnumNestedOption
}
}

0 comments on commit 991292e

Please sign in to comment.