feat!: better errors on empty graphql documents (#1117)

cynic-parser/CHANGELOG.md

@@ -9,6 +9,10 @@ This project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html
 
 ## Unreleased - xxxx-xx-xx
 
+### Breaking Changes
+
+- `Error::span` now returns an `Option<Span>` instead of a `Span`
+
 ## v0.8.7 - 2024-12-03
 
 ### Changes
@@ -23,7 +27,6 @@ This project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html
 - Fixed the `VariableValue` debug impl which was misleading
   ([#1104](https://github.com/obmarg/cynic/pull/1104))
 
-
 ## v0.8.5 - 2024-11-27
 
 ### New Features

cynic-parser/src/errors.rs

@@ -55,25 +55,32 @@ pub enum Error {
 
     /// Variable found in const position
     VariableInConstPosition(usize, String, usize),
+
+    /// The GraphQl document was empty
+    EmptyTypeSystemDocument,
+
+    /// The GraphQl document was empty
+    EmptyExecutableDocument,
 }
 
 impl Error {
-    pub fn span(&self) -> Span {
+    pub fn span(&self) -> Option<Span> {
         match self {
-            Error::InvalidToken { location } => Span::new(*location, *location),
-            Error::UnrecognizedEof { location, .. } => Span::new(*location, *location),
+            Error::InvalidToken { location } => Span::new(*location, *location).into(),
+            Error::UnrecognizedEof { location, .. } => Span::new(*location, *location).into(),
             Error::UnrecognizedToken {
                 token: (start, _, end),
                 ..
-            } => Span::new(*start, *end),
+            } => Span::new(*start, *end).into(),
             Error::ExtraToken {
                 token: (start, _, end),
                 ..
-            } => Span::new(*start, *end),
-            Error::Lexical(error) => error.span(),
-            Error::MalformedStringLiteral(error) => error.span(),
-            Error::MalformedDirectiveLocation(lhs, _, rhs) => Span::new(*lhs, *rhs),
-            Error::VariableInConstPosition(lhs, _, rhs) => Span::new(*lhs, *rhs),
+            } => Span::new(*start, *end).into(),
+            Error::Lexical(error) => error.span().into(),
+            Error::MalformedStringLiteral(error) => error.span().into(),
+            Error::MalformedDirectiveLocation(lhs, _, rhs) => Span::new(*lhs, *rhs).into(),
+            Error::VariableInConstPosition(lhs, _, rhs) => Span::new(*lhs, *rhs).into(),
+            Error::EmptyExecutableDocument | Error::EmptyTypeSystemDocument => None,
         }
     }
 }
@@ -87,7 +94,9 @@ impl std::error::Error for Error {
             | Error::ExtraToken { .. }
             | Error::MalformedStringLiteral(..)
             | Error::MalformedDirectiveLocation(..)
-            | Error::VariableInConstPosition(..) => None,
+            | Error::VariableInConstPosition(..)
+            | Error::EmptyTypeSystemDocument
+            | Error::EmptyExecutableDocument => None,
             Error::Lexical(error) => Some(error),
         }
     }
@@ -157,6 +166,18 @@ impl fmt::Display for Error {
                 )?;
                 Ok(())
             }
+            Error::EmptyExecutableDocument => {
+                write!(
+                    f,
+                    "the graphql document was empty, please provide an operation"
+                )
+            }
+            Error::EmptyTypeSystemDocument => {
+                write!(
+                    f,
+                    "the graphql document was empty, please provide at least one definition"
+                )
+            }
         }
     }
 }

cynic-parser/src/errors/report.rs

