Move to our own Type type for everything that's exposed in trustfall_…

…core (#435) * Move to our own Type type for everything that's exposed in trustfall_core * Switch to our own type implementation * add over max list depth panic * rename InlineModifiers to Modifiers * move from_type to associated function on type * move doc comment to struct instead of impl block * is_nullable() -> nullable() * rename previous is_nullable() usages * merge conflict fix * more is_nullable renames * add docs and doctests to new methods * cargo fmt fix * Update trustfall_core/src/ir/ty.rs Co-authored-by: Predrag Gruevski <[email protected]> * improve tests * add max list depth test with non-null on innermost * Move type-related code into `ir/types/` module and fix impl bug. * Add re-export for `Type` and `NamedTypeValue`. * Add early-return check for scalar-only subtyping. * Rename `base_named_type()` to `base_type()`. * Inline helpers and use `Type` reexport by default. * Return `Result` when parsing a string to a `Type`. * Rename `Type::new()` to `Type::parse()`. * Move type intersection onto `Type` itself. * Move `equal_ignoring_nullability()` method to `Type`. * Move value type-checking fn to a `Type` method. * Move orderability check to a method on `Type`. * Move scalar-only subtyping check into `Type` methods. * Rename `operations` module since it no longer contains any operations. * Add static type names for built-in types. * Minor polish. * Create statics for common types. * Add string capacity when printing. * Add more test coverage. --------- Co-authored-by: Predrag Gruevski <[email protected]> Co-authored-by: Predrag Gruevski <[email protected]>
obi1kenobi · Oct 10, 2023 · 185deb3 · 185deb3
1 parent d99123e
commit 185deb3
Show file tree

Hide file tree

Showing 14 changed files with 1,066 additions and 721 deletions.
diff --git a/trustfall_core/src/frontend/mod.rs b/trustfall_core/src/frontend/mod.rs
@@ -5,10 +5,9 @@ use std::{
 };
 
 use async_graphql_parser::{
-    types::{BaseType, ExecutableDocument, FieldDefinition, Type, TypeDefinition, TypeKind},
+    types::{ExecutableDocument, FieldDefinition, TypeDefinition, TypeKind},
     Positioned,
 };
-use async_graphql_value::Name;
 use smallvec::SmallVec;
 
 use crate::{
@@ -17,12 +16,10 @@ use crate::{
         query::{parse_document, FieldConnection, FieldNode, Query},
     },
     ir::{
-        types::{intersect_types, is_argument_type_valid, NamedTypedValue},
-        Argument, ContextField, EdgeParameters, Eid, FieldRef, FieldValue, FoldSpecificField,
-        FoldSpecificFieldKind, IREdge, IRFold, IRQuery, IRQueryComponent, IRVertex, IndexedQuery,
-        LocalField, Operation, Recursive, TransformationKind, VariableRef, Vid,
-        TYPENAME_META_FIELD, TYPENAME_META_FIELD_ARC, TYPENAME_META_FIELD_NAME,
-        TYPENAME_META_FIELD_TYPE,
+        types::NamedTypedValue, Argument, ContextField, EdgeParameters, Eid, FieldRef, FieldValue,
+        FoldSpecificField, FoldSpecificFieldKind, IREdge, IRFold, IRQuery, IRQueryComponent,
+        IRVertex, IndexedQuery, LocalField, Operation, Recursive, TransformationKind, Type,
+        VariableRef, Vid, TYPENAME_META_FIELD, TYPENAME_META_FIELD_ARC,
     },
     schema::{FieldOrigin, Schema, BUILTIN_SCALARS},
     util::{BTreeMapTryInsertExt, TryCollectUniqueKey},
@@ -71,13 +68,13 @@ pub fn parse_doc(schema: &Schema, document: &ExecutableDocument) -> Result<IRQue
 fn get_field_name_and_type_from_schema<'a>(
     defined_fields: &'a [Positioned<FieldDefinition>],
     field_node: &FieldNode,
-) -> (&'a Name, Arc<str>, Arc<str>, &'a Type) {
+) -> (&'a str, Arc<str>, Arc<str>, Type) {
     if field_node.name.as_ref() == TYPENAME_META_FIELD {
         return (
-            &TYPENAME_META_FIELD_NAME,
+            TYPENAME_META_FIELD,
             TYPENAME_META_FIELD_ARC.clone(),
             TYPENAME_META_FIELD_ARC.clone(),
-            &TYPENAME_META_FIELD_TYPE,
+            Type::new_named_type("String", false),
         );
     }
 
@@ -92,7 +89,12 @@ fn get_field_name_and_type_from_schema<'a>(
             } else {
                 pre_coercion_type_name.clone()
             };
-            return (field_name, pre_coercion_type_name, post_coercion_type_name, field_raw_type);
+            return (
+                field_name,
+                pre_coercion_type_name,
+                post_coercion_type_name,
+                Type::from_type(field_raw_type),
+            );
         }
     }
 
