fix(federation): change HashMap/HashSet usage to IndexMap/IndexSet to avoid non-determinism

apollo-federation/src/link/cost_spec_definition.rs

@@ -1,7 +1,6 @@
-use std::collections::HashMap;
-
 use apollo_compiler::ast::Argument;
 use apollo_compiler::ast::Directive;
+use apollo_compiler::collections::IndexMap;
 use apollo_compiler::name;
 use apollo_compiler::schema::Component;
 use apollo_compiler::schema::EnumType;
@@ -41,7 +40,7 @@ macro_rules! propagate_demand_control_directives {
             subgraph_schema: &FederationSchema,
             source: &$directives_ty,
             dest: &mut $directives_ty,
-            original_directive_names: &HashMap<Name, Name>,
+            original_directive_names: &IndexMap<Name, Name>,
         ) -> Result<(), FederationError> {
             let cost_directive_name = original_directive_names.get(&COST_DIRECTIVE_NAME_IN_SPEC);
             let cost_directive = cost_directive_name.and_then(|name| source.get(name.as_str()));
@@ -75,7 +74,7 @@ macro_rules! propagate_demand_control_directives_to_position {
             subgraph_schema: &mut FederationSchema,
             source: &Node<$source_ty>,
             dest: &$dest_ty,
-            original_directive_names: &HashMap<Name, Name>,
+            original_directive_names: &IndexMap<Name, Name>,
         ) -> Result<(), FederationError> {
             let cost_directive_name = original_directive_names.get(&COST_DIRECTIVE_NAME_IN_SPEC);
             let cost_directive =

apollo-federation/src/link/database.rs

@@ -1,9 +1,9 @@
 use std::borrow::Cow;
-use std::collections::HashMap;
 use std::sync::Arc;
 
 use apollo_compiler::ast::Directive;
 use apollo_compiler::ast::DirectiveLocation;
+use apollo_compiler::collections::IndexMap;
 use apollo_compiler::schema::DirectiveDefinition;
 use apollo_compiler::ty;
 use apollo_compiler::Schema;
@@ -46,10 +46,10 @@ pub fn links_metadata(schema: &Schema) -> Result<Option<LinksMetadata>, LinkErro
     // all of the @link usages (starting with the bootstrapping one) and extract their metadata.
     let link_name_in_schema = &bootstrap_directive.name;
     let mut links = Vec::new();
-    let mut by_identity = HashMap::new();
-    let mut by_name_in_schema = HashMap::new();
-    let mut types_by_imported_name = HashMap::new();
-    let mut directives_by_imported_name = HashMap::new();
+    let mut by_identity = IndexMap::default();
+    let mut by_name_in_schema = IndexMap::default();
+    let mut types_by_imported_name = IndexMap::default();
+    let mut directives_by_imported_name = IndexMap::default();
     let link_applications = schema
         .schema_definition
         .directives

apollo-federation/src/link/mod.rs

@@ -1,10 +1,10 @@
-use std::collections::HashMap;
 use std::fmt;
 use std::str;
 use std::sync::Arc;
 
 use apollo_compiler::ast::Directive;
 use apollo_compiler::ast::Value;
+use apollo_compiler::collections::IndexMap;
 use apollo_compiler::name;
 use apollo_compiler::schema::Component;
 use apollo_compiler::InvalidNameError;
@@ -387,10 +387,10 @@ pub struct LinkedElement {
 #[derive(Default, Eq, PartialEq, Debug)]
 pub struct LinksMetadata {
     pub(crate) links: Vec<Arc<Link>>,
-    pub(crate) by_identity: HashMap<Identity, Arc<Link>>,
-    pub(crate) by_name_in_schema: HashMap<Name, Arc<Link>>,
-    pub(crate) types_by_imported_name: HashMap<Name, (Arc<Link>, Arc<Import>)>,
-    pub(crate) directives_by_imported_name: HashMap<Name, (Arc<Link>, Arc<Import>)>,
+    pub(crate) by_identity: IndexMap<Identity, Arc<Link>>,
+    pub(crate) by_name_in_schema: IndexMap<Name, Arc<Link>>,
+    pub(crate) types_by_imported_name: IndexMap<Name, (Arc<Link>, Arc<Import>)>,
+    pub(crate) directives_by_imported_name: IndexMap<Name, (Arc<Link>, Arc<Import>)>,
 }
 
 impl LinksMetadata {

apollo-federation/src/merge.rs

@@ -1,4 +1,3 @@
-use std::collections::HashSet;
 use std::fmt::Debug;
 use std::fmt::Formatter;
 use std::iter;
@@ -1582,8 +1581,8 @@ fn add_core_feature_inaccessible(supergraph: &mut Schema) {
 // TODO use apollo_compiler::executable::FieldSet
 fn parse_keys<'a>(
     directives: impl Iterator<Item = &'a Component<Directive>> + Sized,
-) -> HashSet<&'a str> {
-    HashSet::from_iter(
+) -> IndexSet<&'a str> {
+    IndexSet::from_iter(
         directives
             .flat_map(|k| {
                 let field_set = directive_string_arg_value(k, &name!("fields")).unwrap();

apollo-federation/src/operation/contains.rs

@@ -1,5 +1,4 @@
-use std::collections::HashMap;
-
+use apollo_compiler::collections::IndexMap;
 use apollo_compiler::executable;
 use apollo_compiler::Name;
 use apollo_compiler::Node;
@@ -146,7 +145,7 @@ fn same_arguments(
     let right = right
         .iter()
         .map(|arg| (&arg.name, arg))
-        .collect::<HashMap<_, _>>();
+        .collect::<IndexMap<_, _>>();
 
     left.iter().all(|arg| {
         right

apollo-federation/src/operation/mod.rs

@@ -13,8 +13,6 @@
 //! [`Field`], and the selection type is [`FieldSelection`].
 
 use std::borrow::Cow;
-use std::collections::HashMap;
-use std::collections::HashSet;
 use std::fmt::Display;
 use std::fmt::Formatter;
 use std::hash::Hash;
@@ -103,7 +101,7 @@ pub struct Operation {
 pub(crate) struct NormalizedDefer {
     pub operation: Operation,
     pub has_defers: bool,
-    pub assigned_defer_labels: HashSet<String>,
+    pub assigned_defer_labels: IndexSet<String>,
     pub defer_conditions: IndexMap<String, IndexSet<String>>,
 }
 
@@ -152,7 +150,7 @@ impl Operation {
         NormalizedDefer {
             operation: self,
             has_defers: false,
-            assigned_defer_labels: HashSet::new(),
+            assigned_defer_labels: IndexSet::default(),
             defer_conditions: IndexMap::default(),
         }
         // TODO(@TylerBloom): Once defer is implement, the above statement needs to be replaced
@@ -164,7 +162,7 @@ impl Operation {
             NormalizedDefer {
                 operation: self,
                 has_defers: false,
-                assigned_defer_labels: HashSet::new(),
+                assigned_defer_labels: IndexSet::default(),
                 defer_conditions: IndexMap::default(),
             }
         }
@@ -2652,7 +2650,8 @@ impl SelectionSet {
             return Ok(self.clone());
         }
 
-        let mut at_current_level: HashMap<FetchDataPathElement, &FieldToAlias> = HashMap::new();
+        let mut at_current_level: IndexMap<FetchDataPathElement, &FieldToAlias> =
+            IndexMap::default();
         let mut remaining: Vec<&FieldToAlias> = Vec::new();
 
         for alias in aliases {
@@ -2929,7 +2928,7 @@ fn compute_aliases_for_non_merging_fields(
     alias_collector: &mut Vec<FieldToAlias>,
     schema: &ValidFederationSchema,
 ) -> Result<(), FederationError> {
-    let mut seen_response_names: HashMap<Name, SeenResponseName> = HashMap::new();
+    let mut seen_response_names: IndexMap<Name, SeenResponseName> = IndexMap::default();
 
     // - `s.selections` must be fragment-spread-free.
     fn rebased_fields_in_set(s: &SelectionSetAtPath) -> impl Iterator<Item = FieldInPath> + '_ {
@@ -3061,7 +3060,7 @@ fn compute_aliases_for_non_merging_fields(
     Ok(())
 }
 
-fn gen_alias_name(base_name: &Name, unavailable_names: &HashMap<Name, SeenResponseName>) -> Name {
+fn gen_alias_name(base_name: &Name, unavailable_names: &IndexMap<Name, SeenResponseName>) -> Name {
     let mut counter = 0usize;
     loop {
         if let Ok(name) = Name::try_from(format!("{base_name}__alias_{counter}")) {
@@ -3419,7 +3418,7 @@ impl NamedFragments {
         //       the outcome of `map_to_expanded_selection_sets`.
         let mut fragments_map: IndexMap<Name, FragmentDependencies> = IndexMap::default();
         for fragment in fragments.values() {
-            let mut fragment_usages = HashMap::new();
+            let mut fragment_usages = IndexMap::default();
             NamedFragments::collect_fragment_usages(&fragment.selection_set, &mut fragment_usages);
             let usages: Vec<Name> = fragment_usages.keys().cloned().collect::<Vec<Name>>();
             fragments_map.insert(
@@ -3431,7 +3430,7 @@ impl NamedFragments {
             );
         }
 
-        let mut removed_fragments: HashSet<Name> = HashSet::new();
+        let mut removed_fragments: IndexSet<Name> = IndexSet::default();
         let mut mapped_fragments = NamedFragments::default();
         while !fragments_map.is_empty() {
             // Note that graphQL specifies that named fragments cannot have cycles (https://spec.graphql.org/draft/#sec-Fragment-spreads-must-not-form-cycles)
@@ -3449,7 +3448,7 @@ impl NamedFragments {
                         // JS code has methods for
                         // * add and throw exception if entry already there
                         // * add_if_not_exists
-                        // Rust HashMap exposes insert (that overwrites) and try_insert (that throws)
+                        // Rust IndexMap exposes insert (that overwrites) and try_insert (that throws)
                         mapped_fragments.insert(normalized);
                     } else {
                         removed_fragments.insert(name.clone());
@@ -3465,7 +3464,7 @@ impl NamedFragments {
     /// Just like our `SelectionSet::used_fragments`, but with apollo-compiler types
     fn collect_fragment_usages(
         selection_set: &executable::SelectionSet,
-        aggregator: &mut HashMap<Name, u32>,
+        aggregator: &mut IndexMap<Name, u32>,
     ) {
         selection_set.selections.iter().for_each(|s| match s {
             executable::Selection::Field(f) => {
@@ -3512,7 +3511,7 @@ impl NamedFragments {
 // Collect fragment usages from operation types.
 
 impl Selection {
-    fn collect_used_fragment_names(&self, aggregator: &mut HashMap<Name, u32>) {
+    fn collect_used_fragment_names(&self, aggregator: &mut IndexMap<Name, u32>) {
         match self {
             Selection::Field(field_selection) => {
                 if let Some(s) = &field_selection.selection_set {
@@ -3533,28 +3532,28 @@ impl Selection {
 }
 
 impl SelectionSet {
-    pub(crate) fn collect_used_fragment_names(&self, aggregator: &mut HashMap<Name, u32>) {
+    pub(crate) fn collect_used_fragment_names(&self, aggregator: &mut IndexMap<Name, u32>) {
         for s in self.selections.values() {
             s.collect_used_fragment_names(aggregator);
         }
     }
 
-    pub(crate) fn used_fragments(&self) -> HashMap<Name, u32> {
-        let mut usages = HashMap::new();
+    pub(crate) fn used_fragments(&self) -> IndexMap<Name, u32> {
+        let mut usages = IndexMap::default();
         self.collect_used_fragment_names(&mut usages);
         usages
     }
 }
 
 impl Fragment {
-    pub(crate) fn collect_used_fragment_names(&self, aggregator: &mut HashMap<Name, u32>) {
+    pub(crate) fn collect_used_fragment_names(&self, aggregator: &mut IndexMap<Name, u32>) {
         self.selection_set.collect_used_fragment_names(aggregator)
     }
 }
 
 impl NamedFragments {
     /// Collect the usages of fragments that are used within the selection of other fragments.
-    pub(crate) fn collect_used_fragment_names(&self, aggregator: &mut HashMap<Name, u32>) {
+    pub(crate) fn collect_used_fragment_names(&self, aggregator: &mut IndexMap<Name, u32>) {
         for fragment in self.fragments.values() {
             fragment
                 .selection_set
@@ -3566,7 +3565,7 @@ impl NamedFragments {
 // Collect used variables from operation types.
 
 pub(crate) struct VariableCollector<'s> {
-    variables: HashSet<&'s Name>,
+    variables: IndexSet<&'s Name>,
 }
 
 impl<'s> VariableCollector<'s> {
@@ -3655,14 +3654,14 @@ impl<'s> VariableCollector<'s> {
     }
 
     /// Consume the collector and return the collected names.
-    pub(crate) fn into_inner(self) -> HashSet<&'s Name> {
+    pub(crate) fn into_inner(self) -> IndexSet<&'s Name> {
         self.variables
     }
 }
 
 impl Fragment {
     /// Returns the variable names that are used by this fragment.
-    pub(crate) fn used_variables(&self) -> HashSet<&'_ Name> {
+    pub(crate) fn used_variables(&self) -> IndexSet<&'_ Name> {
         let mut collector = VariableCollector::new();
         collector.visit_directive_list(&self.directives);
         collector.visit_selection_set(&self.selection_set);
@@ -3673,7 +3672,7 @@ impl Fragment {
 impl SelectionSet {
     /// Returns the variable names that are used by this selection set, including through fragment
     /// spreads.
-    pub(crate) fn used_variables(&self) -> HashSet<&'_ Name> {
+    pub(crate) fn used_variables(&self) -> IndexSet<&'_ Name> {
         let mut collector = VariableCollector::new();
         collector.visit_selection_set(self);
         collector.into_inner()

apollo-federation/src/operation/optimize.rs

@@ -35,10 +35,10 @@
 //! ## `reuse_fragments` methods (putting everything together)
 //! Recursive optimization of selection and selection sets.
 
-use std::collections::HashMap;
-use std::collections::HashSet;
 use std::sync::Arc;
 
+use apollo_compiler::collections::IndexMap;
+use apollo_compiler::collections::IndexSet;
 use apollo_compiler::executable;
 use apollo_compiler::executable::VariableDefinition;
 use apollo_compiler::Name;
@@ -67,7 +67,7 @@ use crate::schema::position::CompositeTypeDefinitionPosition;
 #[derive(Debug)]
 struct ReuseContext<'a> {
     fragments: &'a NamedFragments,
-    operation_variables: Option<HashSet<&'a Name>>,
+    operation_variables: Option<IndexSet<&'a Name>>,
 }
 
 impl<'a> ReuseContext<'a> {
@@ -392,7 +392,7 @@ impl NamedFragments {
 // `Option<FieldsConflictValidator>`. However, `None` validator makes it clearer that validation is
 // unnecessary.
 struct FieldsConflictValidator {
-    by_response_name: HashMap<Name, HashMap<Field, Option<Arc<FieldsConflictValidator>>>>,
+    by_response_name: IndexMap<Name, IndexMap<Field, Option<Arc<FieldsConflictValidator>>>>,
 }
 
 impl FieldsConflictValidator {
@@ -406,7 +406,8 @@ impl FieldsConflictValidator {
 
     fn for_level<'a>(level: &[&'a SelectionSet]) -> Self {
         // Group `level`'s fields by the response-name/field
-        let mut at_level: HashMap<Name, HashMap<Field, Vec<&'a SelectionSet>>> = HashMap::new();
+        let mut at_level: IndexMap<Name, IndexMap<Field, Vec<&'a SelectionSet>>> =
+            IndexMap::default();
         for selection_set in level {
             for field_selection in selection_set.field_selections() {
                 let response_name = field_selection.field.response_name();
@@ -421,10 +422,10 @@ impl FieldsConflictValidator {
         }
 
         // Collect validators per response-name/field
-        let mut by_response_name = HashMap::new();
+        let mut by_response_name = IndexMap::default();
         for (response_name, fields) in at_level {
-            let mut at_response_name: HashMap<Field, Option<Arc<FieldsConflictValidator>>> =
-                HashMap::new();
+            let mut at_response_name: IndexMap<Field, Option<Arc<FieldsConflictValidator>>> =
+                IndexMap::default();
             for (field, selection_sets) in fields {
                 if selection_sets.is_empty() {
                     at_response_name.insert(field, None);
@@ -631,7 +632,7 @@ struct FragmentRestrictionAtType {
 
 #[derive(Default)]
 struct FragmentRestrictionAtTypeCache {
-    map: HashMap<(Name, CompositeTypeDefinitionPosition), Arc<FragmentRestrictionAtType>>,
+    map: IndexMap<(Name, CompositeTypeDefinitionPosition), Arc<FragmentRestrictionAtType>>,
 }
 
 impl FragmentRestrictionAtTypeCache {
@@ -644,8 +645,8 @@ impl FragmentRestrictionAtTypeCache {
         // the lifetime does not really want to work out.
         // (&'cache mut self) -> Result<&'cache FragmentRestrictionAtType>
         match self.map.entry((fragment.name.clone(), ty.clone())) {
-            std::collections::hash_map::Entry::Occupied(entry) => Ok(Arc::clone(entry.get())),
-            std::collections::hash_map::Entry::Vacant(entry) => Ok(Arc::clone(
+            indexmap::map::Entry::Occupied(entry) => Ok(Arc::clone(entry.get())),
+            indexmap::map::Entry::Vacant(entry) => Ok(Arc::clone(
                 entry.insert(Arc::new(fragment.expanded_selection_set_at_type(ty)?)),
             )),
         }
@@ -866,7 +867,7 @@ impl SelectionSet {
     ) {
         // Note: It's not possible for two fragments to include each other. So, we don't need to
         //       worry about inclusion cycles.
-        let included_fragments: HashSet<Name> = applicable_fragments
+        let included_fragments: IndexSet<Name> = applicable_fragments
             .iter()
             .filter(|(fragment, _)| {
                 applicable_fragments
@@ -1249,8 +1250,12 @@ impl NamedFragments {
         )
     }
 
-    fn update_usages(usages: &mut HashMap<Name, u32>, fragment: &Node<Fragment>, usage_count: u32) {
-        let mut inner_usages = HashMap::new();
+    fn update_usages(
+        usages: &mut IndexMap<Name, u32>,
+        fragment: &Node<Fragment>,
+        usage_count: u32,
+    ) {
+        let mut inner_usages = IndexMap::default();
         fragment.collect_used_fragment_names(&mut inner_usages);
 
         for (name, inner_count) in inner_usages {
@@ -1625,7 +1630,7 @@ fn fragment_name(mut index: usize) -> Name {
 struct FragmentGenerator {
     fragments: NamedFragments,
     // XXX(@goto-bus-stop): This is temporary to support mismatch testing with JS!
-    names: HashMap<(String, usize), usize>,
+    names: IndexMap<(String, usize), usize>,
 }
 
 impl FragmentGenerator {