Skip to content

Commit

Permalink
Added option to allow conflicting paths.
Browse files Browse the repository at this point in the history
commit-id:46998f96
  • Loading branch information
orizi committed Nov 27, 2024
1 parent 495e52a commit 0f12366
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 13 deletions.
18 changes: 15 additions & 3 deletions crates/cairo-lang-starknet/src/analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use crate::plugin::consts::{
use crate::plugin::utils::has_derive;

const ALLOW_NO_DEFAULT_VARIANT_ATTR: &str = "starknet::store_no_default_variant";
const ALLOW_COLLIDING_PATHS_ATTR: &str = "starknet::colliding_storage_paths";

/// Plugin to add diagnostics for contracts for bad ABI generation.
#[derive(Default, Debug)]
Expand Down Expand Up @@ -123,7 +124,7 @@ impl AnalyzerPlugin for StorageAnalyzer {
}

fn declared_allows(&self) -> Vec<String> {
vec![ALLOW_NO_DEFAULT_VARIANT_ATTR.to_string()]
vec![ALLOW_NO_DEFAULT_VARIANT_ATTR.to_string(), ALLOW_COLLIDING_PATHS_ATTR.to_string()]
}
}

Expand All @@ -135,11 +136,21 @@ fn add_storage_struct_diags(
id: StructId,
diagnostics: &mut Vec<PluginDiagnostic>,
) {
let paths_data = &mut StorageStructMembers { name_to_paths: OrderedHashMap::default() };
if id.has_attr_with_arg(db, "allow", ALLOW_COLLIDING_PATHS_ATTR) == Ok(true) {
return;
}
let Ok(members) = db.struct_members(id) else {
return;
};
let paths_data = &mut StorageStructMembers { name_to_paths: OrderedHashMap::default() };
for (member_name, member) in members.iter() {
if member.id.stable_ptr(db.upcast()).lookup(db.upcast()).has_attr_with_arg(
db.upcast(),
"allow",
ALLOW_COLLIDING_PATHS_ATTR,
) {
continue;
}
member_analyze(
db,
member,
Expand Down Expand Up @@ -176,7 +187,8 @@ impl StorageStructMembers {
diagnostics.push(PluginDiagnostic::warning(
pointer_to_code,
format!(
"The path `{}` collides with existing path `{}`.",
"The path `{}` collides with existing path `{}`. Fix or add \
`#[allow({ALLOW_COLLIDING_PATHS_ATTR})]` if intentional.",
path_to_member.join("."),
existing_path.join(".")
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1081,7 +1081,7 @@ impl StorageStorageBaseMutDrop of core::traits::Drop::<StorageStorageBaseMut>;
impl StorageStorageBaseMutCopy of core::traits::Copy::<StorageStorageBaseMut>;

//! > expected_diagnostics
warning: Plugin diagnostic: The path `component2_storage.data` collides with existing path `component1_storage.data`.
warning: Plugin diagnostic: The path `component2_storage.data` collides with existing path `component1_storage.data`. Fix or add `#[allow(starknet::colliding_storage_paths)]` if intentional.
--> lib.cairo:26:9
component2_storage: super::component2::Storage,
^****************^
Expand Down Expand Up @@ -1136,7 +1136,7 @@ mod test_contract {
}

//! > expected_diagnostics
warning: Plugin diagnostic: The path `component2_storage.data` collides with existing path `component1_storage.data`.
warning: Plugin diagnostic: The path `component2_storage.data` collides with existing path `component1_storage.data`. Fix or add `#[allow(starknet::colliding_storage_paths)]` if intentional.
--> lib.cairo:26:9
component2_storage: super::component2::Storage,
^****************^
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3601,7 +3601,7 @@ Note: currently with components, only an enum Event directly in the contract is
component!(path: super::component2, storage: component2_storage, event: Comp2Event);
^********^

warning: Plugin diagnostic: The path `component2_storage.data` collides with existing path `component1_storage.data`.
warning: Plugin diagnostic: The path `component2_storage.data` collides with existing path `component1_storage.data`. Fix or add `#[allow(starknet::colliding_storage_paths)]` if intentional.
--> lib.cairo:27:9
component2_storage: super::component2::Storage,
^****************^
Expand Down
56 changes: 49 additions & 7 deletions crates/cairo-lang-starknet/src/test_data/storage_path_check
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ struct Storage {
//! > expected_diagnostics

//! > diagnostics
warning: Plugin diagnostic: The path `has_a.a` collides with existing path `a`.
warning: Plugin diagnostic: The path `has_a.a` collides with existing path `a`. Fix or add `#[allow(starknet::colliding_storage_paths)]` if intentional.
--> lib.cairo:10:5
has_a: HasA,
^***^

warning: Plugin diagnostic: The path `has_a.a` collides with existing path `a`.
warning: Plugin diagnostic: The path `has_a.a` collides with existing path `a`. Fix or add `#[allow(starknet::colliding_storage_paths)]` if intentional.
--> lib.cairo:17:9
pub has_a: HasA,
^***^
Expand Down Expand Up @@ -75,7 +75,7 @@ mod contract_with_and_without_ignored {
//! > expected_diagnostics

//! > diagnostics
warning: Plugin diagnostic: The path `y.member0` collides with existing path `x.ignored_member.member0`.
warning: Plugin diagnostic: The path `y.member0` collides with existing path `x.ignored_member.member0`. Fix or add `#[allow(starknet::colliding_storage_paths)]` if intentional.
--> lib.cairo:25:13
pub y: Struct0,
^
Expand Down Expand Up @@ -115,12 +115,12 @@ mod component_with_and_without_ignored {
//! > expected_diagnostics

//! > diagnostics
warning: Plugin diagnostic: The path `has_a.a` collides with existing path `a`.
warning: Plugin diagnostic: The path `has_a.a` collides with existing path `a`. Fix or add `#[allow(starknet::colliding_storage_paths)]` if intentional.
--> lib.cairo:19:13
pub has_a: HasA,
^***^

warning: Plugin diagnostic: The path `has_has_a.has_a.a` collides with existing path `a`.
warning: Plugin diagnostic: The path `has_has_a.has_a.a` collides with existing path `a`. Fix or add `#[allow(starknet::colliding_storage_paths)]` if intentional.
--> lib.cairo:21:13
pub has_has_a: HasHasA,
^*******^
Expand Down Expand Up @@ -174,12 +174,54 @@ mod contract_with_component {
//! > expected_diagnostics

//! > diagnostics
warning: Plugin diagnostic: The path `y.ignored_member.member0` collides with existing path `x.member0`.
warning: Plugin diagnostic: The path `y.ignored_member.member0` collides with existing path `x.member0`. Fix or add `#[allow(starknet::colliding_storage_paths)]` if intentional.
--> lib.cairo:25:13
pub y: IgnoredMemberStruct,
^

warning: Plugin diagnostic: The path `member.y.ignored_member.member0` collides with existing path `member.x.member0`.
warning: Plugin diagnostic: The path `member.y.ignored_member.member0` collides with existing path `member.x.member0`. Fix or add `#[allow(starknet::colliding_storage_paths)]` if intentional.
--> lib.cairo:35:9
member: component::Storage,
^****^

//! > ==========================================================================

//! > Test ignored colliding paths check.

//! > test_runner_name
test_storage_path_check(expect_diagnostics: false)

//! > cairo_code
#[starknet::storage_node]
struct SingleMember {
pub value: felt252,
}

#[starknet::storage_node]
struct IgnoredCollision1 {
#[flat]
#[allow(starknet::colliding_storage_paths)]
pub a: SingleMember,
#[flat]
pub b: SingleMember,
}

#[starknet::storage_node]
struct IgnoredCollision2 {
#[flat]
pub a: SingleMember,
#[flat]
#[allow(starknet::colliding_storage_paths)]
pub b: SingleMember,
}

#[starknet::storage_node]
#[allow(starknet::colliding_storage_paths)]
struct IgnoredCollision3 {
#[flat]
pub a: SingleMember,
#[flat]
pub b: SingleMember,
}

//! > diagnostics

0 comments on commit 0f12366

Please sign in to comment.