Improve lookahead visibility for aliased fields (#662)

* Improve lookahead visibility for aliased fields
- Add a method to access the children of look ahead structs
- Make the behaviour around accessing aliased lookahead fields more consistent

* Deprecate old Lookahead methods for accessing child selections

juniper/CHANGELOG.md

@@ -32,6 +32,8 @@ See [#618](https://github.com/graphql-rust/juniper/pull/618).
 - Better error messages for all proc macros (see
   [#631](https://github.com/graphql-rust/juniper/pull/631)
 
+-  Improved lookahead visibility for aliased fields (see [#662](https://github.com/graphql-rust/juniper/pull/631))
+
 ## Breaking Changes
 
 - `juniper::graphiql` has moved to `juniper::http::graphiql`
@@ -71,6 +73,8 @@ See [#618](https://github.com/graphql-rust/juniper/pull/618).
   Rename `http::tests::HTTPIntegration` as `http::tests::HttpIntegration`
   and add support for `application/graphql` POST request.
 
+- When using LookAheadMethods to access child selections, children are always found using their alias if it exists rather than their name (see [#662](https://github.com/graphql-rust/juniper/pull/631)). These methods are also deprecated in favour of the new `children` method.
+
 # [[0.14.2] 2019-12-16](https://github.com/graphql-rust/juniper/releases/tag/juniper-0.14.2)
 
 - Fix incorrect validation with non-executed operations [#455](https://github.com/graphql-rust/juniper/issues/455)

juniper/src/executor/look_ahead.rs

@@ -332,14 +332,19 @@ pub struct ConcreteLookAheadSelection<'a, S: 'a> {
 
 /// A set of common methods for `ConcreteLookAheadSelection` and `LookAheadSelection`
 pub trait LookAheadMethods<S> {
-    /// Get the name of the field represented by the current selection
+    /// Get the (potentially aliased) name of the field represented by the current selection
     fn field_name(&self) -> &str;
 
     /// Get the the child selection for a given field
+    /// If a child has an alias, it will only match if the alias matches `name`
+    #[deprecated(note = "please use `children` to access the child selections instead")]
     fn select_child(&self, name: &str) -> Option<&Self>;
 
-    /// Check if a given field exists
+    /// Check if a given child selection with a name exists
+    /// If a child has an alias, it will only match if the alias matches `name`
+    #[deprecated(note = "please use `children` to access the child selections instead")]
     fn has_child(&self, name: &str) -> bool {
+        #[allow(deprecated)]
         self.select_child(name).is_some()
     }
 
@@ -357,8 +362,12 @@ pub trait LookAheadMethods<S> {
         self.arguments().iter().find(|a| a.name == name)
     }
 
-    /// Get the top level children for the current selection
+    /// Get the (possibly aliased) names of the top level children for the current selection
+    #[deprecated(note = "please use `children` to access the child selections instead")]
     fn child_names(&self) -> Vec<&str>;
+
+    /// Get an iterator over the children for the current selection
+    fn children(&self) -> Vec<&Self>;
 }
 
 impl<'a, S> LookAheadMethods<S> for ConcreteLookAheadSelection<'a, S> {
@@ -367,7 +376,7 @@ impl<'a, S> LookAheadMethods<S> for ConcreteLookAheadSelection<'a, S> {
     }
 
     fn select_child(&self, name: &str) -> Option<&Self> {
-        self.children.iter().find(|c| c.name == name)
+        self.children.iter().find(|c| c.field_name() == name)
     }
 
     fn arguments(&self) -> &[LookAheadArgument<S>] {
@@ -375,10 +384,7 @@ impl<'a, S> LookAheadMethods<S> for ConcreteLookAheadSelection<'a, S> {
     }
 
     fn child_names(&self) -> Vec<&str> {
-        self.children
+        self.children.iter().map(|c| c.field_name()).collect()
-            .iter()
-            .map(|c| c.alias.unwrap_or(c.name))
-            .collect()
     }
 
     fn has_arguments(&self) -> bool {
@@ -388,6 +394,10 @@ impl<'a, S> LookAheadMethods<S> for ConcreteLookAheadSelection<'a, S> {
     fn has_children(&self) -> bool {
         !self.children.is_empty()
     }
+
+    fn children(&self) -> Vec<&Self> {
+        self.children.iter().collect()
+    }
 }
 
 impl<'a, S> LookAheadMethods<S> for LookAheadSelection<'a, S> {
@@ -398,7 +408,7 @@ impl<'a, S> LookAheadMethods<S> for LookAheadSelection<'a, S> {
     fn select_child(&self, name: &str) -> Option<&Self> {
         self.children
             .iter()
-            .find(|c| c.inner.name == name)
+            .find(|c| c.inner.field_name() == name)
             .map(|s| &s.inner)
     }
 
@@ -407,10 +417,7 @@ impl<'a, S> LookAheadMethods<S> for LookAheadSelection<'a, S> {
     }
 
     fn child_names(&self) -> Vec<&str> {
-        self.children
+        self.children.iter().map(|c| c.inner.field_name()).collect()
-            .iter()
-            .map(|c| c.inner.alias.unwrap_or(c.inner.name))
-            .collect()
     }
 
     fn has_arguments(&self) -> bool {
@@ -420,6 +427,13 @@ impl<'a, S> LookAheadMethods<S> for LookAheadSelection<'a, S> {
     fn has_children(&self) -> bool {
         !self.children.is_empty()
     }
+
+    fn children(&self) -> Vec<&Self> {
+        self.children
+            .iter()
+            .map(|child_selection| &child_selection.inner)
+            .collect()
+    }
 }
 
 #[cfg(test)]
@@ -1290,6 +1304,7 @@ query Hero {
     }
 
     #[test]
+    #[allow(deprecated)]
     fn check_select_child() {
         let lookahead: LookAheadSelection<DefaultScalarValue> = LookAheadSelection {
             name: "hero",
@@ -1441,12 +1456,14 @@ fragment heroFriendNames on Hero {
     }
 
     #[test]
+    #[allow(deprecated)]
     fn check_visitability() {
         let docs = parse_document_source::<DefaultScalarValue>(
             "
 query Hero {
     hero(episode: EMPIRE) {
         name
+        aliasedName: name
         friends {
             name
         }
@@ -1474,22 +1491,56 @@ query Hero {
             assert_eq!(args[0].value(), &LookAheadValue::Enum("EMPIRE"));
 
             assert!(look_ahead.has_children());
-            assert_eq!(look_ahead.child_names(), vec!["name", "friends"]);
+            assert_eq!(
+                look_ahead.child_names(),
+                vec!["name", "aliasedName", "friends"]
+            );
+            let mut children = look_ahead.children().into_iter();
 
-            let child0 = look_ahead.select_child("name").unwrap();
+            let name_child = children.next().unwrap();
-            assert_eq!(child0.field_name(), "name");
+            assert!(look_ahead.has_child("name"));
-            assert!(!child0.has_arguments());
+            assert_eq!(name_child, look_ahead.select_child("name").unwrap());
-            assert!(!child0.has_children());
+            assert_eq!(name_child.name, "name");
+            assert_eq!(name_child.alias, None);
+            assert_eq!(name_child.field_name(), "name");
+            assert!(!name_child.has_arguments());
+            assert!(!name_child.has_children());
 
-            let child1 = look_ahead.select_child("friends").unwrap();
+            let aliased_name_child = children.next().unwrap();
-            assert_eq!(child1.field_name(), "friends");
+            assert!(look_ahead.has_child("aliasedName"));
-            assert!(!child1.has_arguments());
+            assert_eq!(
-            assert!(child1.has_children());
+                aliased_name_child,
-            assert_eq!(child1.child_names(), vec!["name"]);
+                look_ahead.select_child("aliasedName").unwrap()
+            );
+            assert_eq!(aliased_name_child.name, "name");
+            assert_eq!(aliased_name_child.alias, Some("aliasedName"));
+            assert_eq!(aliased_name_child.field_name(), "aliasedName");
+            assert!(!aliased_name_child.has_arguments());
+            assert!(!aliased_name_child.has_children());
 
-            let child2 = child1.select_child("name").unwrap();
+            let friends_child = children.next().unwrap();
-            assert!(!child2.has_arguments());
+            assert!(look_ahead.has_child("friends"));
-            assert!(!child2.has_children());
+            assert_eq!(friends_child, look_ahead.select_child("friends").unwrap());
+            assert_eq!(friends_child.name, "friends");
+            assert_eq!(friends_child.alias, None);
+            assert_eq!(friends_child.field_name(), "friends");
+            assert!(!friends_child.has_arguments());
+            assert!(friends_child.has_children());
+            assert_eq!(friends_child.child_names(), vec!["name"]);
+
+            assert!(children.next().is_none());
+
+            let mut friends_children = friends_child.children().into_iter();
+            let child = friends_children.next().unwrap();
+            assert!(friends_child.has_child("name"));
+            assert_eq!(child, friends_child.select_child("name").unwrap());
+            assert_eq!(child.name, "name");
+            assert_eq!(child.alias, None);
+            assert_eq!(child.field_name(), "name");
+            assert!(!child.has_arguments());
+            assert!(!child.has_children());
+
+            assert!(friends_children.next().is_none());
         } else {
             panic!("No Operation found");
         }