Improve error handling in graphql-query-derive

Author: tomhoule

examples/github/Cargo.toml

@@ -8,7 +8,7 @@ edition = "2018"
 anyhow = "1.0"
 graphql_client = { path = "../../graphql_client" }
 serde = "^1.0"
-reqwest = { version = "^0.10", features = ["json", "blocking"] }
+reqwest = { version = "^0.11", features = ["json", "blocking"] }
 prettytable-rs = "^0.7"
 structopt = "^0.3"
 dotenv = "^0.13"

examples/hasura/Cargo.toml

@@ -6,11 +6,11 @@ edition = "2018"
 
 [dev-dependencies]
 anyhow = "1.0"
-graphql_client = { path = "../../graphql_client" }
+graphql_client = { path = "../../graphql_client", features = ["reqwest"] }
 serde = "1.0"
 serde_derive = "1.0"
 serde_json = "1.0"
-reqwest = { version = "^0.10", features = ["json", "blocking"] }
+reqwest = { version = "^0.11", features = ["json", "blocking"] }
 prettytable-rs = "0.7.0"
 dotenv = "0.13.0"
 log = "0.4.3"

graphql-introspection-query/src/introspection_response.rs

@@ -286,7 +286,7 @@ impl IntrospectionResponse {
     pub fn as_schema(&self) -> &SchemaContainer {
         match self {
             IntrospectionResponse::FullResponse(full_response) => &full_response.data,
-            IntrospectionResponse::Schema(schema) => &schema,
+            IntrospectionResponse::Schema(schema) => schema,
         }
     }
 

graphql_client_codegen/src/codegen.rs

@@ -298,7 +298,7 @@ fn render_object_literal(
         .fields
         .iter()
         .map(|(name, r#type)| {
-            let field_name = Ident::new(&name, Span::call_site());
+            let field_name = Ident::new(name, Span::call_site());
             let provided_value = object_map.get(name);
             match provided_value {
                 Some(default_value) => {

graphql_client_codegen/src/codegen/selection.rs

@@ -168,7 +168,7 @@ fn calculate_selection<'a>(
                     .iter()
                     .map(|id| (id, context.query.query.get_selection(*id)))
                     .filter_map(|(id, selection)| {
-                        VariantSelection::from_selection(&selection, type_id, context.query)
+                        VariantSelection::from_selection(selection, type_id, context.query)
                             .map(|variant_selection| (*id, selection, variant_selection))
                     })
                     .collect();
@@ -188,7 +188,7 @@ fn calculate_selection<'a>(
 
                 if let Some((selection_id, selection, _variant)) = variant_selections.get(0) {
                     let mut variant_struct_name_str =
-                        full_path_prefix(*selection_id, &context.query);
+                        full_path_prefix(*selection_id, context.query);
                     variant_struct_name_str.reserve(2 + variant_name_str.len());
                     variant_struct_name_str.push_str("On");
                     variant_struct_name_str.push_str(variant_name_str);
@@ -258,7 +258,7 @@ fn calculate_selection<'a>(
 
         match selection {
             Selection::Field(field) => {
-                let (graphql_name, rust_name) = context.field_name(&field);
+                let (graphql_name, rust_name) = context.field_name(field);
                 let schema_field = field.schema_field(context.schema());
                 let field_type_id = schema_field.r#type.id;
 
@@ -295,7 +295,7 @@ fn calculate_selection<'a>(
                         });
                     }
                     TypeId::Object(_) | TypeId::Interface(_) | TypeId::Union(_) => {
-                        let struct_name_string = full_path_prefix(*id, &context.query);
+                        let struct_name_string = full_path_prefix(*id, context.query);
 
                         context.push_field(ExpandedField {
                             struct_id,
@@ -436,7 +436,7 @@ impl<'a> ExpandedVariant<'a> {
     fn render(&self) -> TokenStream {
         let name_ident = Ident::new(&self.name, Span::call_site());
         let optional_type_ident = self.variant_type.as_ref().map(|variant_type| {
-            let ident = Ident::new(&variant_type, Span::call_site());
+            let ident = Ident::new(variant_type, Span::call_site());
             quote!((#ident))
         });
 
@@ -507,7 +507,7 @@ impl<'a> ExpandedSelection<'a> {
 
             // If the type is aliased, stop here.
             if let Some(alias) = self.aliases.iter().find(|alias| alias.struct_id == type_id) {
-                let fragment_name = Ident::new(&alias.name, Span::call_site());
+                let fragment_name = Ident::new(alias.name, Span::call_site());
                 let fragment_name = if alias.boxed {
                     quote!(Box<#fragment_name>)
                 } else {

graphql_client_codegen/src/generated_module.rs

@@ -31,7 +31,7 @@ impl<'a> GeneratedModule<'a> {
     fn build_impls(&self) -> Result<TokenStream, BoxError> {
         Ok(crate::codegen::response_for_query(
             self.root()?,
-            &self.options,
+            self.options,
             BoundQuery {
                 query: self.resolved_query,
                 schema: self.schema,

graphql_client_codegen/src/query.rs

@@ -188,7 +188,7 @@ fn resolve_fragment(
     fragment_definition: &graphql_parser::query::FragmentDefinition,
 ) -> Result<(), QueryValidationError> {
     let graphql_parser::query::TypeCondition::On(on) = &fragment_definition.type_condition;
-    let on = schema.find_type(&on).ok_or_else(|| {
+    let on = schema.find_type(on).ok_or_else(|| {
         QueryValidationError::new(format!(
             "Could not find type `{}` referenced by fragment `{}`",
             on, fragment_definition.name

graphql_query_derive/src/attributes.rs

@@ -1,4 +1,3 @@
-use crate::{BoxError, GeneralError};
 use graphql_client_codegen::deprecation::DeprecationStrategy;
 use graphql_client_codegen::normalization::Normalization;
 
@@ -33,7 +32,10 @@ pub fn extract_attr(ast: &syn::DeriveInput, attr: &str) -> Result<String, syn::E
         }
     }
 
-    Err(syn::Error::new_spanned(ast, "Attribute not found"))
+    Err(syn::Error::new_spanned(
+        &ast,
+        format!("Attribute `{}` not found", attr),
+    ))
 }
 
 /// Extract a list of configuration parameter values specified in the `graphql` attribute.
@@ -75,21 +77,21 @@ pub fn extract_attr_list(ast: &syn::DeriveInput, attr: &str) -> Result<Vec<Strin
 /// Get the deprecation from a struct attribute in the derive case.
 pub fn extract_deprecation_strategy(
     ast: &syn::DeriveInput,
-) -> Result<DeprecationStrategy, BoxError> {
+) -> Result<DeprecationStrategy, syn::Error> {
     extract_attr(&ast, "deprecated")?
         .to_lowercase()
         .as_str()
         .parse()
-        .map_err(|_| GeneralError(DEPRECATION_ERROR.to_owned()).into())
+        .map_err(|_| syn::Error::new_spanned(ast, DEPRECATION_ERROR.to_owned()))
 }
 
 /// Get the deprecation from a struct attribute in the derive case.
-pub fn extract_normalization(ast: &syn::DeriveInput) -> Result<Normalization, BoxError> {
+pub fn extract_normalization(ast: &syn::DeriveInput) -> Result<Normalization, syn::Error> {
     extract_attr(&ast, "normalization")?
         .to_lowercase()
         .as_str()
         .parse()
-        .map_err(|_| GeneralError(NORMALIZATION_ERROR.to_owned()).into())
+        .map_err(|_| syn::Error::new_spanned(ast, NORMALIZATION_ERROR))
 }
 
 #[cfg(test)]

graphql_query_derive/src/lib.rs

@@ -8,66 +8,58 @@ use graphql_client_codegen::{
 };
 use std::{
     env,
-    fmt::Display,
     path::{Path, PathBuf},
 };
 
 use proc_macro2::TokenStream;
 
-type BoxError = Box<dyn std::error::Error + 'static>;
-
-#[derive(Debug)]
-struct GeneralError(String);
-
-impl Display for GeneralError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        f.write_str(&self.0)
-    }
-}
-
-impl std::error::Error for GeneralError {}
-
 #[proc_macro_derive(GraphQLQuery, attributes(graphql))]
 pub fn derive_graphql_query(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
     match graphql_query_derive_inner(input) {
         Ok(ts) => ts,
-        Err(err) => panic!("{:?}", err),
+        Err(err) => err.to_compile_error().into(),
     }
 }
 
 fn graphql_query_derive_inner(
     input: proc_macro::TokenStream,
-) -> Result<proc_macro::TokenStream, BoxError> {
+) -> Result<proc_macro::TokenStream, syn::Error> {
     let input = TokenStream::from(input);
     let ast = syn::parse2(input)?;
     let (query_path, schema_path) = build_query_and_schema_path(&ast)?;
     let options = build_graphql_client_derive_options(&ast, query_path.clone())?;
     Ok(
         generate_module_token_stream(query_path, &schema_path, options)
             .map(Into::into)
-            .map_err(|err| GeneralError(format!("Code generation failed: {}", err)))?,
+            .map_err(|err| {
+                syn::Error::new_spanned(
+                    ast,
+                    format!("Failed to generate GraphQLQuery impl: {}", err),
+                )
+            }),
     )
 }
 
-fn build_query_and_schema_path(input: &syn::DeriveInput) -> Result<(PathBuf, PathBuf), BoxError> {
+fn build_query_and_schema_path(input: &syn::DeriveInput) -> Result<(PathBuf, PathBuf), syn::Error> {
     let cargo_manifest_dir = env::var("CARGO_MANIFEST_DIR").map_err(|_err| {
-        GeneralError("Checking that the CARGO_MANIFEST_DIR env variable is defined.".into())
+        syn::Error::new_spanned(
+            input,
+            "Error checking that the CARGO_MANIFEST_DIR env variable is defined.",
+        )
     })?;
 
-    let query_path = attributes::extract_attr(input, "query_path")
-        .map_err(|err| GeneralError(format!("Error extracting query path. {}", err)))?;
+    let query_path = attributes::extract_attr(input, "query_path")?;
     let query_path = format!("{}/{}", cargo_manifest_dir, query_path);
     let query_path = Path::new(&query_path).to_path_buf();
-    let schema_path = attributes::extract_attr(input, "schema_path")
-        .map_err(|err| GeneralError(format!("Error extracting schema path. {}", err)))?;
+    let schema_path = attributes::extract_attr(input, "schema_path")?;
     let schema_path = Path::new(&cargo_manifest_dir).join(schema_path);
     Ok((query_path, schema_path))
 }
 
 fn build_graphql_client_derive_options(
     input: &syn::DeriveInput,
     query_path: PathBuf,
-) -> Result<GraphQLClientCodegenOptions, BoxError> {
+) -> Result<GraphQLClientCodegenOptions, syn::Error> {
     let variables_derives = attributes::extract_attr(input, "variables_derives").ok();
     let response_derives = attributes::extract_attr(input, "response_derives").ok();
     let custom_scalars_module = attributes::extract_attr(input, "custom_scalars_module").ok();
@@ -96,12 +88,7 @@ fn build_graphql_client_derive_options(
 
     // The user can give a path to a module that provides definitions for the custom scalars.
     if let Some(custom_scalars_module) = custom_scalars_module {
-        let custom_scalars_module = syn::parse_str(&custom_scalars_module).map_err(|err| {
-            GeneralError(format!(
-                "Invalid custom scalars module path: {}. {}",
-                custom_scalars_module, err
-            ))
-        })?;
+        let custom_scalars_module = syn::parse_str(&custom_scalars_module)?;
 
         options.set_custom_scalars_module(custom_scalars_module);
     }