@@ -15,9 +15,12 @@ impl Error {
 
         let mut builder = ariadne::Report::build(ReportKind::Error, (), 0)
             .with_message(message)
-            .with_label(label)
             .with_config(Config::default().with_color(false));
 
+        if let Some(label) = label {
+            builder.add_label(label);
+        }
+
         if let Some(note) = note {
             builder.set_note(note)
         }
@@ -27,68 +30,84 @@ impl Error {
         Report { inner, document }
     }
 
-    fn components(&self) -> (String, Label, Option<String>) {
+    fn components(&self) -> (String, Option<Label>, Option<String>) {
         match self {
             Error::InvalidToken { location } => (
                 "invalid token".into(),
-                Label::new(*location..*location).with_message("could not understand this token"),
+                Label::new(*location..*location)
+                    .with_message("could not understand this token")
+                    .into(),
                 None,
             ),
             Error::UnrecognizedEof { location, expected } => (
                 "unexpected eof".into(),
-                Label::new(*location..*location).with_message("expected another token here"),
+                Label::new(*location..*location)
+                    .with_message("expected another token here")
+                    .into(),
                 Some(format!("expected one of {}", expected.join(", "))),
             ),
             Error::UnrecognizedToken {
                 token: (start, token, end),
                 expected,
             } => (
                 format!("unexpected {}", token),
-                Label::new(*start..*end).with_message("didn't expect to see this"),
+                Label::new(*start..*end)
+                    .with_message("didn't expect to see this")
+                    .into(),
                 Some(format!("expected one of {}", expected.join(", "))),
             ),
             Error::ExtraToken {
                 token: (start, token, end),
             } => (
                 format!("extra {}", token),
-                Label::new(*start..*end).with_message("we expected the document to end here"),
+                Label::new(*start..*end)
+                    .with_message("we expected the document to end here")
+                    .into(),
                 None,
             ),
             Error::Lexical(error) => {
                 let span = error.span();
                 (
                     "invalid token".into(),
-                    Label::new(span.start..span.end).with_message("could not parse a token here"),
+                    Label::new(span.start..span.end)
+                        .with_message("could not parse a token here")
+                        .into(),
                     None,
                 )
             }
             Error::MalformedStringLiteral(error) => {
-                let span = self.span();
+                let span = self.span().unwrap();
                 (
                     error.to_string(),
-                    Label::new(span.start..span.end).with_message("error occurred here"),
+                    Label::new(span.start..span.end)
+                        .with_message("error occurred here")
+                        .into(),
                     None,
                 )
             }
             Error::MalformedDirectiveLocation(_, _, _) => {
-                let span = self.span();
+                let span = self.span().unwrap();
                 (
                     self.to_string(),
                     Label::new(span.start..span.end)
-                        .with_message("this is not a valid directive location"),
+                        .with_message("this is not a valid directive location")
+                        .into(),
                     None,
                 )
             }
 
             Error::VariableInConstPosition(_, _, _) => {
-                let span = self.span();
+                let span = self.span().unwrap();
                 (
                     self.to_string(),
                     Label::new(span.start..span.end)
-                        .with_message("only non-variable values can be used here"),
+                        .with_message("only non-variable values can be used here")
+                        .into(),
                     None,
                 )
             }
+            Error::EmptyExecutableDocument => (self.to_string(), None, Some("graphql documents should contain at least one query, mutation or subscription".into())),
+            Error::EmptyTypeSystemDocument => (self.to_string(), None, Some("graphql documents should contain at least one type, schema or directive definition".into())),
         }
     }
 }

cynic-parser/src/lib.rs

@@ -25,6 +25,10 @@ pub use self::{
 };
 
 pub fn parse_type_system_document(input: &str) -> Result<TypeSystemDocument, Error> {
+    if input.trim().is_empty() {
+        return Err(Error::EmptyTypeSystemDocument);
+    }
+
     let lexer = lexer::Lexer::new(input);
     let mut ast = type_system::writer::TypeSystemAstWriter::new();
 
@@ -34,6 +38,10 @@ pub fn parse_type_system_document(input: &str) -> Result<TypeSystemDocument, Err
 }
 
 pub fn parse_executable_document(input: &str) -> Result<ExecutableDocument, Error> {
+    if input.trim().is_empty() {
+        return Err(Error::EmptyExecutableDocument);
+    }
+
     let lexer = lexer::Lexer::new(input);
     let mut ast = executable::writer::ExecutableAstWriter::new();
 

cynic-parser/tests/report.rs

@@ -6,7 +6,7 @@ fn test_report() {
         .unwrap_err()
         .to_report(document);
 
-    insta::assert_display_snapshot!(report, @r###"
+    insta::assert_display_snapshot!(report, @r#"
     Error: unexpected closing brace ('}')
        ╭─[<unknown>:1:1]
        │
@@ -16,7 +16,7 @@ fn test_report() {
        │ 
        │ Note: expected one of RawIdent, StringLiteral, BlockStringLiteral, schema, query, mutation, subscription, ty, input, true, false, null, implements, interface, "enum", union, scalar, extend, directive, repeatable, on, fragment
     ───╯
-    "###);
+    "#);
 }
 
 #[test]
@@ -28,13 +28,37 @@ fn test_invalid_directive_location() {
         .unwrap_err()
         .to_report(document);
 
-    insta::assert_display_snapshot!(report, @r###"
+    insta::assert_display_snapshot!(report, @r"
     Error: unknown directive location: BLAH. expected one of QUERY, MUTATION, SUBSCRIPTION, FIELD, FRAGMENT_DEFINITION, FRAGMENT_SPREAD, INLINE_FRAGMENT, SCHEMA, SCALAR, OBJECT, FIELD_DEFINITION, ARGUMENT_DEFINITION, INTERFACE, UNION, ENUM, ENUM_VALUE, INPUT_OBJECT, INPUT_FIELD_DEFINITION, VARIABLE_DEFINITION
        ╭─[<unknown>:1:1]
        │
      1 │ directive @Blah on BLAH
        │                    ──┬─  
        │                      ╰─── this is not a valid directive location
     ───╯
-    "###);
+    ");
+}
+
+#[test]
+fn test_empty_type_system_document() {
+    let document = "   ";
+
+    let report = cynic_parser::parse_type_system_document(document)
+        .map(|_| ())
+        .unwrap_err()
+        .to_report(document);
+
+    insta::assert_display_snapshot!(report, @"Error: the graphql document was empty, please provide at least one definition");
+}
+
+#[test]
+fn test_empty_executable_document() {
+    let document = "   ";
+
+    let report = cynic_parser::parse_executable_document(document)
+        .map(|_| ())
+        .unwrap_err()
+        .to_report(document);
+
+    insta::assert_display_snapshot!(report, @"Error: the graphql document was empty, please provide an operation");
 }