diff --git a/lang/syn/src/parser/context.rs b/lang/syn/src/parser/context.rs index 0ffb487984..3372acf9ec 100644 --- a/lang/syn/src/parser/context.rs +++ b/lang/syn/src/parser/context.rs @@ -52,34 +52,92 @@ impl CrateContext { pub fn safety_checks(&self) -> Result<()> { // Check all structs for unsafe field types, i.e. AccountInfo and UncheckedAccount. for ctx in self.modules.values() { - for unsafe_field in ctx.unsafe_struct_fields() { - // Check if unsafe field type has been documented with a /// SAFETY: doc string. - let is_documented = unsafe_field.attrs.iter().any(|attr| { - attr.tokens.clone().into_iter().any(|token| match token { - // Check for doc comments containing CHECK - proc_macro2::TokenTree::Literal(s) => s.to_string().contains("CHECK"), - _ => false, - }) + for struct_item in ctx.structs() { + // Only check structs with #[derive(Accounts)] + let has_accounts_derive = struct_item.attrs.iter().any(|attr| { + match attr.parse_meta() { + Ok(syn::Meta::List(syn::MetaList{path, nested, ..})) => { + path.is_ident("derive") && nested.iter().any(|nested| { + matches!(nested, syn::NestedMeta::Meta(syn::Meta::Path(path)) if path.is_ident("Accounts")) + }) + } + _ => false + } }); - if !is_documented { - let ident = unsafe_field.ident.as_ref().unwrap(); - let span = ident.span(); - // Error if undocumented. - return Err(anyhow!( - r#" + + if has_accounts_derive { + // Check for duplicate field names within the same struct + let mut field_names = std::collections::HashSet::new(); + for field in &struct_item.fields { + if let Some(ident) = &field.ident { + let field_name = ident.to_string(); + if !field_names.insert(field_name.clone()) { + let span = ident.span(); + return Err(anyhow!( + r#" {}:{}:{} - Struct field "{}" is unsafe, but is not documented. + Duplicate field name "{}" found in struct "{}". + Each field in an accounts struct must have a unique name. + Please rename one of the duplicate fields to resolve this conflict. + "#, + ctx.file.canonicalize().unwrap().display(), + span.start().line, + span.start().column, + field_name, + struct_item.ident.to_string() + )); + } + } + } + + for field in &struct_item.fields { + // Check if this field is an unsafe type + let is_unsafe_type = match &field.ty { + syn::Type::Path(syn::TypePath { + path: syn::Path { segments, .. }, + .. + }) => { + segments.len() == 1 && segments[0].ident == "UncheckedAccount" + || segments[0].ident == "AccountInfo" + } + _ => false, + }; + + if is_unsafe_type { + // Check if unsafe field type has been documented with a /// CHECK: doc string. + let is_documented = field.attrs.iter().any(|attr| { + attr.tokens.clone().into_iter().any(|token| match token { + // Check for doc comments containing CHECK + proc_macro2::TokenTree::Literal(s) => { + s.to_string().contains("CHECK") + } + _ => false, + }) + }); + + if !is_documented { + let ident = field.ident.as_ref().unwrap(); + let span = ident.span(); + // Error if undocumented. + return Err(anyhow!( + r#" + {}:{}:{} + Struct field "{}" in struct "{}" is unsafe, but is not documented. Please add a `/// CHECK:` doc comment explaining why no checks through types are necessary. Alternatively, for reasons like quick prototyping, you may disable the safety checks by using the `skip-lint` option. See https://www.anchor-lang.com/docs/the-accounts-struct#safety-checks for more information. "#, - ctx.file.canonicalize().unwrap().display(), - span.start().line, - span.start().column, - ident.to_string() - )); - }; + ctx.file.canonicalize().unwrap().display(), + span.start().line, + span.start().column, + ident.to_string(), + struct_item.ident.to_string() + )); + } + } + } + } } } Ok(()) @@ -221,35 +279,6 @@ impl ParsedModule { }) } - fn unsafe_struct_fields(&self) -> impl Iterator { - let accounts_filter = |item_struct: &&syn::ItemStruct| { - item_struct.attrs.iter().any(|attr| { - match attr.parse_meta() { - Ok(syn::Meta::List(syn::MetaList{path, nested, ..})) => { - path.is_ident("derive") && nested.iter().any(|nested| { - matches!(nested, syn::NestedMeta::Meta(syn::Meta::Path(path)) if path.is_ident("Accounts")) - }) - } - _ => false - } - }) - }; - - self.structs() - .filter(accounts_filter) - .flat_map(|s| &s.fields) - .filter(|f| match &f.ty { - syn::Type::Path(syn::TypePath { - path: syn::Path { segments, .. }, - .. - }) => { - segments.len() == 1 && segments[0].ident == "UncheckedAccount" - || segments[0].ident == "AccountInfo" - } - _ => false, - }) - } - fn enums(&self) -> impl Iterator { self.items.iter().filter_map(|i| match i { syn::Item::Enum(item) => Some(item), diff --git a/tests/safety-checks/programs/duplicate-names/Cargo.toml b/tests/safety-checks/programs/duplicate-names/Cargo.toml new file mode 100644 index 0000000000..0df4748b79 --- /dev/null +++ b/tests/safety-checks/programs/duplicate-names/Cargo.toml @@ -0,0 +1,20 @@ +[package] +name = "duplicate-names" +version = "0.1.0" +description = "Created with Anchor" +edition = "2021" + +[lib] +crate-type = ["cdylib", "lib"] +name = "duplicate_names" + +[features] +no-entrypoint = [] +no-idl = [] +no-log-ix-name = [] +cpi = ["no-entrypoint"] +default = [] +idl-build = ["anchor-lang/idl-build"] + +[dependencies] +anchor-lang = { path = "../../../../lang", features = ["init-if-needed"] } diff --git a/tests/safety-checks/programs/duplicate-names/Xargo.toml b/tests/safety-checks/programs/duplicate-names/Xargo.toml new file mode 100644 index 0000000000..f95d0c5b3c --- /dev/null +++ b/tests/safety-checks/programs/duplicate-names/Xargo.toml @@ -0,0 +1,6 @@ +[target.bpf] +linker = "rust-lld" +rustflags = [ + "-C", "link-arg=--import-memory", + "-C", "link-arg=--export-table", +] diff --git a/tests/safety-checks/programs/duplicate-names/src/lib.rs b/tests/safety-checks/programs/duplicate-names/src/lib.rs new file mode 100644 index 0000000000..8b5fbd1c6c --- /dev/null +++ b/tests/safety-checks/programs/duplicate-names/src/lib.rs @@ -0,0 +1,25 @@ +use anchor_lang::prelude::*; + +declare_id!("Fg6PaFpoGXkYsidMpWTK6W2BeZ7FEfcYkg476zPFsLnS"); + +#[program] +pub mod duplicate_names { + use super::*; + pub fn func_one(ctx: Context) -> Result<()> { + Ok(()) + } + + pub fn func_two(ctx: Context) -> Result<()> { + Ok(()) + } +} + +#[derive(Accounts)] +pub struct FuncOne<'info> { + my_account: UncheckedAccount<'info>, +} + +#[derive(Accounts)] +pub struct FuncTwo<'info> { + my_account: UncheckedAccount<'info>, +} diff --git a/tests/safety-checks/test.sh b/tests/safety-checks/test.sh index 872d6c1f25..37ddc0733d 100755 --- a/tests/safety-checks/test.sh +++ b/tests/safety-checks/test.sh @@ -7,7 +7,7 @@ echo "Building programs" # pushd programs/unchecked-account/ output=$(anchor build 2>&1 > /dev/null) -if ! [[ $output =~ "Struct field \"unchecked\" is unsafe" ]]; then +if ! [[ $output =~ "Struct field \"unchecked\" in struct \"Initialize\" is unsafe" ]]; then echo "Error: expected /// CHECK error" exit 1 fi @@ -18,12 +18,23 @@ popd # pushd programs/account-info/ output=$(anchor build 2>&1 > /dev/null) -if ! [[ $output =~ "Struct field \"unchecked\" is unsafe" ]]; then +if ! [[ $output =~ "Struct field \"unchecked\" in struct \"Initialize\" is unsafe" ]]; then echo "Error: expected /// CHECK error" exit 1 fi popd +# +# Build the duplicate-names variant. +# +pushd programs/duplicate-names/ +output=$(anchor build 2>&1 > /dev/null) +if ! [[ $output =~ "Struct field \"my_account\" in struct \"FuncOne\" is unsafe" ]]; then + echo "Error: expected /// CHECK error for duplicate-names" + exit 1 +fi +popd + # # Build the control variant. #