Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added option to allow conflicting paths. #6743

Merged
merged 1 commit into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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