Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move to our own Type type for everything that's exposed in trustfall_core #435

Merged
merged 37 commits into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
a663307
Move to our own Type type for everything that's exposed in trustfall_…
u9g Aug 6, 2023
f3a5ea6
Switch to our own type implementation
u9g Aug 29, 2023
1bfb81d
add over max list depth panic
u9g Aug 29, 2023
d7db502
rename InlineModifiers to Modifiers
u9g Aug 29, 2023
838b57f
move from_type to associated function on type
u9g Aug 29, 2023
6749bea
move doc comment to struct instead of impl block
u9g Aug 29, 2023
8044bd4
is_nullable() -> nullable()
u9g Aug 29, 2023
9f4135b
Merge branch 'main' into stop-reexporting-gql-type-called-type
u9g Aug 29, 2023
b6f8eaa
rename previous is_nullable() usages
u9g Aug 29, 2023
32449b7
merge conflict fix
u9g Aug 29, 2023
43561ec
more is_nullable renames
u9g Aug 29, 2023
474a79a
add docs and doctests to new methods
u9g Aug 29, 2023
df1c45a
cargo fmt fix
u9g Aug 29, 2023
deaa684
Update trustfall_core/src/ir/ty.rs
u9g Aug 30, 2023
cafff41
improve tests
u9g Aug 30, 2023
10733be
add max list depth test with non-null on innermost
u9g Sep 3, 2023
64e6399
Move type-related code into `ir/types/` module and fix impl bug.
obi1kenobi Oct 7, 2023
0dd6c97
Add re-export for `Type` and `NamedTypeValue`.
obi1kenobi Oct 7, 2023
1a08033
Add early-return check for scalar-only subtyping.
obi1kenobi Oct 7, 2023
93efebd
Rename `base_named_type()` to `base_type()`.
obi1kenobi Oct 7, 2023
653451e
Inline helpers and use `Type` reexport by default.
obi1kenobi Oct 7, 2023
39a7fcb
Return `Result` when parsing a string to a `Type`.
obi1kenobi Oct 7, 2023
9c3990a
Rename `Type::new()` to `Type::parse()`.
obi1kenobi Oct 7, 2023
2ff95dd
Move type intersection onto `Type` itself.
obi1kenobi Oct 7, 2023
a9446a6
Move `equal_ignoring_nullability()` method to `Type`.
obi1kenobi Oct 7, 2023
803bd9a
Move value type-checking fn to a `Type` method.
obi1kenobi Oct 8, 2023
c64c489
Move orderability check to a method on `Type`.
obi1kenobi Oct 8, 2023
aa9c76b
Move scalar-only subtyping check into `Type` methods.
obi1kenobi Oct 8, 2023
71d32c8
Rename `operations` module since it no longer contains any operations.
obi1kenobi Oct 8, 2023
e5df5e2
Merge branch 'main' into stop-reexporting-gql-type-called-type
obi1kenobi Oct 9, 2023
d84478a
Add static type names for built-in types.
obi1kenobi Oct 9, 2023
8118c3f
Minor polish.
obi1kenobi Oct 9, 2023
d437b2c
Merge branch 'main' into stop-reexporting-gql-type-called-type
obi1kenobi Oct 9, 2023
5f07864
Create statics for common types.
obi1kenobi Oct 10, 2023
18e6a39
Add string capacity when printing.
obi1kenobi Oct 10, 2023
097924f
Add more test coverage.
obi1kenobi Oct 10, 2023
3901978
Merge branch 'main' into stop-reexporting-gql-type-called-type
obi1kenobi Oct 10, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 44 additions & 43 deletions trustfall_core/src/frontend/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand All @@ -17,12 +16,12 @@ use crate::{
query::{parse_document, FieldConnection, FieldNode, Query},
},
ir::{
ty::Type,
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,
TYPENAME_META_FIELD, TYPENAME_META_FIELD_ARC,
},
schema::{FieldOrigin, Schema, BUILTIN_SCALARS},
util::{BTreeMapTryInsertExt, TryCollectUniqueKey},
Expand Down Expand Up @@ -72,13 +71,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("String!").unwrap(),
);
}

Expand All @@ -93,7 +92,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),
);
}
}

Expand Down Expand Up @@ -162,7 +166,10 @@ 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!(is_argument_type_valid(
&Type::from_type(&arg.node.ty.node),
&value
));

