Parsing the empty string generates an error with out of bounds location

[samtipton]

Edit after investigation:

Example:

let input = ""; // The empty string
apollo_compiler::ast::Document::parse(input, "example.graphql").unwrap();

called `Result::unwrap()` on an `Err` value: Error: syntax error: Unexpected <EOF>.

The empty document being invalid is correct per spec, though it could be more user-friendly to return an empty document in this case. But if we do return an error, its string formatting should include a source location. For comparison:

let input = " "; // A single space
apollo_compiler::ast::Document::parse(input, "example.graphql").unwrap();

called `Result::unwrap()` on an `Err` value: Error: syntax error: Unexpected <EOF>.
   ╭─[ example.graphql:1:2 ]
   │
 1 │ 
   │  │ 
   │  ╰─ Unexpected <EOF>.
───╯

The underlying bug is in apollo-parser:

let input = ""; // The empty string
dbg!(apollo_parser::Parser::new(input).parse().errors().next().unwrap().index());

The error index is 1 which it out of bounds. It should be 0 instead. When apollo-compiler formats a diagnostic, Ariadne ignores locations that are out of bound.

Original report

---

Description

In SchemaBuilder#built_in() we have the following

            let input = include_str!("../built_in_types.graphql").to_owned();
            let path = "built_in.graphql";
            let id = FileId::BUILT_IN;
            let ast = ast::Document::parser().parse_ast_inner(input, path, id, &mut builder.errors);
            // ..
            builder.add_ast_document(&ast, executable_definitions_are_errors);

However, path is incorrect.

This results in an error when we add additional documents to the schema_builder. Like for example,

        ast::Document::parser().parse_into_schema_builder(&schema_content, file_path.to_string_lossy().as_ref(), &mut schema_builder);

Steps to reproduce

use apollo_compiler::schema::SchemaBuilder;
use clap::Parser as ClapParser;
use std::fs;
use std::ops::Index;
use std::path::{Path, PathBuf};
use apollo_compiler::ast;

#[derive(ClapParser)]
#[command(author, version, about, long_about = None)]
struct Cli {
    /// Directory containing *.graphqls files
    #[arg(short = 'd', long)]
    schema_path: PathBuf,
}

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let cli = Cli::parse();

    // get schema files from directory
    let schema_files = if cli.schema_path.is_dir() {
        collect_schema_files_from_directory(&cli.schema_path)?
    } else {
        return Err(format!("{} is not a directory", &cli.schema_path.display()).into());
    };

    let file_count = schema_files.len();
    if file_count == 0 {
        return Err("No schema files found".into());
    }

    // Create a schema builder to merge schemas
    let mut schema_builder = SchemaBuilder::new();

    // Read and merge all schema files
    for file_path in &schema_files {
        println!("Reading schema from: {}", file_path.display());
        let schema_content = fs::read_to_string(file_path)?;

        // Add to schema builder
        ast::Document::parser().parse_into_schema_builder(&schema_content, file_path.to_string_lossy().as_ref(), &mut schema_builder);
    }

    // Build the final merged schema
    let merged_schema = schema_builder.build();

    // Check for validation errors
    match merged_schema {
        Ok(_) => {
            println!("Successfully validated and merged {} schema files",
                     file_count);

            // Convert the merged schema to SDL (String)
            let merged_schema_sdl = merged_schema.unwrap().to_string();

            println!("Merged schema:");
            println!("{}", merged_schema_sdl);
        },
        Err(errors) => {
            println!("Schema validation errors:");
            for (index, error) in errors.errors.iter().enumerate() {
                println!("{}: {}", error.sources.index(index).path().display(), error);
            }
            return Err("Schema validation failed".into());
        }
    }

    Ok(())
}

/// Collect all *.graphqls files from a directory
fn collect_schema_files_from_directory(dir: &Path) -> Result<Vec<PathBuf>, Box<dyn std::error::Error>> {
    // Check if composite-schema.graphqls exists
    let composite_path = dir.join("composite-schema.graphqls");
    if composite_path.exists() {
        println!("Found composite schema file, using only this file");
        return Ok(vec![composite_path]);
    }

    // Otherwise collect all *.graphqls files
    let mut schema_files = Vec::new();

    for entry in fs::read_dir(dir)? {
        let entry = entry?;
        let path = entry.path();

        if path.is_file() {
            if let Some(extension) = path.extension() {
                if extension == "graphqls" {
                    schema_files.push(path);
                }
            }
        }
    }

    if schema_files.is_empty() {
        println!("No *.graphqls files found in directory: {}", dir.display());
    } else {
        println!("Found {} schema files in directory", schema_files.len());
    }

    Ok(schema_files)
}

Expected result

A schema definition string in output

Actual result

Schema validation errors:
built_in.graphql: Error: syntax error: Unexpected <EOF>.

Environment

• Operating system and version: Mac OSX Sequoia 15.3.1
• Shell (bash/zsh/powershell): zsh
• apollo-rs crate: apollo_compiler
• Crate version: 1.27.0

[SimonSapin]

The path given when parsing with apollo-compiler is only informative, for display in formatted diagnostics. Nothing ever tries to open a file named built_in.graphql, so it doesn’t matter that nothing on the filesystem has that name.

In your code:

for (index, error) in errors.errors.iter().enumerate() {
    println!("{}: {}", error.sources.index(index).path().display(), error);
}

error.sources.index(index) is incorrect. The index of an error inside a DiagnosticList is unrelated to the index of a source inside a SourceMap. So the Unexpected <EOF> error likely comes not from built_in.graphql but some other file which does not contain any definition.

The GraphQL spec grammar defines Document as a list of Definitions. In turn, list means one or more. (Zero or more would be list opt.) So a document that is empty or contains only whitespace or comments is invalid. Maybe for the multi-file scenario it’d be more useful for apollo-compiler to deviate from the spec and accept empty documents.

If you want to programmatically extract the source path for a diagnostic, that looks like this:

use apollo_compiler::diagnostic::ToCliReport;
let path = error
    .error
    .location()
    .and_then(|loc| error.sources.get(&loc.file_id()))
    .map(|source| source.path());

It’s not very convenient and we could add a helper method, but ideally you wouldn’t need to do that in the first place. Formatting the error diagnostic to string like you’re doing uses Ariadne and should include the path. You can even print the errors diagnostic list as a whole.

I can reproduce that parsing the empty string produces an error whose formatting does not include the path. This is a bug. The formatting for a slightly different EOF error looks as expected:

Error: syntax error: expected R_CURLY, got EOF
   ╭─[ example.graphql:1:20 ]
   │
 1 │ type Query { f: Int
   │                    │ 
   │                    ╰─ expected R_CURLY, got EOF
───╯

I think the bug is in apollo-parser specifically when parsing an input of size zero, the error is reported at index 1 which is out of bounds which causes Ariadne to ignore the location:

apollo_parser::Parser::new("").parse().errors().next().unwrap().index()

[SimonSapin]

I’ve edited the issue desciption to describe the underlying bug