Commit 216a79d0 authored by Hero Bird's avatar Hero Bird Committed by GitHub

ink! 2.0: Improve Diagnostics follow-up (#228)

* [lang2/macro] improve diagnostics for missing a #[ink(storage)] struct

* [lang2/macro] add UI test for missing #[ink(storage)] struct

* [lang2/macro] add test for multiple #[ink(storage)] structs

* [lang2/macro] add ui test for storage-impl conflict

* [lang2/macro] implement check that ink! impls are on #[ink(storage)] structs

* [lang2/macro] fix some success tests

* [lang2/macro] add expected output for failure UI test 16

* [lang2/macro] add conflicting ink! marker test

The conflict here is the additional #[ink(event)] marker

* [lang2/macro] utils: add filter_map_ink_attributes

* [lang2/macro] structs now dispatch on whichever ink! marker was provided first

* [lang2/macro] add failure UI test for #[ink(event)] provided with another #[ink(storage)]

* [lang2/macro] improve diagnostics for unsupported ink! markers on structs

* [lang2/macro] add test for unknown struct ink! markers

* [lang2/macro] improve diagnostics for multiple #[ink(storage)] structs

* [lang2/macro] adjust test case for multiple #[ink(storage)] structs

* [lang2/macro] improve error message for unknown method ink! markers

The error diagnostics can be further improved by showing all occurences of unknown ink! markers in an impl block.

* [lang2/macro] add test for unknown ink! method marker

* [lang2/macro] fix minor typo in tests
Co-Authored-By: Michael Müller's avatarMichael Müller <mich@elmueller.net>

* [lang2/macro] fix typo in test
Co-Authored-By: Andrew Jones's avatarAndrew Jones <ascjones@gmail.com>

* [lang2/macro] fix typo in test
Co-Authored-By: Andrew Jones's avatarAndrew Jones <ascjones@gmail.com>
parent 4613943a
Pipeline #56560 failed with stages
in 23 seconds
......@@ -23,7 +23,10 @@ use crate::{
use core::convert::TryFrom;
use either::Either;
use itertools::Itertools as _;
use proc_macro2::Ident;
use proc_macro2::{
Ident,
Span,
};
use std::collections::HashSet;
use syn::{
parse::{
......@@ -383,7 +386,7 @@ impl TryFrom<syn::ImplItemMethod> for ir::Function {
selector: ir::FunctionSelector::from(&method.sig.ident),
}))
}
unknown => Err(format_err!(unknown, "unknown ink! attribute found",)),
_unknown => Err(format_err_span!(attr.span(), "unknown ink! marker",)),
}?;
if kind == ir::FunctionKind::Method {
kind = new_kind;
......@@ -391,7 +394,7 @@ impl TryFrom<syn::ImplItemMethod> for ir::Function {
} else {
Err(format_err_span!(
attr.span(),
"conflicting ink! attribute found",
"conflicting ink! marker",
))
}
})
......@@ -587,24 +590,63 @@ impl TryFrom<syn::Item> for ir::Item {
}
}
syn::Item::Struct(item_struct) => {
let ink_markers = item_struct
.attrs
.iter()
.cloned()
.filter_map(|attr| ir::Marker::try_from(attr).ok())
let markers = utils::filter_map_ink_attributes(&item_struct.attrs)
.collect::<Vec<_>>();
if ink_markers.is_empty() {
Ok(ir::RustItem::from(syn::Item::Struct(item_struct)).into())
} else if ink_markers.iter().any(|marker| marker.is_simple("storage")) {
ir::ItemStorage::try_from(item_struct)
.map(Into::into)
.map(ir::Item::Ink)
} else if ink_markers.iter().any(|marker| marker.is_simple("event")) {
ir::ItemEvent::try_from(item_struct)
.map(Into::into)
.map(ir::Item::Ink)
} else {
bail!(item_struct, "found invalid `#[ink(..)]` marker")
if markers.is_empty() {
return Ok(ir::RustItem::from(syn::Item::Struct(item_struct)).into())
}
let event_marker =
markers.iter().position(|marker| marker.is_simple("event"));
let storage_marker =
markers.iter().position(|marker| marker.is_simple("storage"));
match (storage_marker, event_marker) {
(Some(_storage_marker), None) => {
ir::ItemStorage::try_from(item_struct)
.map(Into::into)
.map(ir::Item::Ink)
},
(None, Some(_event_marker)) => {
ir::ItemEvent::try_from(item_struct)
.map(Into::into)
.map(ir::Item::Ink)
},
(None, None) => {
Err(markers
.iter()
.map(|marker| {
format_err_span!(
marker.span(),
"unsupported ink! marker for struct")
})
.fold(
format_err!(
item_struct,
"encountered unsupported ink! markers for struct",
),
|mut err1, err2| {
err1.combine(err2);
err1
})
)
}
(Some(storage_marker), Some(event_marker)) => {
// Special case: We have both #[ink(storage)] and #[ink(event)].
// This is treated as error but depending on the
// order in which the markers have been provided
// we either treat it as storage or event definition.
//
// We take whatever ink! marker was provided first.
if storage_marker < event_marker {
ir::ItemStorage::try_from(item_struct)
.map(Into::into)
.map(ir::Item::Ink)
} else {
ir::ItemEvent::try_from(item_struct)
.map(Into::into)
.map(ir::Item::Ink)
}
}
}
}
rust_item => Ok(ir::Item::Rust(rust_item.into())),
......@@ -616,7 +658,7 @@ impl TryFrom<syn::Item> for ir::Item {
///
/// # Erros
///
/// - When there is no storage struct.
/// - When there is not exactly one storage struct.
/// - When a contract item is invalid.
fn split_items(
items: Vec<ir::InkItem>,
......@@ -628,19 +670,33 @@ fn split_items(
other => Either::Right(other),
}
});
let storage = if storages.len() != 1 {
Err(storages
.iter()
.map(|storage| format_err!(storage.ident, "conflicting storage struct found"))
.fold1(|mut err1, err2| {
err1.combine(err2);
err1
})
.expect("there must be at least 2 conflicting storages; qed"))
} else {
Ok(storages
.pop()
.expect("there must be exactly one storage in `storages`"))
let storage = match storages.len() {
0 => {
Err(format_err_span!(
Span::call_site(),
"no #[ink(storage)] struct found but expected exactly 1"
))
}
1 => {
Ok(storages
.pop()
.expect("there must be exactly one storage in `storages`"))
}
n => {
Err(storages
.iter()
.map(|storage| {
format_err!(storage.ident, "conflicting storage struct")
})
.fold(
format_err_span!(Span::call_site(), "encountered {} conflicting storage structs", n),
|mut err1, err2| {
err1.combine(err2);
err1
}
)
)
}
}?;
let (events, impl_blocks): (Vec<ir::ItemEvent>, Vec<ir::ItemImpl>) =
non_storage_items.into_iter().partition_map(|item| {
......@@ -654,6 +710,15 @@ fn split_items(
}
}
});
let storage_ident = &storage.ident;
for item_impl in &impl_blocks {
if item_impl.self_ty != storage_ident.to_string() {
bail!(
item_impl.self_ty,
"ink! impl blocks need to be implemented for the #[ink(storage)] struct"
)
}
}
let functions = impl_blocks
.into_iter()
.map(|impl_block| impl_block.functions)
......
......@@ -16,6 +16,7 @@
//! Contains general utilities for the ink! IR module.
use crate::ir;
use proc_macro2::Span;
use syn::{
parse::{
......@@ -73,6 +74,18 @@ where
attrs.into_iter().filter(|attr| is_ink_attribute(attr))
}
/// Yields back the filtered `#[ink(..)]` markers converted into their ink! form if any.
pub fn filter_map_ink_attributes<'a, I>(attrs: I) -> impl Iterator<Item = ir::Marker>
where
I: IntoIterator<Item = &'a syn::Attribute> + 'a,
{
use core::convert::TryFrom as _;
attrs
.into_iter()
.cloned()
.filter_map(|attr| ir::Marker::try_from(attr).ok())
}
/// Returns `true` if the attributes contain any `#[ink(..)]` markers.
pub fn has_ink_attributes<'a, I>(attrs: I) -> bool
where
......
......@@ -37,4 +37,11 @@ fn compile_tests() {
t.compile_fail("tests/ui/fail/11-unsafe-constructor.rs");
t.compile_fail("tests/ui/fail/12-const-constructor.rs");
t.compile_fail("tests/ui/fail/13-abi-constructor.rs");
t.compile_fail("tests/ui/fail/14-missing-storage-struct.rs");
t.compile_fail("tests/ui/fail/15-multiple-storage-structs.rs");
t.compile_fail("tests/ui/fail/16-storage-impl-ident-conflict.rs");
t.compile_fail("tests/ui/fail/17-conflicting-ink-markers.rs");
t.compile_fail("tests/ui/fail/18-conflicting-ink-markers-2.rs");
t.compile_fail("tests/ui/fail/19-unknown-struct-ink-marker.rs");
t.compile_fail("tests/ui/fail/20-unknown-method-marker.rs");
}
#![feature(proc_macro_hygiene)]
use ink_lang2 as ink;
#[ink::contract(version = "0.1.0")]
mod noop {
// We are missing the #[ink(storage)] attribute here
struct Noop {}
impl Noop {
#[ink(constructor)]
fn new(&mut self) {}
#[ink(message)]
fn noop(&self) {}
}
}
fn main() {}
error: no #[ink(storage)] struct found but expected exactly 1
--> $DIR/14-missing-storage-struct.rs:5:1
|
5 | #[ink::contract(version = "0.1.0")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
#![feature(proc_macro_hygiene)]
use ink_lang2 as ink;
#[ink::contract(version = "0.1.0")]
mod noop {
#[ink(storage)]
struct FirstStorage {}
// ink! currently doesn't allow for multiple #[ink(storage)] structs
#[ink(storage)]
struct SecondStorage {}
impl FirstStorage {
#[ink(constructor)]
fn new(&mut self) {}
#[ink(message)]
fn do_something(&self) {}
}
impl SecondStorage {
#[ink(constructor)]
fn new(&mut self) {}
#[ink(message)]
fn do_something(&self) {}
}
}
fn main() {}
error: encountered 2 conflicting storage structs
--> $DIR/15-multiple-storage-structs.rs:5:1
|
5 | #[ink::contract(version = "0.1.0")]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: conflicting storage struct
--> $DIR/15-multiple-storage-structs.rs:8:12
|
8 | struct FirstStorage {}
| ^^^^^^^^^^^^
error: conflicting storage struct
--> $DIR/15-multiple-storage-structs.rs:12:12
|
12 | struct SecondStorage {}
| ^^^^^^^^^^^^^
#![feature(proc_macro_hygiene)]
use ink_lang2 as ink;
#[ink::contract(version = "0.1.0")]
mod noop {
// This test ensures that ink! impl blocks are always
// implemented on the only storage struct definition.
#[ink(storage)]
struct StorageStruct {}
// This ink! impl block is okay.
impl StorageStruct {
#[ink(constructor)]
fn new1(&mut self) {}
#[ink(message)]
fn do_something1(&self) {}
}
// Missing the #[ink(storage)] attribute on purpose.
struct NonStorageStruct {}
// This ink! impl block is invalid in that it implements
// the messages and constructors for a non-existing ink!
// storage struct. We expect a failure here.
impl NonStorageStruct {
#[ink(constructor)]
fn new2(&mut self) {}
#[ink(message)]
fn do_something2(&self) {}
}
}
fn main() {}
error: ink! impl blocks need to be implemented for the #[ink(storage)] struct
--> $DIR/16-storage-impl-ident-conflict.rs:28:10
|
28 | impl NonStorageStruct {
| ^^^^^^^^^^^^^^^^
#![feature(proc_macro_hygiene)]
use ink_lang2 as ink;
#[ink::contract(version = "0.1.0")]
mod noop {
#[ink(storage)]
#[ink(event)] // We cannot have #[ink(event)] if we already have #[ink(storage)]
struct Noop {}
impl Noop {
#[ink(constructor)]
fn new(&mut self) {}
#[ink(message)]
fn noop(&self) {}
}
}
fn main() {}
error: invalid ink! attribute found for `#[ink(storage)]` struct
--> $DIR/17-conflicting-ink-markers.rs:8:10
|
8 | #[ink(event)] // We cannot have #[ink(event)] if we already have #[ink(storage)]
| ^^^^^^^
#![feature(proc_macro_hygiene)]
use ink_lang2 as ink;
#[ink::contract(version = "0.1.0")]
mod noop {
#[ink(storage)]
struct Noop {}
#[ink(event)]
#[ink(storage)] // We cannot have #[ink(storage)] if we already have #[ink(event)]
struct Event {}
impl Noop {
#[ink(constructor)]
fn new(&mut self) {}
#[ink(message)]
fn noop(&self) {}
}
}
fn main() {}
error: invalid ink! attribute found for `#[ink(event)]` struct
--> $DIR/18-conflicting-ink-markers-2.rs:11:10
|
11 | #[ink(storage)] // We cannot have #[ink(storage)] if we already have #[ink(event)]
| ^^^^^^^^^
#![feature(proc_macro_hygiene)]
use ink_lang2 as ink;
#[ink::contract(version = "0.1.0")]
mod noop {
#[ink(storage)]
struct Noop {}
#[ink(unknown_or_unsupported)]
struct HasUnknownMarker {}
impl Noop {
#[ink(constructor)]
fn new(&mut self) {}
#[ink(message)]
fn noop(&self) {}
}
}
fn main() {}
error: encountered unsupported ink! markers for struct
--> $DIR/19-unknown-struct-ink-marker.rs:10:5
|
10 | / #[ink(unknown_or_unsupported)]
11 | | struct HasUnknownMarker {}
| |______________________________^
error: unsupported ink! marker for struct
--> $DIR/19-unknown-struct-ink-marker.rs:10:10
|
10 | #[ink(unknown_or_unsupported)]
| ^^^^^^^^^^^^^^^^^^^^^^^^
#![feature(proc_macro_hygiene)]
use ink_lang2 as ink;
#[ink::contract(version = "0.1.0")]
mod noop {
#[ink(storage)]
struct Noop {}
impl Noop {
#[ink(constructor)]
fn new(&mut self) {}
#[ink(message)]
fn noop(&self) {}
#[ink(unknown_marker)]
fn has_unknown_marker(&self) {}
#[ink(unknown_marker_2)]
fn has_unknown_marker_too(&mut self) {}
}
}
fn main() {}
error: unknown ink! marker
--> $DIR/20-unknown-method-marker.rs:17:14
|
17 | #[ink(unknown_marker)]
| ^^^^^^^^^^^^^^^^
......@@ -18,7 +18,7 @@ mod incrementer {
by: i32,
}
impl Flipper {
impl Incrementer {
#[ink(constructor)]
fn new(&mut self, init_value: i32) {
self.value.set(init_value as i64);
......
......@@ -32,7 +32,7 @@ mod erc20 {
amount: Balance,
}
impl Flipper {
impl Erc20 {
#[ink(constructor)]
fn new(&mut self, initial_supply: Balance) {
let caller = self.env().caller();
......
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