Change decl_storage! to forbid default value for Option types (#3682)

* Forbid default values for Option types in decl_storage!

* fix test errors

* bump impl_version
This commit is contained in:
Ashley
2019-09-25 19:34:31 +12:00
committed by Bastian Köcher
parent 1bf44278d3
commit a6329f8779
5 changed files with 24 additions and 28 deletions
@@ -8,7 +8,7 @@
/// For more guidance on Substrate modules, see the example module /// For more guidance on Substrate modules, see the example module
/// https://github.com/paritytech/substrate/blob/master/srml/example/src/lib.rs /// https://github.com/paritytech/substrate/blob/master/srml/example/src/lib.rs
use support::{decl_module, decl_storage, decl_event, StorageValue, dispatch::Result}; use support::{decl_module, decl_storage, decl_event, dispatch::Result};
use system::ensure_signed; use system::ensure_signed;
/// The module's configuration trait. /// The module's configuration trait.
+1 -1
View File
@@ -85,7 +85,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
// implementation changes and behavior does not, then leave spec_version as // implementation changes and behavior does not, then leave spec_version as
// is and increment impl_version. // is and increment impl_version.
spec_version: 163, spec_version: 163,
impl_version: 163, impl_version: 164,
apis: RUNTIME_API_VERSIONS, apis: RUNTIME_API_VERSIONS,
}; };
+1 -1
View File
@@ -20,7 +20,7 @@ use super::*;
use runtime_io::with_externalities; use runtime_io::with_externalities;
use sr_primitives::traits::OnInitialize; use sr_primitives::traits::OnInitialize;
use sr_staking_primitives::offence::{OffenceDetails, OnOffenceHandler}; use sr_staking_primitives::offence::{OffenceDetails, OnOffenceHandler};
use support::{assert_ok, assert_noop, assert_eq_uvec, StorageLinkedMap}; use support::{assert_ok, assert_noop, assert_eq_uvec};
use mock::*; use mock::*;
use support::traits::{Currency, ReservableCurrency}; use support::traits::{Currency, ReservableCurrency};
@@ -122,7 +122,7 @@ pub fn decl_storage_impl(input: TokenStream) -> TokenStream {
&cratename, &cratename,
&storage_lines, &storage_lines,
&where_clause, &where_clause,
); ).unwrap_or_else(|err| err.to_compile_error());
let decl_store_items = decl_store_items( let decl_store_items = decl_store_items(
&storage_lines, &storage_lines,
@@ -637,7 +637,7 @@ fn decl_storage_items(
cratename: &Ident, cratename: &Ident,
storage_lines: &ext::Punctuated<DeclStorageLine, Token![;]>, storage_lines: &ext::Punctuated<DeclStorageLine, Token![;]>,
where_clause: &Option<WhereClause>, where_clause: &Option<WhereClause>,
) -> TokenStream2 { ) -> syn::Result<TokenStream2> {
let mut impls = TokenStream2::new(); let mut impls = TokenStream2::new();
let InstanceOpts { let InstanceOpts {
@@ -768,6 +768,14 @@ fn decl_storage_items(
} = sline; } = sline;
let type_infos = get_type_infos(storage_type); let type_infos = get_type_infos(storage_type);
if type_infos.is_option && default_value.inner.is_some() {
return Err(syn::Error::new_spanned(
default_value,
"Default values for Option types are not supported"
));
}
let fielddefault = default_value.inner let fielddefault = default_value.inner
.as_ref() .as_ref()
.map(|d| &d.expr) .map(|d| &d.expr)
@@ -808,7 +816,8 @@ fn decl_storage_items(
}; };
impls.extend(implementation) impls.extend(implementation)
} }
impls
Ok(impls)
} }
fn decl_store_items(storage_lines: &ext::Punctuated<DeclStorageLine, Token![;]>) -> TokenStream2 { fn decl_store_items(storage_lines: &ext::Punctuated<DeclStorageLine, Token![;]>) -> TokenStream2 {
@@ -300,10 +300,10 @@ mod tests {
// non-getters: pub / $default // non-getters: pub / $default
/// Hello, this is doc! /// Hello, this is doc!
U32 : Option<u32> = Some(3); U32 : Option<u32>;
pub PUBU32 : Option<u32>; pub PUBU32 : Option<u32>;
U32MYDEF : Option<u32> = None; U32MYDEF : Option<u32>;
pub PUBU32MYDEF : Option<u32> = Some(3); pub PUBU32MYDEF : Option<u32>;
// getters: pub / $default // getters: pub / $default
// we need at least one type which uses T, otherwise GenesisConfig will complain. // we need at least one type which uses T, otherwise GenesisConfig will complain.
@@ -311,17 +311,17 @@ mod tests {
pub PUBGETU32 get(pub_u32_getter) build(|config: &GenesisConfig| config.u32_getter_with_config): u32; pub PUBGETU32 get(pub_u32_getter) build(|config: &GenesisConfig| config.u32_getter_with_config): u32;
GETU32WITHCONFIG get(u32_getter_with_config) config(): u32; GETU32WITHCONFIG get(u32_getter_with_config) config(): u32;
pub PUBGETU32WITHCONFIG get(pub_u32_getter_with_config) config(): u32; pub PUBGETU32WITHCONFIG get(pub_u32_getter_with_config) config(): u32;
GETU32MYDEF get(u32_getter_mydef): Option<u32> = Some(4); GETU32MYDEF get(u32_getter_mydef): Option<u32>;
pub PUBGETU32MYDEF get(pub_u32_getter_mydef) config(): u32 = 3; pub PUBGETU32MYDEF get(pub_u32_getter_mydef) config(): u32 = 3;
GETU32WITHCONFIGMYDEF get(u32_getter_with_config_mydef) config(): u32 = 2; GETU32WITHCONFIGMYDEF get(u32_getter_with_config_mydef) config(): u32 = 2;
pub PUBGETU32WITHCONFIGMYDEF get(pub_u32_getter_with_config_mydef) config(): u32 = 1; pub PUBGETU32WITHCONFIGMYDEF get(pub_u32_getter_with_config_mydef) config(): u32 = 1;
PUBGETU32WITHCONFIGMYDEFOPT get(pub_u32_getter_with_config_mydef_opt) config(): Option<u32> = Some(100); PUBGETU32WITHCONFIGMYDEFOPT get(pub_u32_getter_with_config_mydef_opt) config(): Option<u32>;
// map non-getters: pub / $default // map non-getters: pub / $default
MAPU32 : map u32 => Option<String>; MAPU32 : map u32 => Option<String>;
pub PUBMAPU32 : map u32 => Option<String>; pub PUBMAPU32 : map u32 => Option<String>;
MAPU32MYDEF : map u32 => Option<String> = None; MAPU32MYDEF : map u32 => Option<String>;
pub PUBMAPU32MYDEF : map u32 => Option<String> = Some("hello".into()); pub PUBMAPU32MYDEF : map u32 => Option<String>;
// map getters: pub / $default // map getters: pub / $default
GETMAPU32 get(map_u32_getter): map u32 => String; GETMAPU32 get(map_u32_getter): map u32 => String;
@@ -332,7 +332,7 @@ mod tests {
// linked map // linked map
LINKEDMAPU32 : linked_map u32 => Option<String>; LINKEDMAPU32 : linked_map u32 => Option<String>;
pub PUBLINKEDMAPU32MYDEF : linked_map u32 => Option<String> = Some("hello".into()); pub PUBLINKEDMAPU32MYDEF : linked_map u32 => Option<String>;
GETLINKEDMAPU32 get(linked_map_u32_getter): linked_map u32 => String; GETLINKEDMAPU32 get(linked_map_u32_getter): linked_map u32 => String;
pub PUBGETLINKEDMAPU32MYDEF get(pub_linked_map_u32_getter_mydef): linked_map u32 => String = "pubmap".into(); pub PUBGETLINKEDMAPU32MYDEF get(pub_linked_map_u32_getter_mydef): linked_map u32 => String = "pubmap".into();
@@ -688,7 +688,7 @@ mod tests {
assert_eq!(config.pub_u32_getter_mydef, 3u32); assert_eq!(config.pub_u32_getter_mydef, 3u32);
assert_eq!(config.u32_getter_with_config_mydef, 2u32); assert_eq!(config.u32_getter_with_config_mydef, 2u32);
assert_eq!(config.pub_u32_getter_with_config_mydef, 1u32); assert_eq!(config.pub_u32_getter_with_config_mydef, 1u32);
assert_eq!(config.pub_u32_getter_with_config_mydef_opt, 100u32); assert_eq!(config.pub_u32_getter_with_config_mydef_opt, 0u32);
} }
} }
@@ -781,17 +781,14 @@ mod test_append_and_len {
JustVec: Vec<u32>; JustVec: Vec<u32>;
JustVecWithDefault: Vec<u32> = vec![6, 9]; JustVecWithDefault: Vec<u32> = vec![6, 9];
OptionVec: Option<Vec<u32>>; OptionVec: Option<Vec<u32>>;
OptionVecWithDefault: Option<Vec<u32>> = Some(vec![6, 9]);
MapVec: map u32 => Vec<u32>; MapVec: map u32 => Vec<u32>;
MapVecWithDefault: map u32 => Vec<u32> = vec![6, 9]; MapVecWithDefault: map u32 => Vec<u32> = vec![6, 9];
OptionMapVec: map u32 => Option<Vec<u32>>; OptionMapVec: map u32 => Option<Vec<u32>>;
OptionMapVecWithDefault: map u32 => Option<Vec<u32>> = Some(vec![6, 9]);
LinkedMapVec: linked_map u32 => Vec<u32>; LinkedMapVec: linked_map u32 => Vec<u32>;
LinkedMapVecWithDefault: linked_map u32 => Vec<u32> = vec![6, 9]; LinkedMapVecWithDefault: linked_map u32 => Vec<u32> = vec![6, 9];
OptionLinkedMapVec: linked_map u32 => Option<Vec<u32>>; OptionLinkedMapVec: linked_map u32 => Option<Vec<u32>>;
OptionLinkedMapVecWithDefault: linked_map u32 => Option<Vec<u32>> = Some(vec![6, 9]);
} }
} }
@@ -805,7 +802,6 @@ mod test_append_and_len {
#[test] #[test]
fn default_for_option() { fn default_for_option() {
with_externalities(&mut TestExternalities::default(), || { with_externalities(&mut TestExternalities::default(), || {
assert_eq!(OptionVecWithDefault::get(), Some(vec![6, 9]));
assert_eq!(OptionVec::get(), None); assert_eq!(OptionVec::get(), None);
assert_eq!(JustVec::get(), vec![]); assert_eq!(JustVec::get(), vec![]);
}); });
@@ -882,9 +878,6 @@ mod test_append_and_len {
assert_eq!(OptionVec::get(), None); assert_eq!(OptionVec::get(), None);
assert_eq!(OptionVec::decode_len(), Ok(0)); assert_eq!(OptionVec::decode_len(), Ok(0));
assert_eq!(OptionVecWithDefault::get(), Some(vec![6, 9]));
assert_eq!(OptionVecWithDefault::decode_len(), Ok(2));
// map // map
assert_eq!(MapVec::get(0), vec![]); assert_eq!(MapVec::get(0), vec![]);
assert_eq!(MapVec::decode_len(0), Ok(0)); assert_eq!(MapVec::decode_len(0), Ok(0));
@@ -895,9 +888,6 @@ mod test_append_and_len {
assert_eq!(OptionMapVec::get(0), None); assert_eq!(OptionMapVec::get(0), None);
assert_eq!(OptionMapVec::decode_len(0), Ok(0)); assert_eq!(OptionMapVec::decode_len(0), Ok(0));
assert_eq!(OptionMapVecWithDefault::get(0), Some(vec![6, 9]));
assert_eq!(OptionMapVecWithDefault::decode_len(0), Ok(2));
// linked map // linked map
assert_eq!(LinkedMapVec::get(0), vec![]); assert_eq!(LinkedMapVec::get(0), vec![]);
assert_eq!(LinkedMapVec::decode_len(0), Ok(0)); assert_eq!(LinkedMapVec::decode_len(0), Ok(0));
@@ -907,9 +897,6 @@ mod test_append_and_len {
assert_eq!(OptionLinkedMapVec::get(0), None); assert_eq!(OptionLinkedMapVec::get(0), None);
assert_eq!(OptionLinkedMapVec::decode_len(0), Ok(0)); assert_eq!(OptionLinkedMapVec::decode_len(0), Ok(0));
assert_eq!(OptionLinkedMapVecWithDefault::get(0), Some(vec![6, 9]));
assert_eq!(OptionLinkedMapVecWithDefault::decode_len(0), Ok(2));
}); });
} }
} }