@@ -161,7 +163,7 @@ fn make_edge_parameters(
 
                         // The default value must be a valid type for the parameter,
                         // otherwise the schema itself is invalid.
-                        assert!(is_argument_type_valid(&arg.node.ty.node, &value));
+                        assert!(Type::from_type(&arg.node.ty.node).is_valid_value(&value));
 
                         value
                     })
@@ -175,7 +177,7 @@ fn make_edge_parameters(
             }
             Some(value) => {
                 // Type-check the supplied value against the schema.
-                if !is_argument_type_valid(&arg.node.ty.node, value) {
+                if !Type::from_type(&arg.node.ty.node).is_valid_value(value) {
                     errors.push(FrontendError::InvalidEdgeParameterType(
                         arg_name.to_string(),
                         edge_definition.name.node.to_string(),
@@ -222,7 +224,7 @@ fn make_edge_parameters(
 
 fn infer_variable_type(
     property_name: &str,
-    property_type: &Type,
+    property_type: Type,
     operation: &Operation<(), OperatorArgument>,
 ) -> Result<Type, Box<FilterTypeError>> {
     match operation {
@@ -243,31 +245,31 @@ fn infer_variable_type(
             // Using a "null" valued variable doesn't make sense as a comparison.
             // However, [[1], [2], null] is a valid value to use in the comparison, since
             // there are definitely values that it is smaller than or bigger than.
-            Ok(Type { base: property_type.base.clone(), nullable: false })
+            Ok(property_type.with_nullability(false))
         }
         Operation::Contains(..) | Operation::NotContains(..) => {
             // To be able to check whether the property's value contains the operand,
             // the property needs to be a list. If it's not a list, this is a bad filter.
-            let inner_type = match &property_type.base {
-                BaseType::Named(_) => {
-                    return Err(Box::new(FilterTypeError::ListFilterOperationOnNonListField(
-                        operation.operation_name().to_string(),
-                        property_name.to_string(),
-                        property_type.to_string(),
-                    )))
-                }
-                BaseType::List(inner) => inner.as_ref(),
+            // let value = property_type.value();
+            let inner_type = if let Some(list) = property_type.as_list() {
+                list
+            } else {
+                return Err(Box::new(FilterTypeError::ListFilterOperationOnNonListField(
+                    operation.operation_name().to_string(),
+                    property_name.to_string(),
+                    property_type.to_string(),
+                )));
             };
 
             // We're trying to see if a list of element contains our element, so its type
             // is whatever is inside the list -- nullable or not.
-            Ok(inner_type.clone())
+            Ok(inner_type)
         }
         Operation::OneOf(..) | Operation::NotOneOf(..) => {
             // Whatever the property's type is, the argument must be a non-nullable list of
             // the same type, so that the elements of that list may be checked for equality
             // against that property's value.
-            Ok(Type { base: BaseType::List(Box::new(property_type.clone())), nullable: false })
+            Ok(Type::new_list_type(property_type.clone(), false))
         }
         Operation::HasPrefix(..)
         | Operation::NotHasPrefix(..)
@@ -278,7 +280,7 @@ fn infer_variable_type(
         | Operation::RegexMatches(..)
         | Operation::NotRegexMatches(..) => {
             // Filtering operations involving strings only take non-nullable strings as inputs.
-            Ok(Type { base: BaseType::Named(Name::new("String")), nullable: false })
+            Ok(Type::new_named_type("String", false))
         }
         Operation::IsNull(..) | Operation::IsNotNull(..) => {
             // These are unary operations, there's no place where a variable can be used.
@@ -323,7 +325,7 @@ fn make_filter_expr<LeftT: NamedTypedValue>(
                         variable_name: var_name.clone(),
                         variable_type: infer_variable_type(
                             left_operand.named(),
-                            left_operand.typed(),
+                            left_operand.typed().clone(),
                             &filter_directive.operation,
                         )
                         .map_err(|e| *e)?,
@@ -395,7 +397,7 @@ pub fn make_ir_for_query(schema: &Schema, query: &Query) -> Result<IRQuery, Fron
     let starting_vid = vid_maker.next().unwrap();
 
     let root_parameters = make_edge_parameters(
-        get_edge_definition_from_schema(schema, schema.query_type_name(), root_field_name.as_ref()),
+        get_edge_definition_from_schema(schema, schema.query_type_name(), root_field_name),
         &query.root_connection.arguments,
     );
 
@@ -446,7 +448,7 @@ pub fn make_ir_for_query(schema: &Schema, query: &Query) -> Result<IRQuery, Fron
 
     if errors.is_empty() {
         Ok(IRQuery {
-            root_name: root_field_name.as_ref().to_owned().into(),
+            root_name: root_field_name.into(),
             root_parameters: root_parameters.unwrap(),
             root_component: root_component.into(),
             variables,
@@ -501,7 +503,7 @@ fn fill_in_query_variables(
             .entry(vref.variable_name.clone())
             .or_insert_with(|| vref.variable_type.clone());
 
-        match intersect_types(existing_type, &vref.variable_type) {
+        match existing_type.intersect(&vref.variable_type) {
             Some(intersection) => {
                 *existing_type = intersection;
             }
@@ -606,7 +608,7 @@ where
     #[allow(clippy::type_complexity)]
     let mut properties: BTreeMap<
         (Vid, Arc<str>),
-        (Arc<str>, &'schema Type, SmallVec<[&'query FieldNode; 1]>),
+        (Arc<str>, Type, SmallVec<[&'query FieldNode; 1]>),
     > = Default::default();
 
     output_handler.begin_subcomponent();
@@ -874,13 +876,10 @@ fn get_recurse_implicit_coercion(
 
 #[allow(clippy::too_many_arguments)]
 #[allow(clippy::type_complexity)]
-fn make_vertex<'schema, 'query>(
-    schema: &'schema Schema,
+fn make_vertex<'query>(
+    schema: &Schema,
     property_names_by_vertex: &BTreeMap<Vid, Vec<Arc<str>>>,
-    properties: &BTreeMap<
-        (Vid, Arc<str>),
-        (Arc<str>, &'schema Type, SmallVec<[&'query FieldNode; 1]>),
-    >,
+    properties: &BTreeMap<(Vid, Arc<str>), (Arc<str>, Type, SmallVec<[&'query FieldNode; 1]>)>,
     tags: &mut TagHandler,
     component_path: &ComponentPath,
     vid: Vid,
@@ -977,10 +976,7 @@ fn fill_in_vertex_data<'schema, 'query, V, E>(
     edges: &mut BTreeMap<Eid, (Vid, Vid, &'query FieldConnection)>,
     folds: &mut BTreeMap<Eid, Arc<IRFold>>,
     property_names_by_vertex: &mut BTreeMap<Vid, Vec<Arc<str>>>,
-    properties: &mut BTreeMap<
-        (Vid, Arc<str>),
-        (Arc<str>, &'schema Type, SmallVec<[&'query FieldNode; 1]>),
-    >,
+    properties: &mut BTreeMap<(Vid, Arc<str>), (Arc<str>, Type, SmallVec<[&'query FieldNode; 1]>)>,
     component_path: &mut ComponentPath,
     output_handler: &mut OutputHandler<'query>,
     tags: &mut TagHandler<'query>,
@@ -1098,7 +1094,7 @@ where
             output_handler.end_nested_scope(next_vid);
         } else if BUILTIN_SCALARS.contains(subfield_post_coercion_type.as_ref())
             || schema.scalars.contains_key(subfield_post_coercion_type.as_ref())
-            || subfield_name.as_ref() == TYPENAME_META_FIELD
+            || subfield_name == TYPENAME_META_FIELD
         {
             // Processing a property.
 
@@ -1126,7 +1122,7 @@ where
                 ));
             }
 
-            let subfield_name: Arc<str> = subfield_name.as_ref().to_owned().into();
+            let subfield_name: Arc<str> = subfield_name.into();
             let key = (current_vid, subfield_name.clone());
             properties
                 .entry(key)
@@ -1141,7 +1137,7 @@ where
                         .or_default()
                         .push(subfield_name.clone());
 
-                    (subfield_name, subfield_raw_type, SmallVec::from([subfield]))
+                    (subfield_name, subfield_raw_type.clone(), SmallVec::from([subfield]))
                 });
 
             for output_directive in &subfield.output {
@@ -1188,7 +1184,7 @@ where
                 let tag_field = ContextField {
                     vertex_id: current_vid,
                     field_name: subfield.name.clone(),
-                    field_type: subfield_raw_type.to_owned(),
+                    field_type: subfield_raw_type.clone(),
                 };
 
                 // TODO: handle tags on non-fold-related transformed fields here
@@ -1199,7 +1195,7 @@ where
                 }
             }
         } else {
-            unreachable!("field name: {}", subfield_name.as_ref());
+            unreachable!("field name: {}", subfield_name);
         }
     }
 

diff --git a/trustfall_core/src/interpreter/helpers/correctness.rs b/trustfall_core/src/interpreter/helpers/correctness.rs
@@ -1,12 +1,10 @@
 use std::{collections::BTreeMap, fmt::Debug, num::NonZeroUsize, sync::Arc};
 
-use async_graphql_parser::types::Type;
-
 use crate::{
     interpreter::{Adapter, DataContext, InterpretedQuery, ResolveEdgeInfo, ResolveInfo},
     ir::{
         ContextField, EdgeParameters, Eid, FieldValue, IREdge, IRQuery, IRQueryComponent, IRVertex,
-        TransparentValue, Vid,
+        TransparentValue, Type, Vid,
     },
     schema::{Schema, SchemaAdapter},
     TryIntoStruct,
@@ -171,7 +169,7 @@ fn make_resolve_info_for_property_check(
                 property_name.clone() => ContextField {
                     vertex_id: vid,
                     field_name: property_name.clone(),
-                    field_type: Type::new(property_type).expect("not a valid type"),
+                    field_type: Type::parse(property_type).expect("not a valid type"),
                 }
             },
         }),
@@ -328,7 +326,7 @@ fn make_resolve_edge_info_for_edge_check(
                 property_name.clone() => ContextField {
                     vertex_id: vid,
                     field_name: property_name,
-                    field_type: Type::new("String!").expect("not a valid type"),
+                    field_type: Type::parse("String!").expect("not a valid type"),
                 }
             },
         }),
@@ -496,7 +494,7 @@ fn make_resolve_info_for_type_coercion(
                 typename_property.clone() => ContextField {
                     vertex_id: vid,
                     field_name: typename_property.clone(),
-                    field_type: Type::new("String!").expect("not a valid type"),
+                    field_type: Type::parse("String!").expect("not a valid type"),
                 }
             },
         }),

diff --git a/trustfall_core/src/interpreter/hints/dynamic.rs b/trustfall_core/src/interpreter/hints/dynamic.rs
@@ -1,7 +1,5 @@
 use std::{fmt::Debug, ops::Bound, sync::Arc};
 
-use async_graphql_parser::types::Type;
-
 use crate::{
     interpreter::{
         execution::{
@@ -12,7 +10,9 @@ use crate::{
         Adapter, ContextIterator, ContextOutcomeIterator, InterpretedQuery, TaggedValue,
         VertexIterator,
     },
-    ir::{ContextField, FieldRef, FieldValue, FoldSpecificField, IRQueryComponent, Operation},
+    ir::{
+        ContextField, FieldRef, FieldValue, FoldSpecificField, IRQueryComponent, Operation, Type,
+    },
 };
 
 use super::CandidateValue;

diff --git a/trustfall_core/src/interpreter/hints/vertex_info.rs b/trustfall_core/src/interpreter/hints/vertex_info.rs
@@ -285,7 +285,7 @@ impl<T: InternalVertexInfo + super::sealed::__Sealed> VertexInfo for T {
         let first_filter = relevant_filters.first()?;
 
         let initial_candidate = self.statically_required_property(property).unwrap_or_else(|| {
-            if first_filter.left().field_type.nullable {
+            if first_filter.left().field_type.nullable() {
                 CandidateValue::All
             } else {
                 CandidateValue::Range(Range::full_non_null())
@@ -395,7 +395,7 @@ fn compute_statically_known_candidate<'a, 'b>(
     relevant_filters: impl Iterator<Item = &'a Operation<LocalField, Argument>>,
     query_variables: &'b BTreeMap<Arc<str>, FieldValue>,
 ) -> Option<CandidateValue<&'b FieldValue>> {
-    let is_subject_field_nullable = field.field_type.nullable;
+    let is_subject_field_nullable = field.field_type.nullable();
     super::filters::candidate_from_statically_evaluated_filters(
         relevant_filters,
         query_variables,
@@ -407,13 +407,11 @@ fn compute_statically_known_candidate<'a, 'b>(
 mod tests {
     use std::{ops::Bound, sync::Arc};
 
-    use async_graphql_parser::types::Type;
-
     use crate::{
         interpreter::hints::{
             vertex_info::compute_statically_known_candidate, CandidateValue, Range,
         },
-        ir::{Argument, FieldValue, LocalField, Operation, VariableRef},
+        ir::{Argument, FieldValue, LocalField, Operation, Type, VariableRef},
     };
 
     #[test]
@@ -424,9 +422,9 @@ mod tests {
         let null: Arc<str> = Arc::from("null");
         let list: Arc<str> = Arc::from("my_list");
         let longer_list: Arc<str> = Arc::from("longer_list");
-        let nullable_int_type = Type::new("Int").unwrap();
-        let int_type = Type::new("Int!").unwrap();
-        let list_int_type = Type::new("[Int!]!").unwrap();
+        let nullable_int_type = Type::parse("Int").unwrap();
+        let int_type = Type::parse("Int!").unwrap();
+        let list_int_type = Type::parse("[Int!]!").unwrap();
 
         let first_var = Argument::Variable(VariableRef {
             variable_name: first.clone(),
@@ -603,7 +601,7 @@ mod tests {
     #[test]
     fn use_schema_to_exclude_null_from_range() {
         let first: Arc<str> = Arc::from("first");
-        let int_type = Type::new("Int!").unwrap();
+        let int_type = Type::parse("Int!").unwrap();
 
         let first_var = Argument::Variable(VariableRef {
             variable_name: first.clone(),

diff --git a/trustfall_core/src/interpreter/mod.rs b/trustfall_core/src/interpreter/mod.rs
@@ -1,13 +1,10 @@
 use std::{collections::BTreeMap, fmt::Debug, sync::Arc};
 
-use async_graphql_parser::types::Type;
 use itertools::Itertools;
 use serde::{de::DeserializeOwned, Deserialize, Serialize};
 
 use crate::{
-    ir::{
-        types::is_argument_type_valid, EdgeParameters, Eid, FieldRef, FieldValue, IndexedQuery, Vid,
-    },
+    ir::{EdgeParameters, Eid, FieldRef, FieldValue, IndexedQuery, Type, Vid},
     util::BTreeMapTryInsertExt,
 };
 
@@ -377,7 +374,7 @@ fn validate_argument_type(
     variable_type: &Type,
     argument_value: &FieldValue,
 ) -> Result<(), QueryArgumentsError> {
-    if is_argument_type_valid(variable_type, argument_value) {
+    if variable_type.is_valid_value(argument_value) {
         Ok(())
     } else {
         Err(QueryArgumentsError::ArgumentTypeError(