macro: add types.rs, preserve Span for Expressions

Move the Name type into types.rs and make it hashable.
Expression::Object not contains an Object where the
hashmap's key is a Name and therefore preserves the Span of
all the keys for better error messages.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>

proxmox-api-macro/src/api_def.rs

@@ -1,4 +1,3 @@
-use std::collections::HashMap;
 use std::convert::TryFrom;
 
 use proc_macro2::TokenStream;
@@ -7,7 +6,7 @@ use derive_builder::Builder;
 use failure::{bail, Error};
 use quote::{quote, ToTokens};
 
-use super::parsing::Expression;
+use super::parsing::{Expression, Object};
 
 #[derive(Clone)]
 pub enum CliMode {
@@ -69,7 +68,7 @@ impl CommonTypeDefinition {
         CommonTypeDefinitionBuilder::default()
     }
 
-    pub fn from_object(obj: &mut HashMap<String, Expression>) -> Result<Self, Error> {
+    pub fn from_object(obj: &mut Object) -> Result<Self, Error> {
         let mut def = Self::builder();
 
         if let Some(value) = obj.remove("description") {
@@ -103,7 +102,7 @@ impl ParameterDefinition {
         ParameterDefinitionBuilder::default()
     }
 
-    pub fn from_object(obj: HashMap<String, Expression>) -> Result<Self, Error> {
+    pub fn from_object(obj: Object) -> Result<Self, Error> {
         let mut def = ParameterDefinition::builder();
 
         for (key, value) in obj {

proxmox-api-macro/src/api_macro.rs

@@ -1,5 +1,3 @@
-use std::collections::HashMap;
-
 use proc_macro2::{Delimiter, Ident, Span, TokenStream, TokenTree};
 
 use failure::{bail, format_err, Error};
@@ -48,7 +46,7 @@ pub fn api_macro(attr: TokenStream, item: TokenStream) -> Result<TokenStream, Er
 
 fn handle_function(
     def_span: Span,
-    mut definition: HashMap<String, Expression>,
+    mut definition: Object,
     mut item: syn::ItemFn,
 ) -> Result<TokenStream, Error> {
     if item.decl.generics.lt_token.is_some() {
@@ -96,7 +94,7 @@ fn handle_function(
         .remove("parameters")
         .map(|v| v.expect_object())
         .transpose()?
-        .unwrap_or_else(HashMap::new);
+        .unwrap_or_else(|| Object::new(Span::call_site()));
     let mut parameter_entries = TokenStream::new();
     let mut parameter_verifiers = TokenStream::new();
 
@@ -195,7 +193,7 @@ fn handle_function(
             if !list.is_empty() {
                 list.push_str(", ");
             }
-            list.push_str(&param);
+            list.push_str(param.as_str());
         }
         bail!(
             "api definition contains parameters not found in function declaration: {}",
@@ -389,7 +387,7 @@ fn handle_function(
 fn make_parameter_verifier(
     var: &Ident,
     var_str: &str,
-    info: &mut HashMap<String, Expression>,
+    info: &mut Object,
     out: &mut TokenStream,
 ) -> Result<(), Error> {
     match info.remove("minimum") {
@@ -418,7 +416,7 @@ fn make_parameter_verifier(
 }
 
 fn handle_struct(
-    definition: HashMap<String, Expression>,
+    definition: Object,
     item: &syn::ItemStruct,
 ) -> Result<TokenStream, Error> {
     if item.generics.lt_token.is_some() {
@@ -435,7 +433,7 @@ fn handle_struct(
 }
 
 fn handle_struct_unnamed(
-    mut definition: HashMap<String, Expression>,
+    mut definition: Object,
     name: &Ident,
     item: &syn::FieldsUnnamed,
 ) -> Result<TokenStream, Error> {
@@ -478,7 +476,7 @@ fn handle_struct_unnamed(
 }
 
 fn handle_struct_named(
-    mut definition: HashMap<String, Expression>,
+    mut definition: Object,
     name: &Ident,
     item: &syn::FieldsNamed,
 ) -> Result<TokenStream, Error> {
@@ -522,7 +520,7 @@ fn handle_struct_named(
 
 fn handle_named_struct_fields(
     item: &syn::FieldsNamed,
-    mut field_def: HashMap<String, Expression>,
+    mut field_def: Object,
 ) -> Result<Vec<TokenStream>, Error> {
     let mut verify_entries = Vec::new();
 
@@ -551,7 +549,7 @@ fn handle_named_struct_fields(
             if !missing.is_empty() {
                 missing.push_str(", ");
             }
-            missing.push_str(&key);
+            missing.push_str(key.as_str());
         }
         bail!(
             "the following struct fields are not handled in the api definition: {}",
@@ -569,7 +567,7 @@ fn handle_named_struct_fields(
 /// For enums we automatically implement `ToString`, `FromStr`, and derive `Serialize` and
 /// `Deserialize` via `serde_plain`.
 fn handle_enum(
-    mut definition: HashMap<String, Expression>,
+    mut definition: Object,
     item: &syn::ItemEnum,
 ) -> Result<TokenStream, Error> {
     if item.generics.lt_token.is_some() {

proxmox-api-macro/src/lib.rs

@@ -10,6 +10,7 @@ mod error;
 
 mod api_def;
 mod parsing;
+mod types;
 mod util;
 
 mod api_macro;

proxmox-api-macro/src/parsing.rs

@@ -4,7 +4,9 @@ use proc_macro2::{Delimiter, Group, Ident, Span, TokenStream, TokenTree};
 
 use failure::{bail, Error};
 use quote::quote;
-use syn::{Expr, Lit};
+use syn::{spanned::Spanned, Expr, Lit};
+
+use crate::types::Name;
 
 pub type RawTokenIter = proc_macro2::token_stream::IntoIter;
 pub type TokenIter = std::iter::Peekable<RawTokenIter>;
@@ -117,33 +119,6 @@ pub fn comma_or_end(tokens: &mut TokenIter) -> Result<(), Error> {
     Ok(())
 }
 
-/// A more relaxed version of Ident which allows hyphens.
-pub struct Name(String, Span);
-
-impl Name {
-    pub fn new(name: String, span: Span) -> Result<Self, Error> {
-        let beg = name.as_bytes()[0];
-        if !(beg.is_ascii_alphanumeric() || beg == b'_')
-            || !name
-                .bytes()
-                .all(|b| b.is_ascii_alphanumeric() || b == b'_' || b == b'-')
-        {
-            bail!("`{}` is not a valid name", name);
-        }
-        Ok(Self(name, span))
-    }
-
-    pub fn to_string(&self) -> String {
-        self.0.clone()
-    }
-}
-
-impl From<Ident> for Name {
-    fn from(ident: Ident) -> Name {
-        Name(ident.to_string(), ident.span())
-    }
-}
-
 pub fn need_hyphenated_name(tokens: &mut TokenIter) -> Result<syn::LitStr, Error> {
     let start = need_ident(&mut *tokens)?;
     finish_hyphenated_name(&mut *tokens, start)
@@ -179,7 +154,49 @@ pub fn finish_hyphenated_name(tokens: &mut TokenIter, name: Ident) -> Result<syn
 #[derive(Debug)]
 pub enum Expression {
     Expr(Expr),
-    Object(HashMap<String, Expression>),
+    Object(Object),
+}
+
+#[derive(Debug)]
+pub struct Object {
+    span: Span,
+    map: HashMap<Name, Expression>,
+}
+
+impl std::ops::Deref for Object {
+    type Target = HashMap<Name, Expression>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.map
+    }
+}
+
+impl std::ops::DerefMut for Object {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.map
+    }
+}
+
+impl Object {
+    pub fn new(span: Span) -> Self {
+        Self {
+            span,
+            map: HashMap::new(),
+        }
+    }
+
+    pub fn span(&self) -> Span {
+        self.span
+    }
+}
+
+impl IntoIterator for Object {
+    type Item = <HashMap<Name, Expression> as IntoIterator>::Item;
+    type IntoIter = <HashMap<Name, Expression> as IntoIterator>::IntoIter;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.map.into_iter()
+    }
 }
 
 impl Expression {
@@ -229,7 +246,7 @@ impl Expression {
         }
     }
 
-    pub fn expect_object(self) -> Result<HashMap<String, Expression>, Error> {
+    pub fn expect_object(self) -> Result<Object, Error> {
         match self {
             Expression::Object(obj) => Ok(obj),
             _ => bail!("expected object, found an expression"),
@@ -257,21 +274,20 @@ impl Expression {
     }
 }
 
-pub fn parse_object(tokens: TokenStream) -> Result<HashMap<String, Expression>, Error> {
+pub fn parse_object(tokens: TokenStream) -> Result<Object, Error> {
+    let mut out = Object::new(tokens.span());
     let mut tokens = tokens.into_iter().peekable();
-    let mut out = HashMap::new();
 
     loop {
         let key = match parse_object_key(&mut tokens)? {
             Some(key) => key,
             None => break,
         };
-        let key_name = key.to_string();
 
-        let value = parse_object_value(&mut tokens, &key_name)?;
+        let value = parse_object_value(&mut tokens, &key)?;
 
-        if out.insert(key_name.clone(), value).is_some() {
+        if out.insert(key.clone(), value).is_some() {
-            bail!("duplicate entry: {}", key_name);
+            c_bail!(key.span() => "duplicate entry: {}", key.as_str());
         }
     }
 
@@ -288,7 +304,7 @@ fn parse_object_key(tokens: &mut TokenIter) -> Result<Option<Name>, Error> {
     Ok(Some(key))
 }
 
-fn parse_object_value(tokens: &mut TokenIter, key: &str) -> Result<Expression, Error> {
+fn parse_object_value(tokens: &mut TokenIter, key: &Name) -> Result<Expression, Error> {
     let mut value_tokens = TokenStream::new();
 
     let mut first = true;
@@ -297,7 +313,7 @@ fn parse_object_value(tokens: &mut TokenIter, key: &str) -> Result<Expression, E
             Some(token) => token,
             None => {
                 if first {
-                    bail!("missing value after key '{}'", key);
+                    c_bail!(key.span(), "missing value after key '{}'", key.as_str());
                 }
                 break;
             }

proxmox-api-macro/src/types.rs

@@ -0,0 +1,90 @@
+//! Helper types not found in proc_macro.
+//!
+//! Like a `Name` type which is like an `Ident` but allows hyphens, and is hashable so it can be
+//! used as a key in a `HashMap`.
+
+use proc_macro2::{Ident, Span, TokenStream};
+
+use failure::Error;
+
+/// A more relaxed version of Ident which allows hyphens.
+#[derive(Clone, Debug)]
+pub struct Name(String, Span);
+
+impl Name {
+    pub fn new(name: String, span: Span) -> Result<Self, Error> {
+        let beg = name.as_bytes()[0];
+        if !(beg.is_ascii_alphanumeric() || beg == b'_')
+            || !name
+                .bytes()
+                .all(|b| b.is_ascii_alphanumeric() || b == b'_' || b == b'-')
+        {
+            c_bail!(span, "`{}` is not a valid name", name);
+        }
+        Ok(Self(name, span))
+    }
+
+    pub fn as_str(&self) -> &str {
+        &self.0
+    }
+
+    pub fn to_string(&self) -> String {
+        self.0.clone()
+    }
+
+    pub fn span(&self) -> Span {
+        self.1.clone()
+    }
+}
+
+impl From<Ident> for Name {
+    fn from(ident: Ident) -> Name {
+        Name(ident.to_string(), ident.span())
+    }
+}
+
+impl PartialEq for Name {
+    fn eq(&self, rhs: &Self) -> bool {
+        self.0 == rhs.0
+    }
+
+    fn ne(&self, rhs: &Self) -> bool {
+        self.0 != rhs.0
+    }
+}
+
+impl Eq for Name {}
+
+impl quote::ToTokens for Name {
+    fn to_tokens(&self, tokens: &mut TokenStream) {
+        Ident::new(&self.0, self.1).to_tokens(tokens)
+    }
+}
+
+impl std::borrow::Borrow<String> for Name {
+    fn borrow(&self) -> &String {
+        &self.0
+    }
+}
+
+impl std::borrow::Borrow<str> for Name {
+    fn borrow(&self) -> &str {
+        &self.0
+    }
+}
+
+impl AsRef<str> for Name {
+    fn as_ref(&self) -> &str {
+        &self.0
+    }
+}
+
+impl std::hash::Hash for Name {
+    fn hash<H>(&self, state: &mut H)
+    where
+        H: std::hash::Hasher
+    {
+        std::hash::Hash::hash::<H>(&self.0, state)
+    }
+}
+