Repository: apollographql/apollo-rs (rust)

Issue:
Ast errors: return an iterator
Follow up to [this comment](https://github.com/apollographql/router/pull/136#discussion_r756083538) we might want ast.errors() to return an Iterator.

@lrlna Since the current errors() method takes self by reference, I'll probably go with an `Iterator` and not an `IntoIterator`, what do you think ? :) 

Pull request:
feat: impl Iterator for errors
fixes #119 

ast.errors() now return an `Iter<'_, crate::Error>` which allows us to iterate, but also check for the `len()` (through ExactSizeIterator). It should allow us to use `is_empty()` as well eventually, if / when https://github.com/rust-lang/rust/issues/35428 lands.

I wish we could use `is_empty()`, the ergonomics feel a bit clunky without it. I would totally understand if we decided not to return an impl Iterator, or something else for this reason.

Patch:
diff --git a/crates/apollo-parser/README.md b/crates/apollo-parser/README.md
index bae063b..7aa0203 100644
--- a/crates/apollo-parser/README.md
+++ b/crates/apollo-parser/README.md
@@ -60,8 +60,8 @@ let input = "union SearchResult = Photo | Person | Cat | Dog";
 let parser = Parser::new(input);
 let ast = parser.parse();
 
-// ast.errors() returns an errors slice encountered during lexing and parsing
-assert!(ast.errors().is_empty());
+// ast.errors() returns an iterator with the errors encountered during lexing and parsing
+assert_eq!(0, ast.errors().len());
 
 // ast.document() gets the Document, or root node, of the tree that you can
 // start iterating on.
@@ -93,7 +93,7 @@ type ProductDimension {
 ";
 let parser = Parser::new(input);
 let ast = parser.parse();
-assert!(ast.errors().is_empty());
+assert_eq!(0, ast.errors().len());
 
 let doc = ast.document();
 
@@ -124,7 +124,7 @@ let input = "
   
   let parser = Parser::new(input);
   let ast = parser.parse();
-  assert!(&ast.errors().is_empty());
+  assert_eq!(0, ast.errors().len());
   
   let doc = ast.document();
   
diff --git a/crates/apollo-parser/examples/unused_vars.rs b/crates/apollo-parser/examples/unused_vars.rs
index d580b5a..2e3786e 100644
--- a/crates/apollo-parser/examples/unused_vars.rs
+++ b/crates/apollo-parser/examples/unused_vars.rs
@@ -11,7 +11,7 @@ fn are_variables_unused() {
     let parser = Parser::new(&src);
     let ast = parser.parse();
 
-    assert!(&ast.errors().is_empty());
+    assert_eq!(0, ast.errors().len());
 
     let doc = ast.document();
 
diff --git a/crates/apollo-parser/src/ast/mod.rs b/crates/apollo-parser/src/ast/mod.rs
index e3e6b42..124531f 100644
--- a/crates/apollo-parser/src/ast/mod.rs
+++ b/crates/apollo-parser/src/ast/mod.rs
@@ -42,7 +42,7 @@
 //! let parser = Parser::new(schema);
 //! let ast = parser.parse();
 //!
-//! assert!(ast.errors().is_empty());
+//! assert_eq!(0, ast.errors().len());
 //! let document = ast.document();
 //! for definition in document.definitions() {
 //!     match definition {
diff --git a/crates/apollo-parser/src/error.rs b/crates/apollo-parser/src/error.rs
index 4de687e..5cab263 100644
--- a/crates/apollo-parser/src/error.rs
+++ b/crates/apollo-parser/src/error.rs
@@ -16,7 +16,7 @@ use std::fmt;
 /// let parser = Parser::new(input);
 /// let ast = parser.parse();
 ///
-/// assert!(ast.errors().is_empty());
+/// assert_eq!(0, ast.errors().len());
 ///
 /// let doc = ast.document();
 /// ```
diff --git a/crates/apollo-parser/src/lexer/mod.rs b/crates/apollo-parser/src/lexer/mod.rs
index 3435b8c..2c9ac26 100644
--- a/crates/apollo-parser/src/lexer/mod.rs
+++ b/crates/apollo-parser/src/lexer/mod.rs
@@ -2,6 +2,8 @@ mod cursor;
 mod token;
 mod token_kind;
 
+use std::slice::Iter;
+
 use crate::{lexer::cursor::Cursor, Error};
 
 pub use token::Token;
@@ -59,8 +61,8 @@ impl Lexer {
     }
 
     /// Get a reference to the lexer's tokens.
-    pub(crate) fn errors(&self) -> &[Error] {
-        self.errors.as_slice()
+    pub(crate) fn errors(&self) -> Iter<'_, Error> {
+        self.errors.iter()
     }
 }
 
diff --git a/crates/apollo-parser/src/lib.rs b/crates/apollo-parser/src/lib.rs
index bee6b57..7f05257 100644
--- a/crates/apollo-parser/src/lib.rs
+++ b/crates/apollo-parser/src/lib.rs
@@ -41,7 +41,7 @@
 //!
 //! ## Usage
 //! `apollo-parser` is built to parse both GraphQL schemas and queries according to the latest [October 2021 specification]. It produces
-//! a typed syntax tree that then can we walked extracting all the necessary
+//! a typed syntax tree that then can we walked, extracting all the necessary
 //! information. You can quick start with:
 //!
 //! ```rust
@@ -62,7 +62,7 @@
 //! let ast = parser.parse();
 //!
 //! // ast.errors() returns an errors slice encountered during lexing and parsing
-//! assert!(ast.errors().is_empty());
+//! assert_eq!(0, ast.errors().len());
 //!
 //! // ast.document() get the Document, or root node, of the tree that you can
 //! // start iterating on.
@@ -94,7 +94,7 @@
 //! ";
 //! let parser = Parser::new(input);
 //! let ast = parser.parse();
-//! assert!(ast.errors().is_empty());
+//! assert_eq!(0, ast.errors().len());
 //!
 //! let doc = ast.document();
 //!
@@ -125,7 +125,7 @@
 //!
 //! let parser = Parser::new(input);
 //! let ast = parser.parse();
-//! assert!(&ast.errors().is_empty());
+//! assert_eq!(0, ast.errors().len());
 //!
 //! let doc = ast.document();
 //!
diff --git a/crates/apollo-parser/src/parser/grammar/object.rs b/crates/apollo-parser/src/parser/grammar/object.rs
index 141c64c..b04ba12 100644
--- a/crates/apollo-parser/src/parser/grammar/object.rs
+++ b/crates/apollo-parser/src/parser/grammar/object.rs
@@ -131,7 +131,7 @@ type Business implements NamedEntity & ValuedEntity & CatEntity {
 }";
         let parser = Parser::new(input);
         let ast = parser.parse();
-        assert!(ast.errors().is_empty());
+        assert_eq!(0, ast.errors().len());
 
         let doc = ast.document();
 
diff --git a/crates/apollo-parser/src/parser/grammar/selection.rs b/crates/apollo-parser/src/parser/grammar/selection.rs
index 6fcffec..d3548d2 100644
--- a/crates/apollo-parser/src/parser/grammar/selection.rs
+++ b/crates/apollo-parser/src/parser/grammar/selection.rs
@@ -68,7 +68,7 @@ mod test {
 ";
         let parser = Parser::new(input);
         let ast = parser.parse();
-        assert!(&ast.errors().is_empty());
+        assert_eq!(0, ast.errors().len());
 
         let doc = ast.document();
 
@@ -129,7 +129,7 @@ query GraphQuery($graph_id: ID!, $variant: String) {
 ";
         let parser = Parser::new(input);
         let ast = parser.parse();
-        assert!(&ast.errors().is_empty());
+        assert_eq!(0, ast.errors().len());
 
         let doc = ast.document();
 
diff --git a/crates/apollo-parser/src/parser/grammar/union_.rs b/crates/apollo-parser/src/parser/grammar/union_.rs
index cd9c5dd..cda9597 100644
--- a/crates/apollo-parser/src/parser/grammar/union_.rs
+++ b/crates/apollo-parser/src/parser/grammar/union_.rs
@@ -112,7 +112,7 @@ mod test {
         let input = "union SearchResult = Photo | Person | Cat | Dog";
         let parser = Parser::new(input);
         let ast = parser.parse();
-        assert!(ast.errors().is_empty());
+        assert_eq!(0, ast.errors().len());
 
         let doc = ast.document();
 
diff --git a/crates/apollo-parser/src/parser/grammar/value.rs b/crates/apollo-parser/src/parser/grammar/value.rs
index 8257959..dd097f7 100644
--- a/crates/apollo-parser/src/parser/grammar/value.rs
+++ b/crates/apollo-parser/src/parser/grammar/value.rs
@@ -270,7 +270,7 @@ query GraphQuery($graph_id: ID!, $variant: String) {
         ";
         let parser = Parser::new(input);
         let ast = parser.parse();
-        assert!(&ast.errors().is_empty());
+        assert_eq!(0, ast.errors().len());
 
         let doc = ast.document();
 
diff --git a/crates/apollo-parser/src/parser/mod.rs b/crates/apollo-parser/src/parser/mod.rs
index 8e6195e..512e594 100644
--- a/crates/apollo-parser/src/parser/mod.rs
+++ b/crates/apollo-parser/src/parser/mod.rs
@@ -42,7 +42,7 @@ pub(crate) use token_text::TokenText;
 /// // Parse the query, and return a SyntaxTree.
 /// let ast = parser.parse();
 /// // Check that are no errors. These are not part of the AST.
-/// assert!(&ast.errors().is_empty());
+/// assert_eq!(0, ast.errors().len());
 ///
 /// // Get the document root node
 /// let doc = ast.document();
@@ -65,7 +65,7 @@ pub(crate) use token_text::TokenText;
 /// let parser = Parser::new(core_schema);
 /// let ast = parser.parse();
 ///
-/// assert!(ast.errors().is_empty());
+/// assert_eq!(0, ast.errors().len());
 ///
 /// let document = ast.document();
 /// ```
@@ -91,7 +91,7 @@ impl Parser {
             tokens.push(s);
         }
 
-        for e in lexer.errors().to_owned() {
+        for e in lexer.errors().cloned() {
             errors.push(e);
         }
 
diff --git a/crates/apollo-parser/src/parser/syntax_tree.rs b/crates/apollo-parser/src/parser/syntax_tree.rs
index 09f37cc..c4708d8 100644
--- a/crates/apollo-parser/src/parser/syntax_tree.rs
+++ b/crates/apollo-parser/src/parser/syntax_tree.rs
@@ -1,4 +1,4 @@
-use std::fmt;
+use std::{fmt, slice::Iter};
 
 use rowan::GreenNodeBuilder;
 
@@ -44,8 +44,8 @@ pub struct SyntaxTree {
 
 impl SyntaxTree {
     /// Get a reference to the syntax tree's errors.
-    pub fn errors(&self) -> &[crate::Error] {
-        self.errors.as_ref()
+    pub fn errors(&self) -> Iter<'_, crate::Error> {
+        self.errors.iter()
     }
 
     /// Return the root typed `Document` node.
@@ -159,7 +159,7 @@ mod test {
         ";
         let parser = Parser::new(input);
         let ast = parser.parse();
-        assert!(ast.errors().is_empty());
+        assert_eq!(0, ast.errors().len());
 
         let doc = ast.document();
 


Test patch:
diff --git a/crates/apollo-parser/src/tests.rs b/crates/apollo-parser/src/tests.rs
index bef2542..abae1b1 100644
--- a/crates/apollo-parser/src/tests.rs
+++ b/crates/apollo-parser/src/tests.rs
@@ -6,6 +6,7 @@ use std::{
     fmt::Write,
     fs,
     path::{Path, PathBuf},
+    slice::Iter,
 };
 
 use expect_test::expect_file;
@@ -52,25 +53,24 @@ fn parser_tests() {
     });
 }
 
-fn assert_errors_are_present(errors: &[Error], path: &Path) {
+fn assert_errors_are_present(errors: Iter<'_, Error>, path: &Path) {
     assert!(
-        !errors.is_empty(),
+        errors.len() != 0,
         "There should be errors in the file {:?}",
         path.display()
     );
 }
 
-fn assert_errors_are_absent(errors: &[Error], path: &Path) {
-    assert_eq!(
-        errors,
-        &[] as &[Error],
+fn assert_errors_are_absent(errors: Iter<'_, Error>, path: &Path) {
+    assert!(
+        errors.len() == 0,
         "There should be no errors in the file {:?}",
         path.display(),
     );
 }
 
-/// Concatenate tokens and erorrs.
-fn dump_tokens_and_errors(tokens: &[Token], errors: &[Error]) -> String {
+/// Concatenate tokens and errors.
+fn dump_tokens_and_errors(tokens: &[Token], errors: Iter<'_, Error>) -> String {
     let mut acc = String::new();
     for token in tokens {
         writeln!(acc, "{:?}", token).unwrap();