From ce32e937a0a983e8ea654e52eaf1eb1c2c3efc6a Mon Sep 17 00:00:00 2001 From: Iryna Shestak Date: Wed, 1 Nov 2023 07:15:37 -0600 Subject: [PATCH 01/15] feat(parser): add parse_type method to allow type parsing only --- .../apollo-parser/src/parser/grammar/mod.rs | 2 +- crates/apollo-parser/src/parser/mod.rs | 30 +++++++++++++-- .../apollo-parser/src/parser/syntax_tree.rs | 37 +++++++++++++++++++ 3 files changed, 65 insertions(+), 4 deletions(-) diff --git a/crates/apollo-parser/src/parser/grammar/mod.rs b/crates/apollo-parser/src/parser/grammar/mod.rs index 115872437..e12c0944f 100644 --- a/crates/apollo-parser/src/parser/grammar/mod.rs +++ b/crates/apollo-parser/src/parser/grammar/mod.rs @@ -1,5 +1,6 @@ pub(crate) mod document; pub(crate) mod selection; +pub(crate) mod ty; mod argument; mod description; @@ -15,7 +16,6 @@ mod object; mod operation; mod scalar; mod schema; -mod ty; mod union_; mod value; mod variable; diff --git a/crates/apollo-parser/src/parser/mod.rs b/crates/apollo-parser/src/parser/mod.rs index 30e376c91..98cabcc70 100644 --- a/crates/apollo-parser/src/parser/mod.rs +++ b/crates/apollo-parser/src/parser/mod.rs @@ -8,7 +8,7 @@ pub(crate) mod grammar; use std::{cell::RefCell, rc::Rc}; use crate::{ - cst::{Document, SelectionSet}, + cst::{Document, SelectionSet, Type}, lexer::Lexer, Error, LimitTracker, Token, TokenKind, }; @@ -146,12 +146,15 @@ impl<'a> Parser<'a> { match builder { syntax_tree::SyntaxTreeWrapper::Document(tree) => tree, - syntax_tree::SyntaxTreeWrapper::FieldSet(_) => { + syntax_tree::SyntaxTreeWrapper::Type(_) + | syntax_tree::SyntaxTreeWrapper::FieldSet(_) => { unreachable!("parse constructor can only construct a document") } } } + /// Parse selection set tokens. Useful for specifically parsing selections + /// sets which are part of specific directives, like `@requires`. pub fn parse_selection_set(mut self) -> SyntaxTree { grammar::selection::field_set(&mut self); @@ -166,7 +169,28 @@ impl<'a> Parser<'a> { match builder { syntax_tree::SyntaxTreeWrapper::FieldSet(tree) => tree, - syntax_tree::SyntaxTreeWrapper::Document(_) => { + syntax_tree::SyntaxTreeWrapper::Document(_) + | syntax_tree::SyntaxTreeWrapper::Type(_) => { + unreachable!("parse constructor can only construct a selection set") + } + } + } + + /// Parse type tokens. Useful for specifically parsing field types which are + /// part of specific directives, like `@field`. + pub fn parse_type(mut self) -> SyntaxTree { + grammar::ty::ty(&mut self); + + let builder = Rc::try_unwrap(self.builder) + .expect("More than one reference to builder left") + .into_inner(); + let builder = + builder.finish_type(self.errors, self.recursion_limit, self.lexer.limit_tracker); + + match builder { + syntax_tree::SyntaxTreeWrapper::Type(tree) => tree, + syntax_tree::SyntaxTreeWrapper::FieldSet(_) + | syntax_tree::SyntaxTreeWrapper::Document(_) => { unreachable!("parse constructor can only construct a selection set") } } diff --git a/crates/apollo-parser/src/parser/syntax_tree.rs b/crates/apollo-parser/src/parser/syntax_tree.rs index f4ca6c85e..22c5ee602 100644 --- a/crates/apollo-parser/src/parser/syntax_tree.rs +++ b/crates/apollo-parser/src/parser/syntax_tree.rs @@ -49,6 +49,7 @@ use super::LimitTracker; pub(crate) enum SyntaxTreeWrapper { Document(SyntaxTree), FieldSet(SyntaxTree), + Type(SyntaxTree), } #[derive(PartialEq, Eq, Clone)] @@ -111,6 +112,25 @@ impl SyntaxTree { } } +impl SyntaxTree { + /// Return the root typed `SelectionSet` node. This is used for parsing + /// selection sets defined by @requires directive. + pub fn ty(&self) -> cst::Type { + match self.syntax_node().kind().into() { + SyntaxKind::NAMED_TYPE => cst::Type::NamedType(cst::NamedType { + syntax: self.syntax_node(), + }), + SyntaxKind::LIST_TYPE => cst::Type::ListType(cst::ListType { + syntax: self.syntax_node(), + }), + SyntaxKind::NON_NULL_TYPE => cst::Type::NonNullType(cst::NonNullType { + syntax: self.syntax_node(), + }), + _ => unreachable!("this should only return Type node"), + } + } +} + impl fmt::Debug for SyntaxTree { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fn print(f: &mut fmt::Formatter<'_>, indent: usize, element: SyntaxElement) -> fmt::Result { @@ -228,6 +248,23 @@ impl SyntaxTreeBuilder { _phantom: PhantomData, }) } + + pub(crate) fn finish_type( + self, + errors: Vec, + recursion_limit: LimitTracker, + token_limit: LimitTracker, + ) -> SyntaxTreeWrapper { + SyntaxTreeWrapper::Type(SyntaxTree { + green: self.builder.finish(), + // TODO: keep the errors in the builder rather than pass it in here? + errors, + // TODO: keep the recursion and token limits in the builder rather than pass it in here? + recursion_limit, + token_limit, + _phantom: PhantomData, + }) + } } #[cfg(test)] From 990e36bc8f39e08d5107f063376f77b88f50ec4c Mon Sep 17 00:00:00 2001 From: Iryna Shestak Date: Thu, 2 Nov 2023 05:43:09 -0600 Subject: [PATCH 02/15] setup field type parsing in apollo-compiler --- crates/apollo-compiler/src/ast/from_cst.rs | 10 ++++++++- crates/apollo-compiler/src/parser.rs | 22 ++++++++++++++++++ crates/apollo-compiler/src/schema/mod.rs | 26 ++++++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/crates/apollo-compiler/src/ast/from_cst.rs b/crates/apollo-compiler/src/ast/from_cst.rs index 1d16edc90..ef996d1e8 100644 --- a/crates/apollo-compiler/src/ast/from_cst.rs +++ b/crates/apollo-compiler/src/ast/from_cst.rs @@ -27,7 +27,7 @@ impl Document { } /// Similar to `TryFrom`, but with an `Option` return type because AST uses Option a lot. -trait Convert { +pub(crate) trait Convert { type Target; fn convert(&self, file_id: FileId) -> Option; } @@ -567,6 +567,14 @@ impl Convert for cst::Type { } } +pub(crate) fn convert_type(ty: &cst::Type, file_id: FileId) -> ast::Type { + match ty { + cst::Type::NamedType(name) => todo!(), + cst::Type::ListType(inner) => todo!(), + cst::Type::NonNullType(inner) => todo!(), + } +} + impl Convert for cst::FieldDefinition { type Target = ast::FieldDefinition; diff --git a/crates/apollo-compiler/src/parser.rs b/crates/apollo-compiler/src/parser.rs index f3ca853a8..7be0b271a 100644 --- a/crates/apollo-compiler/src/parser.rs +++ b/crates/apollo-compiler/src/parser.rs @@ -1,6 +1,7 @@ use crate::ast; use crate::ast::Document; use crate::executable; +use crate::schema; use crate::schema::SchemaBuilder; use crate::validation::Details; use crate::validation::Diagnostics; @@ -234,6 +235,27 @@ impl Parser { } } + /// Parse the given source of a field type. + /// + /// `path` is the filesystem path (or arbitrary string) used in diagnostics + /// to identify this source file to users. + /// + /// Parsing is fault-tolerant, so a type node is always returned. + /// TODO: document how to validate + pub fn parse_field_type( + &mut self, + source_text: impl Into, + path: impl AsRef, + ) -> schema::FieldType { + let (tree, source_file) = + self.parse_common(source_text.into(), path.as_ref().to_owned(), |parser| { + parser.parse_type() + }); + let file_id = FileId::new(); + let ast = ast::from_cst::convert_type(&tree.ty(), file_id); + todo!() + } + /// What level of recursion was reached during the last call to a `parse_*` method. /// /// Collecting this on a corpus of documents can help decide diff --git a/crates/apollo-compiler/src/schema/mod.rs b/crates/apollo-compiler/src/schema/mod.rs index 459608060..bdebfcfa2 100644 --- a/crates/apollo-compiler/src/schema/mod.rs +++ b/crates/apollo-compiler/src/schema/mod.rs @@ -143,6 +143,20 @@ pub struct InputObjectType { pub fields: IndexMap>, } +#[derive(Debug, Clone)] +pub struct FieldType { + /// If this field type was originally parsed from a source file, + /// this map contains one entry for that file and its ID. + /// + /// The document may have been modified since. + pub sources: crate::SourceMap, + + /// Errors that occurred when building this field type, + /// either parsing a source file or converting from AST. + build_errors: Vec, + pub ty: Type, +} + /// AST node that has been skipped during conversion to `Schema` #[derive(thiserror::Error, Debug, Clone)] pub(crate) enum BuildError { @@ -851,6 +865,18 @@ impl Directives { serialize_method!(); } +impl FieldType { + /// Parse the given source with a field type. + /// + /// `path` is the filesystem path (or arbitrary string) used in diagnostics + /// to identify this source file to users. + /// + /// Create a [`Parser`] to use different parser configuration. + pub fn parse(source_text: impl Into, path: impl AsRef) -> Self { + Parser::new().parse_field_type(source_text, path) + } +} + impl std::fmt::Debug for Directives { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { self.0.fmt(f) From c3fc58fbaf0be6a060ef2b7b6e0158346cf90c51 Mon Sep 17 00:00:00 2001 From: Iryna Shestak Date: Tue, 7 Nov 2023 20:37:54 -0600 Subject: [PATCH 03/15] add field type to schema in compiler --- crates/apollo-compiler/src/ast/from_cst.rs | 8 -------- crates/apollo-compiler/src/parser.rs | 11 +++++++++-- crates/apollo-compiler/src/schema/mod.rs | 10 ++++++++-- crates/apollo-compiler/src/schema/validation.rs | 11 +++++++++++ 4 files changed, 28 insertions(+), 12 deletions(-) diff --git a/crates/apollo-compiler/src/ast/from_cst.rs b/crates/apollo-compiler/src/ast/from_cst.rs index ef996d1e8..1584ef437 100644 --- a/crates/apollo-compiler/src/ast/from_cst.rs +++ b/crates/apollo-compiler/src/ast/from_cst.rs @@ -567,14 +567,6 @@ impl Convert for cst::Type { } } -pub(crate) fn convert_type(ty: &cst::Type, file_id: FileId) -> ast::Type { - match ty { - cst::Type::NamedType(name) => todo!(), - cst::Type::ListType(inner) => todo!(), - cst::Type::NonNullType(inner) => todo!(), - } -} - impl Convert for cst::FieldDefinition { type Target = ast::FieldDefinition; diff --git a/crates/apollo-compiler/src/parser.rs b/crates/apollo-compiler/src/parser.rs index 7be0b271a..e0257df29 100644 --- a/crates/apollo-compiler/src/parser.rs +++ b/crates/apollo-compiler/src/parser.rs @@ -1,7 +1,9 @@ use crate::ast; +use crate::ast::from_cst::Convert; use crate::ast::Document; use crate::executable; use crate::schema; +use crate::schema::FieldType; use crate::schema::SchemaBuilder; use crate::validation::Details; use crate::validation::Diagnostics; @@ -252,8 +254,13 @@ impl Parser { parser.parse_type() }); let file_id = FileId::new(); - let ast = ast::from_cst::convert_type(&tree.ty(), file_id); - todo!() + match tree.ty().convert(file_id) { + Some(ty) => return Ok(FieldType{ + sources: Arc::new([(file_id, source_file)]), + ty, + }), + None => , + } } /// What level of recursion was reached during the last call to a `parse_*` method. diff --git a/crates/apollo-compiler/src/schema/mod.rs b/crates/apollo-compiler/src/schema/mod.rs index bdebfcfa2..850194ded 100644 --- a/crates/apollo-compiler/src/schema/mod.rs +++ b/crates/apollo-compiler/src/schema/mod.rs @@ -150,10 +150,10 @@ pub struct FieldType { /// /// The document may have been modified since. pub sources: crate::SourceMap, - - /// Errors that occurred when building this field type, + /// Errors that occurred when building this schema, /// either parsing a source file or converting from AST. build_errors: Vec, + pub ty: Type, } @@ -875,6 +875,12 @@ impl FieldType { pub fn parse(source_text: impl Into, path: impl AsRef) -> Self { Parser::new().parse_field_type(source_text, path) } + + pub fn validate(&self, schema: &Schema) -> Result<(), Diagnostics> { + let mut errors = Diagnostics::new(Some(schema.sources.clone()), self.sources.clone()); + validation::validate_field_type(&mut errors, schema, self); + errors.into_result() + } } impl std::fmt::Debug for Directives { diff --git a/crates/apollo-compiler/src/schema/validation.rs b/crates/apollo-compiler/src/schema/validation.rs index deb8a6035..60e6c6f43 100644 --- a/crates/apollo-compiler/src/schema/validation.rs +++ b/crates/apollo-compiler/src/schema/validation.rs @@ -1,4 +1,5 @@ use super::BuildError; +use crate::schema::FieldType; use crate::validation::Details; use crate::validation::Diagnostics; use crate::FileId; @@ -75,3 +76,13 @@ fn compiler_validation(errors: &mut Diagnostics, schema: &Schema) -> Vec Date: Wed, 8 Nov 2023 21:36:59 -0600 Subject: [PATCH 04/15] parse_field_type to return an option --- crates/apollo-compiler/src/parser.rs | 14 ++++----- crates/apollo-compiler/src/schema/mod.rs | 11 +++---- .../apollo-compiler/src/schema/validation.rs | 6 +--- crates/apollo-compiler/tests/field_type.rs | 29 +++++++++++++++++++ crates/apollo-compiler/tests/main.rs | 1 + 5 files changed, 41 insertions(+), 20 deletions(-) create mode 100644 crates/apollo-compiler/tests/field_type.rs diff --git a/crates/apollo-compiler/src/parser.rs b/crates/apollo-compiler/src/parser.rs index e0257df29..3c21d4179 100644 --- a/crates/apollo-compiler/src/parser.rs +++ b/crates/apollo-compiler/src/parser.rs @@ -3,7 +3,6 @@ use crate::ast::from_cst::Convert; use crate::ast::Document; use crate::executable; use crate::schema; -use crate::schema::FieldType; use crate::schema::SchemaBuilder; use crate::validation::Details; use crate::validation::Diagnostics; @@ -248,19 +247,18 @@ impl Parser { &mut self, source_text: impl Into, path: impl AsRef, - ) -> schema::FieldType { + ) -> Option { let (tree, source_file) = self.parse_common(source_text.into(), path.as_ref().to_owned(), |parser| { parser.parse_type() }); let file_id = FileId::new(); - match tree.ty().convert(file_id) { - Some(ty) => return Ok(FieldType{ - sources: Arc::new([(file_id, source_file)]), + tree.ty().convert(file_id).map(|ty| { + return schema::FieldType { + sources: Arc::new([(file_id, source_file)].into()), ty, - }), - None => , - } + }; + }) } /// What level of recursion was reached during the last call to a `parse_*` method. diff --git a/crates/apollo-compiler/src/schema/mod.rs b/crates/apollo-compiler/src/schema/mod.rs index 850194ded..2b0a34363 100644 --- a/crates/apollo-compiler/src/schema/mod.rs +++ b/crates/apollo-compiler/src/schema/mod.rs @@ -150,9 +150,6 @@ pub struct FieldType { /// /// The document may have been modified since. pub sources: crate::SourceMap, - /// Errors that occurred when building this schema, - /// either parsing a source file or converting from AST. - build_errors: Vec, pub ty: Type, } @@ -872,13 +869,13 @@ impl FieldType { /// to identify this source file to users. /// /// Create a [`Parser`] to use different parser configuration. - pub fn parse(source_text: impl Into, path: impl AsRef) -> Self { + pub fn parse(source_text: impl Into, path: impl AsRef) -> Option { Parser::new().parse_field_type(source_text, path) } - pub fn validate(&self, schema: &Schema) -> Result<(), Diagnostics> { - let mut errors = Diagnostics::new(Some(schema.sources.clone()), self.sources.clone()); - validation::validate_field_type(&mut errors, schema, self); + pub fn validate(&self) -> Result<(), Diagnostics> { + let mut errors = Diagnostics::new(Some(self.sources.clone()), self.sources.clone()); + validation::validate_field_type(&mut errors, self); errors.into_result() } } diff --git a/crates/apollo-compiler/src/schema/validation.rs b/crates/apollo-compiler/src/schema/validation.rs index 60e6c6f43..1c2f40249 100644 --- a/crates/apollo-compiler/src/schema/validation.rs +++ b/crates/apollo-compiler/src/schema/validation.rs @@ -77,11 +77,7 @@ fn compiler_validation(errors: &mut Diagnostics, schema: &Schema) -> Vec Date: Wed, 8 Nov 2023 22:00:02 -0600 Subject: [PATCH 05/15] parse_field_type returns Result<(FieldType, Diagnostics), Diagnostics> --- crates/apollo-compiler/src/parser.rs | 24 +++++++++++-------- crates/apollo-compiler/src/schema/mod.rs | 11 ++++----- .../apollo-compiler/src/schema/validation.rs | 7 ------ crates/apollo-compiler/tests/field_type.rs | 19 +++++++++------ 4 files changed, 30 insertions(+), 31 deletions(-) diff --git a/crates/apollo-compiler/src/parser.rs b/crates/apollo-compiler/src/parser.rs index 3c21d4179..c668c15bc 100644 --- a/crates/apollo-compiler/src/parser.rs +++ b/crates/apollo-compiler/src/parser.rs @@ -240,25 +240,29 @@ impl Parser { /// /// `path` is the filesystem path (or arbitrary string) used in diagnostics /// to identify this source file to users. - /// - /// Parsing is fault-tolerant, so a type node is always returned. - /// TODO: document how to validate pub fn parse_field_type( &mut self, source_text: impl Into, path: impl AsRef, - ) -> Option { + ) -> Result<(schema::FieldType, Diagnostics), Diagnostics> { let (tree, source_file) = self.parse_common(source_text.into(), path.as_ref().to_owned(), |parser| { parser.parse_type() }); let file_id = FileId::new(); - tree.ty().convert(file_id).map(|ty| { - return schema::FieldType { - sources: Arc::new([(file_id, source_file)].into()), - ty, - }; - }) + + let sources: crate::SourceMap = Arc::new([(file_id, source_file)].into()); + let mut errors = Diagnostics::new(Some(sources.clone()), sources.clone()); + for (file_id, source) in sources.iter() { + source.validate_parse_errors(&mut errors, *file_id) + } + + if let Some(ty) = tree.ty().convert(file_id) { + let field_type = schema::FieldType { sources, ty }; + return Ok((field_type, errors)); + } else { + return Err(errors); + } } /// What level of recursion was reached during the last call to a `parse_*` method. diff --git a/crates/apollo-compiler/src/schema/mod.rs b/crates/apollo-compiler/src/schema/mod.rs index 2b0a34363..5ec2ef0bf 100644 --- a/crates/apollo-compiler/src/schema/mod.rs +++ b/crates/apollo-compiler/src/schema/mod.rs @@ -869,15 +869,12 @@ impl FieldType { /// to identify this source file to users. /// /// Create a [`Parser`] to use different parser configuration. - pub fn parse(source_text: impl Into, path: impl AsRef) -> Option { + pub fn parse( + source_text: impl Into, + path: impl AsRef, + ) -> Result<(Self, Diagnostics), Diagnostics> { Parser::new().parse_field_type(source_text, path) } - - pub fn validate(&self) -> Result<(), Diagnostics> { - let mut errors = Diagnostics::new(Some(self.sources.clone()), self.sources.clone()); - validation::validate_field_type(&mut errors, self); - errors.into_result() - } } impl std::fmt::Debug for Directives { diff --git a/crates/apollo-compiler/src/schema/validation.rs b/crates/apollo-compiler/src/schema/validation.rs index 1c2f40249..deb8a6035 100644 --- a/crates/apollo-compiler/src/schema/validation.rs +++ b/crates/apollo-compiler/src/schema/validation.rs @@ -1,5 +1,4 @@ use super::BuildError; -use crate::schema::FieldType; use crate::validation::Details; use crate::validation::Diagnostics; use crate::FileId; @@ -76,9 +75,3 @@ fn compiler_validation(errors: &mut Diagnostics, schema: &Schema) -> Vec panic!("this input should have no type"), + Err(diag) => { + let errors = diag.to_string_no_color(); + assert!(errors.contains("expected item type"), "{errors}"); + assert!(errors.contains("expected R_BRACK, got EOF"), "{errors}"); + } + } } From 4548ff9cf2663fbd384cbb2efd41c4bf27afe0af Mon Sep 17 00:00:00 2001 From: Iryna Shestak Date: Wed, 8 Nov 2023 22:05:50 -0600 Subject: [PATCH 06/15] clippy --- crates/apollo-compiler/src/parser.rs | 4 ++-- crates/apollo-parser/src/parser/syntax_tree.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/apollo-compiler/src/parser.rs b/crates/apollo-compiler/src/parser.rs index c668c15bc..d3e8c2914 100644 --- a/crates/apollo-compiler/src/parser.rs +++ b/crates/apollo-compiler/src/parser.rs @@ -259,9 +259,9 @@ impl Parser { if let Some(ty) = tree.ty().convert(file_id) { let field_type = schema::FieldType { sources, ty }; - return Ok((field_type, errors)); + Ok((field_type, errors)) } else { - return Err(errors); + Err(errors) } } diff --git a/crates/apollo-parser/src/parser/syntax_tree.rs b/crates/apollo-parser/src/parser/syntax_tree.rs index 22c5ee602..7fd338ee2 100644 --- a/crates/apollo-parser/src/parser/syntax_tree.rs +++ b/crates/apollo-parser/src/parser/syntax_tree.rs @@ -116,7 +116,7 @@ impl SyntaxTree { /// Return the root typed `SelectionSet` node. This is used for parsing /// selection sets defined by @requires directive. pub fn ty(&self) -> cst::Type { - match self.syntax_node().kind().into() { + match self.syntax_node().kind() { SyntaxKind::NAMED_TYPE => cst::Type::NamedType(cst::NamedType { syntax: self.syntax_node(), }), From aad6b2b22259dac3e7786e89edb33864f05ca2f2 Mon Sep 17 00:00:00 2001 From: Iryna Shestak Date: Wed, 8 Nov 2023 22:10:13 -0600 Subject: [PATCH 07/15] FieldType struct does not need sources, as we create diagnostics right away --- crates/apollo-compiler/src/parser.rs | 2 +- crates/apollo-compiler/src/schema/mod.rs | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/crates/apollo-compiler/src/parser.rs b/crates/apollo-compiler/src/parser.rs index d3e8c2914..63bcb88ed 100644 --- a/crates/apollo-compiler/src/parser.rs +++ b/crates/apollo-compiler/src/parser.rs @@ -258,7 +258,7 @@ impl Parser { } if let Some(ty) = tree.ty().convert(file_id) { - let field_type = schema::FieldType { sources, ty }; + let field_type = schema::FieldType { ty }; Ok((field_type, errors)) } else { Err(errors) diff --git a/crates/apollo-compiler/src/schema/mod.rs b/crates/apollo-compiler/src/schema/mod.rs index 5ec2ef0bf..ee7ddf96b 100644 --- a/crates/apollo-compiler/src/schema/mod.rs +++ b/crates/apollo-compiler/src/schema/mod.rs @@ -145,12 +145,6 @@ pub struct InputObjectType { #[derive(Debug, Clone)] pub struct FieldType { - /// If this field type was originally parsed from a source file, - /// this map contains one entry for that file and its ID. - /// - /// The document may have been modified since. - pub sources: crate::SourceMap, - pub ty: Type, } From 9e27f2e2e11204c6cff146167089db3c15709de6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Tue, 14 Nov 2023 16:35:19 +0100 Subject: [PATCH 08/15] Parse directly into `ast::Type` --- crates/apollo-compiler/src/ast/impls.rs | 13 +++++++++ crates/apollo-compiler/src/parser.rs | 8 +++--- crates/apollo-compiler/src/schema/mod.rs | 20 -------------- crates/apollo-compiler/tests/field_type.rs | 31 ++++++++++++---------- 4 files changed, 33 insertions(+), 39 deletions(-) diff --git a/crates/apollo-compiler/src/ast/impls.rs b/crates/apollo-compiler/src/ast/impls.rs index 1fbf84a69..687653138 100644 --- a/crates/apollo-compiler/src/ast/impls.rs +++ b/crates/apollo-compiler/src/ast/impls.rs @@ -634,6 +634,19 @@ impl Type { } } + /// Parse the given source with a field type. + /// + /// `path` is the filesystem path (or arbitrary string) used in diagnostics + /// to identify this source file to users. + /// + /// Create a [`Parser`] to use different parser configuration. + pub fn parse( + source_text: impl Into, + path: impl AsRef, + ) -> Result { + Parser::new().parse_field_type(source_text, path) + } + serialize_method!(); } diff --git a/crates/apollo-compiler/src/parser.rs b/crates/apollo-compiler/src/parser.rs index d510527b7..0d18d6320 100644 --- a/crates/apollo-compiler/src/parser.rs +++ b/crates/apollo-compiler/src/parser.rs @@ -2,7 +2,6 @@ use crate::ast; use crate::ast::from_cst::Convert; use crate::ast::Document; use crate::executable; -use crate::schema; use crate::schema::SchemaBuilder; use crate::validation::Details; use crate::validation::DiagnosticList; @@ -244,7 +243,7 @@ impl Parser { &mut self, source_text: impl Into, path: impl AsRef, - ) -> Result<(schema::FieldType, Diagnostics), Diagnostics> { + ) -> Result { let (tree, source_file) = self.parse_common(source_text.into(), path.as_ref().to_owned(), |parser| { parser.parse_type() @@ -252,14 +251,13 @@ impl Parser { let file_id = FileId::new(); let sources: crate::SourceMap = Arc::new([(file_id, source_file)].into()); - let mut errors = Diagnostics::new(Some(sources.clone()), sources.clone()); + let mut errors = DiagnosticList::new(Some(sources.clone()), sources.clone()); for (file_id, source) in sources.iter() { source.validate_parse_errors(&mut errors, *file_id) } if let Some(ty) = tree.ty().convert(file_id) { - let field_type = schema::FieldType { ty }; - Ok((field_type, errors)) + Ok(ty) } else { Err(errors) } diff --git a/crates/apollo-compiler/src/schema/mod.rs b/crates/apollo-compiler/src/schema/mod.rs index 9b6b172e2..740cf5124 100644 --- a/crates/apollo-compiler/src/schema/mod.rs +++ b/crates/apollo-compiler/src/schema/mod.rs @@ -147,11 +147,6 @@ pub struct InputObjectType { pub fields: IndexMap>, } -#[derive(Debug, Clone)] -pub struct FieldType { - pub ty: Type, -} - /// AST node that has been skipped during conversion to `Schema` #[derive(thiserror::Error, Debug, Clone)] pub(crate) enum BuildError { @@ -871,21 +866,6 @@ impl DirectiveList { serialize_method!(); } -impl FieldType { - /// Parse the given source with a field type. - /// - /// `path` is the filesystem path (or arbitrary string) used in diagnostics - /// to identify this source file to users. - /// - /// Create a [`Parser`] to use different parser configuration. - pub fn parse( - source_text: impl Into, - path: impl AsRef, - ) -> Result<(Self, Diagnostics), Diagnostics> { - Parser::new().parse_field_type(source_text, path) - } -} - impl std::fmt::Debug for DirectiveList { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { self.0.fmt(f) diff --git a/crates/apollo-compiler/tests/field_type.rs b/crates/apollo-compiler/tests/field_type.rs index c24693532..db403ff53 100644 --- a/crates/apollo-compiler/tests/field_type.rs +++ b/crates/apollo-compiler/tests/field_type.rs @@ -1,30 +1,33 @@ -use apollo_compiler::schema::FieldType; +use apollo_compiler::schema::Type; #[test] fn test_valid_field_type() { let input = "String!"; - let field_type = FieldType::parse(input, "field_type.graphql").expect("expected a field type"); - assert!(field_type.1.is_empty()); + let field_type = Type::parse(input, "field_type.graphql").expect("expected a field type"); + assert_eq!(field_type.to_string(), input); let input = "[[[[[Int!]!]!]!]!]!"; - let field_type = FieldType::parse(input, "field_type.graphql").expect("expected a field type"); - assert!(field_type.1.is_empty()); + let field_type = Type::parse(input, "field_type.graphql").expect("expected a field type"); + assert_eq!(field_type.to_string(), input); } #[test] fn test_invalid_field_type() { let input = "[[String]"; - let field_type = FieldType::parse(input, "field_type.graphql").expect("expected a field type"); - let errors = field_type.1.to_string_no_color(); - assert!( - errors.contains("Error: syntax error: expected R_BRACK, got EOF"), - "{errors}" - ); + match Type::parse(input, "field_type.graphql") { + Ok(parsed) => panic!("Field type should fail to parse, instead got `{parsed}`"), + Err(errors) => { + let errors = errors.to_string_no_color(); + assert!( + errors.contains("Error: syntax error: expected R_BRACK, got EOF"), + "{errors}" + ); + } + } let input = "[]"; - let field_type = FieldType::parse(input, "field_type.graphql"); - match field_type { - Ok(_) => panic!("this input should have no type"), + match Type::parse(input, "field_type.graphql") { + Ok(parsed) => panic!("Field type should fail to parse, instead got `{parsed}`"), Err(diag) => { let errors = diag.to_string_no_color(); assert!(errors.contains("expected item type"), "{errors}"); From 9018f3afbc0ffbd4e0ec428bae08a176c081ed7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Tue, 14 Nov 2023 16:39:06 +0100 Subject: [PATCH 09/15] naming --- crates/apollo-compiler/src/ast/impls.rs | 2 +- crates/apollo-compiler/src/parser.rs | 6 +++--- crates/apollo-parser/src/parser/mod.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/apollo-compiler/src/ast/impls.rs b/crates/apollo-compiler/src/ast/impls.rs index 687653138..c90d40848 100644 --- a/crates/apollo-compiler/src/ast/impls.rs +++ b/crates/apollo-compiler/src/ast/impls.rs @@ -644,7 +644,7 @@ impl Type { source_text: impl Into, path: impl AsRef, ) -> Result { - Parser::new().parse_field_type(source_text, path) + Parser::new().parse_type(source_text, path) } serialize_method!(); diff --git a/crates/apollo-compiler/src/parser.rs b/crates/apollo-compiler/src/parser.rs index 0d18d6320..b418219b4 100644 --- a/crates/apollo-compiler/src/parser.rs +++ b/crates/apollo-compiler/src/parser.rs @@ -196,7 +196,7 @@ impl Parser { self.parse_ast(source_text, path).to_mixed() } - /// Parse the given source a selection set with optional outer brackets. + /// Parse the given source text as a selection set with optional outer brackets. /// /// `path` is the filesystem path (or arbitrary string) used in diagnostics /// to identify this source file to users. @@ -235,11 +235,11 @@ impl Parser { } } - /// Parse the given source of a field type. + /// Parse the given source text as a reference to a type. /// /// `path` is the filesystem path (or arbitrary string) used in diagnostics /// to identify this source file to users. - pub fn parse_field_type( + pub fn parse_type( &mut self, source_text: impl Into, path: impl AsRef, diff --git a/crates/apollo-parser/src/parser/mod.rs b/crates/apollo-parser/src/parser/mod.rs index 98cabcc70..be927bc0f 100644 --- a/crates/apollo-parser/src/parser/mod.rs +++ b/crates/apollo-parser/src/parser/mod.rs @@ -191,7 +191,7 @@ impl<'a> Parser<'a> { syntax_tree::SyntaxTreeWrapper::Type(tree) => tree, syntax_tree::SyntaxTreeWrapper::FieldSet(_) | syntax_tree::SyntaxTreeWrapper::Document(_) => { - unreachable!("parse constructor can only construct a selection set") + unreachable!("parse constructor can only construct a type") } } } From c3c8dcda953fcb8826dd8539742203824b0b6097 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Tue, 14 Nov 2023 17:01:26 +0100 Subject: [PATCH 10/15] Return Err() on parse errors --- crates/apollo-compiler/src/parser.rs | 7 +++++-- crates/apollo-parser/src/parser/mod.rs | 4 +--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/apollo-compiler/src/parser.rs b/crates/apollo-compiler/src/parser.rs index b418219b4..f295c5efa 100644 --- a/crates/apollo-compiler/src/parser.rs +++ b/crates/apollo-compiler/src/parser.rs @@ -256,8 +256,11 @@ impl Parser { source.validate_parse_errors(&mut errors, *file_id) } - if let Some(ty) = tree.ty().convert(file_id) { - Ok(ty) + if errors.is_empty() { + if let Some(ty) = tree.ty().convert(file_id) { + return Ok(ty); + } + unreachable!("conversion is infallible if there were no syntax errors"); } else { Err(errors) } diff --git a/crates/apollo-parser/src/parser/mod.rs b/crates/apollo-parser/src/parser/mod.rs index be927bc0f..ffc4b01d4 100644 --- a/crates/apollo-parser/src/parser/mod.rs +++ b/crates/apollo-parser/src/parser/mod.rs @@ -323,9 +323,7 @@ impl<'a> Parser<'a> { /// Consume the next token if it is `kind` or emit an error /// otherwise. pub(crate) fn expect(&mut self, token: TokenKind, kind: SyntaxKind) { - let current = if let Some(current) = self.current() { - current - } else { + let Some(current) = self.current() else { return; }; let is_eof = current.kind == TokenKind::Eof; From cefdf91034615f06fbcd274b4a5376e31f42b9b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Tue, 14 Nov 2023 17:12:44 +0100 Subject: [PATCH 11/15] tweak unreachable! messages --- crates/apollo-parser/src/parser/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/apollo-parser/src/parser/mod.rs b/crates/apollo-parser/src/parser/mod.rs index ffc4b01d4..54f84c016 100644 --- a/crates/apollo-parser/src/parser/mod.rs +++ b/crates/apollo-parser/src/parser/mod.rs @@ -171,7 +171,7 @@ impl<'a> Parser<'a> { syntax_tree::SyntaxTreeWrapper::FieldSet(tree) => tree, syntax_tree::SyntaxTreeWrapper::Document(_) | syntax_tree::SyntaxTreeWrapper::Type(_) => { - unreachable!("parse constructor can only construct a selection set") + unreachable!("parse_selection_set constructor can only construct a selection set") } } } @@ -191,7 +191,7 @@ impl<'a> Parser<'a> { syntax_tree::SyntaxTreeWrapper::Type(tree) => tree, syntax_tree::SyntaxTreeWrapper::FieldSet(_) | syntax_tree::SyntaxTreeWrapper::Document(_) => { - unreachable!("parse constructor can only construct a type") + unreachable!("parse_type constructor can only construct a type") } } } From ae39d7906f4e32a781347b2124386d6fcec80668 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e?= Date: Tue, 14 Nov 2023 17:53:17 +0100 Subject: [PATCH 12/15] Do not pass the same SourceMap twice Co-authored-by: Simon Sapin --- crates/apollo-compiler/src/parser.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/apollo-compiler/src/parser.rs b/crates/apollo-compiler/src/parser.rs index f295c5efa..75e1b286b 100644 --- a/crates/apollo-compiler/src/parser.rs +++ b/crates/apollo-compiler/src/parser.rs @@ -251,7 +251,7 @@ impl Parser { let file_id = FileId::new(); let sources: crate::SourceMap = Arc::new([(file_id, source_file)].into()); - let mut errors = DiagnosticList::new(Some(sources.clone()), sources.clone()); + let mut errors = DiagnosticList::new(None, sources.clone()); for (file_id, source) in sources.iter() { source.validate_parse_errors(&mut errors, *file_id) } From 81ee543698a4b3ac1e9845ab9eeb288c95fa9d92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e?= Date: Tue, 14 Nov 2023 17:54:44 +0100 Subject: [PATCH 13/15] Improve doc comments for `Parser::parse_{type,selection_set}` Co-authored-by: Simon Sapin --- crates/apollo-parser/src/parser/mod.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/apollo-parser/src/parser/mod.rs b/crates/apollo-parser/src/parser/mod.rs index 54f84c016..1de85defa 100644 --- a/crates/apollo-parser/src/parser/mod.rs +++ b/crates/apollo-parser/src/parser/mod.rs @@ -153,8 +153,9 @@ impl<'a> Parser<'a> { } } - /// Parse selection set tokens. Useful for specifically parsing selections - /// sets which are part of specific directives, like `@requires`. + /// Parse a selection set with optional outer braces. + /// This is the expected format of the string value of the `fields` argument of some directives + /// like [`@requires`](https://www.apollographql.com/docs/federation/federated-types/federated-directives/#requires). pub fn parse_selection_set(mut self) -> SyntaxTree { grammar::selection::field_set(&mut self); @@ -176,8 +177,9 @@ impl<'a> Parser<'a> { } } - /// Parse type tokens. Useful for specifically parsing field types which are - /// part of specific directives, like `@field`. + /// Parse a GraphQL type. + /// This is the expected format of the string value of the `type` argument + /// of some directives like [`@field`](https://specs.apollo.dev/join/v0.3/#@field). pub fn parse_type(mut self) -> SyntaxTree { grammar::ty::ty(&mut self); From 2dd6a68618484b0751f577a743df6fb1c4b75ef8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Wed, 15 Nov 2023 12:46:56 +0100 Subject: [PATCH 14/15] doc tweak --- crates/apollo-compiler/src/ast/impls.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/apollo-compiler/src/ast/impls.rs b/crates/apollo-compiler/src/ast/impls.rs index c90d40848..10a26083a 100644 --- a/crates/apollo-compiler/src/ast/impls.rs +++ b/crates/apollo-compiler/src/ast/impls.rs @@ -34,7 +34,7 @@ impl Document { Self::parser().parse_ast(source_text, path) } - /// Returns [`Diagnostics`] for cases where parsed input does not match + /// Returns [`DiagnosticList`] for cases where parsed input does not match /// the GraphQL grammar or where the parser reached a token limit or recursion limit. /// /// Does not perform any validation beyond this syntactic level. @@ -634,7 +634,7 @@ impl Type { } } - /// Parse the given source with a field type. + /// Parse the given source text as a reference to a type. /// /// `path` is the filesystem path (or arbitrary string) used in diagnostics /// to identify this source file to users. From 6792069e9fa89fee97ce9f4d570ee220b15616ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9e=20Kooi?= Date: Wed, 15 Nov 2023 12:47:06 +0100 Subject: [PATCH 15/15] changelogs --- crates/apollo-compiler/CHANGELOG.md | 12 ++++++++++++ crates/apollo-parser/CHANGELOG.md | 22 ++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/crates/apollo-compiler/CHANGELOG.md b/crates/apollo-compiler/CHANGELOG.md index bc433cc89..60b399def 100644 --- a/crates/apollo-compiler/CHANGELOG.md +++ b/crates/apollo-compiler/CHANGELOG.md @@ -32,6 +32,18 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm [SimonSapin]: https://github.com/SimonSapin [pull/739]: https://github.com/apollographql/apollo-rs/pull/739 +- **Add parsing an `ast::Type` from a string - [lrlna] and [goto-bus-stop], [pull/718] fixing [issue/715]** + + Parses GraphQL type syntax: + ```rust + use apollo_compiler::ast::Type; + let ty = Type::parse("[ListItem!]!")?; + ``` + +[lrlna]: https://github.com/lrlna +[goto-bus-stop]: https://github.com/goto-bus-stop +[pull/718]: https://github.com/apollographql/apollo-rs/pull/718 +[issue/715]: https://github.com/apollographql/apollo-rs/issues/715 # [1.0.0-beta.6](https://crates.io/crates/apollo-compiler/1.0.0-beta.6) - 2023-11-10 diff --git a/crates/apollo-parser/CHANGELOG.md b/crates/apollo-parser/CHANGELOG.md index f3b6c8e34..0e51820bb 100644 --- a/crates/apollo-parser/CHANGELOG.md +++ b/crates/apollo-parser/CHANGELOG.md @@ -16,6 +16,28 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## Maintenance ## Documentation --> +# [unreleased](https://crates.io/crates/apollo-parser/x.x.x) - 2023-xx-xx + +## Features +- **`parse_type` parses a selection set with optional outer brackets - [lrlna], [pull/718] fixing [issue/715]** + This returns a `SyntaxTree` which instead of `.document() -> cst::Document` + has `.type() -> cst::Type`. + This is intended to parse the string value of a [`@field(type:)` argument][fieldtype] + used in some Apollo Federation directives. + ```rust + let source = r#"[[NestedList!]]!"#; + + let parser = Parser::new(source); + let cst: SyntaxTree = parser.parse_type(); + let errors = cst.errors().collect::>(); + assert_eq!(errors.len(), 0); + ``` + +[lrlna]: https://github.com/lrlna +[pull/718]: https://github.com/apollographql/apollo-rs/pull/718 +[issue/715]: https://github.com/apollographql/apollo-rs/issues/715 +[fieldtype]: https://specs.apollo.dev/join/v0.3/#@field + # [0.7.3]([unreleased](https://crates.io/crates/apollo-parser/0.7.3)) - 2023-11-07 ## Fixes