Add spans for better errors

Author: Bennett Hardwick

cryptonamo-derive/src/cryptonamo.rs

@@ -42,7 +42,7 @@ pub(crate) fn derive_cryptonamo(
                 .flat_map(|x| x.ident.as_ref().map(|x| x.to_string()))
                 .collect();
 
-            let mut compound_indexes: HashMap<String, Vec<(String, String)>> = Default::default();
+            let mut compound_indexes: HashMap<String, Vec<(String, String, Span)>> = Default::default();
 
             for field in &fields_named.named {
                 let ident = &field.ident;
@@ -51,10 +51,10 @@ pub(crate) fn derive_cryptonamo(
 
                 // Parse the meta for the field
                 for attr in &field.attrs {
-                    let mut query: Option<(String, String)> = None;
-                    let mut compound_index_name: Option<(String, Span)> = None;
-
                     if attr.path().is_ident("cryptonamo") {
+                        let mut query: Option<(String, String, Span)> = None;
+                        let mut compound_index_name: Option<(String, Span)> = None;
+
                         attr.parse_nested_meta(|meta| {
                             let directive = meta.path.get_ident().map(|i| i.to_string());
                             match directive.as_deref() {
@@ -70,13 +70,14 @@ pub(crate) fn derive_cryptonamo(
                                 }
                                 Some("query") => {
                                     let value = meta.value()?;
+                                    let index_type_span = value.span();
                                     let index_type = value.parse::<LitStr>()?.value();
                                     let index_name = ident
                                         .as_ref()
                                         .ok_or(meta.error("no index type specified"))?
                                         .to_string();
 
-                                    query = Some(( index_name, index_type ));
+                                    query = Some(( index_name, index_type, index_type_span ));
 
                                     Ok(())
                                 }
@@ -112,26 +113,26 @@ pub(crate) fn derive_cryptonamo(
                                 _ => Err(meta.error("unsupported field attribute")),
                             }
                         })?;
-                    }
 
-                    match (query, compound_index_name) {
-                        (Some((index_name, index_type)), Some((compound_index_name, _))) => {
-                            compound_indexes
-                                .entry(compound_index_name)
-                                .or_default()
-                                .push((index_name, index_type));
-                        }
+                        match (query, compound_index_name) {
+                            (Some((index_name, index_type, span)), Some((compound_index_name, _))) => {
+                                compound_indexes
+                                    .entry(compound_index_name)
+                                    .or_default()
+                                    .push((index_name, index_type, span));
+                            }
 
-                        (Some((index_name, index_type)), None) => {
-                            settings.add_index(index_name, index_type.as_ref())?;
-                        }
+                            (Some((index_name, index_type, span)), None) => {
+                                settings.add_index(index_name, index_type.as_ref(), span)?;
+                            }
 
-                        (None, Some((compound_index_name, span))) => {
-                            return Err(syn::Error::new(span,  format!("Compound attribute was specified but no query options were. Specify how this field should be queried with the attribute #[cryptonamo(query = <option>, compound = \"{compound_index_name}\")]")));
-                        }
+                            (None, Some((compound_index_name, span))) => {
+                                return Err(syn::Error::new(span,  format!("Compound attribute was specified but no query options were. Specify how this field should be queried with the attribute #[cryptonamo(query = <option>, compound = \"{compound_index_name}\")]")));
+                            }
 
-                        (None, None) => {}
-                    };
+                            (None, None) => {}
+                        };
+                    }
                 }
 
                 if !skip {

cryptonamo-derive/src/settings.rs

@@ -1,3 +1,4 @@
+use proc_macro2::Span;
 use quote::format_ident;
 use std::collections::HashMap;
 use syn::LitStr;
@@ -100,32 +101,22 @@ impl Settings {
         }
     }
 
-    fn create_index(name: String, index_type: &str) -> Result<IndexType, syn::Error> {
-        match index_type {
-            "exact" | "prefix" => Ok(IndexType::single(name, index_type.to_string())),
-            _ => Err(syn::Error::new_spanned(
-                index_type,
-                format!("Unsupported index type: {}", index_type),
-            )),
-        }
-    }
-
     pub(crate) fn add_compound_index(
         &mut self,
         name: String,
-        parts: Vec<(String, String)>,
+        parts: Vec<(String, String, Span)>,
     ) -> Result<(), syn::Error> {
         let name_parts = name.split("#").collect::<Vec<_>>();
 
         if name_parts.len() > parts.len() {
             let missing_fields = name_parts
                 .iter()
-                .filter(|x| !parts.iter().find(|(y, _)| x == &y).is_some())
+                .filter(|x| !parts.iter().find(|(y, _, _)| x == &y).is_some())
                 .cloned()
                 .collect::<Vec<_>>();
 
-            return Err(syn::Error::new_spanned(
-                    "",
+            return Err(syn::Error::new(
+                    Span::call_site(),
                     format!(
                         "Not all fields were annotated with the #[cryptonamo(compound)] attribute. Missing fields: {}",
                         missing_fields.join(",")
@@ -136,13 +127,13 @@ impl Settings {
         if parts.len() > name_parts.len() {
             let extra_fields = parts
                 .iter()
-                .map(|(x, _)| x)
+                .map(|(x, _, _)| x)
                 .filter(|x| !name_parts.iter().find(|y| x == y).is_some())
                 .cloned()
                 .collect::<Vec<_>>();
 
-            return Err(syn::Error::new_spanned(
-                    "",
+            return Err(syn::Error::new(
+                    Span::call_site(),
                     format!(
                         "Too many fields were annotated with the #[cryptonamo(compound)] attribute. Extra fields: {}",
                         extra_fields.join(",")
@@ -154,32 +145,38 @@ impl Settings {
 
         let field = name_parts_iter.next().unwrap();
 
+        let (field, index_type, index_type_span) = parts
+            .iter()
+            .find(|x| x.0 == field)
+            .ok_or_else(|| {
+                syn::Error::new(
+                    Span::call_site(),
+                    format!("Internal error: index was not specified for field \"{field}\""),
+                )
+            })?
+            .clone();
+
+        Self::validate_index_type(index_type.as_str(), index_type_span)?;
+
         let mut index = IndexType::Compound1 {
             name: name.clone(),
-            index: parts
-                .iter()
-                .find(|x| x.0 == field)
-                .ok_or_else(|| {
-                    syn::Error::new_spanned(
-                        "",
-                        format!("Internal error: index was not specified for field \"{field}\""),
-                    )
-                })?
-                .clone(),
+            index: (field, index_type),
         };
 
         while let Some(field) = name_parts_iter.next() {
-            let (field, index_type) = parts
+            let (field, index_type, index_type_span) = parts
                 .iter()
                 .find(|x| x.0 == field)
                 .ok_or_else(|| {
-                    syn::Error::new_spanned(
-                        "",
+                    syn::Error::new(
+                        Span::call_site(),
                         format!("Internal error: index was not specified for field \"{field}\""),
                     )
                 })?
                 .clone();
 
+            Self::validate_index_type(index_type.as_str(), index_type_span)?;
+
             index = index.and(field, index_type)?;
         }
 
@@ -188,15 +185,32 @@ impl Settings {
         Ok(())
     }
 
+    fn validate_index_type(index_type: &str, index_type_span: Span) -> Result<(), syn::Error> {
+        if !matches!(index_type.as_ref(), "exact" | "prefix") {
+            Err(syn::Error::new(
+                index_type_span,
+                format!("Unsupported index type: {}", index_type),
+            ))
+        } else {
+            Ok(())
+        }
+    }
+
     // TODO: Add an IndexOptions enum so we can pass those through as well
     pub(crate) fn add_index(
         &mut self,
         name: impl Into<String>,
         index_type: &str,
+        index_type_span: Span,
     ) -> Result<(), syn::Error> {
         let name = name.into();
-        self.indexes
-            .insert(name.clone(), Self::create_index(name, index_type)?);
+
+        Self::validate_index_type(index_type, index_type_span)?;
+
+        self.indexes.insert(
+            name.clone(),
+            IndexType::single(name, index_type.to_string()),
+        );
 
         Ok(())
     }

tests/compile_tests.rs

@@ -5,4 +5,6 @@ fn ui_tests() {
     t.compile_fail("tests/ui/compound-index-missing-field.rs");
     t.compile_fail("tests/ui/compound-index-missing-config.rs");
     t.compile_fail("tests/ui/compound-index-too-many-fields.rs");
+    t.compile_fail("tests/ui/index-unsupported.rs");
+    t.compile_fail("tests/ui/compound-index-unsupported.rs");
 }

tests/ui/compound-index-missing-config.rs

@@ -1,6 +1,7 @@
 use cryptonamo::Cryptonamo;
 
 #[derive(Cryptonamo)]
+#[cryptonamo(partition_key = "email")]
 struct User {
     #[cryptonamo(compound = "email#name")]
     email: String,

tests/ui/compound-index-missing-config.stderr

@@ -1,5 +1,5 @@
 error: Compound attribute was specified but no query options were. Specify how this field should be queried with the attribute #[cryptonamo(query = <option>, compound = "email#name")]
- --> tests/ui/compound-index-missing-config.rs:5:41
+ --> tests/ui/compound-index-missing-config.rs:6:41
   |
-5 |     #[cryptonamo(compound = "email#name")]
+6 |     #[cryptonamo(compound = "email#name")]
   |                                         ^

tests/ui/compound-index-unsupported.rs

@@ -0,0 +1,12 @@
+use cryptonamo::Cryptonamo;
+
+#[derive(Debug, Cryptonamo)]
+#[cryptonamo(partition_key = "email")]
+struct User {
+    #[cryptonamo(query = "prefix", compound = "email#name")]
+    email: String,
+    #[cryptonamo(query = "blah", compound = "email#name")]
+    name: String,
+}
+
+fn main() {}

tests/ui/compound-index-unsupported.stderr

@@ -0,0 +1,5 @@
+error: Unsupported index type: blah
+ --> tests/ui/compound-index-unsupported.rs:8:26
+  |
+8 |     #[cryptonamo(query = "blah", compound = "email#name")]
+  |                          ^^^^^^

tests/ui/index-unsupported.rs

@@ -0,0 +1,9 @@
+use cryptonamo::Cryptonamo;
+
+#[derive(Cryptonamo)]
+struct User {
+    #[cryptonamo(query = "blah")]
+    email: String,
+}
+
+fn main() {}

tests/ui/index-unsupported.stderr

@@ -0,0 +1,5 @@
+error: Unsupported index type: blah
+ --> tests/ui/index-unsupported.rs:5:26
+  |
+5 |     #[cryptonamo(query = "blah")]
+  |                          ^^^^^^