Unverified Commit 000bca05 authored by Hero Bird's avatar Hero Bird Committed by GitHub
Browse files

Generalize conflicting attribute error reporting (#628)



* [lang/ir] generalize conflicting attribute error reporting

# Conflicts:
#	crates/lang/ir/src/ir/chain_extension.rs

* [lang/macro] add UI tests for payable constructors

* fix docs (review suggestion)
Co-authored-by: Michael Müller's avatarMichael Müller <michi@parity.io>

* fix docs (review suggestion)
Co-authored-by: Michael Müller's avatarMichael Müller <michi@parity.io>
Co-authored-by: Michael Müller's avatarMichael Müller <michi@parity.io>
parent 59a7f5ed
Pipeline #120001 failed with stages
in 50 minutes and 59 seconds
......@@ -573,6 +573,12 @@ where
///
/// Returns the partitioned ink! and non-ink! attributes.
///
/// # Parameters
///
/// The `is_conflicting_attr` closure returns `Ok` if the attribute does not conflict,
/// returns `Err(None)` if the attribute conflicts but without providing further reasoning
/// and `Err(Some(reason))` if the attribute conflicts given additional context information.
///
/// # Errors
///
/// - If there are invalid ink! attributes.
......@@ -587,7 +593,7 @@ pub fn sanitize_attributes<I, C>(
) -> Result<(InkAttribute, Vec<syn::Attribute>), syn::Error>
where
I: IntoIterator<Item = syn::Attribute>,
C: FnMut(&AttributeArg) -> bool,
C: FnMut(&ir::AttributeFrag) -> Result<(), Option<syn::Error>>,
{
let (ink_attrs, other_attrs) = ir::partition_attributes(attrs)?;
let normalized = ir::InkAttribute::from_expanded(ink_attrs).map_err(|err| {
......@@ -600,7 +606,7 @@ where
is_valid_first,
))
})?;
normalized.ensure_no_conflicts(|arg| is_conflicting_attr(arg.kind()))?;
normalized.ensure_no_conflicts(|arg| is_conflicting_attr(arg))?;
Ok((normalized, other_attrs))
}
......@@ -684,21 +690,44 @@ impl InkAttribute {
///
/// The given `is_conflicting` describes for every ink! attribute argument
/// found in `self` if it is in conflict.
///
/// # Parameters
///
/// The `is_conflicting_attr` closure returns `Ok` if the attribute does not conflict,
/// returns `Err(None)` if the attribute conflicts but without providing further reasoning
/// and `Err(Some(reason))` if the attribute conflicts given additional context information.
pub fn ensure_no_conflicts<'a, P>(
&'a self,
mut is_conflicting: P,
) -> Result<(), syn::Error>
where
P: FnMut(&'a ir::AttributeFrag) -> bool,
P: FnMut(&'a ir::AttributeFrag) -> Result<(), Option<syn::Error>>,
{
let mut err: Option<syn::Error> = None;
for arg in self.args() {
if is_conflicting(arg) {
return Err(format_err!(
if let Err(reason) = is_conflicting(arg) {
let conflict_err = format_err!(
arg.span(),
"encountered conflicting ink! attribute argument",
))
);
match &mut err {
Some(err) => {
err.combine(conflict_err);
}
None => {
err = Some(conflict_err);
}
}
if let Some(reason) = reason {
err.as_mut()
.expect("must be `Some` at this point")
.combine(reason);
}
}
}
if let Some(err) = err {
return Err(err)
}
Ok(())
}
}
......
......@@ -486,13 +486,13 @@ impl ChainExtension {
item_method.span(),
item_method.attrs.clone(),
&ir::AttributeArgKind::Extension,
|c| {
!matches!(
c,
|arg| {
match arg.kind() {
ir::AttributeArg::Extension(_)
| ir::AttributeArg::HandleStatus(_)
| ir::AttributeArg::ReturnsResult(_)
)
| ir::AttributeArg::HandleStatus(_)
| ir::AttributeArg::ReturnsResult(_) => Ok(()),
_ => Err(None),
}
},
)?;
if let Some(receiver) = item_method.sig.receiver() {
......
......@@ -86,7 +86,12 @@ impl TryFrom<syn::ItemStruct> for Event {
struct_span,
item_struct.attrs,
&ir::AttributeArgKind::Event,
|kind| !matches!(kind, ir::AttributeArg::Event | ir::AttributeArg::Anonymous),
|arg| {
match arg.kind() {
ir::AttributeArg::Event | ir::AttributeArg::Anonymous => Ok(()),
_ => Err(None),
}
},
)?;
if !item_struct.generics.params.is_empty() {
return Err(format_err_spanned!(
......
......@@ -86,7 +86,12 @@ impl TryFrom<syn::ItemStruct> for Storage {
struct_span,
item_struct.attrs,
&ir::AttributeArgKind::Storage,
|kind| !matches!(kind, ir::AttributeArg::Storage),
|arg| {
match arg.kind() {
ir::AttributeArg::Storage => Ok(()),
_ => Err(None),
}
},
)?;
if !item_struct.generics.params.is_empty() {
return Err(format_err_spanned!(
......
......@@ -153,11 +153,19 @@ impl Constructor {
method_item.span(),
method_item.attrs.clone(),
&ir::AttributeArgKind::Constructor,
|kind| {
!matches!(
kind,
ir::AttributeArg::Constructor | ir::AttributeArg::Selector(_)
)
|arg| {
match arg.kind() {
ir::AttributeArg::Constructor | ir::AttributeArg::Selector(_) => {
Ok(())
}
ir::AttributeArg::Payable => {
Err(Some(format_err!(
arg.span(),
"constructors are implicitly payable"
)))
}
_ => Err(None),
}
},
)
}
......
......@@ -180,13 +180,13 @@ impl Message {
method_item.span(),
method_item.attrs.clone(),
&ir::AttributeArgKind::Message,
|kind| {
!matches!(
kind,
|arg| {
match arg.kind() {
ir::AttributeArg::Message
| ir::AttributeArg::Payable
| ir::AttributeArg::Selector(_)
)
| ir::AttributeArg::Payable
| ir::AttributeArg::Selector(_) => Ok(()),
_ => Err(None),
}
},
)
}
......
......@@ -299,10 +299,12 @@ impl TryFrom<syn::ItemImpl> for ItemImpl {
err.into_combine(format_err!(impl_block_span, "at this invocation",))
})?;
normalized.ensure_no_conflicts(|arg| {
!matches!(
arg.kind(),
ir::AttributeArg::Implementation | ir::AttributeArg::Namespace(_)
)
match arg.kind() {
ir::AttributeArg::Implementation | ir::AttributeArg::Namespace(_) => {
Ok(())
}
_ => Err(None),
}
})?;
namespace = normalized.namespace();
}
......
......@@ -498,7 +498,12 @@ impl InkTrait {
constructor.span(),
constructor.attrs.clone(),
&ir::AttributeArgKind::Constructor,
|c| !matches!(c, ir::AttributeArg::Constructor),
|arg| {
match arg.kind() {
ir::AttributeArg::Constructor => Ok(()),
_ => Err(None),
}
},
)?;
if let Some(receiver) = constructor.sig.receiver() {
return Err(format_err_spanned!(
......@@ -545,7 +550,12 @@ impl InkTrait {
message.span(),
message.attrs.clone(),
&ir::AttributeArgKind::Message,
|c| !matches!(c, ir::AttributeArg::Message),
|arg| {
match arg.kind() {
ir::AttributeArg::Message => Ok(()),
_ => Err(None),
}
},
)?;
match message.sig.receiver() {
None | Some(syn::FnArg::Typed(_)) => {
......
......@@ -36,6 +36,8 @@ fn compile_tests() {
t.compile_fail("tests/ui/fail/C-11-unsafe-constructor.rs");
t.compile_fail("tests/ui/fail/C-12-const-constructor.rs");
t.compile_fail("tests/ui/fail/C-13-abi-constructor.rs");
t.compile_fail("tests/ui/fail/C-14-payable-constructor.rs");
t.compile_fail("tests/ui/fail/C-15-payable-trait-constructor.rs");
t.compile_fail("tests/ui/fail/H-01-invalid-dyn-alloc.rs");
t.compile_fail("tests/ui/fail/H-02-invalid-as-dependency.rs");
......
use ink_lang as ink;
#[ink::contract]
mod noop {
#[ink(storage)]
pub struct Noop {}
impl Noop {
#[ink(constructor, payable)]
pub fn abi_constructor() -> Self {
Self {}
}
#[ink(message)]
pub fn noop(&self) {}
}
}
fn main() {}
error: encountered conflicting ink! attribute argument
--> $DIR/C-14-payable-constructor.rs:9:28
|
9 | #[ink(constructor, payable)]
| ^^^^^^^
error: constructors are implicitly payable
--> $DIR/C-14-payable-constructor.rs:9:28
|
9 | #[ink(constructor, payable)]
| ^^^^^^^
use ink_lang as ink;
#[ink::trait_definition]
pub trait Constructor {
#[ink(constructor)]
fn constructor() -> Self;
}
#[ink::contract]
mod noop {
#[ink(storage)]
pub struct Noop {}
impl Constructor for Noop {
#[ink(constructor, payable)]
fn constructor() -> Self {
Self {}
}
}
impl Noop {
#[ink(message)]
pub fn noop(&self) {}
}
}
fn main() {}
error: encountered conflicting ink! attribute argument
--> $DIR/C-15-payable-trait-constructor.rs:15:28
|
15 | #[ink(constructor, payable)]
| ^^^^^^^
error: constructors are implicitly payable
--> $DIR/C-15-payable-trait-constructor.rs:15:28
|
15 | #[ink(constructor, payable)]
| ^^^^^^^
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment