From 8222c5d3b777448efc85e049640e55747adf74fa Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 10 Aug 2022 23:04:22 -0700 Subject: [PATCH 1/2] fix `clippy::format_push_string` issue --- graphql_client/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/graphql_client/src/lib.rs b/graphql_client/src/lib.rs index 0dbae617..d4cf7f94 100644 --- a/graphql_client/src/lib.rs +++ b/graphql_client/src/lib.rs @@ -32,7 +32,7 @@ pub mod reqwest; use serde::{Deserialize, Serialize}; use std::collections::HashMap; -use std::fmt::{self, Display}; +use std::fmt::{self, Display, Write}; /// A convenience trait that can be used to build a GraphQL request body. /// @@ -215,7 +215,7 @@ impl Display for Error { fragments .iter() .fold(String::new(), |mut acc, item| { - acc.push_str(&format!("{}/", item)); + let _ = write!(acc, "{}/", item); acc }) .trim_end_matches('/') From 1dc6d20340e7d58e00fd4d94216173fb63b53e04 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 10 Aug 2022 23:04:32 -0700 Subject: [PATCH 2/2] s/Hash/BTree/g in codegen to make the output deterministic 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. --- graphql_client_codegen/src/lib.rs | 14 +++++------ graphql_client_codegen/src/query.rs | 12 +++++----- graphql_client_codegen/src/schema.rs | 24 +++++++++---------- .../src/schema/json_conversion.rs | 1 - 4 files changed, 25 insertions(+), 26 deletions(-) diff --git a/graphql_client_codegen/src/lib.rs b/graphql_client_codegen/src/lib.rs index c23a2b91..9d9d74f3 100644 --- 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; -type CacheMap = std::sync::Mutex>; +type CacheMap = std::sync::Mutex>; lazy_static! { static ref SCHEMA_CACHE: CacheMap = CacheMap::default(); @@ -57,7 +57,7 @@ pub fn generate_module_token_stream( schema_path: &std::path::Path, options: GraphQLClientCodegenOptions, ) -> Result { - 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 index 12741dae..d70001a8 100644 --- 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, operations: Vec, - selection_parent_idx: HashMap, + selection_parent_idx: BTreeMap, selections: Vec, variables: Vec, } @@ -620,8 +620,8 @@ impl ResolvedVariable { #[derive(Debug, Default)] pub(crate) struct UsedTypes { - pub(crate) types: HashSet, - fragments: HashSet, + pub(crate) types: BTreeSet, + fragments: BTreeSet, } impl UsedTypes { diff --git a/graphql_client_codegen/src/schema.rs b/graphql_client_codegen/src/schema.rs index 6af5c84e..2be24c3e 100644 --- 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, stored_enums: Vec, stored_inputs: Vec, - names: HashMap, + names: BTreeMap, pub(crate) query_type: Option, pub(crate) mutation_type: Option, @@ -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> for Schema diff --git a/graphql_client_codegen/src/schema/json_conversion.rs b/graphql_client_codegen/src/schema/json_conversion.rs index 0eb2d008..52a92cc9 100644 --- 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"))