From 710b79e20109c9460a5bbc643dd98839ebbb3434 Mon Sep 17 00:00:00 2001 From: rooooooooob Date: Fri, 28 Jun 2024 02:13:51 +0900 Subject: [PATCH] Add @doc comment DSL (#238) This allows us to specify doc-comments for auto-generated types to avoid users having to manually put in comments afterwards which would be overwritten every re-gen. There is support for type-level (after the definition), field-level and variant-level (for group/type-choices). --- docs/docs/comment_dsl.mdx | 50 ++++++++++++++++++++++++++++++++ src/comment_ast.rs | 37 +++++++++++++++++++++++- src/generation.rs | 61 +++++++++++++++++++++++++++++---------- src/intermediate.rs | 18 ++++++++++-- src/parsing.rs | 39 ++++++++++++++++++++----- tests/core/input.cddl | 12 ++++++++ tests/core/tests.rs | 15 ++++++++++ 7 files changed, 206 insertions(+), 26 deletions(-) diff --git a/docs/docs/comment_dsl.mdx b/docs/docs/comment_dsl.mdx index ac90aea..994b8aa 100644 --- a/docs/docs/comment_dsl.mdx +++ b/docs/docs/comment_dsl.mdx @@ -161,6 +161,56 @@ Note that as this is at the field-level it must handle the tag as well as the `u For more examples see `tests/custom_serialization` (used in the `core` and `core_no_wasm` tests) and `tests/custom_serialization_preserve` (used in the `preserve-encodings` test). +## @doc + +This can be placed at field-level, struct-level or variant-level to specify a comment to be placed as a rust doc-comment. + +```cddl +docs = [ + foo: text, ; @doc this is a field-level comment + bar: uint, ; @doc bar is a u64 +] ; @doc struct documentation here + +docs_groupchoice = [ + ; @name first @doc comment-about-first + 0, uint // + ; @doc comments about second @name second + text +] ; @doc type-level comment +``` + +Will generate: +```rust +/// struct documentation here +#[derive(Clone, Debug)] +pub struct Docs { + /// this is a field-level comment + pub foo: String, + /// bar is a u64 + pub bar: u64, +} + +impl Docs { + /// * `foo` - this is a field-level comment + /// * `bar` - bar is a u64 + pub fn new(foo: String, bar: u64) -> Self { + Self { foo, bar } + } +} + +/// type-level comment +#[derive(Clone, Debug)] +pub enum DocsGroupchoice { + /// comment-about-first + First(u64), + /// comments about second + Second(String), +} +``` + +Due to the comment dsl parsing this doc comment cannot contain the character `@`. + + ## _CDDL_CODEGEN_EXTERN_TYPE_ While not as a comment, this allows you to compose in hand-written structs into a cddl spec. diff --git a/src/comment_ast.rs b/src/comment_ast.rs index 9efbaf9..d9756b3 100644 --- a/src/comment_ast.rs +++ b/src/comment_ast.rs @@ -15,6 +15,7 @@ pub struct RuleMetadata { pub custom_json: bool, pub custom_serialize: Option, pub custom_deserialize: Option, + pub comment: Option, } pub fn merge_metadata(r1: &RuleMetadata, r2: &RuleMetadata) -> RuleMetadata { @@ -53,6 +54,13 @@ pub fn merge_metadata(r1: &RuleMetadata, r2: &RuleMetadata) -> RuleMetadata { (val @ Some(_), _) => val.cloned(), (_, val) => val.cloned(), }, + comment: match (r1.comment.as_ref(), r2.comment.as_ref()) { + (Some(val1), Some(val2)) => { + panic!("Key \"comment\" specified twice: {:?} {:?}", val1, val2) + } + (val @ Some(_), _) => val.cloned(), + (_, val) => val.cloned(), + }, }; merged.verify(); merged @@ -66,6 +74,7 @@ enum ParseResult { CustomJson, CustomSerialize(String), CustomDeserialize(String), + Comment(String), } impl RuleMetadata { @@ -120,6 +129,14 @@ impl RuleMetadata { } } } + ParseResult::Comment(comment) => match base.comment.as_ref() { + Some(old) => { + panic!("Key \"comment\" specified twice: {:?} {:?}", old, comment) + } + None => { + base.comment = Some(comment.to_string()); + } + }, } } base.verify(); @@ -188,6 +205,13 @@ fn tag_custom_deserialize(input: &str) -> IResult<&str, ParseResult> { )) } +fn tag_comment(input: &str) -> IResult<&str, ParseResult> { + let (input, _) = tag("@doc")(input)?; + let (input, comment) = take_while1(|c| c != '@')(input)?; + + Ok((input, ParseResult::Comment(comment.trim().to_string()))) +} + fn whitespace_then_tag(input: &str) -> IResult<&str, ParseResult> { let (input, _) = take_while(char::is_whitespace)(input)?; let (input, result) = alt(( @@ -198,6 +222,7 @@ fn whitespace_then_tag(input: &str) -> IResult<&str, ParseResult> { tag_custom_json, tag_custom_serialize, tag_custom_deserialize, + tag_comment, ))(input)?; Ok((input, result)) @@ -242,6 +267,7 @@ fn parse_comment_name() { custom_json: false, custom_serialize: None, custom_deserialize: None, + comment: None, } )) ); @@ -261,6 +287,7 @@ fn parse_comment_newtype() { custom_json: false, custom_serialize: None, custom_deserialize: None, + comment: None, } )) ); @@ -280,6 +307,7 @@ fn parse_comment_newtype_and_name() { custom_json: false, custom_serialize: None, custom_deserialize: None, + comment: None, } )) ); @@ -299,6 +327,7 @@ fn parse_comment_newtype_and_name_and_used_as_key() { custom_json: false, custom_serialize: None, custom_deserialize: None, + comment: None, } )) ); @@ -318,6 +347,7 @@ fn parse_comment_used_as_key() { custom_json: false, custom_serialize: None, custom_deserialize: None, + comment: None, } )) ); @@ -337,6 +367,7 @@ fn parse_comment_newtype_and_name_inverse() { custom_json: false, custom_serialize: None, custom_deserialize: None, + comment: None, } )) ); @@ -356,6 +387,7 @@ fn parse_comment_name_noalias() { custom_json: false, custom_serialize: None, custom_deserialize: None, + comment: None, } )) ); @@ -375,6 +407,7 @@ fn parse_comment_newtype_and_custom_json() { custom_json: true, custom_serialize: None, custom_deserialize: None, + comment: None, } )) ); @@ -400,6 +433,7 @@ fn parse_comment_custom_serialize_deserialize() { custom_json: false, custom_serialize: Some("foo".to_string()), custom_deserialize: Some("bar".to_string()), + comment: None, } )) ); @@ -409,7 +443,7 @@ fn parse_comment_custom_serialize_deserialize() { #[test] fn parse_comment_all_except_no_alias() { assert_eq!( - rule_metadata("@newtype @name baz @custom_serialize foo @custom_deserialize bar @used_as_key @custom_json"), + rule_metadata("@newtype @name baz @custom_serialize foo @custom_deserialize bar @used_as_key @custom_json @doc this is a doc comment"), Ok(( "", RuleMetadata { @@ -420,6 +454,7 @@ fn parse_comment_all_except_no_alias() { custom_json: true, custom_serialize: Some("foo".to_string()), custom_deserialize: Some("bar".to_string()), + comment: Some("this is a doc comment".to_string()), } )) ); diff --git a/src/generation.rs b/src/generation.rs index f08219a..3f71e47 100644 --- a/src/generation.rs +++ b/src/generation.rs @@ -5162,6 +5162,7 @@ fn codegen_struct( } wasm_new.vis("pub"); let mut wasm_new_args = Vec::new(); + let mut wasm_new_comments = Vec::new(); for field in &record.fields { // Fixed values don't need constructors or getters or fields in the rust code if !field.rust_type.is_fixed_value() { @@ -5249,6 +5250,9 @@ fn codegen_struct( .from_wasm_boundary_clone(types, &field.name, false) .into_iter(), )); + if let Some(comment) = &field.rule_metadata.comment { + wasm_new_comments.push(format!("* `{}` - {}", field.name, comment)); + } // do we want setters here later for mandatory types covered by new? // getter let mut getter = codegen::Function::new(&field.name); @@ -5278,6 +5282,12 @@ fn codegen_struct( wasm_new_args.join(", ") )); } + if !wasm_new_comments.is_empty() { + wasm_new.doc(wasm_new_comments.join("\n")); + } + if let Some(doc) = config.doc.as_ref() { + wrapper.s.doc(doc); + } wrapper.s_impl.push_fn(wasm_new); wrapper.push(gen_scope, types); } @@ -5287,6 +5297,9 @@ fn codegen_struct( // Struct (fields) + constructor let (mut native_struct, mut native_impl) = create_base_rust_struct(types, name, false, cli); native_struct.vis("pub"); + if let Some(doc) = config.doc.as_ref() { + native_struct.doc(doc); + } let mut native_new = codegen::Function::new("new"); let (ctor_ret, ctor_before) = if new_can_fail { ("Result", "Ok(Self") @@ -5298,6 +5311,7 @@ fn codegen_struct( if new_can_fail { native_new_block.after(")"); } + let mut native_new_comments = Vec::new(); // for clippy we generate a Default impl if new has no args let mut new_arg_count = 0; for field in &record.fields { @@ -5313,37 +5327,35 @@ fn codegen_struct( } // Fixed values only exist in (de)serialization code (outside of preserve-encodings=true) if !field.rust_type.is_fixed_value() { - if let Some(default_value) = &field.rust_type.config.default { - // field - native_struct.field( - &format!("pub {}", field.name), - field.rust_type.for_rust_member(types, false, cli), - ); + let mut codegen_field = if let Some(default_value) = &field.rust_type.config.default { // new native_new_block.line(format!( "{}: {},", field.name, default_value.to_primitive_str_assign() )); + // field + codegen::Field::new( + &format!("pub {}", field.name), + field.rust_type.for_rust_member(types, false, cli), + ) } else if field.optional { + // new + native_new_block.line(format!("{}: None,", field.name)); // field - native_struct.field( + codegen::Field::new( &format!("pub {}", field.name), format!( "Option<{}>", field.rust_type.for_rust_member(types, false, cli) ), - ); - // new - native_new_block.line(format!("{}: None,", field.name)); + ) } else { - // field - native_struct.field( - &format!("pub {}", field.name), - field.rust_type.for_rust_member(types, false, cli), - ); // new native_new.arg(&field.name, field.rust_type.for_rust_move(types, cli)); + if let Some(comment) = &field.rule_metadata.comment { + native_new_comments.push(format!("* `{}` - {}", field.name, comment)); + } new_arg_count += 1; native_new_block.line(format!("{},", field.name)); if let Some(bounds) = field.rust_type.config.bounds.as_ref() { @@ -5363,9 +5375,21 @@ fn codegen_struct( } } } + // field + codegen::Field::new( + &format!("pub {}", field.name), + field.rust_type.for_rust_member(types, false, cli), + ) + }; + if let Some(comment) = &field.rule_metadata.comment { + codegen_field.doc(comment); } + native_struct.push_field(codegen_field); } } + if !native_new_comments.is_empty() { + native_new.doc(native_new_comments.join("\n")); + } let len_encoding_var = if cli.preserve_encodings { let encoding_name = RustIdent::new(CDDLIdent::new(format!("{name}Encoding"))); native_struct.field( @@ -6850,6 +6874,9 @@ fn generate_enum( // rust enum containing the data let mut e = codegen::Enum::new(name.to_string()); e.vis("pub"); + if let Some(doc) = config.doc.as_ref() { + e.doc(doc); + } let mut e_impl = codegen::Impl::new(name.to_string()); // instead of using create_serialize_impl() and having the length encoded there, we want to make it easier // to offer definite length encoding even if we're mixing plain group members and non-plain group members (or mixed length plain ones) @@ -6960,6 +6987,10 @@ fn generate_enum( } } } + if let Some(doc) = &variant.doc { + // we must repurpose annotations since there is no doc support on enum variants + v.annotation(format!("/// {doc}")); + } e.push_variant(v); // new (particularly useful if we have encoding variables) let mut new_func = codegen::Function::new(&format!("new_{variant_var_name}")); diff --git a/src/intermediate.rs b/src/intermediate.rs index 9654bb4..d348e5a 100644 --- a/src/intermediate.rs +++ b/src/intermediate.rs @@ -2121,22 +2121,34 @@ pub struct EnumVariant { pub name: VariantIdent, pub data: EnumVariantData, pub serialize_as_embedded_group: bool, + pub doc: Option, } impl EnumVariant { - pub fn new(name: VariantIdent, rust_type: RustType, serialize_as_embedded_group: bool) -> Self { + pub fn new( + name: VariantIdent, + rust_type: RustType, + serialize_as_embedded_group: bool, + doc: Option, + ) -> Self { Self { name, data: EnumVariantData::RustType(rust_type), serialize_as_embedded_group, + doc, } } - pub fn new_embedded(name: VariantIdent, embedded_record: RustRecord) -> Self { + pub fn new_embedded( + name: VariantIdent, + embedded_record: RustRecord, + doc: Option, + ) -> Self { Self { name, data: EnumVariantData::Inlined(embedded_record), serialize_as_embedded_group: false, + doc, } } @@ -2295,6 +2307,7 @@ pub struct RustStructConfig { pub custom_json: bool, pub custom_serialize: Option, pub custom_deserialize: Option, + pub doc: Option, } impl From> for RustStructConfig { @@ -2304,6 +2317,7 @@ impl From> for RustStructConfig { custom_json: rule_metadata.custom_json, custom_serialize: rule_metadata.custom_serialize.clone(), custom_deserialize: rule_metadata.custom_deserialize.clone(), + doc: rule_metadata.comment.clone(), }, None => Self::default(), } diff --git a/src/parsing.rs b/src/parsing.rs index 948b047..aba1efc 100644 --- a/src/parsing.rs +++ b/src/parsing.rs @@ -779,14 +779,20 @@ pub fn create_variants_from_type_choices( .iter() .map(|choice| { let rust_type = rust_type_from_type1(types, parent_visitor, &choice.type1, cli); - let base_name = match RuleMetadata::from(choice.type1.comments_after_type.as_ref()) { + let rule_metadata = RuleMetadata::from(choice.type1.comments_after_type.as_ref()); + let base_name = match &rule_metadata { RuleMetadata { name: Some(name), .. - } => convert_to_camel_case(&name), + } => convert_to_camel_case(name), _ => rust_type.for_variant().to_string(), }; let variant_name = append_number_if_duplicate(&mut variant_names_used, base_name); - EnumVariant::new(VariantIdent::new_custom(variant_name), rust_type, false) + EnumVariant::new( + VariantIdent::new_custom(variant_name), + rust_type, + false, + rule_metadata.comment.clone(), + ) }) .collect() } @@ -1594,7 +1600,12 @@ pub fn parse_group( }); let variant_ident = VariantIdent::new_custom(convert_to_camel_case(&ident_name)); - EnumVariant::new(variant_ident, ty, serialize_as_embedded) + EnumVariant::new( + variant_ident, + ty, + serialize_as_embedded, + rule_metadata.comment.clone(), + ) // None => { // // TODO: Weird case, group choice with only one fixed-value field. // // What should we do here? In the future we could make this a @@ -1632,15 +1643,27 @@ pub fn parse_group( RustStructType::Record(record) => record, _ => unreachable!(), }; - EnumVariant::new_embedded(name, embedded_record) + EnumVariant::new_embedded( + name, + embedded_record, + rule_metadata.comment.clone(), + ) } else { - EnumVariant::new(name, variant_ident.into(), true) + EnumVariant::new( + name, + variant_ident.into(), + true, + rule_metadata.comment.clone(), + ) } } }) .collect(); - let rule_metadata = RuleMetadata::from( - get_comment_after(parent_visitor, &CDDLType::from(group), None).as_ref(), + let rule_metadata = merge_metadata( + &RuleMetadata::from( + get_comment_after(parent_visitor, &CDDLType::from(group), None).as_ref(), + ), + parent_rule_metadata, ); types.register_rust_struct( parent_visitor, diff --git a/tests/core/input.cddl b/tests/core/input.cddl index 3ffbe2d..1c8cbd8 100644 --- a/tests/core/input.cddl +++ b/tests/core/input.cddl @@ -265,3 +265,15 @@ struct_with_custom_serialization = [ tagged1: #6.9(custom_bytes), tagged2: #6.9(uint), ; @custom_serialize write_tagged_uint_str @custom_deserialize read_tagged_uint_str ] + +docs = [ + foo: text, ; @doc this is a field-level comment + bar: uint, ; @doc bar is a u64 +] ; @doc struct documentation here + +docs_groupchoice = [ + ; @name first @doc comment-about-first + 0, uint // + ; @doc comments about second @name second + text +] ; @doc type-level comment \ No newline at end of file diff --git a/tests/core/tests.rs b/tests/core/tests.rs index 7273fe0..444ade0 100644 --- a/tests/core/tests.rs +++ b/tests/core/tests.rs @@ -562,4 +562,19 @@ mod tests { let from_bytes = WrapperList::from_cbor_bytes(&bytes).unwrap(); deser_test(&from_bytes); } + + #[test] + fn docs() { + use std::str::FromStr; + // reading the file is the only way to test for comments being generated + let lib_rs_with_tests = std::fs::read_to_string(std::path::PathBuf::from_str("src").unwrap().join("lib.rs")).unwrap(); + // lib.rs includes this very test (and thus those strings we're searching for) so we need to strip that part + let lib_rs = &lib_rs_with_tests[..lib_rs_with_tests.find("#[cfg(test)]").unwrap()]; + assert!(lib_rs.contains("this is a field-level comment")); + assert!(lib_rs.contains("bar is a u64")); + assert!(lib_rs.contains("struct documentation here")); + assert!(lib_rs.contains("comment-about-first")); + assert!(lib_rs.contains("comments about second")); + assert!(lib_rs.contains("type-level comment")); + } }