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 <mike@groble.me>

.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.
+    // Convert the Rust schema into the GraphQL Schema Definition Language.
-    let result = schema.as_schema_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
+    /// Returns this [`RootNode`] as a [`String`] containing the schema in [SDL (schema definition language)].
-    /// [GraphQL Schema Language](https://graphql.org/learn/schema/#type-language)
+    ///
-    /// format.
+    /// # Sorted
-    pub fn as_schema_language(&self) -> String {
+    ///
-        self.as_parser_document().to_string()
+    /// 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")]
+    #[cfg(feature = "schema-language")]
-    /// The schema definition as a [`graphql_parser`](https://crates.io/crates/graphql-parser)
+    /// Returns this [`RootNode`] as a [`graphql_parser`]'s [`Document`].
-    /// [`Document`](https://docs.rs/graphql-parser/latest/graphql_parser/schema/struct.Document.html).
+    ///
-    pub fn as_parser_document(&'a self) -> Document<'a, &'a str> {
+    /// # 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 {
+mod root_node_test {
-
+    #[cfg(feature = "schema-language")]
-    #[cfg(feature = "graphql-parser")]
+    mod as_document {
-    mod graphql_parser_integration {
         use crate::{graphql_object, EmptyMutation, EmptySubscription, RootNode};
 
-        #[test]
+        struct Query;
-        fn graphql_parser_doc() {
+
-            struct Query;
+        #[graphql_object]
-            #[graphql_object]
+        impl Query {
-            impl Query {
+            fn blah() -> bool {
-                fn blah() -> bool {
+                true
-                    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]
+        #[derive(GraphQLObject, Default)]
-        fn schema_language() {
+        struct Cake {
-            #[derive(GraphQLObject, Default)]
+            fresh: bool,
-            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(GraphQLObject, Default)]
+
-            struct IceCream {
+            /// This is whatever's description.
-                cold: bool,
+            fn whatever() -> String {
+                "foo".into()
             }
-            #[derive(GraphQLUnion)]
+
-            enum GlutenFree {
+            fn arr(stuff: Vec<Coordinate>) -> Option<&'static str> {
-                Cake(Cake),
+                (!stuff.is_empty()).then_some("stuff")
-                IceCream(IceCream),
             }
-            #[derive(GraphQLEnum)]
+
-            enum Fruit {
+            fn fruit() -> Fruit {
-                Apple,
+                Fruit::Apple
-                Orange,
             }
-            #[derive(GraphQLInputObject)]
+
-            struct Coordinate {
+            fn gluten_free(flavor: String) -> GlutenFree {
-                latitude: f64,
+                if flavor == "savory" {
-                longitude: f64,
+                    GlutenFree::Cake(Cake::default())
-            }
+                } else {
-            struct Query;
+                    GlutenFree::IceCream(IceCream::default())
-            #[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
                 }
             }
 
-            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 {
+                union GlutenFree = Cake | IceCream
-                    latitude: Float!
+                "#,
-                    longitude: Float!
-                }
-                schema {
-                  query: Query
-                }
-            "#,
             )
             .unwrap();
-            assert_eq!(ast.to_string(), schema.as_schema_language());
+
+            assert_eq!(actual.as_sdl(), expected.to_string());
         }
     }
 }

juniper/src/schema/translate/graphql_parser.rs

@@ -307,3 +307,113 @@ where
         vec![]
     }
 }
+
+/// Sorts the provided [`Document`] in the "type-then-name" manner.
+pub(crate) fn sort_schema_document<'a, T: Text<'a>>(document: &mut Document<'a, T>) {
+    document.definitions.sort_by(move |a, b| {
+        let type_cmp = sort_value::by_type(a).cmp(&sort_value::by_type(b));
+        let name_cmp = sort_value::by_is_directive(a)
+            .cmp(&sort_value::by_is_directive(b))
+            .then(sort_value::by_name(a).cmp(&sort_value::by_name(b)))
+            .then(sort_value::by_directive(a).cmp(&sort_value::by_directive(b)));
+        type_cmp.then(name_cmp)
+    })
+}
+
+/// Evaluation of a [`Definition`] weights for sorting.
+mod sort_value {
+    use graphql_parser::schema::{Definition, Text, TypeDefinition, TypeExtension};
+
+    /// Returns a [`Definition`] sorting weight by its type.
+    pub(super) fn by_type<'a, T>(definition: &Definition<'a, T>) -> u8
+    where
+        T: Text<'a>,
+    {
+        match definition {
+            Definition::SchemaDefinition(_) => 0,
+            Definition::DirectiveDefinition(_) => 1,
+            Definition::TypeDefinition(t) => match t {
+                TypeDefinition::Enum(_) => 2,
+                TypeDefinition::InputObject(_) => 4,
+                TypeDefinition::Interface(_) => 6,
+                TypeDefinition::Scalar(_) => 8,
+                TypeDefinition::Object(_) => 10,
+                TypeDefinition::Union(_) => 12,
+            },
+            Definition::TypeExtension(e) => match e {
+                TypeExtension::Enum(_) => 3,
+                TypeExtension::InputObject(_) => 5,
+                TypeExtension::Interface(_) => 7,
+                TypeExtension::Scalar(_) => 9,
+                TypeExtension::Object(_) => 11,
+                TypeExtension::Union(_) => 13,
+            },
+        }
+    }
+
+    /// Returns a [`Definition`] sorting weight by its name.
+    pub(super) fn by_name<'b, 'a, T>(definition: &'b Definition<'a, T>) -> Option<&'b T::Value>
+    where
+        T: Text<'a>,
+    {
+        match definition {
+            Definition::SchemaDefinition(_) => None,
+            Definition::DirectiveDefinition(d) => Some(&d.name),
+            Definition::TypeDefinition(t) => match t {
+                TypeDefinition::Enum(d) => Some(&d.name),
+                TypeDefinition::InputObject(d) => Some(&d.name),
+                TypeDefinition::Interface(d) => Some(&d.name),
+                TypeDefinition::Scalar(d) => Some(&d.name),
+                TypeDefinition::Object(d) => Some(&d.name),
+                TypeDefinition::Union(d) => Some(&d.name),
+            },
+            Definition::TypeExtension(e) => match e {
+                TypeExtension::Enum(d) => Some(&d.name),
+                TypeExtension::InputObject(d) => Some(&d.name),
+                TypeExtension::Interface(d) => Some(&d.name),
+                TypeExtension::Scalar(d) => Some(&d.name),
+                TypeExtension::Object(d) => Some(&d.name),
+                TypeExtension::Union(d) => Some(&d.name),
+            },
+        }
+    }
+
+    /// Returns a [`Definition`] sorting weight by its directive.
+    pub(super) fn by_directive<'b, 'a, T>(definition: &'b Definition<'a, T>) -> Option<&'b T::Value>
+    where
+        T: Text<'a>,
+    {
+        match definition {
+            Definition::SchemaDefinition(_) => None,
+            Definition::DirectiveDefinition(_) => None,
+            Definition::TypeDefinition(t) => match t {
+                TypeDefinition::Enum(d) => d.directives.first().map(|d| &d.name),
+                TypeDefinition::InputObject(d) => d.directives.first().map(|d| &d.name),
+                TypeDefinition::Interface(d) => d.directives.first().map(|d| &d.name),
+                TypeDefinition::Scalar(d) => d.directives.first().map(|d| &d.name),
+                TypeDefinition::Object(d) => d.directives.first().map(|d| &d.name),
+                TypeDefinition::Union(d) => d.directives.first().map(|d| &d.name),
+            },
+            Definition::TypeExtension(e) => match e {
+                TypeExtension::Enum(d) => d.directives.first().map(|d| &d.name),
+                TypeExtension::InputObject(d) => d.directives.first().map(|d| &d.name),
+                TypeExtension::Interface(d) => d.directives.first().map(|d| &d.name),
+                TypeExtension::Scalar(d) => d.directives.first().map(|d| &d.name),
+                TypeExtension::Object(d) => d.directives.first().map(|d| &d.name),
+                TypeExtension::Union(d) => d.directives.first().map(|d| &d.name),
+            },
+        }
+    }
+
+    /// Returns a [`Definition`] sorting weight by whether it represents a directive.
+    pub(super) fn by_is_directive<'a, T>(definition: &Definition<'a, T>) -> u8
+    where
+        T: Text<'a>,
+    {
+        match definition {
+            Definition::SchemaDefinition(_) => 0,
+            Definition::DirectiveDefinition(_) => 1,
+            _ => 2,
+        }
+    }
+}

juniper/src/schema/translate/mod.rs

@@ -4,5 +4,5 @@ pub trait SchemaTranslator<'a, T> {
     fn translate_schema<S: 'a + ScalarValue>(s: &'a SchemaType<S>) -> T;
 }
 
-#[cfg(feature = "graphql-parser")]
+#[cfg(feature = "schema-language")]
 pub mod graphql_parser;

juniper/src/tests/fixtures/starwars/schema_language.rs

@@ -1,6 +1,6 @@
 #![allow(missing_docs)]
 
-/// The schema as a static/hardcoded GraphQL Schema Language.
+/// The schema as a static/hardcoded GraphQL SDL (schema definition language).
 pub const STATIC_GRAPHQL_SCHEMA_DEFINITION: &str = include_str!("starwars.graphql");
 
 #[cfg(test)]
@@ -24,7 +24,7 @@ mod tests {
             EmptySubscription::<Database>::new(),
         );
 
-        dbg!("{}", schema.as_schema_language());
+        //dbg!("{}", schema.as_sdl());
 
         // `include_str()` keeps line endings. `git` will sadly by default
         // convert them, making this test fail without runtime tweaks on
@@ -32,11 +32,10 @@ mod tests {
         //
         // See https://github.com/rust-lang/rust/pull/63681.
         #[cfg(windows)]
-        let expected = &STATIC_GRAPHQL_SCHEMA_DEFINITION.replace("\r\n", "\n");
+        let expected = STATIC_GRAPHQL_SCHEMA_DEFINITION.replace("\r\n", "\n");
-
         #[cfg(not(windows))]
         let expected = STATIC_GRAPHQL_SCHEMA_DEFINITION;
 
-        assert_eq!(expected, &schema.as_schema_language());
+        assert_eq!(schema.as_sdl(), expected);
     }
 }

juniper/src/tests/fixtures/starwars/starwars.graphql

@@ -1,22 +1,5 @@
-"A mechanical creature in the Star Wars universe."
+schema {
-type Droid implements Character {
+  query: Query
-  "The id of the droid"
-  id: String!
-  "The name of the droid"
-  name: String
-  "The friends of the droid"
-  friends: [Character!]!
-  "Which movies they appear in"
-  appearsIn: [Episode!]!
-  "The primary function of the droid"
-  primaryFunction: String
-}
-
-"The root query object of the schema"
-type Query {
-  human("id of the human" id: String!): Human
-  droid("id of the droid" id: String!): Droid
-  hero("If omitted, returns the hero of the whole saga. If provided, returns the hero of that particular episode" episode: Episode): Character
 }
 
 enum Episode {
@@ -37,6 +20,20 @@ interface Character {
   appearsIn: [Episode!]!
 }
 
+"A mechanical creature in the Star Wars universe."
+type Droid implements Character {
+  "The id of the droid"
+  id: String!
+  "The name of the droid"
+  name: String
+  "The friends of the droid"
+  friends: [Character!]!
+  "Which movies they appear in"
+  appearsIn: [Episode!]!
+  "The primary function of the droid"
+  primaryFunction: String
+}
+
 "A humanoid creature in the Star Wars universe."
 type Human implements Character {
   "The id of the human"
@@ -51,6 +48,9 @@ type Human implements Character {
   homePlanet: String
 }
 
-schema {
+"The root query object of the schema"
-  query: Query
+type Query {
+  human("id of the human" id: String!): Human
+  droid("id of the droid" id: String!): Droid
+  hero("If omitted, returns the hero of the whole saga. If provided, returns the hero of that particular episode" episode: Episode): Character
 }