Sort order as "type-then-name" in generated schema's SDL (#1237, #1134)

- rename `RootNode::as_schema_language()` method as `RootNode::as_sdl()`
- rename `RootNode::as_parser_document()` method as `RootNode::as_document()`
- merge `graphql-parser` and `schema-language` Cargo features

Co-authored-by: Michael Groble <[email protected]>

.github/workflows/ci.yml

@@ -107,7 +107,6 @@ jobs:
           - { feature: chrono-clock, crate: juniper }
           - { feature: chrono-tz, crate: juniper }
           - { feature: expose-test-schema, crate: juniper }
-          - { feature: graphql-parser, crate: juniper }
           - { feature: rust_decimal, crate: juniper }
           - { feature: schema-language, crate: juniper }
           - { feature: time, crate: juniper }

book/src/schema/schemas_and_mutations.md

@@ -89,17 +89,17 @@ fn main() {
         EmptySubscription::<()>::new(),
     );
 
-    // Convert the Rust schema into the GraphQL Schema Language.
-    let result = schema.as_schema_language();
+    // Convert the Rust schema into the GraphQL Schema Definition Language.
+    let result = schema.as_sdl();
 
     let expected = "\
-type Query {
-  hello: String!
-}
-
 schema {
   query: Query
 }
+
+type Query {
+  hello: String!
+}
 ";
 #   #[cfg(not(target_os = "windows"))]
     assert_eq!(result, expected);

juniper/CHANGELOG.md

