From 1fd5c10327284954093fa5fd5727e56cf9a49498 Mon Sep 17 00:00:00 2001 From: Christian Legnitto <christian@legnitto.com> Date: Thu, 7 Jun 2018 17:45:42 -0700 Subject: [PATCH] Add support for using doc comments as descriptions Fixes https://github.com/graphql-rust/juniper/issues/194. --- changelog/master.md | 12 +- juniper_codegen/src/derive_enum.rs | 6 + juniper_codegen/src/derive_input_object.rs | 6 + juniper_codegen/src/derive_object.rs | 6 + juniper_codegen/src/util.rs | 64 +++++++- juniper_tests/Cargo.toml | 1 + juniper_tests/src/codegen/derive_enum.rs | 49 ++++++ .../src/codegen/derive_input_object.rs | 48 ++++++ juniper_tests/src/codegen/derive_object.rs | 149 ++++++++++++++++++ juniper_tests/src/lib.rs | 2 + 10 files changed, 337 insertions(+), 6 deletions(-) diff --git a/changelog/master.md b/changelog/master.md index d64d5c74..5dbb8138 100644 --- a/changelog/master.md +++ b/changelog/master.md @@ -1,6 +1,7 @@ # [master] yyyy-mm-dd ## Changes + * Changed serialization of `NaiveDate` when using the optional `chronos` support. **Note:** while this is not a Rust breaking change, if you relied on the serialization format (perhaps by storing serialized data in a database or making asumptions in your client code written in another language) it could be a breaking change for your application. @@ -8,7 +9,7 @@ [#151](https://github.com/graphql-rust/juniper/pull/151) * The `GraphQLObject`, `GraphQLInputObject`, and `GraphQLEnum` custom derives will reject - invalid [names](http://facebook.github.io/graphql/October2016/#Name) at compile time. + invalid [names](http://facebook.github.io/graphql/October2016/#Name) at compile time. [#170](https://github.com/graphql-rust/juniper/pull/170) @@ -19,4 +20,11 @@ fractional part could not be decoded (because they are represented without a decimal part `.0`). - [#179](https://github.com/graphql-rust/juniper/pull/179) \ No newline at end of file + [#179](https://github.com/graphql-rust/juniper/pull/179) + +* The `GraphQLObject`, `GraphQLInputObject`, and `GraphQLEnum` custom derives + now parse doc strings and use them as descriptions. This behavior can be + overridden by using an explicit GraphQL `description` annotation such as + `#[graphql(description = "my description")]`. + + [#194](https://github.com/graphql-rust/juniper/issues/194) diff --git a/juniper_codegen/src/derive_enum.rs b/juniper_codegen/src/derive_enum.rs index 90ad866b..5fef7d4b 100644 --- a/juniper_codegen/src/derive_enum.rs +++ b/juniper_codegen/src/derive_enum.rs @@ -28,6 +28,9 @@ impl EnumAttrs { internal: false, }; + // Check doc comments for description. + res.description = get_doc_comment(&input.attrs); + // Check attributes for name and description. if let Some(items) = get_graphl_attr(&input.attrs) { for item in items { @@ -74,6 +77,9 @@ impl EnumVariantAttrs { fn from_input(variant: &Variant) -> EnumVariantAttrs { let mut res = EnumVariantAttrs::default(); + // Check doc comments for description. + res.description = get_doc_comment(&variant.attrs); + // Check attributes for name and description. if let Some(items) = get_graphl_attr(&variant.attrs) { for item in items { diff --git a/juniper_codegen/src/derive_input_object.rs b/juniper_codegen/src/derive_input_object.rs index 653a5651..f01c28b3 100644 --- a/juniper_codegen/src/derive_input_object.rs +++ b/juniper_codegen/src/derive_input_object.rs @@ -25,6 +25,9 @@ impl ObjAttrs { fn from_input(input: &DeriveInput) -> ObjAttrs { let mut res = ObjAttrs::default(); + // Check doc comments for description. + res.description = get_doc_comment(&input.attrs); + // Check attributes for name and description. if let Some(items) = get_graphl_attr(&input.attrs) { for item in items { @@ -72,6 +75,9 @@ impl ObjFieldAttrs { fn from_input(variant: &Field) -> ObjFieldAttrs { let mut res = ObjFieldAttrs::default(); + // Check doc comments for description. + res.description = get_doc_comment(&variant.attrs); + // Check attributes for name and description. if let Some(items) = get_graphl_attr(&variant.attrs) { for item in items { diff --git a/juniper_codegen/src/derive_object.rs b/juniper_codegen/src/derive_object.rs index 78417b60..98d32f75 100644 --- a/juniper_codegen/src/derive_object.rs +++ b/juniper_codegen/src/derive_object.rs @@ -19,6 +19,9 @@ impl ObjAttrs { fn from_input(input: &DeriveInput) -> ObjAttrs { let mut res = ObjAttrs::default(); + // Check doc comments for description. + res.description = get_doc_comment(&input.attrs); + // Check attributes for name and description. if let Some(items) = get_graphl_attr(&input.attrs) { for item in items { @@ -56,6 +59,9 @@ impl ObjFieldAttrs { fn from_input(variant: &Field) -> ObjFieldAttrs { let mut res = ObjFieldAttrs::default(); + // Check doc comments for description. + res.description = get_doc_comment(&variant.attrs); + // Check attributes for name and description. if let Some(items) = get_graphl_attr(&variant.attrs) { for item in items { diff --git a/juniper_codegen/src/util.rs b/juniper_codegen/src/util.rs index 784e664e..fb3986a4 100644 --- a/juniper_codegen/src/util.rs +++ b/juniper_codegen/src/util.rs @@ -1,11 +1,67 @@ use syn::{ Attribute, Meta, + MetaNameValue, NestedMeta, Lit, }; use regex::Regex; +// Gets doc comment. +pub fn get_doc_comment(attrs: &Vec<Attribute>) -> Option<String> { + if let Some(items) = get_doc_attr(attrs) { + if let Some(doc_strings) = get_doc_strings(&items) { + return Some(join_doc_strings(&doc_strings)); + } + } + None +} + +// Concatenates doc strings into one string. +fn join_doc_strings(docs: &Vec<String>) -> String { + let s: String = docs.iter() + // Convert empty comments to newlines. + .map(|x| if x == "" { "\n".to_string() } else { x.clone() }) + .collect::<Vec<String>>() + .join(" "); + // Clean up spacing on empty lines. + s.replace(" \n ", "\n") +} + +// Gets doc strings from doc comment attributes. +fn get_doc_strings(items: &Vec<MetaNameValue>) -> Option<Vec<String>> { + let mut docs = Vec::new(); + for item in items { + match item.lit { + Lit::Str(ref strlit) => { + docs.push(strlit.value().trim().to_string()); + }, + _ => panic!("doc attributes only have string literal"), + } + } + if !docs.is_empty() { + return Some(docs); + } + None +} + +// Gets doc comment attributes. +fn get_doc_attr(attrs: &Vec<Attribute>) -> Option<Vec<MetaNameValue>> { + let mut docs = Vec::new(); + for attr in attrs { + match attr.interpret_meta() { + Some(Meta::NameValue(ref nv)) if nv.ident == "doc" => { + docs.push(nv.clone()) + } + _ => {} + } + } + if !docs.is_empty() { + return Some(docs); + } + None +} + // Get the nested items of a a #[graphql(...)] attribute. pub fn get_graphl_attr(attrs: &Vec<Attribute>) -> Option<Vec<NestedMeta>> { for attr in attrs { @@ -124,12 +180,12 @@ pub fn is_valid_name(field_name: &str) -> bool { #[test] fn test_is_valid_name(){ assert_eq!(is_valid_name("yesItIs"), true); - assert_eq!(is_valid_name("NoitIsnt"), true); - assert_eq!(is_valid_name("iso6301"), true); - assert_eq!(is_valid_name("thisIsATest"), true); + assert_eq!(is_valid_name("NoitIsnt"), true); + assert_eq!(is_valid_name("iso6301"), true); + assert_eq!(is_valid_name("thisIsATest"), true); assert_eq!(is_valid_name("i6Op"), true); assert_eq!(is_valid_name("i!"), false); - assert_eq!(is_valid_name(""), false); + assert_eq!(is_valid_name(""), false); assert_eq!(is_valid_name("aTest"), true); assert_eq!(is_valid_name("__Atest90"), true); } diff --git a/juniper_tests/Cargo.toml b/juniper_tests/Cargo.toml index 73a72dbe..04441bf5 100644 --- a/juniper_tests/Cargo.toml +++ b/juniper_tests/Cargo.toml @@ -8,6 +8,7 @@ serde_json = { version = "1" } [dev-dependencies] fnv = "1.0.3" +indexmap = "1.0" [[test]] name = "integration_tests" diff --git a/juniper_tests/src/codegen/derive_enum.rs b/juniper_tests/src/codegen/derive_enum.rs index 2e1ea626..25a50ef8 100644 --- a/juniper_tests/src/codegen/derive_enum.rs +++ b/juniper_tests/src/codegen/derive_enum.rs @@ -11,6 +11,33 @@ enum SomeEnum { #[graphql(name = "FULL", description = "field descr", deprecated = "depr")] Full, } +/// Enum doc. +#[derive(GraphQLEnum)] +enum DocEnum { + /// Variant doc. + Foo, +} + +/// Doc 1. +/// Doc 2. +/// +/// Doc 4. +#[derive(GraphQLEnum, Debug, PartialEq)] +enum MultiDocEnum { + /// Variant 1. + /// Variant 2. + Foo, +} + +/// This is not used as the description. +#[derive(GraphQLEnum, Debug, PartialEq)] +#[graphql(description = "enum override")] +enum OverrideDocEnum { + /// This is not used as the description. + #[graphql(description = "variant override")] + Foo, +} + #[test] fn test_derived_enum() { // Ensure that rename works. @@ -43,3 +70,25 @@ fn test_derived_enum() { Some(SomeEnum::Full) ); } + +#[test] +fn test_doc_comment() { + let mut registry = juniper::Registry::new(FnvHashMap::default()); + let meta = DocEnum::meta(&(), &mut registry); + assert_eq!(meta.description(), Some(&"Enum doc.".to_string())); +} + +#[test] +fn test_multi_doc_comment() { + let mut registry = juniper::Registry::new(FnvHashMap::default()); + let meta = MultiDocEnum::meta(&(), &mut registry); + assert_eq!(meta.description(), Some(&"Doc 1. Doc 2.\nDoc 4.".to_string())); +} + +#[test] +fn test_doc_comment_override() { + let mut registry = juniper::Registry::new(FnvHashMap::default()); + let meta = OverrideDocEnum::meta(&(), &mut registry); + assert_eq!(meta.description(), Some(&"enum override".to_string())); + +} diff --git a/juniper_tests/src/codegen/derive_input_object.rs b/juniper_tests/src/codegen/derive_input_object.rs index d1d9e51d..e1c7bc68 100644 --- a/juniper_tests/src/codegen/derive_input_object.rs +++ b/juniper_tests/src/codegen/derive_input_object.rs @@ -13,6 +13,33 @@ struct Input { #[graphql(default)] other: Option<bool>, } +/// Object comment. +#[derive(GraphQLInputObject, Debug, PartialEq)] +struct DocComment { + /// Field comment. + regular_field: bool, +} + +/// Doc 1. +/// Doc 2. +/// +/// Doc 4. +#[derive(GraphQLInputObject, Debug, PartialEq)] +struct MultiDocComment { + /// Field 1. + /// Field 2. + regular_field: bool, +} + +/// This is not used as the description. +#[derive(GraphQLInputObject, Debug, PartialEq)] +#[graphql(description = "obj override")] +struct OverrideDocComment { + /// This is not used as the description. + #[graphql(description = "field override")] + regular_field: bool, +} + #[test] fn test_derived_input_object() { assert_eq!(Input::name(&()), Some("MyInput")); @@ -57,3 +84,24 @@ fn test_derived_input_object() { } ); } + +#[test] +fn test_doc_comment() { + let mut registry = juniper::Registry::new(FnvHashMap::default()); + let meta = DocComment::meta(&(), &mut registry); + assert_eq!(meta.description(), Some(&"Object comment.".to_string())); +} + +#[test] +fn test_multi_doc_comment() { + let mut registry = juniper::Registry::new(FnvHashMap::default()); + let meta = MultiDocComment::meta(&(), &mut registry); + assert_eq!(meta.description(), Some(&"Doc 1. Doc 2.\nDoc 4.".to_string())); +} + +#[test] +fn test_doc_comment_override() { + let mut registry = juniper::Registry::new(FnvHashMap::default()); + let meta = OverrideDocComment::meta(&(), &mut registry); + assert_eq!(meta.description(), Some(&"obj override".to_string())); +} diff --git a/juniper_tests/src/codegen/derive_object.rs b/juniper_tests/src/codegen/derive_object.rs index 06340d14..016d3c1c 100644 --- a/juniper_tests/src/codegen/derive_object.rs +++ b/juniper_tests/src/codegen/derive_object.rs @@ -1,5 +1,7 @@ #[cfg(test)] use fnv::FnvHashMap; +#[cfg(test)] +use indexmap::IndexMap; #[cfg(test)] use juniper::{self, execute, EmptyMutation, GraphQLType, RootNode, Value, Variables}; @@ -18,6 +20,33 @@ struct Nested { struct Query; +/// Object comment. +#[derive(GraphQLObject, Debug, PartialEq)] +struct DocComment { + /// Field comment. + regular_field: bool, +} + +/// Doc 1. +/// Doc 2. +/// +/// Doc 4. +#[derive(GraphQLObject, Debug, PartialEq)] +struct MultiDocComment { + /// Field 1. + /// Field 2. + regular_field: bool, +} + +/// This is not used as the description. +#[derive(GraphQLObject, Debug, PartialEq)] +#[graphql(description = "obj override")] +struct OverrideDocComment { + /// This is not used as the description. + #[graphql(description = "field override")] + regular_field: bool, +} + graphql_object!(Query: () |&self| { field obj() -> Obj { Obj{ @@ -34,8 +63,68 @@ graphql_object!(Query: () |&self| { } } } + + field doc() -> DocComment { + DocComment{ + regular_field: true, + } + } + + field multi_doc() -> MultiDocComment { + MultiDocComment{ + regular_field: true, + } + } + + field override_doc() -> OverrideDocComment { + OverrideDocComment{ + regular_field: true, + } + } }); +#[test] +fn test_doc_comment() { + let mut registry = juniper::Registry::new(FnvHashMap::default()); + let meta = DocComment::meta(&(), &mut registry); + assert_eq!(meta.description(), Some(&"Object comment.".to_string())); + + check_descriptions( + "DocComment", + &Value::string("Object comment."), + "regularField", + &Value::string("Field comment."), + ); +} + +#[test] +fn test_multi_doc_comment() { + let mut registry = juniper::Registry::new(FnvHashMap::default()); + let meta = MultiDocComment::meta(&(), &mut registry); + assert_eq!(meta.description(), Some(&"Doc 1. Doc 2.\nDoc 4.".to_string())); + + check_descriptions( + "MultiDocComment", + &Value::string("Doc 1. Doc 2.\nDoc 4."), + "regularField", + &Value::string("Field 1. Field 2."), + ); +} + +#[test] +fn test_doc_comment_override() { + let mut registry = juniper::Registry::new(FnvHashMap::default()); + let meta = OverrideDocComment::meta(&(), &mut registry); + assert_eq!(meta.description(), Some(&"obj override".to_string())); + + check_descriptions( + "OverrideDocComment", + &Value::string("obj override"), + "regularField", + &Value::string("field override"), + ); +} + #[test] fn test_derived_object() { assert_eq!(Obj::name(&()), Some("MyObj")); @@ -124,3 +213,63 @@ fn test_derived_object_nested() { )) ); } + +#[cfg(test)] +fn check_descriptions(object_name: &str, object_description: &Value, field_name: &str, field_value: &Value ) { + let doc = format!(r#" + {{ + __type(name: "{}") {{ + name, + description, + fields {{ + name + description + }} + }} + }} + "#, object_name); + run_type_info_query(&doc, |(type_info, values)| { + assert_eq!(type_info.get("name"), Some(&Value::string(object_name))); + assert_eq!(type_info.get("description"), Some(object_description)); + assert!( + values.contains(&Value::object( + vec![ + ("name", Value::string(field_name)), + ("description", field_value.clone()), + ].into_iter() + .collect(), + )) + ); + }); +} + +#[cfg(test)] +fn run_type_info_query<F>(doc: &str, f: F) +where + F: Fn((&IndexMap<String, Value>, &Vec<Value>)) -> (), +{ + let schema = RootNode::new(Query, EmptyMutation::<()>::new()); + + let (result, errs) = + execute(doc, None, &schema, &Variables::new(), &()).expect("Execution failed"); + + assert_eq!(errs, []); + + println!("Result: {:?}", result); + + let type_info = result + .as_object_value() + .expect("Result is not an object") + .get("__type") + .expect("__type field missing") + .as_object_value() + .expect("__type field not an object value"); + + let fields = type_info + .get("fields") + .expect("fields field missing") + .as_list_value() + .expect("fields not a list"); + + f((type_info, fields)); +} diff --git a/juniper_tests/src/lib.rs b/juniper_tests/src/lib.rs index 1be7ebe0..a392cfca 100644 --- a/juniper_tests/src/lib.rs +++ b/juniper_tests/src/lib.rs @@ -6,5 +6,7 @@ extern crate serde_json; #[cfg(test)] extern crate fnv; +#[cfg(test)] +extern crate indexmap; mod codegen;