Cleanup code, and allow to use graphql_parser_hive_fork instead of …

…`graphql_parser` (#60)
dotansimha · Nov 12, 2024 · 46701aa · 46701aa
1 parent a6cdb8c
commit 46701aa
Show file tree

Hide file tree

Showing 38 changed files with 746 additions and 593 deletions.
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -18,7 +18,15 @@ jobs:
         uses: Swatinem/rust-cache@v1
 
       - name: Test Rust
-        run: cargo test
+        run: |
+          # features=graphql_parser_fork
+          cargo test --features graphql_parser_fork --no-default-features
+          # features=graphql_parser
+          cargo test
 
       - name: Build Rust
-        run: cargo build --release
+        run: |
+          # features=graphql_parser_fork
+          cargo build --release --features graphql_parser_fork --no-default-features
+          # features=graphql_parser
+          cargo build --release
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -11,11 +11,14 @@ documentation = "https://github.com/dotansimha/graphql-tools-rs"
 authors = ["Dotan Simha <[email protected]>"]
 
 [dependencies]
-graphql-parser = "^0.4.0"
+graphql-parser = { version = "^0.4.0", optional = true }
+graphql-parser-hive-fork = { version = "^0.5.0", optional = true }
 lazy_static = "1.4.0"
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"
 serde_with = "2.2.0"
 
-[dev-dependencies]
-graphql-parser = "0.4.0"
+[features]
+default = ["graphql_parser"]
+graphql_parser_fork = ["dep:graphql-parser-hive-fork"]
+graphql_parser = ["dep:graphql-parser"]
diff --git a/README.md b/README.md
@@ -2,8 +2,6 @@
 
 [Documentation](https://docs.rs/graphql-tools) | [Crate](https://crates.io/crates/graphql-tools) | [GitHub](https://github.com/dotansimha/graphql-tools-rs)
 
-> **Note: this crate is still under development (see roadmap below)**
-
 The [`graphql_tools` crate](https://crates.io/crates/graphql-tools) implements tooling around GraphQL for Rust libraries. Most of the tools are based on `trait`s and `struct`s implemented in [`graphql_parser` crate](https://crates.io/crates/graphql-parser).
 
 The goal of this library is to create a common layer of tools that has similar/improved APIs to [`graphql-js` reference implementation](https://github.com/graphql/graphql-js) and [`graphql-tools` from the JS/TS ecosystem](https://github.com/ardatan/graphql-tools).
@@ -25,23 +23,16 @@ Or, if you are using [`cargo-edit`](https://github.com/killercup/cargo-edit):
 cargo add graphql-tools
 ```
 
-### Roadmap and progress
-
-- [ ] Better documentation 
-- [x] AST Visitor for GraphQL schema (`graphql_parser::schema::Document`)
-- [x] AST Visitor for GraphQL operations (`graphql_parser::operation::Document`) 
-- [x] AST Visitor with TypeInfo
-- [x] AST tools (ongoing)
-- [x] `struct` extensions
-- [x] GraphQL Validation engine
-- [x] Validation rules
-- [x] GraphQL operations transformer
+By default, this crate is using the [`graphql-parser`](https://github.com/graphql-rust/graphql-parser) library for parsing. If you wish to use an alternative implementation such as [`graphql-hive/graphql-parser-hive-fork`](https://github.com/graphql-hive/graphql-parser-hive-fork), use the following `features` setup:
 
-> If you have an idea / missing feature, feel free to open an issue / start a GitHub discussion!
+```toml
+[dependencies]
+graphql-tools = { version = "...", features = "graphql_parser_fork", default-features = false }
+```
 
 #### Validation Rules
 
-> This comparison is based on `graphql-js` refernece implementation. 
+> This comparison is based on `graphql-js` refernece implementation.
 
 - [x] ExecutableDefinitions (not actually needed)
 - [x] UniqueOperationNames
@@ -68,4 +59,4 @@ cargo add graphql-tools
 - [x] ProvidedRequiredArguments
 - [x] VariablesInAllowedPosition
 - [x] OverlappingFieldsCanBeMerged
-- [ ] UniqueInputFieldNames (blocked by https://github.com/graphql-rust/graphql-parser/issues/59)
+- [ ] UniqueInputFieldNames (blocked by https://github.com/graphql-rust/graphql-parser/issues/59)
diff --git a/src/ast/collect_fields.rs b/src/ast/collect_fields.rs
@@ -47,7 +47,7 @@ fn does_fragment_condition_match<'a>(
                         return interface_type.is_implemented_by(current_selection_set_type)
                     }
                     TypeDefinition::Union(union_type) => {
-                        return union_type.has_sub_type(&current_selection_set_type.name())
+                        return union_type.has_sub_type(current_selection_set_type.name())
                     }
                     _ => return false,
                 }
@@ -70,14 +70,14 @@ fn collect_fields_inner<'a>(
 ) {
     selection_set.items.iter().for_each(|item| match item {
         Selection::Field(f) => {
-            let existing = result_arr.entry(f.name.clone()).or_insert(vec![]);
+            let existing = result_arr.entry(f.name.clone()).or_default();
             existing.push(f.clone());
         }
         Selection::InlineFragment(f) => {
             if does_fragment_condition_match(&f.type_condition, parent_type, context) {
                 collect_fields_inner(
                     &f.selection_set,
-                    &parent_type,
+                    parent_type,
                     known_fragments,
                     context,
                     result_arr,
@@ -86,22 +86,21 @@ fn collect_fields_inner<'a>(
             }
         }
         Selection::FragmentSpread(f) => {
-            if visited_fragments_names
+            if !visited_fragments_names
                 .iter()
-                .find(|name| f.fragment_name.eq(*name))
-                .is_none()
+                .any(|name| f.fragment_name.eq(name))
             {
                 visited_fragments_names.push(f.fragment_name.clone());
 
                 if let Some(fragment) = known_fragments.get(f.fragment_name.as_str()) {
                     if does_fragment_condition_match(
                         &Some(fragment.type_condition.clone()),
-                        &parent_type,
+                        parent_type,
                         context,
                     ) {
                         collect_fields_inner(
                             &fragment.selection_set,
-                            &parent_type,
+                            parent_type,
                             known_fragments,
                             context,
                             result_arr,

diff --git a/src/ast/ext.rs b/src/ast/ext.rs
@@ -57,7 +57,7 @@ impl OperationDefinitionExtension for OperationDefinition {
     fn selection_set(&self) -> &SelectionSet {
         match self {
             OperationDefinition::Query(query) => &query.selection_set,
-            OperationDefinition::SelectionSet(selection_set) => &selection_set,
+            OperationDefinition::SelectionSet(selection_set) => selection_set,
             OperationDefinition::Mutation(mutation) => &mutation.selection_set,
             OperationDefinition::Subscription(subscription) => &subscription.selection_set,
         }
@@ -156,7 +156,7 @@ impl SchemaDocumentExtension for schema::Document {
         self.schema_definition()
             .subscription
             .as_ref()
-            .and_then(|name| self.object_type_by_name(&name))
+            .and_then(|name| self.object_type_by_name(name))
     }
 
     fn object_type_by_name(&self, name: &str) -> Option<&ObjectType> {
@@ -185,7 +185,7 @@ impl SchemaDocumentExtension for schema::Document {
             self.type_by_name(sub_type_name),
             self.type_by_name(super_type_name),
         ) {
-            super_type.is_abstract_type() && self.is_possible_type(&super_type, &sub_type)
+            super_type.is_abstract_type() && self.is_possible_type(super_type, sub_type)
         } else {
             false
         }
@@ -206,7 +206,7 @@ impl SchemaDocumentExtension for schema::Document {
             TypeDefinition::Interface(interface_typedef) => {
                 let implementes_interfaces = possible_type.interfaces();
 
-                return implementes_interfaces.contains(&interface_typedef.name);
+                implementes_interfaces.contains(&interface_typedef.name)
             }
             _ => false,
         }
@@ -248,12 +248,12 @@ impl SchemaDocumentExtension for schema::Document {
         // If superType type is an abstract type, check if it is super type of maybeSubType.
         // Otherwise, the child type is not a valid subtype of the parent type.
         if let (Some(sub_type), Some(super_type)) = (
-            self.type_by_name(&sub_type.inner_type()),
-            self.type_by_name(&super_type.inner_type()),
+            self.type_by_name(sub_type.inner_type()),
+            self.type_by_name(super_type.inner_type()),
         ) {
             return super_type.is_abstract_type()
                 && (sub_type.is_interface_type() || sub_type.is_object_type())
-                && self.is_possible_type(&super_type, &sub_type);
+                && self.is_possible_type(super_type, sub_type);
         }
 
         false
@@ -350,7 +350,7 @@ pub trait InputValueHelpers {
 impl InputValueHelpers for InputValue {
     fn is_required(&self) -> bool {
         if let Type::NonNullType(_inner_type) = &self.value_type {
-            if let None = &self.default_value {
+            if self.default_value.is_none() {
                 return true;
             }
         }
@@ -394,22 +394,20 @@ impl ImplementingInterfaceExtension for TypeDefinition {
     fn has_sub_type(&self, other_type: &TypeDefinition) -> bool {
         match self {
             TypeDefinition::Interface(interface_type) => {
-                return interface_type.is_implemented_by(other_type)
+                interface_type.is_implemented_by(other_type)
             }
             TypeDefinition::Union(union_type) => return union_type.has_sub_type(other_type.name()),
-            _ => return false,
+            _ => false,
         }
     }
 
     fn has_concrete_sub_type(&self, concrete_type: &ObjectType) -> bool {
         match self {
             TypeDefinition::Interface(interface_type) => {
-                return interface_type.is_implemented_by(concrete_type)
-            }
-            TypeDefinition::Union(union_type) => {
-                return union_type.has_sub_type(&concrete_type.name)
+                interface_type.is_implemented_by(concrete_type)
             }
-            _ => return false,
+            TypeDefinition::Union(union_type) => union_type.has_sub_type(&concrete_type.name),
+            _ => false,
         }
     }
 }
@@ -487,17 +485,13 @@ pub trait SubTypeExtension {
 
 impl SubTypeExtension for UnionType {
     fn has_sub_type(&self, other_type_name: &str) -> bool {
-        self.types.iter().find(|v| other_type_name.eq(*v)).is_some()
+        self.types.iter().any(|v| other_type_name.eq(v))
     }
 }
 
 impl AbstractTypeDefinitionExtension for InterfaceType {
     fn is_implemented_by(&self, other_type: &dyn ImplementingInterfaceExtension) -> bool {
-        other_type
-            .interfaces()
-            .iter()
-            .find(|v| self.name.eq(*v))
-            .is_some()
+        other_type.interfaces().iter().any(|v| self.name.eq(v))
     }
 }
 
@@ -627,31 +621,19 @@ impl TypeDefinitionExtension for schema::TypeDefinition {
     }
 
     fn is_object_type(&self) -> bool {
-        match self {
-            schema::TypeDefinition::Object(_o) => true,
-            _ => false,
-        }
+        matches!(self, schema::TypeDefinition::Object(_o))
     }
 
     fn is_union_type(&self) -> bool {
-        match self {
-            schema::TypeDefinition::Union(_o) => true,
-            _ => false,
-        }
+        matches!(self, schema::TypeDefinition::Union(_o))
     }
 
     fn is_enum_type(&self) -> bool {
-        match self {
-            schema::TypeDefinition::Enum(_o) => true,
-            _ => false,
-        }
+        matches!(self, schema::TypeDefinition::Enum(_o))
     }
 
     fn is_scalar_type(&self) -> bool {
-        match self {
-            schema::TypeDefinition::Scalar(_o) => true,
-            _ => false,
-        }
+        matches!(self, schema::TypeDefinition::Scalar(_o))
     }
 }