@@ -55,6 +55,9 @@ All user visible changes to `juniper` crate will be documented in this file. Thi
 - Made `LookAheadMethods::children()` method to return slice instead of `Vec`. ([#1200])
 - Abstracted `Spanning::start` and `Spanning::end` fields into separate struct `Span`. ([#1207], [#1208])
 - Added `Span` to `Arguments` and `LookAheadArguments`. ([#1206], [#1209])
+- Removed `graphql-parser-integration` and `graphql-parser` [Cargo feature]s by merging them into `schema-language` [Cargo feature]. ([#1237])
+- Renamed `RootNode::as_schema_language()` method as `RootNode::as_sdl()`. ([#1237]) 
+- Renamed `RootNode::as_parser_document()` method as `RootNode::as_document()`. ([#1237]) 
 
 ### Added
 
@@ -89,6 +92,7 @@ All user visible changes to `juniper` crate will be documented in this file. Thi
 - Incorrect input value coercion with defaults. ([#1080], [#1073])
 - Incorrect error when explicit `null` provided for `null`able list input parameter. ([#1086], [#1085])
 - Stack overflow on nested GraphQL fragments. ([CVE-2022-31173])
+- Unstable definitions order in schema generated by `RootNode::as_sdl()`. ([#1237], [#1134])
 
 [#103]: /../../issues/103
 [#113]: /../../issues/113
@@ -132,6 +136,7 @@ All user visible changes to `juniper` crate will be documented in this file. Thi
 [#1086]: /../../pull/1086
 [#1118]: /../../issues/1118
 [#1119]: /../../pull/1119
+[#1134]: /../../issues/1134
 [#1138]: /../../issues/1138
 [#1145]: /../../pull/1145
 [#1147]: /../../pull/1147
@@ -149,6 +154,7 @@ All user visible changes to `juniper` crate will be documented in this file. Thi
 [#1227]: /../../pull/1227
 [#1228]: /../../pull/1228
 [#1235]: /../../pull/1235
+[#1237]: /../../pull/1237
 [ba1ed85b]: /../../commit/ba1ed85b3c3dd77fbae7baf6bc4e693321a94083
 [CVE-2022-31173]: /../../security/advisories/GHSA-4rx6-g5vg-5f3j
 

juniper/Cargo.toml

@@ -39,10 +39,9 @@ chrono = ["dep:chrono"]
 chrono-clock = ["chrono", "chrono/clock"]
 chrono-tz = ["dep:chrono-tz", "dep:regex"]
 expose-test-schema = ["dep:anyhow", "dep:serde_json"]
-graphql-parser = ["dep:graphql-parser", "dep:void"]
 js = ["chrono?/wasmbind", "time?/wasm-bindgen", "uuid?/js"]
 rust_decimal = ["dep:rust_decimal"]
-schema-language = ["graphql-parser"]
+schema-language = ["dep:graphql-parser", "dep:void"]
 time = ["dep:time"]
 url = ["dep:url"]
 uuid = ["dep:uuid"]

juniper/src/schema/model.rs

@@ -1,7 +1,7 @@
 use std::{borrow::Cow, fmt};
 
 use fnv::FnvHashMap;
-#[cfg(feature = "graphql-parser")]
+#[cfg(feature = "schema-language")]
 use graphql_parser::schema::Document;
 
 use crate::{
@@ -13,9 +13,6 @@ use crate::{
     GraphQLEnum,
 };
 
-#[cfg(feature = "graphql-parser")]
-use crate::schema::translate::{graphql_parser::GraphQLParserTranslator, SchemaTranslator};
-
 /// Root query node of a schema
 ///
 /// This brings the mutation, subscription and query types together,
@@ -221,17 +218,40 @@ where
     }
 
     #[cfg(feature = "schema-language")]
-    /// The schema definition as a `String` in the
-    /// [GraphQL Schema Language](https://graphql.org/learn/schema/#type-language)
-    /// format.
-    pub fn as_schema_language(&self) -> String {
-        self.as_parser_document().to_string()
+    /// Returns this [`RootNode`] as a [`String`] containing the schema in [SDL (schema definition language)].
+    ///
+    /// # Sorted
+    ///
+    /// The order of the generated definitions is stable and is sorted in the "type-then-name" manner.
+    ///
+    /// If another sorting order is required, then the [`as_document()`] method should be used, which allows to sort the
+    /// returned [`Document`] in the desired manner and then to convert it [`to_string()`].
+    ///
+    /// [`as_document()`]: RootNode::as_document
+    /// [`to_string()`]: ToString::to_string
+    /// [0]: https://graphql.org/learn/schema#type-language
+    #[must_use]
+    pub fn as_sdl(&self) -> String {
+        use crate::schema::translate::graphql_parser::sort_schema_document;
+
+        let mut doc = self.as_document();
+        sort_schema_document(&mut doc);
+        doc.to_string()
     }
 
-    #[cfg(feature = "graphql-parser")]
-    /// The schema definition as a [`graphql_parser`](https://crates.io/crates/graphql-parser)
-    /// [`Document`](https://docs.rs/graphql-parser/latest/graphql_parser/schema/struct.Document.html).
-    pub fn as_parser_document(&'a self) -> Document<'a, &'a str> {
+    #[cfg(feature = "schema-language")]
+    /// Returns this [`RootNode`] as a [`graphql_parser`]'s [`Document`].
+    ///
+    /// # Unsorted
+    ///
+    /// The order of the generated definitions in the returned [`Document`] is NOT stable and may change without any
+    /// real schema changes.
+    #[must_use]
+    pub fn as_document(&'a self) -> Document<'a, &'a str> {
+        use crate::schema::translate::{
+            graphql_parser::GraphQLParserTranslator, SchemaTranslator as _,
+        };
+
         GraphQLParserTranslator::translate_schema(&self.schema)
     }
 }
@@ -666,119 +686,141 @@ impl<'a, S> fmt::Display for TypeType<'a, S> {
 }
 
 #[cfg(test)]
-mod test {
-
-    #[cfg(feature = "graphql-parser")]
-    mod graphql_parser_integration {
+mod root_node_test {
+    #[cfg(feature = "schema-language")]
+    mod as_document {
         use crate::{graphql_object, EmptyMutation, EmptySubscription, RootNode};
 
-        #[test]
-        fn graphql_parser_doc() {
-            struct Query;
-            #[graphql_object]
-            impl Query {
-                fn blah() -> bool {
-                    true
-                }
+        struct Query;
+
+        #[graphql_object]
+        impl Query {
+            fn blah() -> bool {
+                true
             }
+        }
+
+        #[test]
+        fn generates_correct_document() {
             let schema = RootNode::new(
                 Query,
                 EmptyMutation::<()>::new(),
                 EmptySubscription::<()>::new(),
             );
             let ast = graphql_parser::parse_schema::<&str>(
+                //language=GraphQL
                 r#"
                 type Query {
-                  blah: Boolean!
+                    blah: Boolean!
                 }
 
                 schema {
                   query: Query
                 }
-            "#,
+                "#,
             )
             .unwrap();
-            assert_eq!(ast.to_string(), schema.as_parser_document().to_string());
+
+            assert_eq!(ast.to_string(), schema.as_document().to_string());
         }
     }
 
     #[cfg(feature = "schema-language")]
-    mod schema_language {
+    mod as_sdl {
         use crate::{
             graphql_object, EmptyMutation, EmptySubscription, GraphQLEnum, GraphQLInputObject,
             GraphQLObject, GraphQLUnion, RootNode,
         };
 
-        #[test]
-        fn schema_language() {
-            #[derive(GraphQLObject, Default)]
-            struct Cake {
-                fresh: bool,
-            }
-            #[derive(GraphQLObject, Default)]
-            struct IceCream {
-                cold: bool,
+        #[derive(GraphQLObject, Default)]
+        struct Cake {
+            fresh: bool,
+        }
+
+        #[derive(GraphQLObject, Default)]
+        struct IceCream {
+            cold: bool,
+        }
+
+        #[derive(GraphQLUnion)]
+        enum GlutenFree {
+            Cake(Cake),
+            IceCream(IceCream),
+        }
+
+        #[derive(GraphQLEnum)]
+        enum Fruit {
+            Apple,
+            Orange,
+        }
+
+        #[derive(GraphQLInputObject)]
+        struct Coordinate {
+            latitude: f64,
+            longitude: f64,
+        }
+
+        struct Query;
+
+        #[graphql_object]
+        impl Query {
+            fn blah() -> bool {
+                true
             }
-            #[derive(GraphQLUnion)]
-            enum GlutenFree {
-                Cake(Cake),
-                IceCream(IceCream),
+
+            /// This is whatever's description.
+            fn whatever() -> String {
+                "foo".into()
             }
-            #[derive(GraphQLEnum)]
-            enum Fruit {
-                Apple,
-                Orange,
+
+            fn arr(stuff: Vec<Coordinate>) -> Option<&'static str> {
+                (!stuff.is_empty()).then_some("stuff")
             }
-            #[derive(GraphQLInputObject)]
-            struct Coordinate {
-                latitude: f64,
-                longitude: f64,
+
+            fn fruit() -> Fruit {
+                Fruit::Apple
             }
-            struct Query;
-            #[graphql_object]
-            impl Query {
-                fn blah() -> bool {
-                    true
-                }
-                /// This is whatever's description.
-                fn whatever() -> String {
-                    "foo".into()
-                }
-                fn arr(stuff: Vec<Coordinate>) -> Option<&'static str> {
-                    (!stuff.is_empty()).then_some("stuff")
-                }
-                fn fruit() -> Fruit {
-                    Fruit::Apple
-                }
-                fn gluten_free(flavor: String) -> GlutenFree {
-                    if flavor == "savory" {
-                        GlutenFree::Cake(Cake::default())
-                    } else {
-                        GlutenFree::IceCream(IceCream::default())
-                    }
-                }
-                #[deprecated]
-                fn old() -> i32 {
-                    42
-                }
-                #[deprecated(note = "This field is deprecated, use another.")]
-                fn really_old() -> f64 {
-                    42.0
+
+            fn gluten_free(flavor: String) -> GlutenFree {
+                if flavor == "savory" {
+                    GlutenFree::Cake(Cake::default())
+                } else {
+                    GlutenFree::IceCream(IceCream::default())
                 }
             }
 
-            let schema = RootNode::new(
+            #[deprecated]
+            fn old() -> i32 {
+                42
+            }
+
+            #[deprecated(note = "This field is deprecated, use another.")]
+            fn really_old() -> f64 {
+                42.0
+            }
+        }
+
+        #[test]
+        fn generates_correct_sdl() {
+            let actual = RootNode::new(
                 Query,
                 EmptyMutation::<()>::new(),
                 EmptySubscription::<()>::new(),
             );
-            let ast = graphql_parser::parse_schema::<&str>(
+            let expected = graphql_parser::parse_schema::<&str>(
+                //language=GraphQL
                 r#"
-                union GlutenFree = Cake | IceCream
+                schema {
+                  query: Query
+                }
                 enum Fruit {
                     APPLE
                     ORANGE
                 }
+                input Coordinate {
+                    latitude: Float!
+                    longitude: Float!
+                }
                 type Cake {
                     fresh: Boolean!
                 }
@@ -795,17 +837,12 @@ mod test {
                   old: Int! @deprecated
                   reallyOld: Float! @deprecated(reason: "This field is deprecated, use another.")
                 }
-                input Coordinate {
-                    latitude: Float!
-                    longitude: Float!
-                }
-                schema {
-                  query: Query
-                }
-            "#,
+                union GlutenFree = Cake | IceCream
+                "#,
             )
             .unwrap();
-            assert_eq!(ast.to_string(), schema.as_schema_language());
+
+            assert_eq!(actual.as_sdl(), expected.to_string());
         }
     }
 }