Skip to content

Commit

Permalink
feat(sol-macro): improve error messages (#345)
Browse files Browse the repository at this point in the history
  • Loading branch information
DaniPopes authored Oct 8, 2023
1 parent 27375c8 commit 2168ae3
Show file tree
Hide file tree
Showing 11 changed files with 161 additions and 100 deletions.
1 change: 1 addition & 0 deletions crates/sol-macro/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ hex.workspace = true
# json
alloy-json-abi = { workspace = true, optional = true }
serde_json = { workspace = true, optional = true }
proc-macro-error = "1.0.4"

[features]
json = ["dep:alloy-json-abi", "dep:serde_json"]
6 changes: 6 additions & 0 deletions crates/sol-macro/src/expand/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,19 @@ use syn::{ext::IdentExt, parse_quote, Attribute, Result};
///
/// ```ignore (pseudo-code)
/// pub mod #name {
/// #(#items)*
///
/// pub enum #{name}Calls {
/// ...
/// }
///
/// pub enum #{name}Errors {
/// ...
/// }
///
/// pub enum #{name}Events {
/// ...
/// }
/// }
/// ```
pub(super) fn expand(cx: &ExpCtxt<'_>, contract: &ItemContract) -> Result<TokenStream> {
Expand Down
123 changes: 64 additions & 59 deletions crates/sol-macro/src/expand/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ pub fn expand(ast: File) -> Result<TokenStream> {
ExpCtxt::new(&ast).expand()
}

/// The expansion context.
struct ExpCtxt<'ast> {
all_items: Vec<&'ast Item>,
custom_types: HashMap<SolIdent, Type>,
Expand Down Expand Up @@ -61,25 +62,31 @@ impl<'ast> ExpCtxt<'ast> {
}

fn expand(mut self) -> Result<TokenStream> {
let mut abort = false;
let mut tokens = TokenStream::new();

if let Err(e) = self.parse_file_attributes() {
tokens.extend(e.into_compile_error());
}

self.visit_file(self.ast);

if self.all_items.len() > 1 {
self.resolve_custom_types()?;
self.mk_overloads_map()?;
self.resolve_custom_types();
if self.mk_overloads_map().is_err() {
abort = true;
}
}

if abort {
return Ok(tokens)
}

for item in &self.ast.items {
// TODO: Dummy items
let t = match self.expand_item(item) {
Ok(t) => t,
Err(e) => {
// TODO: Dummy items
e.into_compile_error()
}
Err(e) => e.into_compile_error(),
};
tokens.extend(t);
}
Expand Down Expand Up @@ -118,6 +125,7 @@ impl<'ast> ExpCtxt<'ast> {
map.reserve(self.all_items.len());
for &item in &self.all_items {
let (name, ty) = match item {
Item::Contract(c) => (&c.name, c.as_type()),
Item::Enum(e) => (&e.name, e.as_type()),
Item::Struct(s) => (&s.name, s.as_type()),
Item::Udt(u) => (&u.name, u.ty.clone()),
Expand All @@ -128,14 +136,13 @@ impl<'ast> ExpCtxt<'ast> {
self.custom_types = map;
}

fn resolve_custom_types(&mut self) -> Result<()> {
fn resolve_custom_types(&mut self) {
self.mk_types_map();
// you won't get me this time, borrow checker
// SAFETY: no data races, we don't modify the map while we're iterating
// I think this is safe anyway
let map_ref: &mut HashMap<SolIdent, Type> =
unsafe { &mut *(&mut self.custom_types as *mut _) };
let map = &self.custom_types;
for ty in map_ref.values_mut() {
let mut i = 0;
ty.visit_mut(|ty| {
Expand All @@ -146,25 +153,25 @@ impl<'ast> ExpCtxt<'ast> {
let Type::Custom(name) = &*ty else {
unreachable!()
};
let Some(resolved) = map.get(name.last_tmp()) else {
let Some(resolved) = self.try_custom_type(name) else {
return
};
ty.clone_from(resolved);
i += 1;
});
if i >= RESOLVE_LIMIT {
let msg = "\
failed to resolve types.\n\
This is likely due to an infinitely recursive type definition.\n\
If you believe this is a bug, please file an issue at \
https://github.com/alloy-rs/core/issues/new/choose";
return Err(Error::new(ty.span(), msg))
abort!(
ty.span(),
"failed to resolve types.\n\
This is likely due to an infinitely recursive type definition.\n\
If you believe this is a bug, please file an issue at \
https://github.com/alloy-rs/core/issues/new/choose"
);
}
}
Ok(())
}

fn mk_overloads_map(&mut self) -> Result<()> {
fn mk_overloads_map(&mut self) -> std::result::Result<(), ()> {
let all_orig_names: Vec<_> = self
.overloaded_items
.values()
Expand All @@ -173,25 +180,21 @@ impl<'ast> ExpCtxt<'ast> {
.collect();
let mut overloads_map = std::mem::take(&mut self.overloads);

// report all errors at the end
let mut errors = Vec::new();
let mut failed = false;

for functions in self.overloaded_items.values().filter(|fs| fs.len() >= 2) {
// check for same parameters
for (i, &a) in functions.iter().enumerate() {
for &b in functions.iter().skip(i + 1) {
if a.eq_by_types(b) {
let msg = format!(
failed = true;
emit_error!(
a.span(),
"{} with same name and parameter types defined twice",
a.desc()
);
let mut err = syn::Error::new(a.span(), msg);
a.desc();

let msg = "other declaration is here";
let note = syn::Error::new(b.span(), msg);

err.combine(note);
errors.push(err);
note = b.span() => "other declaration is here";
);
}
}
}
Expand All @@ -202,27 +205,27 @@ impl<'ast> ExpCtxt<'ast> {
};
let new_name = format!("{old_name}_{i}");
if let Some(other) = all_orig_names.iter().find(|x| x.0 == new_name) {
let msg = format!(
failed = true;
emit_error!(
old_name.span(),
"{} `{old_name}` is overloaded, \
but the generated name `{new_name}` is already in use",
item.desc()
);
let mut err = syn::Error::new(old_name.span(), msg);
item.desc();

let msg = "other declaration is here";
let note = syn::Error::new(other.span(), msg);

err.combine(note);
errors.push(err);
note = other.span() => "other declaration is here";
)
}

overloads_map.insert(item.signature(self), new_name);
}
}

utils::combine_errors(errors).map(|()| {
self.overloads = overloads_map;
})
if failed {
return Err(())
}

self.overloads = overloads_map;
Ok(())
}
}

Expand Down Expand Up @@ -310,28 +313,32 @@ impl<'a> OverloadedItem<'a> {
// utils
impl<'ast> ExpCtxt<'ast> {
#[allow(dead_code)]
fn get_item(&self, name: &SolPath) -> &Item {
match self.try_get_item(name) {
fn item(&self, name: &SolPath) -> &Item {
match self.try_item(name) {
Some(item) => item,
None => panic!("unresolved item: {name}"),
None => abort!(name.span(), "unresolved item: {}", name),
}
}

fn try_get_item(&self, name: &SolPath) -> Option<&Item> {
let name = name.last_tmp();
fn try_item(&self, name: &SolPath) -> Option<&Item> {
let name = name.last();
self.all_items
.iter()
.find(|item| item.name() == Some(name))
.copied()
.find(|item| item.name() == Some(name))
}

fn custom_type(&self, name: &SolPath) -> &Type {
match self.custom_types.get(name.last_tmp()) {
match self.try_custom_type(name) {
Some(item) => item,
None => panic!("unresolved item: {name}"),
None => abort!(name.span(), "unresolved custom type: {}", name),
}
}

fn try_custom_type(&self, name: &SolPath) -> Option<&Type> {
self.custom_types.get(name.last())
}

/// Returns the name of the function, adjusted for overloads.
fn function_name(&self, function: &ItemFunction) -> SolIdent {
self.overloaded_name(function.into())
Expand Down Expand Up @@ -489,24 +496,22 @@ impl<'ast> ExpCtxt<'ast> {
where
I: IntoIterator<Item = &'a VariableDeclaration>,
{
let mut errors = Vec::new();
let mut errored = false;
for param in params {
param.ty.visit(|ty| {
if let Type::Custom(name) = ty {
if !self.custom_types.contains_key(name.last_tmp()) {
let e = syn::Error::new(name.span(), "unresolved type");
errors.push(e);
if !self.custom_types.contains_key(name.last()) {
let note = (!errored).then(|| {
errored = true;
"Custom types must be declared inside of the same scope they are referenced in,\n\
or \"imported\" as a UDT with `type ... is (...);`"
});
emit_error!(name.span(), "unresolved type"; help =? note);
}
}
});
}
utils::combine_errors(errors).map_err(|mut e| {
let note =
"Custom types must be declared inside of the same scope they are referenced in,\n\
or \"imported\" as a UDT with `type ... is (...);`";
e.combine(Error::new(Span::call_site(), note));
e
})
Ok(())
}
}

Expand Down
5 changes: 3 additions & 2 deletions crates/sol-macro/src/expand/struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,14 @@ fn expand_encode_type_fns(
let mut fields = fields.clone();
fields.visit_types_mut(|ty| {
let Type::Custom(name) = ty else { return };
match cx.try_get_item(name) {
match cx.try_item(name) {
// keep as custom
Some(Item::Struct(_)) | None => {}
// convert to underlying
Some(Item::Contract(_)) => *ty = Type::Address(ty.span(), None),
Some(Item::Enum(_)) => *ty = Type::Uint(ty.span(), NonZeroU16::new(8)),
Some(Item::Udt(udt)) => *ty = udt.ty.clone(),
Some(item) => panic!("Invalid type in struct field: {item:?}"),
Some(item) => abort!(item.span(), "Invalid type in struct field: {:?}", item),
}
});

Expand Down
18 changes: 9 additions & 9 deletions crates/sol-macro/src/expand/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,8 @@ pub(super) fn type_base_data_size(cx: &ExpCtxt<'_>, ty: &Type) -> usize {
.map(|ty| type_base_data_size(cx, ty))
.sum(),

Type::Custom(name) => match cx.try_get_item(name) {
Some(Item::Enum(_)) => 32,
Type::Custom(name) => match cx.try_item(name) {
Some(Item::Contract(_)) | Some(Item::Enum(_)) => 32,
Some(Item::Error(error)) => error
.parameters
.types()
Expand All @@ -266,7 +266,7 @@ pub(super) fn type_base_data_size(cx: &ExpCtxt<'_>, ty: &Type) -> usize {
.map(|ty| type_base_data_size(cx, ty))
.sum(),
Some(Item::Udt(udt)) => type_base_data_size(cx, &udt.ty),
Some(item) => panic!("Invalid item in param list: {item:?}"),
Some(item) => abort!(item.span(), "Invalid type in struct field: {:?}", item),
None => 0,
},

Expand All @@ -293,8 +293,8 @@ pub(super) fn can_derive_default(cx: &ExpCtxt<'_>, ty: &Type) -> bool {
}
}

Type::Custom(name) => match cx.try_get_item(name) {
Some(Item::Enum(_)) => false,
Type::Custom(name) => match cx.try_item(name) {
Some(Item::Contract(_)) | Some(Item::Enum(_)) => false,
Some(Item::Error(error)) => error
.parameters
.types()
Expand All @@ -307,7 +307,7 @@ pub(super) fn can_derive_default(cx: &ExpCtxt<'_>, ty: &Type) -> bool {
strukt.fields.types().all(|ty| can_derive_default(cx, ty))
}
Some(Item::Udt(udt)) => can_derive_default(cx, &udt.ty),
Some(item) => panic!("Invalid item in param list: {item:?}"),
Some(item) => abort!(item.span(), "Invalid type in struct field: {:?}", item),
_ => false,
},

Expand All @@ -331,8 +331,8 @@ pub(super) fn can_derive_builtin_traits(cx: &ExpCtxt<'_>, ty: &Type) -> bool {
}
}

Type::Custom(name) => match cx.try_get_item(name) {
Some(Item::Enum(_)) => true,
Type::Custom(name) => match cx.try_item(name) {
Some(Item::Contract(_)) | Some(Item::Enum(_)) => true,
Some(Item::Error(error)) => error
.parameters
.types()
Expand All @@ -346,7 +346,7 @@ pub(super) fn can_derive_builtin_traits(cx: &ExpCtxt<'_>, ty: &Type) -> bool {
.types()
.all(|ty| can_derive_builtin_traits(cx, ty)),
Some(Item::Udt(udt)) => can_derive_builtin_traits(cx, &udt.ty),
Some(item) => panic!("Invalid item in param list: {item:?}"),
Some(item) => abort!(item.span(), "Invalid type in struct field: {:?}", item),
_ => false,
},

Expand Down
3 changes: 3 additions & 0 deletions crates/sol-macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
#![deny(unused_must_use, rust_2018_idioms)]
#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]

#[macro_use]
extern crate proc_macro_error;
extern crate syn_solidity as ast;

use proc_macro::TokenStream;
Expand Down Expand Up @@ -192,6 +194,7 @@ mod utils;
#[doc = include_str!("../doctests/json.rs")]
/// ```
#[proc_macro]
#[proc_macro_error]
pub fn sol(input: TokenStream) -> TokenStream {
parse_macro_input!(input as input::SolInput)
.expand()
Expand Down
Loading

0 comments on commit 2168ae3

Please sign in to comment.