value
})
Expand All @@ -176,7 +183,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 !is_argument_type_valid(&Type::from_type(&arg.node.ty.node), value) {
errors.push(FrontendError::InvalidEdgeParameterType(
arg_name.to_string(),
edge_definition.name.node.to_string(),
Expand Down Expand Up @@ -223,7 +230,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 {
Expand All @@ -244,31 +251,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(..)
Expand All @@ -279,7 +286,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.
Expand Down Expand Up @@ -324,7 +331,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)?,
Expand Down Expand Up @@ -396,7 +403,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,
);

Expand Down Expand Up @@ -447,7 +454,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,
Expand Down Expand Up @@ -607,7 +614,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();
Expand Down Expand Up @@ -875,13 +882,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,
Expand Down Expand Up @@ -978,10 +982,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>,
Expand Down Expand Up @@ -1099,7 +1100,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.

Expand Down Expand Up @@ -1127,7 +1128,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)
Expand All @@ -1142,7 +1143,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]))
u9g marked this conversation as resolved.
Show resolved Hide resolved
});

for output_directive in &subfield.output {
Expand Down Expand Up @@ -1189,7 +1190,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(),
u9g marked this conversation as resolved.
Show resolved Hide resolved
};

// TODO: handle tags on non-fold-related transformed fields here
Expand All @@ -1200,7 +1201,7 @@ where
}
}
} else {
unreachable!("field name: {}", subfield_name.as_ref());
unreachable!("field name: {}", subfield_name);
}
}

Expand Down
6 changes: 2 additions & 4 deletions trustfall_core/src/interpreter/helpers/correctness.rs
Original file line number Diff line number Diff line change
@@ -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,
ty::Type, ContextField, EdgeParameters, Eid, FieldValue, IREdge, IRQuery, IRQueryComponent,
IRVertex, TransparentValue, Vid,
},
schema::{Schema, SchemaAdapter},
TryIntoStruct,
Expand Down
7 changes: 4 additions & 3 deletions trustfall_core/src/interpreter/hints/dynamic.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use std::{fmt::Debug, ops::Bound, sync::Arc};

use async_graphql_parser::types::Type;

use crate::{
interpreter::{
execution::{
Expand All @@ -12,7 +10,10 @@ use crate::{
Adapter, ContextIterator, ContextOutcomeIterator, InterpretedQuery, TaggedValue,
VertexIterator,
},
ir::{ContextField, FieldRef, FieldValue, FoldSpecificField, IRQueryComponent, Operation},
ir::{
ty::Type, ContextField, FieldRef, FieldValue, FoldSpecificField, IRQueryComponent,
Operation,
},
};

use super::CandidateValue;
Expand Down
8 changes: 3 additions & 5 deletions trustfall_core/src/interpreter/hints/vertex_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ impl<T: InternalVertexInfo + super::sealed::__Sealed> VertexInfo for T {
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())
Expand Down Expand Up @@ -397,7 +397,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,
Expand All @@ -409,13 +409,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::{ty::Type, Argument, FieldValue, LocalField, Operation, VariableRef},
};

#[test]
Expand Down
4 changes: 2 additions & 2 deletions trustfall_core/src/interpreter/mod.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
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,
ty::Type, types::is_argument_type_valid, EdgeParameters, Eid, FieldRef, FieldValue,
IndexedQuery, Vid,
},
util::BTreeMapTryInsertExt,
};
Expand Down
13 changes: 4 additions & 9 deletions trustfall_core/src/ir/indexed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ use std::{
sync::Arc,
};

use async_graphql_parser::types::{BaseType, Type};
use serde::{Deserialize, Serialize};

use crate::util::BTreeMapTryInsertExt;

use super::{
types::is_scalar_only_subtype, Argument, Eid, IREdge, IRFold, IRQuery, IRQueryComponent, Vid,
ty::Type, types::is_scalar_only_subtype, Argument, Eid, IREdge, IRFold, IRQuery,
IRQueryComponent, Vid,
};

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
Expand All @@ -29,8 +29,6 @@ pub struct IndexedQuery {
pub struct Output {
pub name: Arc<str>,

#[serde(serialize_with = "crate::ir::serialization::serde_type_serializer")]
obi1kenobi marked this conversation as resolved.
Show resolved Hide resolved
#[serde(deserialize_with = "crate::ir::serialization::serde_type_deserializer")]
pub value_type: Type,

pub vid: Vid,
Expand Down Expand Up @@ -101,13 +99,10 @@ fn get_output_type(
) -> Type {
let mut wrapped_output_type = field_type.clone();
if component_optional_vertices.contains(&output_at) {
wrapped_output_type.nullable = true;
wrapped_output_type = wrapped_output_type.with_nullability(true);
obi1kenobi marked this conversation as resolved.
Show resolved Hide resolved
}
for is_fold_optional in are_folds_optional.iter().rev() {
wrapped_output_type = Type {
base: BaseType::List(Box::new(wrapped_output_type)),
nullable: *is_fold_optional,
};
wrapped_output_type = Type::new_list_type(wrapped_output_type, *is_fold_optional);
}
wrapped_output_type
}
Expand Down
Loading