diff --git a/crates/cairo-lang-starknet/cairo_level_tests/lib.cairo b/crates/cairo-lang-starknet/cairo_level_tests/lib.cairo index c846b14c3dc..d9c973a18fc 100644 --- a/crates/cairo-lang-starknet/cairo_level_tests/lib.cairo +++ b/crates/cairo-lang-starknet/cairo_level_tests/lib.cairo @@ -31,3 +31,5 @@ mod collections_test; mod component_usage_test; #[cfg(test)] mod flat_storage_test; +#[cfg(test)] +mod renamed_storage_test; diff --git a/crates/cairo-lang-starknet/cairo_level_tests/renamed_storage_test.cairo b/crates/cairo-lang-starknet/cairo_level_tests/renamed_storage_test.cairo new file mode 100644 index 00000000000..2c8255ffde9 --- /dev/null +++ b/crates/cairo-lang-starknet/cairo_level_tests/renamed_storage_test.cairo @@ -0,0 +1,47 @@ +use core::starknet::storage::{StoragePointerWriteAccess, StoragePointerReadAccess}; + +#[starknet::storage_node] +struct A { + pub a: felt252, +} + +#[starknet::storage_node] +struct B { + #[rename("a")] + pub b: felt252, +} + +#[starknet::storage_node] +struct AB { + #[flat] + pub a: A, + #[allow(starknet::colliding_storage_paths)] + #[flat] + pub b: B, +} + +#[starknet::contract] +mod contract { + #[storage] + pub struct Storage { + pub a: super::A, + #[allow(starknet::colliding_storage_paths)] + #[rename("a")] + pub b: super::B, + pub ab: super::AB, + } +} + +#[test] +fn rename_test() { + let mut state = contract::contract_state_for_testing(); + state.a.a.write(1); + assert_eq!(state.b.b.read(), 1); + state.b.b.write(2); + assert_eq!(state.a.a.read(), 2); + assert_eq!(state.ab.a.a.read(), 0); + state.ab.a.a.write(3); + assert_eq!(state.ab.b.b.read(), 3); + state.ab.b.b.write(4); + assert_eq!(state.ab.a.a.read(), 4); +} diff --git a/crates/cairo-lang-starknet/src/analyzer.rs b/crates/cairo-lang-starknet/src/analyzer.rs index 23189487692..b2569743c62 100644 --- a/crates/cairo-lang-starknet/src/analyzer.rs +++ b/crates/cairo-lang-starknet/src/analyzer.rs @@ -16,9 +16,10 @@ use smol_str::SmolStr; use crate::abi::{ABIError, AbiBuilder, BuilderConfig}; use crate::contract::module_contract; use crate::plugin::consts::{ - COMPONENT_ATTR, CONTRACT_ATTR, EMBEDDABLE_ATTR, FLAT_ATTR, STORAGE_ATTR, STORAGE_NODE_ATTR, - STORAGE_STRUCT_NAME, STORE_TRAIT, SUBSTORAGE_ATTR, + COMPONENT_ATTR, CONTRACT_ATTR, EMBEDDABLE_ATTR, STORAGE_ATTR, STORAGE_NODE_ATTR, + STORAGE_STRUCT_NAME, STORE_TRAIT, }; +use crate::plugin::storage_interfaces::{StorageMemberKind, get_member_storage_config}; use crate::plugin::utils::has_derive; const ALLOW_NO_DEFAULT_VARIANT_ATTR: &str = "starknet::store_no_default_variant"; @@ -218,14 +219,12 @@ fn member_analyze( diagnostics: &mut Vec, ) { user_data_path.push(member_name.clone()); - if !(member.id.stable_ptr(db.upcast()).lookup(db.upcast()).has_attr(db.upcast(), FLAT_ATTR) - || member - .id - .stable_ptr(db.upcast()) - .lookup(db.upcast()) - .has_attr(db.upcast(), SUBSTORAGE_ATTR)) - { - paths_data.handle(member_name, user_data_path.clone(), pointer_to_code, diagnostics); + let member_ast = member.id.stable_ptr(db.upcast()).lookup(db.upcast()); + // Ignoring diagnostics as these would have been reported previously. + let config = get_member_storage_config(db.upcast(), &member_ast, &mut vec![]); + if config.kind == StorageMemberKind::Basic { + let name = config.rename.map(Into::into).unwrap_or(member_name); + paths_data.handle(name, user_data_path.clone(), pointer_to_code, diagnostics); user_data_path.pop(); return; } diff --git a/crates/cairo-lang-starknet/src/plugin/consts.rs b/crates/cairo-lang-starknet/src/plugin/consts.rs index 84859834a82..27c8a1df5fd 100644 --- a/crates/cairo-lang-starknet/src/plugin/consts.rs +++ b/crates/cairo-lang-starknet/src/plugin/consts.rs @@ -41,6 +41,7 @@ pub const EMBEDDABLE_AS_ATTR: &str = "embeddable_as"; pub const COMPONENT_INLINE_MACRO: &str = "component"; pub const HAS_COMPONENT_TRAIT: &str = "HasComponent"; pub const SUBSTORAGE_ATTR: &str = "substorage"; +pub const RENAME_ATTR: &str = "rename"; pub const NESTED_ATTR: &str = "nested"; pub const FLAT_ATTR: &str = "flat"; pub const KEY_ATTR: &str = "key"; diff --git a/crates/cairo-lang-starknet/src/plugin/derive/store.rs b/crates/cairo-lang-starknet/src/plugin/derive/store.rs index 6af40a0bcae..711dd249cc9 100644 --- a/crates/cairo-lang-starknet/src/plugin/derive/store.rs +++ b/crates/cairo-lang-starknet/src/plugin/derive/store.rs @@ -7,7 +7,9 @@ use indent::indent_by; use indoc::formatdoc; use crate::plugin::consts::STORE_TRAIT; -use crate::plugin::storage_interfaces::handle_storage_interface_struct; +use crate::plugin::storage_interfaces::{ + handle_storage_interface_struct, struct_members_storage_configs, +}; /// Returns the rewrite node for the `#[derive(starknet::Store)]` attribute. pub fn handle_store_derive( @@ -22,7 +24,9 @@ pub fn handle_store_derive( // a sub-pointers implementation. let store_trait_code = handle_struct_store(db, struct_ast)?; let sub_pointers_code = if !struct_ast.members(db).elements(db).is_empty() { - handle_storage_interface_struct(db, struct_ast, metadata).into_rewrite_node() + let configs = struct_members_storage_configs(db, struct_ast, diagnostics); + handle_storage_interface_struct(db, struct_ast, &configs, metadata) + .into_rewrite_node() } else { RewriteNode::Text("".to_string()) }; diff --git a/crates/cairo-lang-starknet/src/plugin/plugin_test_data/contracts/with_component_diagnostics b/crates/cairo-lang-starknet/src/plugin/plugin_test_data/contracts/with_component_diagnostics index 269c84f9df9..d27945c22f8 100644 --- a/crates/cairo-lang-starknet/src/plugin/plugin_test_data/contracts/with_component_diagnostics +++ b/crates/cairo-lang-starknet/src/plugin/plugin_test_data/contracts/with_component_diagnostics @@ -1969,6 +1969,11 @@ impl StorageStorageBaseMutDrop of core::traits::Drop::; impl StorageStorageBaseMutCopy of core::traits::Copy::; //! > expected_diagnostics +error: Plugin diagnostic: Only #[substorage(v0)] is supported. + --> lib.cairo:19:9 + #[substorage] + ^***********^ + error: Plugin diagnostic: `substorage` attribute is only allowed for members of type [some_path::]Storage` --> lib.cairo:14:9 #[substorage(v0)] diff --git a/crates/cairo-lang-starknet/src/plugin/storage.rs b/crates/cairo-lang-starknet/src/plugin/storage.rs index 93937d129c5..37ce76896c6 100644 --- a/crates/cairo-lang-starknet/src/plugin/storage.rs +++ b/crates/cairo-lang-starknet/src/plugin/storage.rs @@ -4,10 +4,14 @@ use cairo_lang_syntax::node::db::SyntaxGroup; use cairo_lang_syntax::node::helpers::QueryAttrs; use cairo_lang_syntax::node::{Terminal, TypedSyntaxNode, ast}; use indoc::formatdoc; +use itertools::zip_eq; use super::starknet_module::generation_data::StarknetModuleCommonGenerationData; use super::starknet_module::{StarknetModuleKind, backwards_compatible_storage}; -use super::storage_interfaces::handle_storage_interface_struct; +use super::storage_interfaces::{ + StorageMemberConfig, StorageMemberKind, handle_storage_interface_struct, + struct_members_storage_configs, +}; use super::{CONCRETE_COMPONENT_STATE_NAME, CONTRACT_STATE_NAME, FLAT_ATTR, STORAGE_STRUCT_NAME}; use crate::plugin::SUBSTORAGE_ATTR; @@ -31,8 +35,9 @@ pub fn handle_storage_struct( let mut substorage_members_struct_code = vec![]; let mut substorage_members_init_code = vec![]; let mut storage_struct_members = vec![]; - for member in struct_ast.members(db).elements(db) { - if member.has_attr(db, SUBSTORAGE_ATTR) { + let configs = struct_members_storage_configs(db, &struct_ast, diagnostics); + for (member, config) in zip_eq(struct_ast.members(db).elements(db), &configs) { + if config.kind == StorageMemberKind::SubStorage { if let Some((struct_code, init_code)) = get_substorage_member_code(db, &member, metadata) { @@ -49,7 +54,7 @@ pub fn handle_storage_struct( } } let SimpleMemberGeneratedCode { struct_code, struct_code_mut, init_code, storage_member } = - get_simple_member_code(db, &member, metadata); + get_simple_member_code(db, &member, config, metadata); members_struct_code.push(struct_code); members_struct_code_mut.push(struct_code_mut); members_init_code.push(init_code); @@ -70,7 +75,7 @@ pub fn handle_storage_struct( "".to_string() }; let storage_base_code = - handle_storage_interface_struct(db, &struct_ast, metadata).into_rewrite_node(); + handle_storage_interface_struct(db, &struct_ast, &configs, metadata).into_rewrite_node(); data.state_struct_code = RewriteNode::interpolate_patched( &formatdoc!( " @@ -211,10 +216,11 @@ struct SimpleMemberGeneratedCode { fn get_simple_member_code( db: &dyn SyntaxGroup, member: &ast::Member, + config: &StorageMemberConfig, metadata: &MacroPluginMetadata<'_>, ) -> SimpleMemberGeneratedCode { let member_wrapper_type = RewriteNode::text( - if member.has_attr(db, SUBSTORAGE_ATTR) || member.has_attr(db, FLAT_ATTR) { + if matches!(config.kind, StorageMemberKind::SubStorage | StorageMemberKind::Flat) { "FlattenedStorage" } else { "StorageBase" @@ -226,11 +232,14 @@ fn get_simple_member_code( RewriteNode::from_ast(&member.visibility(db)) }; let member_name = RewriteNode::from_ast_trimmed(&member.name(db)); + let member_selector_name = + config.rename.as_deref().map_or_else(|| member_name.clone(), RewriteNode::text); let patches = [ ("attributes".to_string(), RewriteNode::from_ast(&member.attributes(db))), ("member_visibility".to_string(), member_visibility.clone()), ("member_wrapper_type".to_string(), member_wrapper_type.clone()), ("member_name".to_string(), member_name), + ("member_selector_name".to_string(), member_selector_name), ("member_type".to_string(), RewriteNode::from_ast_trimmed(&member.type_clause(db).ty(db))), ] .into(); @@ -256,7 +265,7 @@ fn get_simple_member_code( } else { RewriteNode::interpolate_patched( "\n $member_name$: starknet::storage::StorageBase{ address: \ - selector!(\"$member_name$\") },", + selector!(\"$member_selector_name$\") },", &patches, ) } diff --git a/crates/cairo-lang-starknet/src/plugin/storage_interfaces.rs b/crates/cairo-lang-starknet/src/plugin/storage_interfaces.rs index eff19f6f292..d49a9d601ec 100644 --- a/crates/cairo-lang-starknet/src/plugin/storage_interfaces.rs +++ b/crates/cairo-lang-starknet/src/plugin/storage_interfaces.rs @@ -4,17 +4,21 @@ use cairo_lang_defs::plugin::{ }; use cairo_lang_defs::plugin_utils::extract_single_unnamed_arg; use cairo_lang_filesystem::ids::CodeMapping; +use cairo_lang_syntax::attribute::structured::{ + AttributeArg, AttributeArgVariant, AttributeStructurize, +}; use cairo_lang_syntax::node::ast::OptionArgListParenthesized; use cairo_lang_syntax::node::db::SyntaxGroup; use cairo_lang_syntax::node::helpers::QueryAttrs; use cairo_lang_syntax::node::{TypedSyntaxNode, ast}; use indoc::formatdoc; +use itertools::zip_eq; use super::starknet_module::backwards_compatible_storage; -use super::utils::has_derive; +use super::utils::{has_derive, validate_v0}; use super::{ - FLAT_ATTR, STORAGE_ATTR, STORAGE_NODE_ATTR, STORAGE_SUB_POINTERS_ATTR, STORE_TRAIT, - SUBSTORAGE_ATTR, + FLAT_ATTR, RENAME_ATTR, STORAGE_ATTR, STORAGE_NODE_ATTR, STORAGE_SUB_POINTERS_ATTR, + STORE_TRAIT, SUBSTORAGE_ATTR, }; /// Generates an impl for the `starknet::StorageNode` to point to a generate a struct to to be @@ -85,8 +89,9 @@ impl MacroPlugin for StorageInterfacesPlugin { if !diagnostics.is_empty() { return PluginResult { code: None, diagnostics, remove_original_item: false }; } + let configs = struct_members_storage_configs(db, &struct_ast, &mut diagnostics); let (content, code_mappings) = - handle_storage_interface_struct(db, &struct_ast, metadata).build(); + handle_storage_interface_struct(db, &struct_ast, &configs, metadata).build(); PluginResult { code: Some(PluginGeneratedFile { name: "storage_node".into(), @@ -95,7 +100,7 @@ impl MacroPlugin for StorageInterfacesPlugin { aux_data: None, diagnostics_note: Default::default(), }), - diagnostics: vec![], + diagnostics, remove_original_item: false, } } @@ -159,7 +164,11 @@ impl MacroPlugin for StorageInterfacesPlugin { } fn declared_attributes(&self) -> Vec { - vec![STORAGE_NODE_ATTR.to_string(), STORAGE_SUB_POINTERS_ATTR.to_string()] + vec![ + STORAGE_NODE_ATTR.to_string(), + STORAGE_SUB_POINTERS_ATTR.to_string(), + RENAME_ATTR.to_string(), + ] } fn phantom_type_attributes(&self) -> Vec { @@ -376,7 +385,7 @@ impl<'a> StorageInterfaceInfo<'a> { " let __$field_name$_value__ = \ starknet::storage::PendingStoragePathTrait::new( @self, - selector!(\"$field_name$\") + selector!(\"$field_selector_name$\") ); " .to_string() @@ -406,7 +415,7 @@ impl<'a> StorageInterfaceInfo<'a> { .to_string() } else { " let __$field_name$_value__ = starknet::storage::StorageBase \ - {__base_address__: selector!(\"$field_name$\")}; + {__base_address__: selector!(\"$field_selector_name$\")}; " .to_string() } @@ -429,6 +438,7 @@ impl<'a> StorageInterfaceInfo<'a> { fn handle_storage_interface_for_interface_type( db: &dyn SyntaxGroup, struct_ast: &ast::ItemStruct, + configs: &[StorageMemberConfig], metadata: &MacroPluginMetadata<'_>, storage_node_type: StorageInterfaceType, builder: &mut PatchBuilder<'_>, @@ -437,12 +447,12 @@ fn handle_storage_interface_for_interface_type( StorageInterfaceInfo { db, node_type: storage_node_type.clone(), is_mutable: false }; // Create Info with type and false for is_mutable add_interface_struct_definition(db, builder, struct_ast, &storage_node_info, metadata); - add_interface_impl(db, builder, struct_ast, &storage_node_info); + add_interface_impl(db, builder, struct_ast, configs, &storage_node_info); let mutable_storage_node_info = StorageInterfaceInfo { db, node_type: storage_node_type, is_mutable: true }; // Create Info with type and true for is_mutable add_interface_struct_definition(db, builder, struct_ast, &mutable_storage_node_info, metadata); - add_interface_impl(db, builder, struct_ast, &mutable_storage_node_info); + add_interface_impl(db, builder, struct_ast, configs, &mutable_storage_node_info); } /// Adds the storage interface structs (two variants, mutable and immutable) and their constructor @@ -453,6 +463,7 @@ fn handle_storage_interface_for_interface_type( pub fn handle_storage_interface_struct<'a>( db: &'a dyn SyntaxGroup, struct_ast: &ast::ItemStruct, + configs: &[StorageMemberConfig], metadata: &MacroPluginMetadata<'_>, ) -> PatchBuilder<'a> { // Run for both StorageNode and StorageTrait @@ -474,6 +485,7 @@ pub fn handle_storage_interface_struct<'a>( handle_storage_interface_for_interface_type( db, struct_ast, + configs, metadata, interface_type, &mut builder, @@ -566,6 +578,7 @@ fn add_interface_impl( db: &dyn SyntaxGroup, builder: &mut PatchBuilder<'_>, struct_ast: &ast::ItemStruct, + configs: &[StorageMemberConfig], storage_node_info: &StorageInterfaceInfo<'_>, ) { let struct_name = RewriteNode::from_ast_trimmed(&struct_ast.name(db)); @@ -590,15 +603,21 @@ fn add_interface_impl( )); let fields = struct_ast.members(db).elements(db); - let mut fields_iter = fields.iter().peekable(); - while let Some(field) = fields_iter.next() { + let mut fields_iter = zip_eq(&fields, configs).peekable(); + while let Some((field, config)) = fields_iter.next() { let field_name = RewriteNode::from_ast_trimmed(&field.name(db)); let field_type = RewriteNode::from_ast_trimmed(&field.type_clause(db).ty(db)); + let field_selector_name = + config.rename.as_deref().map_or_else(|| field_name.clone(), RewriteNode::text); let is_last = fields_iter.peek().is_none(); builder.add_modified(RewriteNode::interpolate_patched( &storage_node_info.node_constructor_field_init_code(is_last, field), - &[("field_name".to_string(), field_name), ("field_type".to_string(), field_type)] - .into(), + &[ + ("field_selector_name".to_string(), field_selector_name), + ("field_name".to_string(), field_name), + ("field_type".to_string(), field_type), + ] + .into(), )); } @@ -719,3 +738,99 @@ fn add_node_enum_impl( } builder.add_str(" }\n}\n}\n"); } + +/// The configuration of a storage struct member. +#[derive(Debug)] +pub struct StorageMemberConfig { + pub kind: StorageMemberKind, + pub rename: Option, +} + +/// The kind of a storage struct member. +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum StorageMemberKind { + /// A basic storage member - would be stored in a separate storage slot. + Basic, + /// A flat storage member - would be stored in the same storage slot as the struct. + Flat, + /// A sub-storage member - would be stored in the same storage slot, but used for components. + SubStorage, +} + +/// Gets the storage configuration for members of a struct. +pub fn struct_members_storage_configs( + db: &dyn SyntaxGroup, + struct_ast: &ast::ItemStruct, + diagnostics: &mut Vec, +) -> Vec { + struct_ast + .members(db) + .elements(db) + .into_iter() + .map(|member| get_member_storage_config(db, &member, diagnostics)) + .collect() +} + +/// Gets the storage configuration of a struct member. +pub fn get_member_storage_config( + db: &dyn SyntaxGroup, + member: &ast::Member, + diagnostics: &mut Vec, +) -> StorageMemberConfig { + let mut result = StorageMemberConfig { kind: StorageMemberKind::Basic, rename: None }; + for attr in member.query_attr(db, FLAT_ATTR) { + let attr = attr.structurize(db); + if result.kind != StorageMemberKind::Basic { + diagnostics.push(PluginDiagnostic::error( + attr.stable_ptr, + "Multiple storage attributes are not allowed.".to_string(), + )); + } + if !attr.args.is_empty() { + diagnostics.push(PluginDiagnostic::error( + attr.args_stable_ptr, + "Unexpected arguments.".to_string(), + )); + } + result.kind = StorageMemberKind::Flat; + } + for attr in member.query_attr(db, SUBSTORAGE_ATTR) { + if result.kind != StorageMemberKind::Basic { + diagnostics.push(PluginDiagnostic::error( + &attr, + "Multiple storage attributes are not allowed.".to_string(), + )); + } + validate_v0(db, diagnostics, &attr, SUBSTORAGE_ATTR); + result.kind = StorageMemberKind::SubStorage; + } + for attr in member.query_attr(db, RENAME_ATTR) { + if result.kind != StorageMemberKind::Basic { + diagnostics.push(PluginDiagnostic::error( + &attr, + "The `rename` attribute cannot be used with other storage attributes.".to_string(), + )); + } + if result.rename.is_some() { + diagnostics.push(PluginDiagnostic::error( + &attr, + "Multiple `rename` attributes are not allowed.".to_string(), + )); + } + let attr = attr.structurize(db); + let [ + AttributeArg { + variant: AttributeArgVariant::Unnamed(ast::Expr::String(value)), .. + }, + ] = &attr.args[..] + else { + diagnostics.push(PluginDiagnostic::error( + attr.args_stable_ptr, + "Unexpected arguments, expected single string argument.".to_string(), + )); + continue; + }; + result.rename = value.string_value(db); + } + result +} diff --git a/crates/cairo-lang-starknet/src/plugin/utils.rs b/crates/cairo-lang-starknet/src/plugin/utils.rs index 4e9680d7751..a152b35883a 100644 --- a/crates/cairo-lang-starknet/src/plugin/utils.rs +++ b/crates/cairo-lang-starknet/src/plugin/utils.rs @@ -225,7 +225,7 @@ pub fn has_v0_attribute_ex( } /// Assuming the attribute is `name`, validates it's #[`name`(v0)]. -fn validate_v0( +pub fn validate_v0( db: &dyn SyntaxGroup, diagnostics: &mut Vec, attr: &Attribute, diff --git a/crates/cairo-lang-starknet/src/test_data/storage_path_check b/crates/cairo-lang-starknet/src/test_data/storage_path_check index ab7946af0eb..64f60fe27b0 100644 --- a/crates/cairo-lang-starknet/src/test_data/storage_path_check +++ b/crates/cairo-lang-starknet/src/test_data/storage_path_check @@ -225,3 +225,24 @@ struct IgnoredCollision3 { } //! > diagnostics + +//! > ========================================================================== + +//! > Test collision by renames. + +//! > test_runner_name +test_storage_path_check(expect_diagnostics: true) + +//! > cairo_code +#[starknet::storage_node] +struct IgnoredCollision1 { + pub a: felt252, + #[rename("a")] + pub b: felt252, +} + +//! > diagnostics +warning: Plugin diagnostic: The path `b` collides with existing path `a`. Fix or add `#[allow(starknet::colliding_storage_paths)]` if intentional. + --> lib.cairo:5:9 + pub b: felt252, + ^