s/Hash/BTree/g in codegen to make the output deterministic

bolinfest · bolinfest · commit 1dc6d20340e7 · 2022-08-10T23:04:32.000-07:00
We were seeing cases where `#[derive(GraphqlQuery)]` was not generating
consistent output. This appeared to be due to the use of `HashMap` and
`HashSet`, whose iteration order is not guaranteed to be consistent.
Switching to `BTreeMap` and `BTreeSet` appears to fix the issue.
diff --git a/graphql_client_codegen/src/lib.rs b/graphql_client_codegen/src/lib.rs
@@ -29,7 +29,7 @@ mod tests;
 
 pub use crate::codegen_options::{CodegenMode, GraphQLClientCodegenOptions};
 
-use std::{collections::HashMap, fmt::Display, io};
+use std::{collections::BTreeMap, fmt::Display, io};
 
 #[derive(Debug)]
 struct GeneralError(String);
@@ -43,7 +43,7 @@ impl Display for GeneralError {
 impl std::error::Error for GeneralError {}
 
 type BoxError = Box<dyn std::error::Error + Send + Sync + 'static>;
-type CacheMap<T> = std::sync::Mutex<HashMap<std::path::PathBuf, T>>;
+type CacheMap<T> = std::sync::Mutex<BTreeMap<std::path::PathBuf, T>>;
 
 lazy_static! {
     static ref SCHEMA_CACHE: CacheMap<schema::Schema> = CacheMap::default();
@@ -57,7 +57,7 @@ pub fn generate_module_token_stream(
     schema_path: &std::path::Path,
     options: GraphQLClientCodegenOptions,
 ) -> Result<TokenStream, BoxError> {
-    use std::collections::hash_map;
+    use std::collections::btree_map;
 
     let schema_extension = schema_path
         .extension()
@@ -69,8 +69,8 @@ pub fn generate_module_token_stream(
     let schema: schema::Schema = {
         let mut lock = SCHEMA_CACHE.lock().expect("schema cache is poisoned");
         match lock.entry(schema_path.to_path_buf()) {
-            hash_map::Entry::Occupied(o) => o.get().clone(),
-            hash_map::Entry::Vacant(v) => {
+            btree_map::Entry::Occupied(o) => o.get().clone(),
+            btree_map::Entry::Vacant(v) => {
                 schema_string = read_file(v.key())?;
                 let schema = match schema_extension {
                     "graphql" | "gql" => {
@@ -93,8 +93,8 @@ pub fn generate_module_token_stream(
     let (query_string, query) = {
         let mut lock = QUERY_CACHE.lock().expect("query cache is poisoned");
         match lock.entry(query_path) {
-            hash_map::Entry::Occupied(o) => o.get().clone(),
-            hash_map::Entry::Vacant(v) => {
+            btree_map::Entry::Occupied(o) => o.get().clone(),
+            btree_map::Entry::Vacant(v) => {
                 let query_string = read_file(v.key())?;
                 let query = graphql_parser::parse_query(&query_string)
                     .map_err(|err| GeneralError(format!("Query parser error: {}", err)))?
diff --git a/graphql_client_codegen/src/query.rs b/graphql_client_codegen/src/query.rs
@@ -19,7 +19,7 @@ use crate::{
     },
 };
 use std::{
-    collections::{HashMap, HashSet},
+    collections::{BTreeMap, BTreeSet},
     fmt::Display,
 };
 
@@ -42,7 +42,7 @@ impl QueryValidationError {
     }
 }
 
-#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord)]
 pub(crate) struct SelectionId(u32);
 #[derive(Debug, Clone, Copy, PartialEq)]
 pub(crate) struct OperationId(u32);
@@ -53,7 +53,7 @@ impl OperationId {
     }
 }
 
-#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, PartialOrd, Ord)]
 pub(crate) struct ResolvedFragmentId(u32);
 
 #[derive(Debug, Clone, Copy)]
@@ -509,7 +509,7 @@ where
 pub(crate) struct Query {
     fragments: Vec<ResolvedFragment>,
     operations: Vec<ResolvedOperation>,
-    selection_parent_idx: HashMap<SelectionId, SelectionParent>,
+    selection_parent_idx: BTreeMap<SelectionId, SelectionParent>,
     selections: Vec<Selection>,
     variables: Vec<ResolvedVariable>,
 }
@@ -620,8 +620,8 @@ impl ResolvedVariable {
 
 #[derive(Debug, Default)]
 pub(crate) struct UsedTypes {
-    pub(crate) types: HashSet<TypeId>,
-    fragments: HashSet<ResolvedFragmentId>,
+    pub(crate) types: BTreeSet<TypeId>,
+    fragments: BTreeSet<ResolvedFragmentId>,
 }
 
 impl UsedTypes {
diff --git a/graphql_client_codegen/src/schema.rs b/graphql_client_codegen/src/schema.rs
@@ -6,7 +6,7 @@ mod tests;
 
 use crate::query::UsedTypes;
 use crate::type_qualifiers::GraphqlTypeQualifier;
-use std::collections::{HashMap, HashSet};
+use std::collections::{BTreeMap, BTreeSet};
 
 pub(crate) const DEFAULT_SCALARS: &[&str] = &["ID", "String", "Int", "Float", "Boolean"];
 
@@ -44,25 +44,25 @@ pub(crate) enum StoredFieldParent {
     Interface(InterfaceId),
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Hash, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Hash, Eq, PartialOrd, Ord)]
 pub(crate) struct ObjectId(u32);
 
 #[derive(Debug, Clone, Copy, PartialEq, Hash, Eq)]
 pub(crate) struct ObjectFieldId(usize);
 
-#[derive(Debug, Clone, Copy, PartialEq, Hash, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Hash, Eq, PartialOrd, Ord)]
 pub(crate) struct InterfaceId(usize);
 
-#[derive(Debug, Clone, Copy, PartialEq, Hash, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Hash, Eq, PartialOrd, Ord)]
 pub(crate) struct ScalarId(usize);
 
-#[derive(Debug, Clone, Copy, PartialEq, Hash, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Hash, Eq, PartialOrd, Ord)]
 pub(crate) struct UnionId(usize);
 
-#[derive(Debug, Clone, Copy, PartialEq, Hash, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Hash, Eq, PartialOrd, Ord)]
 pub(crate) struct EnumId(usize);
 
-#[derive(Debug, Clone, Copy, PartialEq, Hash, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Hash, Eq, PartialOrd, Ord)]
 pub(crate) struct InputId(u32);
 
 #[derive(Debug, Clone, Copy, PartialEq)]
@@ -98,7 +98,7 @@ pub(crate) struct StoredScalar {
     pub(crate) name: String,
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Hash, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Hash, Eq, PartialOrd, Ord)]
 pub(crate) enum TypeId {
     Object(ObjectId),
     Scalar(ScalarId),
@@ -222,7 +222,7 @@ pub(crate) struct Schema {
     stored_scalars: Vec<StoredScalar>,
     stored_enums: Vec<StoredEnum>,
     stored_inputs: Vec<StoredInputType>,
-    names: HashMap<String, TypeId>,
+    names: BTreeMap<String, TypeId>,
 
     pub(crate) query_type: Option<ObjectId>,
     pub(crate) mutation_type: Option<ObjectId>,
@@ -239,7 +239,7 @@ impl Schema {
             stored_scalars: Vec::with_capacity(DEFAULT_SCALARS.len()),
             stored_enums: Vec::new(),
             stored_inputs: Vec::new(),
-            names: HashMap::new(),
+            names: BTreeMap::new(),
             query_type: None,
             mutation_type: None,
             subscription_type: None,
@@ -404,7 +404,7 @@ impl StoredInputType {
         &'a self,
         input_id: InputId,
         schema: &'a Schema,
-        visited_types: &mut HashSet<&'a str>,
+        visited_types: &mut BTreeSet<&'a str>,
     ) -> bool {
         visited_types.insert(&self.name);
         // The input type is recursive if any of its members contains it, without indirection
@@ -440,7 +440,7 @@ impl StoredInputType {
 
 pub(crate) fn input_is_recursive_without_indirection(input_id: InputId, schema: &Schema) -> bool {
     let input = schema.get_input(input_id);
-    let mut visited_types = HashSet::<&str>::new();
+    let mut visited_types = BTreeSet::<&str>::new();
     input.contains_type_without_indirection(input_id, schema, &mut visited_types)
 }
 impl<'doc, T> std::convert::From<graphql_parser::schema::Document<'doc, T>> for Schema
diff --git a/graphql_client_codegen/src/schema/json_conversion.rs b/graphql_client_codegen/src/schema/json_conversion.rs
@@ -14,7 +14,6 @@ pub(super) fn build_schema(src: IntrospectionResponse) -> Schema {
 
 fn build_names_map(src: &mut JsonSchema, schema: &mut Schema) {
     let names = &mut schema.names;
-    names.reserve(types_mut(src).count());
 
     unions_mut(src)
         .map(|u| u.name.as_ref().expect("union name"))
