From 4b3d9ae9789b2bb59748de620978b836257bb513 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Tue, 25 Nov 2025 06:20:18 -0800 Subject: [PATCH 1/9] feat(compiler): add ExecutableDocumentBuilder for multi-file support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add ExecutableDocumentBuilder following the same pattern as SchemaBuilder, enabling users to build executable documents from multiple source files. Key features: - Build executable documents from multiple GraphQL files - Proper collision detection for operations and fragments across files - Optional schema validation during building - Source tracking for all definitions - Backwards compatible with existing document_from_ast function API additions: - ExecutableDocumentBuilder struct with new(), add_ast_document(), and build() methods - ExecutableDocument::builder() convenience method - Parser::parse_into_executable_builder() for multi-file parsing Example usage: ```rust let mut builder = ExecutableDocument::builder(Some(&schema)); Parser::new().parse_into_executable_builder( Some(&schema), "query GetUser { user { id } }", "query1.graphql", &mut builder, ); Parser::new().parse_into_executable_builder( Some(&schema), "query GetPost { post { title } }", "query2.graphql", &mut builder, ); let document = builder.build().unwrap(); ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .gitignore | 4 +- .../src/executable/from_ast.rs | 329 +++++++++++++----- crates/apollo-compiler/src/executable/mod.rs | 40 +++ crates/apollo-compiler/src/parser.rs | 56 +++ 4 files changed, 332 insertions(+), 97 deletions(-) diff --git a/.gitignore b/.gitignore index 89f39f8df..375e0ef19 100644 --- a/.gitignore +++ b/.gitignore @@ -9,5 +9,7 @@ Cargo.lock # These are backup files generated by rustfmt **/*.rs.bk -# ignore IDEA fileas +# ignore IDEA files .idea + +.claude \ No newline at end of file diff --git a/crates/apollo-compiler/src/executable/from_ast.rs b/crates/apollo-compiler/src/executable/from_ast.rs index 74ce323f1..d32fdd172 100644 --- a/crates/apollo-compiler/src/executable/from_ast.rs +++ b/crates/apollo-compiler/src/executable/from_ast.rs @@ -1,127 +1,264 @@ use super::*; use crate::ty; +use crate::validation::WithErrors; +use std::sync::Arc; pub(crate) struct BuildErrors<'a> { pub(crate) errors: &'a mut DiagnosticList, pub(crate) path: SelectionPath, } -pub(crate) fn document_from_ast( - schema: Option<&Schema>, - document: &ast::Document, - errors: &mut DiagnosticList, - type_system_definitions_are_errors: bool, -) -> ExecutableDocument { - let mut operations = OperationMap::default(); - let mut multiple_anonymous = false; - let mut fragments = IndexMap::with_hasher(Default::default()); - let mut errors = BuildErrors { - errors, - path: SelectionPath { - nested_fields: Vec::new(), - // overwritten: - root: ExecutableDefinitionName::AnonymousOperation(ast::OperationType::Query), - }, - }; - for definition in &document.definitions { - debug_assert!(errors.path.nested_fields.is_empty()); - match definition { - ast::Definition::OperationDefinition(operation) => { - if let Some(name) = &operation.name { - if let Some(anonymous) = &operations.anonymous { - errors.errors.push( - anonymous.location(), - BuildError::AmbiguousAnonymousOperation, - ) - } - if let Entry::Vacant(entry) = operations.named.entry(name.clone()) { - errors.path.root = ExecutableDefinitionName::NamedOperation( - operation.operation_type, - name.clone(), - ); - if let Some(op) = Operation::from_ast(schema, &mut errors, operation) { - entry.insert(operation.same_location(op)); +/// A builder for constructing an [`ExecutableDocument`] from multiple AST documents. +/// +/// This builder allows you to parse and combine executable definitions (operations and fragments) +/// from multiple source files into a single [`ExecutableDocument`]. +/// +/// # Example +/// +/// ```rust +/// use apollo_compiler::{Schema, ExecutableDocument}; +/// use apollo_compiler::parser::Parser; +/// # let schema_src = "type Query { user: User } type User { id: ID }"; +/// # let schema = Schema::parse_and_validate(schema_src, "schema.graphql").unwrap(); +/// +/// // Create a builder +/// let mut builder = ExecutableDocument::builder(Some(&schema)); +/// +/// // Add operations from multiple files +/// Parser::new().parse_into_executable_builder( +/// Some(&schema), +/// "query GetUser { user { id } }", +/// "query1.graphql", +/// &mut builder, +/// ); +/// +/// // Build the final document +/// let document = builder.build().unwrap(); +/// ``` +#[derive(Clone)] +pub struct ExecutableDocumentBuilder { + /// The executable document being built + pub(crate) document: ExecutableDocument, + /// Optional schema for type checking during build + schema: Option, + /// Accumulated diagnostics + pub(crate) errors: DiagnosticList, + /// Track if we've seen multiple anonymous operations + multiple_anonymous: bool, +} + +impl Default for ExecutableDocumentBuilder { + fn default() -> Self { + Self::new(None) + } +} + +impl ExecutableDocumentBuilder { + /// Creates a new [`ExecutableDocumentBuilder`]. + /// + /// # Arguments + /// + /// * `schema` - Optional schema for type checking. If provided, the builder will validate + /// operations and fragments against the schema while building. + pub fn new(schema: Option<&Schema>) -> Self { + Self { + document: ExecutableDocument::new(), + schema: schema.cloned(), + errors: DiagnosticList::new(Default::default()), + multiple_anonymous: false, + } + } + + /// Adds an AST document to the executable document being built. + /// + /// # Arguments + /// + /// * `document` - The AST document to add + /// * `type_system_definitions_are_errors` - If true, type system definitions (types, directives, etc.) + /// in the document will be reported as errors + pub fn add_ast_document( + &mut self, + document: &ast::Document, + type_system_definitions_are_errors: bool, + ) { + Arc::make_mut(&mut self.errors.sources) + .extend(document.sources.iter().map(|(k, v)| (*k, v.clone()))); + self.add_ast_document_not_adding_sources(document, type_system_definitions_are_errors); + } + + pub(crate) fn add_ast_document_not_adding_sources( + &mut self, + document: &ast::Document, + type_system_definitions_are_errors: bool, + ) { + let schema = self.schema.as_ref(); + let mut errors = BuildErrors { + errors: &mut self.errors, + path: SelectionPath { + nested_fields: Vec::new(), + // overwritten: + root: ExecutableDefinitionName::AnonymousOperation(ast::OperationType::Query), + }, + }; + + for definition in &document.definitions { + debug_assert!(errors.path.nested_fields.is_empty()); + match definition { + ast::Definition::OperationDefinition(operation) => { + if let Some(name) = &operation.name { + // Named operation + if let Some(anonymous) = &self.document.operations.anonymous { + errors.errors.push( + anonymous.location(), + BuildError::AmbiguousAnonymousOperation, + ) + } + if let Entry::Vacant(entry) = + self.document.operations.named.entry(name.clone()) + { + errors.path.root = ExecutableDefinitionName::NamedOperation( + operation.operation_type, + name.clone(), + ); + if let Some(op) = Operation::from_ast(schema, &mut errors, operation) { + entry.insert(operation.same_location(op)); + } else { + errors.errors.push( + operation.location(), + BuildError::UndefinedRootOperation { + operation_type: operation.operation_type.name(), + }, + ) + } } else { + let (key, _) = self + .document + .operations + .named + .get_key_value(name) + .unwrap(); errors.errors.push( - operation.location(), - BuildError::UndefinedRootOperation { - operation_type: operation.operation_type.name(), + name.location(), + BuildError::OperationNameCollision { + name_at_previous_location: key.clone(), }, + ); + } + } else { + // Anonymous operation + if let Some(previous) = &self.document.operations.anonymous { + if !self.multiple_anonymous { + self.multiple_anonymous = true; + errors.errors.push( + previous.location(), + BuildError::AmbiguousAnonymousOperation, + ) + } + errors.errors.push( + operation.location(), + BuildError::AmbiguousAnonymousOperation, ) + } else if !self.document.operations.named.is_empty() { + errors.errors.push( + operation.location(), + BuildError::AmbiguousAnonymousOperation, + ) + } else { + errors.path.root = ExecutableDefinitionName::AnonymousOperation( + operation.operation_type, + ); + if let Some(op) = Operation::from_ast(schema, &mut errors, operation) { + self.document.operations.anonymous = + Some(operation.same_location(op)); + } else { + errors.errors.push( + operation.location(), + BuildError::UndefinedRootOperation { + operation_type: operation.operation_type.name(), + }, + ) + } + } + } + } + ast::Definition::FragmentDefinition(fragment) => { + if let Entry::Vacant(entry) = + self.document.fragments.entry(fragment.name.clone()) + { + errors.path.root = + ExecutableDefinitionName::Fragment(fragment.name.clone()); + if let Some(node) = Fragment::from_ast(schema, &mut errors, fragment) { + entry.insert(fragment.same_location(node)); } } else { - let (key, _) = operations.named.get_key_value(name).unwrap(); + let (key, _) = self + .document + .fragments + .get_key_value(&fragment.name) + .unwrap(); errors.errors.push( - name.location(), - BuildError::OperationNameCollision { + fragment.name.location(), + BuildError::FragmentNameCollision { name_at_previous_location: key.clone(), }, - ); - } - } else if let Some(previous) = &operations.anonymous { - if !multiple_anonymous { - multiple_anonymous = true; - errors - .errors - .push(previous.location(), BuildError::AmbiguousAnonymousOperation) + ) } - errors.errors.push( - operation.location(), - BuildError::AmbiguousAnonymousOperation, - ) - } else if !operations.named.is_empty() { - errors.errors.push( - operation.location(), - BuildError::AmbiguousAnonymousOperation, - ) - } else { - errors.path.root = - ExecutableDefinitionName::AnonymousOperation(operation.operation_type); - if let Some(op) = Operation::from_ast(schema, &mut errors, operation) { - operations.anonymous = Some(operation.same_location(op)); - } else { + } + _ => { + if type_system_definitions_are_errors { errors.errors.push( - operation.location(), - BuildError::UndefinedRootOperation { - operation_type: operation.operation_type.name(), + definition.location(), + BuildError::TypeSystemDefinition { + name: definition.name().cloned(), + describe: definition.describe(), }, ) } } } - ast::Definition::FragmentDefinition(fragment) => { - if let Entry::Vacant(entry) = fragments.entry(fragment.name.clone()) { - errors.path.root = ExecutableDefinitionName::Fragment(fragment.name.clone()); - if let Some(node) = Fragment::from_ast(schema, &mut errors, fragment) { - entry.insert(fragment.same_location(node)); - } - } else { - let (key, _) = fragments.get_key_value(&fragment.name).unwrap(); - errors.errors.push( - fragment.name.location(), - BuildError::FragmentNameCollision { - name_at_previous_location: key.clone(), - }, - ) - } - } - _ => { - if type_system_definitions_are_errors { - errors.errors.push( - definition.location(), - BuildError::TypeSystemDefinition { - name: definition.name().cloned(), - describe: definition.describe(), - }, - ) - } - } } + + // Merge sources into the document + Arc::make_mut(&mut self.document.sources) + .extend(document.sources.iter().map(|(k, v)| (*k, v.clone()))); } + + /// Returns the executable document built from all added AST documents. + #[allow(clippy::result_large_err)] // Typically not called very often + pub fn build(self) -> Result> { + let (document, errors) = self.build_inner(); + errors.into_result_with(document) + } + + pub(crate) fn build_inner(mut self) -> (ExecutableDocument, DiagnosticList) { + self.document.sources = self.errors.sources.clone(); + (self.document, self.errors) + } +} + +pub(crate) fn document_from_ast( + schema: Option<&Schema>, + document: &ast::Document, + errors: &mut DiagnosticList, + type_system_definitions_are_errors: bool, +) -> ExecutableDocument { + // Use the builder internally but maintain the same API + let mut builder = ExecutableDocumentBuilder { + document: ExecutableDocument::new(), + schema: schema.cloned(), + errors: std::mem::replace(errors, DiagnosticList::new(Default::default())), + multiple_anonymous: false, + }; + + builder.add_ast_document_not_adding_sources(document, type_system_definitions_are_errors); + + let (doc, new_errors) = builder.build_inner(); + *errors = new_errors; + ExecutableDocument { sources: document.sources.clone(), - operations, - fragments, + operations: doc.operations, + fragments: doc.fragments, } } diff --git a/crates/apollo-compiler/src/executable/mod.rs b/crates/apollo-compiler/src/executable/mod.rs index 36ff1b168..f1093f0a8 100644 --- a/crates/apollo-compiler/src/executable/mod.rs +++ b/crates/apollo-compiler/src/executable/mod.rs @@ -70,6 +70,7 @@ pub(crate) mod from_ast; mod serialize; pub(crate) mod validation; +pub use self::from_ast::ExecutableDocumentBuilder; pub use crate::ast::Argument; use crate::ast::ArgumentByNameError; pub use crate::ast::Directive; @@ -348,6 +349,45 @@ impl ExecutableDocument { Self::default() } + /// Returns a new builder for creating an ExecutableDocument from multiple AST documents. + /// + /// The builder allows you to parse and combine executable definitions (operations and fragments) + /// from multiple source files into a single [`ExecutableDocument`]. + /// + /// # Arguments + /// + /// * `schema` - Optional schema for type checking. If provided, the builder will validate + /// operations and fragments against the schema while building. + /// + /// # Example + /// + /// ```rust + /// use apollo_compiler::{Schema, ExecutableDocument}; + /// use apollo_compiler::parser::Parser; + /// # let schema_src = "type Query { user: User, post: Post } type User { id: ID } type Post { title: String }"; + /// # let schema = Schema::parse_and_validate(schema_src, "schema.graphql").unwrap(); + /// + /// let mut builder = ExecutableDocument::builder(Some(&schema)); + /// + /// Parser::new().parse_into_executable_builder( + /// Some(&schema), + /// "query GetUser { user { id } }", + /// "query1.graphql", + /// &mut builder, + /// ); + /// Parser::new().parse_into_executable_builder( + /// Some(&schema), + /// "query GetPost { post { title } }", + /// "query2.graphql", + /// &mut builder, + /// ); + /// + /// let document = builder.build().unwrap(); + /// ``` + pub fn builder(schema: Option<&Valid>) -> from_ast::ExecutableDocumentBuilder { + from_ast::ExecutableDocumentBuilder::new(schema.map(|s| s.as_ref())) + } + /// Parse an executable document with the default configuration. /// /// `path` is the filesystem path (or arbitrary string) used in diagnostics diff --git a/crates/apollo-compiler/src/parser.rs b/crates/apollo-compiler/src/parser.rs index 4b9ed8c02..25bb481cc 100644 --- a/crates/apollo-compiler/src/parser.rs +++ b/crates/apollo-compiler/src/parser.rs @@ -258,6 +258,62 @@ impl Parser { errors.into_result_with(document) } + /// Parse the given source text as an additional input to an executable document builder. + /// + /// `path` is the filesystem path (or arbitrary string) used in diagnostics + /// to identify this source file to users. + /// + /// This can be used to build an executable document from multiple source files. + /// + /// # Arguments + /// + /// * `schema` - Optional schema for type checking. If provided, operations and fragments + /// will be validated against the schema while building. + /// * `source_text` - The GraphQL source text to parse + /// * `path` - Path used in diagnostics to identify this source file + /// * `builder` - The builder to add parsed definitions to + /// + /// # Example + /// + /// ```rust + /// use apollo_compiler::{Schema, ExecutableDocument}; + /// use apollo_compiler::parser::Parser; + /// # let schema_src = "type Query { user: User, post: Post } type User { id: ID } type Post { title: String }"; + /// # let schema = Schema::parse_and_validate(schema_src, "schema.graphql").unwrap(); + /// + /// let mut builder = ExecutableDocument::builder(Some(&schema)); + /// let mut parser = Parser::new(); + /// + /// parser.parse_into_executable_builder( + /// Some(&schema), + /// "query GetUser { user { id } }", + /// "query1.graphql", + /// &mut builder, + /// ); + /// parser.parse_into_executable_builder( + /// Some(&schema), + /// "query GetPost { post { title } }", + /// "query2.graphql", + /// &mut builder, + /// ); + /// + /// let document = builder.build().unwrap(); + /// ``` + /// + /// Errors (if any) are recorded in the builder and returned by + /// [`ExecutableDocumentBuilder::build`]. + pub fn parse_into_executable_builder( + &mut self, + _schema: Option<&Valid>, + source_text: impl Into, + path: impl AsRef, + builder: &mut executable::ExecutableDocumentBuilder, + ) { + let ast = self.parse_ast_inner(source_text, path, FileId::new(), &mut builder.errors); + let type_system_definitions_are_errors = true; + builder.add_ast_document(&ast, type_system_definitions_are_errors); + } + pub(crate) fn parse_executable_inner( &mut self, schema: &Valid, From 04b96227c20c56637afb4ef56c5c022531be20df Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Tue, 25 Nov 2025 06:44:00 -0800 Subject: [PATCH 2/9] test(compiler): add comprehensive tests for ExecutableDocumentBuilder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add 8 new tests covering: - Building documents from multiple files - Combining queries and fragments from separate files - Operation name collision detection across files - Fragment name collision detection across files - Building without a schema (validation-free mode) - Source information preservation - Anonymous/named operation mixing (error case) - Multiple fragments in single query All tests pass successfully, verifying the builder handles multi-file scenarios correctly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../src/executable/from_ast.rs | 8 +- crates/apollo-compiler/tests/executable.rs | 268 ++++++++++++++++++ 2 files changed, 270 insertions(+), 6 deletions(-) diff --git a/crates/apollo-compiler/src/executable/from_ast.rs b/crates/apollo-compiler/src/executable/from_ast.rs index d32fdd172..790ac7e8b 100644 --- a/crates/apollo-compiler/src/executable/from_ast.rs +++ b/crates/apollo-compiler/src/executable/from_ast.rs @@ -131,12 +131,8 @@ impl ExecutableDocumentBuilder { ) } } else { - let (key, _) = self - .document - .operations - .named - .get_key_value(name) - .unwrap(); + let (key, _) = + self.document.operations.named.get_key_value(name).unwrap(); errors.errors.push( name.location(), BuildError::OperationNameCollision { diff --git a/crates/apollo-compiler/tests/executable.rs b/crates/apollo-compiler/tests/executable.rs index 828f1639e..ac8c7dce8 100644 --- a/crates/apollo-compiler/tests/executable.rs +++ b/crates/apollo-compiler/tests/executable.rs @@ -302,3 +302,271 @@ fn iter_all_fields() { ["f1", "inner", "f2", "f3"] ); } + +// ExecutableDocumentBuilder tests + +#[test] +fn builder_from_multiple_files() { + let schema_src = r#" + type Query { + user: User + post: Post + } + type User { id: ID! name: String } + type Post { id: ID! title: String } + "#; + let schema = Schema::parse_and_validate(schema_src, "schema.graphql").unwrap(); + + let query1 = "query GetUser { user { id name } }"; + let query2 = "query GetPost { post { id title } }"; + + let mut builder = ExecutableDocument::builder(Some(&schema)); + Parser::new().parse_into_executable_builder( + Some(&schema), + query1, + "query1.graphql", + &mut builder, + ); + Parser::new().parse_into_executable_builder( + Some(&schema), + query2, + "query2.graphql", + &mut builder, + ); + + let doc = builder.build().unwrap(); + + assert_eq!(doc.operations.named.len(), 2); + assert!(doc.operations.named.contains_key("GetUser")); + assert!(doc.operations.named.contains_key("GetPost")); +} + +#[test] +fn builder_with_fragments_from_multiple_files() { + let schema_src = r#" + type Query { user: User } + type User { id: ID! name: String email: String } + "#; + let schema = Schema::parse_and_validate(schema_src, "schema.graphql").unwrap(); + + let query = r#" + query GetUser { user { ...UserFields } } + "#; + let fragment = r#" + fragment UserFields on User { id name email } + "#; + + let mut builder = ExecutableDocument::builder(Some(&schema)); + Parser::new().parse_into_executable_builder( + Some(&schema), + query, + "query.graphql", + &mut builder, + ); + Parser::new().parse_into_executable_builder( + Some(&schema), + fragment, + "fragment.graphql", + &mut builder, + ); + + let doc = builder.build().unwrap(); + + assert_eq!(doc.operations.named.len(), 1); + assert_eq!(doc.fragments.len(), 1); + assert!(doc.operations.named.contains_key("GetUser")); + assert!(doc.fragments.contains_key("UserFields")); +} + +#[test] +fn builder_detects_operation_name_collision() { + let schema_src = "type Query { field: String }"; + let schema = Schema::parse_and_validate(schema_src, "schema.graphql").unwrap(); + + let query1 = "query GetData { field }"; + let query2 = "query GetData { field }"; + + let mut builder = ExecutableDocument::builder(Some(&schema)); + Parser::new().parse_into_executable_builder( + Some(&schema), + query1, + "query1.graphql", + &mut builder, + ); + Parser::new().parse_into_executable_builder( + Some(&schema), + query2, + "query2.graphql", + &mut builder, + ); + + let result = builder.build(); + assert!(result.is_err()); + + let err = result.unwrap_err(); + let error_messages: Vec = err.errors.iter().map(|e| e.error.to_string()).collect(); + + assert!(error_messages + .iter() + .any(|msg| msg.contains("GetData") && msg.contains("multiple times"))); +} + +#[test] +fn builder_detects_fragment_name_collision() { + let schema_src = "type Query { user: User } type User { id: ID }"; + let schema = Schema::parse_and_validate(schema_src, "schema.graphql").unwrap(); + + let fragment1 = "fragment UserData on User { id }"; + let fragment2 = "fragment UserData on User { id }"; + + let mut builder = ExecutableDocument::builder(Some(&schema)); + Parser::new().parse_into_executable_builder( + Some(&schema), + fragment1, + "fragment1.graphql", + &mut builder, + ); + Parser::new().parse_into_executable_builder( + Some(&schema), + fragment2, + "fragment2.graphql", + &mut builder, + ); + + let result = builder.build(); + assert!(result.is_err()); + + let err = result.unwrap_err(); + let error_messages: Vec = err.errors.iter().map(|e| e.error.to_string()).collect(); + + assert!(error_messages + .iter() + .any(|msg| msg.contains("UserData") && msg.contains("multiple times"))); +} + +#[test] +fn builder_without_schema() { + let query1 = "query GetData { field }"; + let query2 = "query GetMore { other }"; + + let mut builder = ExecutableDocument::builder(None); + Parser::new().parse_into_executable_builder(None, query1, "query1.graphql", &mut builder); + Parser::new().parse_into_executable_builder(None, query2, "query2.graphql", &mut builder); + + let doc = builder.build().unwrap(); + + assert_eq!(doc.operations.named.len(), 2); + assert!(doc.operations.named.contains_key("GetData")); + assert!(doc.operations.named.contains_key("GetMore")); +} + +#[test] +fn builder_preserves_source_information() { + let schema_src = "type Query { field: String }"; + let schema = Schema::parse_and_validate(schema_src, "schema.graphql").unwrap(); + + let query1 = "query Q1 { field }"; + let query2 = "query Q2 { field }"; + + let mut builder = ExecutableDocument::builder(Some(&schema)); + Parser::new().parse_into_executable_builder( + Some(&schema), + query1, + "query1.graphql", + &mut builder, + ); + Parser::new().parse_into_executable_builder( + Some(&schema), + query2, + "query2.graphql", + &mut builder, + ); + + let doc = builder.build().unwrap(); + + // Verify that source information is tracked + assert_eq!(doc.sources.len(), 2); + assert_eq!(doc.sources[0].path(), "query1.graphql"); + assert_eq!(doc.sources[1].path(), "query2.graphql"); +} + +#[test] +fn builder_handles_anonymous_and_named_operations() { + let schema_src = "type Query { field: String }"; + let schema = Schema::parse_and_validate(schema_src, "schema.graphql").unwrap(); + + let anonymous = "{ field }"; + let named = "query GetData { field }"; + + let mut builder = ExecutableDocument::builder(Some(&schema)); + Parser::new().parse_into_executable_builder( + Some(&schema), + anonymous, + "anonymous.graphql", + &mut builder, + ); + Parser::new().parse_into_executable_builder( + Some(&schema), + named, + "named.graphql", + &mut builder, + ); + + let result = builder.build(); + // Should error because mixing anonymous and named operations is ambiguous + assert!(result.is_err()); +} + +#[test] +fn builder_with_multiple_fragments_used_in_query() { + let schema_src = r#" + type Query { user: User } + type User { + id: ID! + profile: Profile + settings: Settings + } + type Profile { name: String bio: String } + type Settings { theme: String notifications: Boolean } + "#; + let schema = Schema::parse_and_validate(schema_src, "schema.graphql").unwrap(); + + let query = r#" + query GetUser { + user { + id + ...ProfileFields + ...SettingsFields + } + } + "#; + let profile_fragment = "fragment ProfileFields on Profile { name bio }"; + let settings_fragment = "fragment SettingsFields on Settings { theme notifications }"; + + let mut builder = ExecutableDocument::builder(Some(&schema)); + Parser::new().parse_into_executable_builder( + Some(&schema), + query, + "query.graphql", + &mut builder, + ); + Parser::new().parse_into_executable_builder( + Some(&schema), + profile_fragment, + "profile.graphql", + &mut builder, + ); + Parser::new().parse_into_executable_builder( + Some(&schema), + settings_fragment, + "settings.graphql", + &mut builder, + ); + + let doc = builder.build().unwrap(); + + assert_eq!(doc.operations.named.len(), 1); + assert_eq!(doc.fragments.len(), 2); + assert!(doc.fragments.contains_key("ProfileFields")); + assert!(doc.fragments.contains_key("SettingsFields")); +} From f71a00e66dc68fef56b8dab92ae0b913d6b1469f Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Tue, 25 Nov 2025 16:09:29 -0800 Subject: [PATCH 3/9] Don't clone schema, just hold a reference to it --- .../src/executable/from_ast.rs | 25 +++++++++++-------- crates/apollo-compiler/src/executable/mod.rs | 2 +- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/crates/apollo-compiler/src/executable/from_ast.rs b/crates/apollo-compiler/src/executable/from_ast.rs index 790ac7e8b..08cdb7f2a 100644 --- a/crates/apollo-compiler/src/executable/from_ast.rs +++ b/crates/apollo-compiler/src/executable/from_ast.rs @@ -36,34 +36,34 @@ pub(crate) struct BuildErrors<'a> { /// let document = builder.build().unwrap(); /// ``` #[derive(Clone)] -pub struct ExecutableDocumentBuilder { +pub struct ExecutableDocumentBuilder<'schema> { /// The executable document being built pub(crate) document: ExecutableDocument, /// Optional schema for type checking during build - schema: Option, + schema: Option<&'schema Schema>, /// Accumulated diagnostics pub(crate) errors: DiagnosticList, /// Track if we've seen multiple anonymous operations multiple_anonymous: bool, } -impl Default for ExecutableDocumentBuilder { +impl Default for ExecutableDocumentBuilder<'_> { fn default() -> Self { Self::new(None) } } -impl ExecutableDocumentBuilder { +impl<'schema> ExecutableDocumentBuilder<'schema> { /// Creates a new [`ExecutableDocumentBuilder`]. /// /// # Arguments /// /// * `schema` - Optional schema for type checking. If provided, the builder will validate /// operations and fragments against the schema while building. - pub fn new(schema: Option<&Schema>) -> Self { + pub fn new(schema: Option<&'schema Schema>) -> Self { Self { document: ExecutableDocument::new(), - schema: schema.cloned(), + schema, errors: DiagnosticList::new(Default::default()), multiple_anonymous: false, } @@ -91,7 +91,6 @@ impl ExecutableDocumentBuilder { document: &ast::Document, type_system_definitions_are_errors: bool, ) { - let schema = self.schema.as_ref(); let mut errors = BuildErrors { errors: &mut self.errors, path: SelectionPath { @@ -120,7 +119,9 @@ impl ExecutableDocumentBuilder { operation.operation_type, name.clone(), ); - if let Some(op) = Operation::from_ast(schema, &mut errors, operation) { + if let Some(op) = + Operation::from_ast(self.schema, &mut errors, operation) + { entry.insert(operation.same_location(op)); } else { errors.errors.push( @@ -163,7 +164,9 @@ impl ExecutableDocumentBuilder { errors.path.root = ExecutableDefinitionName::AnonymousOperation( operation.operation_type, ); - if let Some(op) = Operation::from_ast(schema, &mut errors, operation) { + if let Some(op) = + Operation::from_ast(self.schema, &mut errors, operation) + { self.document.operations.anonymous = Some(operation.same_location(op)); } else { @@ -183,7 +186,7 @@ impl ExecutableDocumentBuilder { { errors.path.root = ExecutableDefinitionName::Fragment(fragment.name.clone()); - if let Some(node) = Fragment::from_ast(schema, &mut errors, fragment) { + if let Some(node) = Fragment::from_ast(self.schema, &mut errors, fragment) { entry.insert(fragment.same_location(node)); } } else { @@ -241,7 +244,7 @@ pub(crate) fn document_from_ast( // Use the builder internally but maintain the same API let mut builder = ExecutableDocumentBuilder { document: ExecutableDocument::new(), - schema: schema.cloned(), + schema, errors: std::mem::replace(errors, DiagnosticList::new(Default::default())), multiple_anonymous: false, }; diff --git a/crates/apollo-compiler/src/executable/mod.rs b/crates/apollo-compiler/src/executable/mod.rs index f1093f0a8..85813b8ed 100644 --- a/crates/apollo-compiler/src/executable/mod.rs +++ b/crates/apollo-compiler/src/executable/mod.rs @@ -384,7 +384,7 @@ impl ExecutableDocument { /// /// let document = builder.build().unwrap(); /// ``` - pub fn builder(schema: Option<&Valid>) -> from_ast::ExecutableDocumentBuilder { + pub fn builder(schema: Option<&Valid>) -> from_ast::ExecutableDocumentBuilder<'_> { from_ast::ExecutableDocumentBuilder::new(schema.map(|s| s.as_ref())) } From 6da8915db3f4cdf7d40955f0a331f16c34d969a3 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Tue, 25 Nov 2025 16:18:03 -0800 Subject: [PATCH 4/9] tidy comments and revert if / elseif change --- .../src/executable/from_ast.rs | 63 ++++++++----------- 1 file changed, 26 insertions(+), 37 deletions(-) diff --git a/crates/apollo-compiler/src/executable/from_ast.rs b/crates/apollo-compiler/src/executable/from_ast.rs index 08cdb7f2a..7e25800d4 100644 --- a/crates/apollo-compiler/src/executable/from_ast.rs +++ b/crates/apollo-compiler/src/executable/from_ast.rs @@ -105,7 +105,6 @@ impl<'schema> ExecutableDocumentBuilder<'schema> { match definition { ast::Definition::OperationDefinition(operation) => { if let Some(name) = &operation.name { - // Named operation if let Some(anonymous) = &self.document.operations.anonymous { errors.errors.push( anonymous.location(), @@ -141,42 +140,34 @@ impl<'schema> ExecutableDocumentBuilder<'schema> { }, ); } + } else if let Some(previous) = &self.document.operations.anonymous { + if !self.multiple_anonymous { + self.multiple_anonymous = true; + errors + .errors + .push(previous.location(), BuildError::AmbiguousAnonymousOperation) + } + errors.errors.push( + operation.location(), + BuildError::AmbiguousAnonymousOperation, + ) + } else if !self.document.operations.named.is_empty() { + errors.errors.push( + operation.location(), + BuildError::AmbiguousAnonymousOperation, + ) } else { - // Anonymous operation - if let Some(previous) = &self.document.operations.anonymous { - if !self.multiple_anonymous { - self.multiple_anonymous = true; - errors.errors.push( - previous.location(), - BuildError::AmbiguousAnonymousOperation, - ) - } - errors.errors.push( - operation.location(), - BuildError::AmbiguousAnonymousOperation, - ) - } else if !self.document.operations.named.is_empty() { + errors.path.root = + ExecutableDefinitionName::AnonymousOperation(operation.operation_type); + if let Some(op) = Operation::from_ast(self.schema, &mut errors, operation) { + self.document.operations.anonymous = Some(operation.same_location(op)); + } else { errors.errors.push( operation.location(), - BuildError::AmbiguousAnonymousOperation, + BuildError::UndefinedRootOperation { + operation_type: operation.operation_type.name(), + }, ) - } else { - errors.path.root = ExecutableDefinitionName::AnonymousOperation( - operation.operation_type, - ); - if let Some(op) = - Operation::from_ast(self.schema, &mut errors, operation) - { - self.document.operations.anonymous = - Some(operation.same_location(op)); - } else { - errors.errors.push( - operation.location(), - BuildError::UndefinedRootOperation { - operation_type: operation.operation_type.name(), - }, - ) - } } } } @@ -217,7 +208,6 @@ impl<'schema> ExecutableDocumentBuilder<'schema> { } } - // Merge sources into the document Arc::make_mut(&mut self.document.sources) .extend(document.sources.iter().map(|(k, v)| (*k, v.clone()))); } @@ -241,7 +231,6 @@ pub(crate) fn document_from_ast( errors: &mut DiagnosticList, type_system_definitions_are_errors: bool, ) -> ExecutableDocument { - // Use the builder internally but maintain the same API let mut builder = ExecutableDocumentBuilder { document: ExecutableDocument::new(), schema, @@ -270,7 +259,7 @@ impl Operation { let ty = if let Some(s) = schema { s.root_operation(ast.operation_type)?.clone() } else { - // Hack for validate_standalone_excutable + // Hack for validate_standalone_executable ast.operation_type.default_type_name().clone() }; let mut selection_set = SelectionSet::new(ty); @@ -388,7 +377,7 @@ impl SelectionSet { ) } Err(schema::FieldLookupError::NoSuchType) => { - // `self.ty` is the name of a type not definied in the schema. + // `self.ty` is the name of a type not defined in the schema. // It can come from: // * A root operation type, or a field definition: // the schema is invalid, no need to record another error here. From 8296091740ad26cbc429e72c22e1cf112b50b12a Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Tue, 25 Nov 2025 16:48:17 -0800 Subject: [PATCH 5/9] builder takes a mutable reference to DiagnosticList --- .../src/executable/from_ast.rs | 47 ++++++--------- crates/apollo-compiler/src/executable/mod.rs | 16 +++-- crates/apollo-compiler/src/parser.rs | 7 ++- crates/apollo-compiler/tests/executable.rs | 58 +++++++++++-------- 4 files changed, 70 insertions(+), 58 deletions(-) diff --git a/crates/apollo-compiler/src/executable/from_ast.rs b/crates/apollo-compiler/src/executable/from_ast.rs index 7e25800d4..b72d9847e 100644 --- a/crates/apollo-compiler/src/executable/from_ast.rs +++ b/crates/apollo-compiler/src/executable/from_ast.rs @@ -1,6 +1,5 @@ use super::*; use crate::ty; -use crate::validation::WithErrors; use std::sync::Arc; pub(crate) struct BuildErrors<'a> { @@ -18,11 +17,13 @@ pub(crate) struct BuildErrors<'a> { /// ```rust /// use apollo_compiler::{Schema, ExecutableDocument}; /// use apollo_compiler::parser::Parser; +/// use apollo_compiler::validation::DiagnosticList; /// # let schema_src = "type Query { user: User } type User { id: ID }"; /// # let schema = Schema::parse_and_validate(schema_src, "schema.graphql").unwrap(); /// /// // Create a builder -/// let mut builder = ExecutableDocument::builder(Some(&schema)); +/// let mut errors = DiagnosticList::new(Default::default()); +/// let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); /// /// // Add operations from multiple files /// Parser::new().parse_into_executable_builder( @@ -33,38 +34,34 @@ pub(crate) struct BuildErrors<'a> { /// ); /// /// // Build the final document -/// let document = builder.build().unwrap(); +/// let document = builder.build(); +/// // Check for errors +/// assert!(errors.is_empty()); /// ``` -#[derive(Clone)] -pub struct ExecutableDocumentBuilder<'schema> { +pub struct ExecutableDocumentBuilder<'schema, 'errors> { /// The executable document being built pub(crate) document: ExecutableDocument, /// Optional schema for type checking during build schema: Option<&'schema Schema>, /// Accumulated diagnostics - pub(crate) errors: DiagnosticList, + pub(crate) errors: &'errors mut DiagnosticList, /// Track if we've seen multiple anonymous operations multiple_anonymous: bool, } -impl Default for ExecutableDocumentBuilder<'_> { - fn default() -> Self { - Self::new(None) - } -} - -impl<'schema> ExecutableDocumentBuilder<'schema> { +impl<'schema, 'errors> ExecutableDocumentBuilder<'schema, 'errors> { /// Creates a new [`ExecutableDocumentBuilder`]. /// /// # Arguments /// /// * `schema` - Optional schema for type checking. If provided, the builder will validate /// operations and fragments against the schema while building. - pub fn new(schema: Option<&'schema Schema>) -> Self { + /// * `errors` - Mutable reference to a DiagnosticList where errors will be accumulated + pub fn new(schema: Option<&'schema Schema>, errors: &'errors mut DiagnosticList) -> Self { Self { document: ExecutableDocument::new(), schema, - errors: DiagnosticList::new(Default::default()), + errors, multiple_anonymous: false, } } @@ -213,15 +210,13 @@ impl<'schema> ExecutableDocumentBuilder<'schema> { } /// Returns the executable document built from all added AST documents. - #[allow(clippy::result_large_err)] // Typically not called very often - pub fn build(self) -> Result> { - let (document, errors) = self.build_inner(); - errors.into_result_with(document) + pub fn build(self) -> ExecutableDocument { + self.build_inner() } - pub(crate) fn build_inner(mut self) -> (ExecutableDocument, DiagnosticList) { + pub(crate) fn build_inner(mut self) -> ExecutableDocument { self.document.sources = self.errors.sources.clone(); - (self.document, self.errors) + self.document } } @@ -231,17 +226,11 @@ pub(crate) fn document_from_ast( errors: &mut DiagnosticList, type_system_definitions_are_errors: bool, ) -> ExecutableDocument { - let mut builder = ExecutableDocumentBuilder { - document: ExecutableDocument::new(), - schema, - errors: std::mem::replace(errors, DiagnosticList::new(Default::default())), - multiple_anonymous: false, - }; + let mut builder = ExecutableDocumentBuilder::new(schema, errors); builder.add_ast_document_not_adding_sources(document, type_system_definitions_are_errors); - let (doc, new_errors) = builder.build_inner(); - *errors = new_errors; + let doc = builder.build_inner(); ExecutableDocument { sources: document.sources.clone(), diff --git a/crates/apollo-compiler/src/executable/mod.rs b/crates/apollo-compiler/src/executable/mod.rs index 85813b8ed..874bbb134 100644 --- a/crates/apollo-compiler/src/executable/mod.rs +++ b/crates/apollo-compiler/src/executable/mod.rs @@ -358,16 +358,19 @@ impl ExecutableDocument { /// /// * `schema` - Optional schema for type checking. If provided, the builder will validate /// operations and fragments against the schema while building. + /// * `errors` - Mutable reference to a DiagnosticList where errors will be accumulated /// /// # Example /// /// ```rust /// use apollo_compiler::{Schema, ExecutableDocument}; /// use apollo_compiler::parser::Parser; + /// use apollo_compiler::validation::DiagnosticList; /// # let schema_src = "type Query { user: User, post: Post } type User { id: ID } type Post { title: String }"; /// # let schema = Schema::parse_and_validate(schema_src, "schema.graphql").unwrap(); /// - /// let mut builder = ExecutableDocument::builder(Some(&schema)); + /// let mut errors = DiagnosticList::new(Default::default()); + /// let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); /// /// Parser::new().parse_into_executable_builder( /// Some(&schema), @@ -382,10 +385,15 @@ impl ExecutableDocument { /// &mut builder, /// ); /// - /// let document = builder.build().unwrap(); + /// let document = builder.build(); + /// // Check for errors + /// assert!(errors.is_empty()); /// ``` - pub fn builder(schema: Option<&Valid>) -> from_ast::ExecutableDocumentBuilder<'_> { - from_ast::ExecutableDocumentBuilder::new(schema.map(|s| s.as_ref())) + pub fn builder<'schema, 'errors>( + schema: Option<&'schema Valid>, + errors: &'errors mut DiagnosticList, + ) -> from_ast::ExecutableDocumentBuilder<'schema, 'errors> { + from_ast::ExecutableDocumentBuilder::new(schema.map(|s| s.as_ref()), errors) } /// Parse an executable document with the default configuration. diff --git a/crates/apollo-compiler/src/parser.rs b/crates/apollo-compiler/src/parser.rs index 25bb481cc..6697cf796 100644 --- a/crates/apollo-compiler/src/parser.rs +++ b/crates/apollo-compiler/src/parser.rs @@ -278,10 +278,12 @@ impl Parser { /// ```rust /// use apollo_compiler::{Schema, ExecutableDocument}; /// use apollo_compiler::parser::Parser; + /// use apollo_compiler::validation::DiagnosticList; /// # let schema_src = "type Query { user: User, post: Post } type User { id: ID } type Post { title: String }"; /// # let schema = Schema::parse_and_validate(schema_src, "schema.graphql").unwrap(); /// - /// let mut builder = ExecutableDocument::builder(Some(&schema)); + /// let mut errors = DiagnosticList::new(Default::default()); + /// let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); /// let mut parser = Parser::new(); /// /// parser.parse_into_executable_builder( @@ -297,7 +299,8 @@ impl Parser { /// &mut builder, /// ); /// - /// let document = builder.build().unwrap(); + /// let document = builder.build(); + /// assert!(errors.is_empty()); /// ``` /// /// Errors (if any) are recorded in the builder and returned by diff --git a/crates/apollo-compiler/tests/executable.rs b/crates/apollo-compiler/tests/executable.rs index ac8c7dce8..867aa2f75 100644 --- a/crates/apollo-compiler/tests/executable.rs +++ b/crates/apollo-compiler/tests/executable.rs @@ -1,4 +1,5 @@ use apollo_compiler::parser::Parser; +use apollo_compiler::validation::DiagnosticList; use apollo_compiler::ExecutableDocument; use apollo_compiler::Schema; @@ -320,7 +321,8 @@ fn builder_from_multiple_files() { let query1 = "query GetUser { user { id name } }"; let query2 = "query GetPost { post { id title } }"; - let mut builder = ExecutableDocument::builder(Some(&schema)); + let mut errors = DiagnosticList::new(Default::default()); + let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); Parser::new().parse_into_executable_builder( Some(&schema), query1, @@ -334,7 +336,8 @@ fn builder_from_multiple_files() { &mut builder, ); - let doc = builder.build().unwrap(); + let doc = builder.build(); + assert!(errors.is_empty(), "Expected no errors, got: {}", errors); assert_eq!(doc.operations.named.len(), 2); assert!(doc.operations.named.contains_key("GetUser")); @@ -356,7 +359,8 @@ fn builder_with_fragments_from_multiple_files() { fragment UserFields on User { id name email } "#; - let mut builder = ExecutableDocument::builder(Some(&schema)); + let mut errors = DiagnosticList::new(Default::default()); + let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); Parser::new().parse_into_executable_builder( Some(&schema), query, @@ -370,7 +374,8 @@ fn builder_with_fragments_from_multiple_files() { &mut builder, ); - let doc = builder.build().unwrap(); + let doc = builder.build(); + assert!(errors.is_empty(), "Expected no errors, got: {}", errors); assert_eq!(doc.operations.named.len(), 1); assert_eq!(doc.fragments.len(), 1); @@ -386,7 +391,8 @@ fn builder_detects_operation_name_collision() { let query1 = "query GetData { field }"; let query2 = "query GetData { field }"; - let mut builder = ExecutableDocument::builder(Some(&schema)); + let mut errors = DiagnosticList::new(Default::default()); + let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); Parser::new().parse_into_executable_builder( Some(&schema), query1, @@ -400,11 +406,10 @@ fn builder_detects_operation_name_collision() { &mut builder, ); - let result = builder.build(); - assert!(result.is_err()); + let _doc = builder.build(); + assert!(!errors.is_empty(), "Expected errors for operation name collision"); - let err = result.unwrap_err(); - let error_messages: Vec = err.errors.iter().map(|e| e.error.to_string()).collect(); + let error_messages: Vec = errors.iter().map(|e| e.error.to_string()).collect(); assert!(error_messages .iter() @@ -419,7 +424,8 @@ fn builder_detects_fragment_name_collision() { let fragment1 = "fragment UserData on User { id }"; let fragment2 = "fragment UserData on User { id }"; - let mut builder = ExecutableDocument::builder(Some(&schema)); + let mut errors = DiagnosticList::new(Default::default()); + let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); Parser::new().parse_into_executable_builder( Some(&schema), fragment1, @@ -433,11 +439,10 @@ fn builder_detects_fragment_name_collision() { &mut builder, ); - let result = builder.build(); - assert!(result.is_err()); + let _doc = builder.build(); + assert!(!errors.is_empty(), "Expected errors for fragment name collision"); - let err = result.unwrap_err(); - let error_messages: Vec = err.errors.iter().map(|e| e.error.to_string()).collect(); + let error_messages: Vec = errors.iter().map(|e| e.error.to_string()).collect(); assert!(error_messages .iter() @@ -449,11 +454,13 @@ fn builder_without_schema() { let query1 = "query GetData { field }"; let query2 = "query GetMore { other }"; - let mut builder = ExecutableDocument::builder(None); + let mut errors = DiagnosticList::new(Default::default()); + let mut builder = ExecutableDocument::builder(None, &mut errors); Parser::new().parse_into_executable_builder(None, query1, "query1.graphql", &mut builder); Parser::new().parse_into_executable_builder(None, query2, "query2.graphql", &mut builder); - let doc = builder.build().unwrap(); + let doc = builder.build(); + assert!(errors.is_empty(), "Expected no errors, got: {}", errors); assert_eq!(doc.operations.named.len(), 2); assert!(doc.operations.named.contains_key("GetData")); @@ -468,7 +475,8 @@ fn builder_preserves_source_information() { let query1 = "query Q1 { field }"; let query2 = "query Q2 { field }"; - let mut builder = ExecutableDocument::builder(Some(&schema)); + let mut errors = DiagnosticList::new(Default::default()); + let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); Parser::new().parse_into_executable_builder( Some(&schema), query1, @@ -482,7 +490,8 @@ fn builder_preserves_source_information() { &mut builder, ); - let doc = builder.build().unwrap(); + let doc = builder.build(); + assert!(errors.is_empty(), "Expected no errors, got: {}", errors); // Verify that source information is tracked assert_eq!(doc.sources.len(), 2); @@ -498,7 +507,8 @@ fn builder_handles_anonymous_and_named_operations() { let anonymous = "{ field }"; let named = "query GetData { field }"; - let mut builder = ExecutableDocument::builder(Some(&schema)); + let mut errors = DiagnosticList::new(Default::default()); + let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); Parser::new().parse_into_executable_builder( Some(&schema), anonymous, @@ -512,9 +522,9 @@ fn builder_handles_anonymous_and_named_operations() { &mut builder, ); - let result = builder.build(); + let _doc = builder.build(); // Should error because mixing anonymous and named operations is ambiguous - assert!(result.is_err()); + assert!(!errors.is_empty(), "Expected errors for mixing anonymous and named operations"); } #[test] @@ -543,7 +553,8 @@ fn builder_with_multiple_fragments_used_in_query() { let profile_fragment = "fragment ProfileFields on Profile { name bio }"; let settings_fragment = "fragment SettingsFields on Settings { theme notifications }"; - let mut builder = ExecutableDocument::builder(Some(&schema)); + let mut errors = DiagnosticList::new(Default::default()); + let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); Parser::new().parse_into_executable_builder( Some(&schema), query, @@ -563,7 +574,8 @@ fn builder_with_multiple_fragments_used_in_query() { &mut builder, ); - let doc = builder.build().unwrap(); + let doc = builder.build(); + assert!(errors.is_empty(), "Expected no errors, got: {}", errors); assert_eq!(doc.operations.named.len(), 1); assert_eq!(doc.fragments.len(), 2); From 95cb5010ff6db44d4973e4bc4109d27e30b64e80 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Tue, 25 Nov 2025 16:51:29 -0800 Subject: [PATCH 6/9] cleanup unused param --- .../src/executable/from_ast.rs | 1 - crates/apollo-compiler/src/executable/mod.rs | 2 - crates/apollo-compiler/src/parser.rs | 3 - crates/apollo-compiler/tests/executable.rs | 118 +++++------------- 4 files changed, 28 insertions(+), 96 deletions(-) diff --git a/crates/apollo-compiler/src/executable/from_ast.rs b/crates/apollo-compiler/src/executable/from_ast.rs index b72d9847e..bf45295c0 100644 --- a/crates/apollo-compiler/src/executable/from_ast.rs +++ b/crates/apollo-compiler/src/executable/from_ast.rs @@ -27,7 +27,6 @@ pub(crate) struct BuildErrors<'a> { /// /// // Add operations from multiple files /// Parser::new().parse_into_executable_builder( -/// Some(&schema), /// "query GetUser { user { id } }", /// "query1.graphql", /// &mut builder, diff --git a/crates/apollo-compiler/src/executable/mod.rs b/crates/apollo-compiler/src/executable/mod.rs index 874bbb134..7debe2d11 100644 --- a/crates/apollo-compiler/src/executable/mod.rs +++ b/crates/apollo-compiler/src/executable/mod.rs @@ -373,13 +373,11 @@ impl ExecutableDocument { /// let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); /// /// Parser::new().parse_into_executable_builder( - /// Some(&schema), /// "query GetUser { user { id } }", /// "query1.graphql", /// &mut builder, /// ); /// Parser::new().parse_into_executable_builder( - /// Some(&schema), /// "query GetPost { post { title } }", /// "query2.graphql", /// &mut builder, diff --git a/crates/apollo-compiler/src/parser.rs b/crates/apollo-compiler/src/parser.rs index 6697cf796..0f08e8e1a 100644 --- a/crates/apollo-compiler/src/parser.rs +++ b/crates/apollo-compiler/src/parser.rs @@ -287,13 +287,11 @@ impl Parser { /// let mut parser = Parser::new(); /// /// parser.parse_into_executable_builder( - /// Some(&schema), /// "query GetUser { user { id } }", /// "query1.graphql", /// &mut builder, /// ); /// parser.parse_into_executable_builder( - /// Some(&schema), /// "query GetPost { post { title } }", /// "query2.graphql", /// &mut builder, @@ -307,7 +305,6 @@ impl Parser { /// [`ExecutableDocumentBuilder::build`]. pub fn parse_into_executable_builder( &mut self, - _schema: Option<&Valid>, source_text: impl Into, path: impl AsRef, builder: &mut executable::ExecutableDocumentBuilder, diff --git a/crates/apollo-compiler/tests/executable.rs b/crates/apollo-compiler/tests/executable.rs index 867aa2f75..c8de92473 100644 --- a/crates/apollo-compiler/tests/executable.rs +++ b/crates/apollo-compiler/tests/executable.rs @@ -323,18 +323,8 @@ fn builder_from_multiple_files() { let mut errors = DiagnosticList::new(Default::default()); let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); - Parser::new().parse_into_executable_builder( - Some(&schema), - query1, - "query1.graphql", - &mut builder, - ); - Parser::new().parse_into_executable_builder( - Some(&schema), - query2, - "query2.graphql", - &mut builder, - ); + Parser::new().parse_into_executable_builder(query1, "query1.graphql", &mut builder); + Parser::new().parse_into_executable_builder(query2, "query2.graphql", &mut builder); let doc = builder.build(); assert!(errors.is_empty(), "Expected no errors, got: {}", errors); @@ -361,18 +351,8 @@ fn builder_with_fragments_from_multiple_files() { let mut errors = DiagnosticList::new(Default::default()); let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); - Parser::new().parse_into_executable_builder( - Some(&schema), - query, - "query.graphql", - &mut builder, - ); - Parser::new().parse_into_executable_builder( - Some(&schema), - fragment, - "fragment.graphql", - &mut builder, - ); + Parser::new().parse_into_executable_builder(query, "query.graphql", &mut builder); + Parser::new().parse_into_executable_builder(fragment, "fragment.graphql", &mut builder); let doc = builder.build(); assert!(errors.is_empty(), "Expected no errors, got: {}", errors); @@ -393,21 +373,14 @@ fn builder_detects_operation_name_collision() { let mut errors = DiagnosticList::new(Default::default()); let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); - Parser::new().parse_into_executable_builder( - Some(&schema), - query1, - "query1.graphql", - &mut builder, - ); - Parser::new().parse_into_executable_builder( - Some(&schema), - query2, - "query2.graphql", - &mut builder, - ); + Parser::new().parse_into_executable_builder(query1, "query1.graphql", &mut builder); + Parser::new().parse_into_executable_builder(query2, "query2.graphql", &mut builder); let _doc = builder.build(); - assert!(!errors.is_empty(), "Expected errors for operation name collision"); + assert!( + !errors.is_empty(), + "Expected errors for operation name collision" + ); let error_messages: Vec = errors.iter().map(|e| e.error.to_string()).collect(); @@ -426,21 +399,14 @@ fn builder_detects_fragment_name_collision() { let mut errors = DiagnosticList::new(Default::default()); let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); - Parser::new().parse_into_executable_builder( - Some(&schema), - fragment1, - "fragment1.graphql", - &mut builder, - ); - Parser::new().parse_into_executable_builder( - Some(&schema), - fragment2, - "fragment2.graphql", - &mut builder, - ); + Parser::new().parse_into_executable_builder(fragment1, "fragment1.graphql", &mut builder); + Parser::new().parse_into_executable_builder(fragment2, "fragment2.graphql", &mut builder); let _doc = builder.build(); - assert!(!errors.is_empty(), "Expected errors for fragment name collision"); + assert!( + !errors.is_empty(), + "Expected errors for fragment name collision" + ); let error_messages: Vec = errors.iter().map(|e| e.error.to_string()).collect(); @@ -456,8 +422,8 @@ fn builder_without_schema() { let mut errors = DiagnosticList::new(Default::default()); let mut builder = ExecutableDocument::builder(None, &mut errors); - Parser::new().parse_into_executable_builder(None, query1, "query1.graphql", &mut builder); - Parser::new().parse_into_executable_builder(None, query2, "query2.graphql", &mut builder); + Parser::new().parse_into_executable_builder(query1, "query1.graphql", &mut builder); + Parser::new().parse_into_executable_builder(query2, "query2.graphql", &mut builder); let doc = builder.build(); assert!(errors.is_empty(), "Expected no errors, got: {}", errors); @@ -477,18 +443,8 @@ fn builder_preserves_source_information() { let mut errors = DiagnosticList::new(Default::default()); let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); - Parser::new().parse_into_executable_builder( - Some(&schema), - query1, - "query1.graphql", - &mut builder, - ); - Parser::new().parse_into_executable_builder( - Some(&schema), - query2, - "query2.graphql", - &mut builder, - ); + Parser::new().parse_into_executable_builder(query1, "query1.graphql", &mut builder); + Parser::new().parse_into_executable_builder(query2, "query2.graphql", &mut builder); let doc = builder.build(); assert!(errors.is_empty(), "Expected no errors, got: {}", errors); @@ -509,22 +465,15 @@ fn builder_handles_anonymous_and_named_operations() { let mut errors = DiagnosticList::new(Default::default()); let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); - Parser::new().parse_into_executable_builder( - Some(&schema), - anonymous, - "anonymous.graphql", - &mut builder, - ); - Parser::new().parse_into_executable_builder( - Some(&schema), - named, - "named.graphql", - &mut builder, - ); + Parser::new().parse_into_executable_builder(anonymous, "anonymous.graphql", &mut builder); + Parser::new().parse_into_executable_builder(named, "named.graphql", &mut builder); let _doc = builder.build(); // Should error because mixing anonymous and named operations is ambiguous - assert!(!errors.is_empty(), "Expected errors for mixing anonymous and named operations"); + assert!( + !errors.is_empty(), + "Expected errors for mixing anonymous and named operations" + ); } #[test] @@ -555,20 +504,9 @@ fn builder_with_multiple_fragments_used_in_query() { let mut errors = DiagnosticList::new(Default::default()); let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); + Parser::new().parse_into_executable_builder(query, "query.graphql", &mut builder); + Parser::new().parse_into_executable_builder(profile_fragment, "profile.graphql", &mut builder); Parser::new().parse_into_executable_builder( - Some(&schema), - query, - "query.graphql", - &mut builder, - ); - Parser::new().parse_into_executable_builder( - Some(&schema), - profile_fragment, - "profile.graphql", - &mut builder, - ); - Parser::new().parse_into_executable_builder( - Some(&schema), settings_fragment, "settings.graphql", &mut builder, From 1e809e9badc14dc352c1be046f607cb7646b8ce9 Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Tue, 25 Nov 2025 17:03:15 -0800 Subject: [PATCH 7/9] add test for multiple diagnostic sources --- crates/apollo-compiler/tests/executable.rs | 88 ++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/crates/apollo-compiler/tests/executable.rs b/crates/apollo-compiler/tests/executable.rs index c8de92473..c3ac6a363 100644 --- a/crates/apollo-compiler/tests/executable.rs +++ b/crates/apollo-compiler/tests/executable.rs @@ -1,3 +1,4 @@ +use apollo_compiler::diagnostic::ToCliReport; use apollo_compiler::parser::Parser; use apollo_compiler::validation::DiagnosticList; use apollo_compiler::ExecutableDocument; @@ -520,3 +521,90 @@ fn builder_with_multiple_fragments_used_in_query() { assert!(doc.fragments.contains_key("ProfileFields")); assert!(doc.fragments.contains_key("SettingsFields")); } + +#[test] +fn builder_accumulates_diagnostics_from_multiple_sources() { + let schema_src = r#" + type Query { user: User } + type User { id: ID! name: String } + "#; + let schema = Schema::parse_and_validate(schema_src, "schema.graphql").unwrap(); + + let query1 = r#" + query GetUser { + user { + id + nonexistentField + } + } + "#; + + let query2 = r#" + query GetUserProfile { + user { + id + anotherUndefinedField + } + } + "#; + + let fragment1 = "fragment UserFields on User { id name }"; + let fragment2 = "fragment UserFields on User { id }"; + let fragment3 = "fragment UserName on User { name }"; + + let mut errors = DiagnosticList::new(Default::default()); + let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); + + Parser::new().parse_into_executable_builder(query1, "query1.graphql", &mut builder); + Parser::new().parse_into_executable_builder(query2, "query2.graphql", &mut builder); + Parser::new().parse_into_executable_builder(fragment1, "fragment1.graphql", &mut builder); + Parser::new().parse_into_executable_builder(fragment2, "fragment2.graphql", &mut builder); + Parser::new().parse_into_executable_builder(fragment3, "fragment3.graphql", &mut builder); + + let doc = builder.build(); + + // Verify we collected multiple errors from different sources + assert!(!errors.is_empty(), "Expected errors from multiple sources"); + assert!( + errors.len() >= 3, + "Expected at least 3 errors (2 undefined fields + 1 fragment collision), got {}", + errors.len() + ); + + let error_messages: Vec = errors.iter().map(|e| e.error.to_string()).collect(); + + assert_eq!(error_messages.len(), 3); + assert_eq!( + error_messages[0], + "type `User` does not have a field `nonexistentField`" + ); + assert_eq!( + error_messages[1], + "type `User` does not have a field `anotherUndefinedField`" + ); + assert_eq!( + error_messages[2], + "the fragment `UserFields` is defined multiple times in the document" + ); + + assert_eq!(doc.operations.named.len(), 2); + assert!(doc.operations.named.contains_key("GetUser")); + assert!(doc.operations.named.contains_key("GetUserProfile")); + + // Only 2 fragments should be present (UserFields once, UserName once) + // The duplicate UserFields from fragment2 should not overwrite fragment1 + assert_eq!(doc.fragments.len(), 2, "Expected 2 unique fragments"); + assert!(doc.fragments.contains_key("UserFields")); + assert!(doc.fragments.contains_key("UserName")); + + // Verify source tracking is correct - we should have diagnostics from multiple files + let diagnostic_sources: std::collections::HashSet<_> = errors + .iter() + .filter_map(|e| e.error.location().map(|loc| loc.file_id())) + .collect(); + + assert!( + diagnostic_sources.len() == 3, + "Errors should come from 3 different source files" + ); +} From d7c9f750984a9c789e71f7b0d3a4d5ed1cdaa95f Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Tue, 25 Nov 2025 17:14:43 -0800 Subject: [PATCH 8/9] add .parse() function --- .../src/executable/from_ast.rs | 36 ++++++++ crates/apollo-compiler/tests/executable.rs | 86 +++++++++---------- 2 files changed, 76 insertions(+), 46 deletions(-) diff --git a/crates/apollo-compiler/src/executable/from_ast.rs b/crates/apollo-compiler/src/executable/from_ast.rs index bf45295c0..f20f3008a 100644 --- a/crates/apollo-compiler/src/executable/from_ast.rs +++ b/crates/apollo-compiler/src/executable/from_ast.rs @@ -65,6 +65,42 @@ impl<'schema, 'errors> ExecutableDocumentBuilder<'schema, 'errors> { } } + /// Parses a GraphQL executable document from source text and adds it to the builder. + /// + /// This is a convenience method that creates a parser and calls + /// [`Parser::parse_into_executable_builder`](crate::parser::Parser::parse_into_executable_builder). + /// + /// # Arguments + /// + /// * `source_text` - The GraphQL source text to parse + /// * `path` - The filesystem path (or arbitrary string) used in diagnostics to identify this source file + /// + /// # Example + /// + /// ```rust + /// use apollo_compiler::{Schema, ExecutableDocument}; + /// use apollo_compiler::validation::DiagnosticList; + /// # let schema_src = "type Query { user: User } type User { id: ID }"; + /// # let schema = Schema::parse_and_validate(schema_src, "schema.graphql").unwrap(); + /// + /// let mut errors = DiagnosticList::new(Default::default()); + /// let doc = ExecutableDocument::builder(Some(&schema), &mut errors) + /// .parse("query GetUser { user { id } }", "query1.graphql") + /// .parse("query GetMore { user { id } }", "query2.graphql") + /// .build(); + /// + /// assert!(errors.is_empty()); + /// assert_eq!(doc.operations.named.len(), 2); + /// ``` + pub fn parse( + mut self, + source_text: impl Into, + path: impl AsRef, + ) -> Self { + Parser::new().parse_into_executable_builder(source_text, path, &mut self); + self + } + /// Adds an AST document to the executable document being built. /// /// # Arguments diff --git a/crates/apollo-compiler/tests/executable.rs b/crates/apollo-compiler/tests/executable.rs index c3ac6a363..50ac06039 100644 --- a/crates/apollo-compiler/tests/executable.rs +++ b/crates/apollo-compiler/tests/executable.rs @@ -323,11 +323,11 @@ fn builder_from_multiple_files() { let query2 = "query GetPost { post { id title } }"; let mut errors = DiagnosticList::new(Default::default()); - let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); - Parser::new().parse_into_executable_builder(query1, "query1.graphql", &mut builder); - Parser::new().parse_into_executable_builder(query2, "query2.graphql", &mut builder); + let doc = ExecutableDocument::builder(Some(&schema), &mut errors) + .parse(query1, "query1.graphql") + .parse(query2, "query2.graphql") + .build(); - let doc = builder.build(); assert!(errors.is_empty(), "Expected no errors, got: {}", errors); assert_eq!(doc.operations.named.len(), 2); @@ -351,11 +351,11 @@ fn builder_with_fragments_from_multiple_files() { "#; let mut errors = DiagnosticList::new(Default::default()); - let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); - Parser::new().parse_into_executable_builder(query, "query.graphql", &mut builder); - Parser::new().parse_into_executable_builder(fragment, "fragment.graphql", &mut builder); + let doc = ExecutableDocument::builder(Some(&schema), &mut errors) + .parse(query, "query.graphql") + .parse(fragment, "fragment.graphql") + .build(); - let doc = builder.build(); assert!(errors.is_empty(), "Expected no errors, got: {}", errors); assert_eq!(doc.operations.named.len(), 1); @@ -373,11 +373,11 @@ fn builder_detects_operation_name_collision() { let query2 = "query GetData { field }"; let mut errors = DiagnosticList::new(Default::default()); - let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); - Parser::new().parse_into_executable_builder(query1, "query1.graphql", &mut builder); - Parser::new().parse_into_executable_builder(query2, "query2.graphql", &mut builder); + let _doc = ExecutableDocument::builder(Some(&schema), &mut errors) + .parse(query1, "query1.graphql") + .parse(query2, "query2.graphql") + .build(); - let _doc = builder.build(); assert!( !errors.is_empty(), "Expected errors for operation name collision" @@ -399,11 +399,11 @@ fn builder_detects_fragment_name_collision() { let fragment2 = "fragment UserData on User { id }"; let mut errors = DiagnosticList::new(Default::default()); - let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); - Parser::new().parse_into_executable_builder(fragment1, "fragment1.graphql", &mut builder); - Parser::new().parse_into_executable_builder(fragment2, "fragment2.graphql", &mut builder); + let _doc = ExecutableDocument::builder(Some(&schema), &mut errors) + .parse(fragment1, "fragment1.graphql") + .parse(fragment2, "fragment2.graphql") + .build(); - let _doc = builder.build(); assert!( !errors.is_empty(), "Expected errors for fragment name collision" @@ -422,11 +422,11 @@ fn builder_without_schema() { let query2 = "query GetMore { other }"; let mut errors = DiagnosticList::new(Default::default()); - let mut builder = ExecutableDocument::builder(None, &mut errors); - Parser::new().parse_into_executable_builder(query1, "query1.graphql", &mut builder); - Parser::new().parse_into_executable_builder(query2, "query2.graphql", &mut builder); + let doc = ExecutableDocument::builder(None, &mut errors) + .parse(query1, "query1.graphql") + .parse(query2, "query2.graphql") + .build(); - let doc = builder.build(); assert!(errors.is_empty(), "Expected no errors, got: {}", errors); assert_eq!(doc.operations.named.len(), 2); @@ -443,11 +443,11 @@ fn builder_preserves_source_information() { let query2 = "query Q2 { field }"; let mut errors = DiagnosticList::new(Default::default()); - let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); - Parser::new().parse_into_executable_builder(query1, "query1.graphql", &mut builder); - Parser::new().parse_into_executable_builder(query2, "query2.graphql", &mut builder); + let doc = ExecutableDocument::builder(Some(&schema), &mut errors) + .parse(query1, "query1.graphql") + .parse(query2, "query2.graphql") + .build(); - let doc = builder.build(); assert!(errors.is_empty(), "Expected no errors, got: {}", errors); // Verify that source information is tracked @@ -465,11 +465,11 @@ fn builder_handles_anonymous_and_named_operations() { let named = "query GetData { field }"; let mut errors = DiagnosticList::new(Default::default()); - let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); - Parser::new().parse_into_executable_builder(anonymous, "anonymous.graphql", &mut builder); - Parser::new().parse_into_executable_builder(named, "named.graphql", &mut builder); + let _doc = ExecutableDocument::builder(Some(&schema), &mut errors) + .parse(anonymous, "anonymous.graphql") + .parse(named, "named.graphql") + .build(); - let _doc = builder.build(); // Should error because mixing anonymous and named operations is ambiguous assert!( !errors.is_empty(), @@ -504,16 +504,12 @@ fn builder_with_multiple_fragments_used_in_query() { let settings_fragment = "fragment SettingsFields on Settings { theme notifications }"; let mut errors = DiagnosticList::new(Default::default()); - let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); - Parser::new().parse_into_executable_builder(query, "query.graphql", &mut builder); - Parser::new().parse_into_executable_builder(profile_fragment, "profile.graphql", &mut builder); - Parser::new().parse_into_executable_builder( - settings_fragment, - "settings.graphql", - &mut builder, - ); + let doc = ExecutableDocument::builder(Some(&schema), &mut errors) + .parse(query, "query.graphql") + .parse(profile_fragment, "profile.graphql") + .parse(settings_fragment, "settings.graphql") + .build(); - let doc = builder.build(); assert!(errors.is_empty(), "Expected no errors, got: {}", errors); assert_eq!(doc.operations.named.len(), 1); @@ -553,15 +549,13 @@ fn builder_accumulates_diagnostics_from_multiple_sources() { let fragment3 = "fragment UserName on User { name }"; let mut errors = DiagnosticList::new(Default::default()); - let mut builder = ExecutableDocument::builder(Some(&schema), &mut errors); - - Parser::new().parse_into_executable_builder(query1, "query1.graphql", &mut builder); - Parser::new().parse_into_executable_builder(query2, "query2.graphql", &mut builder); - Parser::new().parse_into_executable_builder(fragment1, "fragment1.graphql", &mut builder); - Parser::new().parse_into_executable_builder(fragment2, "fragment2.graphql", &mut builder); - Parser::new().parse_into_executable_builder(fragment3, "fragment3.graphql", &mut builder); - - let doc = builder.build(); + let doc = ExecutableDocument::builder(Some(&schema), &mut errors) + .parse(query1, "query1.graphql") + .parse(query2, "query2.graphql") + .parse(fragment1, "fragment1.graphql") + .parse(fragment2, "fragment2.graphql") + .parse(fragment3, "fragment3.graphql") + .build(); // Verify we collected multiple errors from different sources assert!(!errors.is_empty(), "Expected errors from multiple sources"); From 8e02d44e9104ff9efe628b336472ca15526a02bf Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Wed, 26 Nov 2025 07:02:51 -0800 Subject: [PATCH 9/9] clippy --- crates/apollo-compiler/src/executable/from_ast.rs | 2 +- crates/apollo-compiler/src/parser.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/apollo-compiler/src/executable/from_ast.rs b/crates/apollo-compiler/src/executable/from_ast.rs index f20f3008a..354b5bfd2 100644 --- a/crates/apollo-compiler/src/executable/from_ast.rs +++ b/crates/apollo-compiler/src/executable/from_ast.rs @@ -124,7 +124,7 @@ impl<'schema, 'errors> ExecutableDocumentBuilder<'schema, 'errors> { type_system_definitions_are_errors: bool, ) { let mut errors = BuildErrors { - errors: &mut self.errors, + errors: self.errors, path: SelectionPath { nested_fields: Vec::new(), // overwritten: diff --git a/crates/apollo-compiler/src/parser.rs b/crates/apollo-compiler/src/parser.rs index 0f08e8e1a..54ad7a911 100644 --- a/crates/apollo-compiler/src/parser.rs +++ b/crates/apollo-compiler/src/parser.rs @@ -309,7 +309,7 @@ impl Parser { path: impl AsRef, builder: &mut executable::ExecutableDocumentBuilder, ) { - let ast = self.parse_ast_inner(source_text, path, FileId::new(), &mut builder.errors); + let ast = self.parse_ast_inner(source_text, path, FileId::new(), builder.errors); let type_system_definitions_are_errors = true; builder.add_ast_document(&ast, type_system_definitions_are_errors); }