Skip to content

Conversation

@blockiosaurus
Copy link
Contributor

This PR adds a ShankAccounts macro to give the option of sourcing Shank's IDL generation from a struct of AccountInfos rather than the current syntax which can make Instruction enums overly long. The account struct also has an autogenerated context() function which will turn a slice of AccountInfos into the struct.

#[derive(ShankAccounts)]
pub struct CreateVaultAccounts<'info> {
    #[account(mut, signer, desc = "The payer and authority")]
    pub payer: &'info AccountInfo<'info>,

    #[account(mut, desc = "The vault account to create")]
    pub vault: &'info AccountInfo<'info>,

    #[account(optional, desc = "Optional new authority")]
    pub optional_authority: Option<&'info AccountInfo<'info>>,

    #[account(desc = "System program")]
    pub system_program: &'info AccountInfo<'info>,
}

@coderabbitai
Copy link

coderabbitai bot commented Sep 19, 2025

Summary by CodeRabbit

  • New Features

    • Introduces ShankAccounts derive for defining account structs and using them in instructions via #[accounts(...)], with optional and optional signer support.
    • IDL generation now expands accounts from structs, including flags (mut, signer, optional).
    • Adds a lightweight Context wrapper and auto-generated context() for type-safe account access and remaining accounts.
    • Optional Solana integration via a feature flag.
  • Documentation

    • New guides and examples demonstrating ShankAccounts, optional accounts, context usage, and Anchor-style patterns.
  • Tests

    • Extensive fixtures and integration tests covering struct-based accounts, IDL output, and context behavior.

Walkthrough

Introduces a new ShankAccounts derive macro and end-to-end integration: macro parsing/generation, IDL extraction/expansion for struct-based accounts, a generic Context type, feature-gated Solana support, examples, and comprehensive tests. IDL generation now resolves #[accounts(StructName)] by expanding fields, with fallback to placeholder when unresolved.

Changes

Cohort / File(s) Summary
Docs
EXAMPLE_USAGE.md, shank-idl/ACCOUNTS_STRUCT_IDL.md
Adds usage and IDL behavior docs for ShankAccounts, optional accounts, and struct-based account expansion vs placeholder handling.
IDL core integration
shank-idl/src/accounts_extraction.rs, shank-idl/src/file.rs, shank-idl/src/idl_instruction.rs, shank-idl/src/lib.rs
Adds AST-based extraction of ShankAccounts, wires accounts map into instruction-to-IDL conversion, introduces accounts-aware conversion paths and expansion logic with fallback, and registers the new module.
Macro implementation: Accounts derive
shank-macro-impl/src/accounts/account_field.rs, shank-macro-impl/src/accounts/account_struct.rs, shank-macro-impl/src/accounts/mod.rs, shank-macro-impl/src/lib.rs
Implements parsing of #[account(...)] on struct fields, aggregates into account structs, generates __shank_accounts() and, when lifetimes present, a context() constructor; exposes the Accounts module and derive marker.
Macro implementation: Instruction/attributes
shank-macro-impl/src/instruction/account_attrs.rs, shank-macro-impl/src/instruction/instruction.rs
Adds AccountsSource (Inline vs Struct), expands InstructionAccount metadata, parses #[accounts(StructName)], prevents mixing with inline accounts, and carries source through InstructionVariant.
Macro crate API
shank-macro/src/accounts.rs, shank-macro/src/lib.rs
Adds derive path and public ShankAccounts macro, updates ShankInstruction to recognize accounts attribute.
Core crate: Context and traits
shank/src/context.rs, shank/src/lib.rs
Adds generic Context<'a, T, U=()> and AccountInfoRef trait; re-exports Context.
Workspace config and feature flag
shank/Cargo.toml
Adds optional solana-program dependency with feature flag; adds macrotest dev-dependency and features block.
Examples
shank/examples/accounts_struct_example.rs, shank/examples/anchor_style_example.rs, shank/examples/context_usage_example.rs, shank/examples/full_solana_example.rs
Demonstrates ShankAccounts with struct-based accounts, optional accounts, context() usage, and mixed old/new styles; includes feature-gated Solana paths and mocks when disabled.
IDL fixtures (single-file)
shank-idl/tests/fixtures/instructions/single_file/*.rs, .../*.json
Adds Rust and JSON fixtures covering simple, standard, and complex accounts-struct scenarios for IDL generation validation.
IDL tests
shank-idl/tests/instructions.rs, shank-idl/tests/generate_expected_idl.rs, shank-idl/tests/test_shank_accounts_expansion.rs
Adds tests to compare parsed IDL against fixtures; includes ignored generators and an expansion check ensuring struct accounts expand to individual items.
Shank macro expansion tests
shank/tests/expand_tests.rs, shank/tests/expand/*
Uses macrotest to snapshot expanded output for basic, optional, complex, no-constraints, custom lifetime, and empty struct cases.
Shank context and IDL unit tests
shank/tests/*context*, shank/tests/*accounts*, shank/tests/*idl*, shank/tests/instruction_*
Adds broad test coverage for __shank_accounts() metadata, context() construction, optional handling, integration with instructions, error handling, and cross-feature compilation with/without solana-program.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Parser as Rust Parser
  participant Extract as extract_shank_accounts_structs
  participant Map as Accounts Map (HashMap)
  participant Enum as Instruction Enum Parser
  participant IDL as IdlInstruction::try_from_with_accounts

  Parser->>Extract: Iterate ItemStructs
  Extract->>Map: Insert StructName -> Vec<InstructionAccount>
  Parser->>Enum: Parse InstructionVariant + attributes
  Enum->>IDL: Try convert variant with accounts_map
  alt Inline accounts
    IDL->>IDL: Map inline accounts to IdlAccountItems
  else Struct accounts
    IDL->>Map: Lookup StructName
    alt Found
      Map-->>IDL: Vec<InstructionAccount>
      IDL->>IDL: Expand to IdlAccountItems
    else Not found
      IDL->>IDL: Emit placeholder account item
    end
  end
  IDL-->>Parser: IdlInstruction(s)
Loading
sequenceDiagram
  autonumber
  participant Caller as Program Logic
  participant Gen as ShankAccounts::context()
  participant Ctx as Context<'a, T, U>
  note over Gen: Generated by ShankAccounts derive

  Caller->>Gen: context(&[AccountInfo])
  Gen->>Gen: Validate min accounts length
  Gen->>Gen: For Option<&AccountInfo>: if key == crate::ID then None else Some(&acc[i])
  Gen->>Ctx: Construct Context { accounts: T, remaining_accounts }
  Ctx-->>Caller: Typed access (ctx.accounts), remaining_accounts
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Suggested reviewers

  • pileks
  • danenbm

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title "Adding ShankAccounts Macro" is a concise, single-sentence summary that directly reflects the primary change in the patch (introduction of a ShankAccounts derive macro, associated context()/IDL generation, and related integration). It is specific, on-topic, and readable for a teammate scanning PR history.
Description Check ✅ Passed The PR description explains the intent (derive macro to source IDL from an account struct), includes a representative code example, and matches the changes shown in the diff (generated context(), optional account handling, and IDL implications), so it is directly related to the changeset. The level of detail is adequate for this check and not off-topic.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/shank-accounts

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 81

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
shank-idl/src/idl_instruction.rs (1)

264-273: Same off-by-one in the accounts-aware path.

Mirror the fix in try_from_with_accounts.

-        ensure!(
-            discriminant < u8::MAX as usize,
+        ensure!(
+            discriminant <= u8::MAX as usize,
             anyhow!(
                 "Instruction variant discriminants have to be <= u8::MAX ({}), \
                     but the discriminant of variant '{}' is {}",
                 u8::MAX,
                 ident,
                 discriminant
             )
         );
shank-macro-impl/src/instruction/account_attrs.rs (1)

222-249: Index-position validation is good; consider duplicate-name detection.

Multiple inline accounts with the same name should error early to avoid confusing IDL.

If desired, I can add a small pass to check for duplicate InstructionAccount.name values and return a ParseError.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5e1caff and 10aa392.

📒 Files selected for processing (61)
  • EXAMPLE_USAGE.md (1 hunks)
  • shank-idl/ACCOUNTS_STRUCT_IDL.md (1 hunks)
  • shank-idl/src/accounts_extraction.rs (1 hunks)
  • shank-idl/src/file.rs (3 hunks)
  • shank-idl/src/idl_instruction.rs (4 hunks)
  • shank-idl/src/lib.rs (1 hunks)
  • shank-idl/tests/fixtures/instructions/single_file/instruction_with_accounts_struct.json (1 hunks)
  • shank-idl/tests/fixtures/instructions/single_file/instruction_with_accounts_struct.rs (1 hunks)
  • shank-idl/tests/fixtures/instructions/single_file/instruction_with_complex_accounts_struct.json (1 hunks)
  • shank-idl/tests/fixtures/instructions/single_file/instruction_with_complex_accounts_struct.rs (1 hunks)
  • shank-idl/tests/fixtures/instructions/single_file/instruction_with_simple_accounts_struct.json (1 hunks)
  • shank-idl/tests/fixtures/instructions/single_file/instruction_with_simple_accounts_struct.rs (1 hunks)
  • shank-idl/tests/generate_expected_idl.rs (1 hunks)
  • shank-idl/tests/instructions.rs (1 hunks)
  • shank-idl/tests/test_shank_accounts_expansion.rs (1 hunks)
  • shank-macro-impl/src/accounts/account_field.rs (1 hunks)
  • shank-macro-impl/src/accounts/account_struct.rs (1 hunks)
  • shank-macro-impl/src/accounts/mod.rs (1 hunks)
  • shank-macro-impl/src/instruction/account_attrs.rs (3 hunks)
  • shank-macro-impl/src/instruction/instruction.rs (3 hunks)
  • shank-macro-impl/src/lib.rs (2 hunks)
  • shank-macro/src/accounts.rs (1 hunks)
  • shank-macro/src/lib.rs (4 hunks)
  • shank/Cargo.toml (1 hunks)
  • shank/examples/accounts_struct_example.rs (1 hunks)
  • shank/examples/anchor_style_example.rs (1 hunks)
  • shank/examples/context_usage_example.rs (1 hunks)
  • shank/examples/full_solana_example.rs (1 hunks)
  • shank/src/context.rs (1 hunks)
  • shank/src/lib.rs (1 hunks)
  • shank/tests/accounts_macro_test.rs (1 hunks)
  • shank/tests/basic_context_test.rs (1 hunks)
  • shank/tests/context_api_test.rs (1 hunks)
  • shank/tests/context_api_working_test.rs (1 hunks)
  • shank/tests/context_generation_test.rs (1 hunks)
  • shank/tests/context_issue_demo.rs (1 hunks)
  • shank/tests/context_method_demo.rs (1 hunks)
  • shank/tests/context_method_test.rs (1 hunks)
  • shank/tests/context_runtime_test.rs (1 hunks)
  • shank/tests/context_usage_test.rs (1 hunks)
  • shank/tests/enhanced_accounts_test.rs (1 hunks)
  • shank/tests/error_handling_test.rs (1 hunks)
  • shank/tests/expand/basic_accounts.expanded.rs (1 hunks)
  • shank/tests/expand/basic_accounts.rs (1 hunks)
  • shank/tests/expand/complex_constraints.expanded.rs (1 hunks)
  • shank/tests/expand/complex_constraints.rs (1 hunks)
  • shank/tests/expand/custom_lifetime.expanded.rs (1 hunks)
  • shank/tests/expand/custom_lifetime.rs (1 hunks)
  • shank/tests/expand/empty_struct.expanded.rs (1 hunks)
  • shank/tests/expand/empty_struct.rs (1 hunks)
  • shank/tests/expand/no_constraints.expanded.rs (1 hunks)
  • shank/tests/expand/no_constraints.rs (1 hunks)
  • shank/tests/expand/optional_accounts.expanded.rs (1 hunks)
  • shank/tests/expand/optional_accounts.rs (1 hunks)
  • shank/tests/expand_tests.rs (1 hunks)
  • shank/tests/final_context_test.rs (1 hunks)
  • shank/tests/idl_generation_test.rs (1 hunks)
  • shank/tests/instruction_integration_test.rs (1 hunks)
  • shank/tests/instruction_with_accounts_struct_test.rs (1 hunks)
  • shank/tests/simple_context_test.rs (1 hunks)
  • shank/tests/working_context_demo.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/tests/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place test files and fixtures under each crate’s tests/ directory

Files:

  • shank-idl/tests/fixtures/instructions/single_file/instruction_with_accounts_struct.json
  • shank/tests/expand_tests.rs
  • shank-idl/tests/fixtures/instructions/single_file/instruction_with_complex_accounts_struct.json
  • shank/tests/expand/empty_struct.rs
  • shank-idl/tests/fixtures/instructions/single_file/instruction_with_simple_accounts_struct.json
  • shank-idl/tests/generate_expected_idl.rs
  • shank/tests/expand/empty_struct.expanded.rs
  • shank/tests/expand/basic_accounts.rs
  • shank-idl/tests/test_shank_accounts_expansion.rs
  • shank/tests/working_context_demo.rs
  • shank-idl/tests/instructions.rs
  • shank/tests/expand/complex_constraints.rs
  • shank/tests/basic_context_test.rs
  • shank/tests/simple_context_test.rs
  • shank/tests/expand/custom_lifetime.rs
  • shank/tests/context_method_demo.rs
  • shank/tests/instruction_with_accounts_struct_test.rs
  • shank/tests/expand/basic_accounts.expanded.rs
  • shank/tests/final_context_test.rs
  • shank/tests/expand/optional_accounts.rs
  • shank/tests/expand/complex_constraints.expanded.rs
  • shank/tests/idl_generation_test.rs
  • shank-idl/tests/fixtures/instructions/single_file/instruction_with_accounts_struct.rs
  • shank/tests/expand/no_constraints.rs
  • shank/tests/context_runtime_test.rs
  • shank/tests/error_handling_test.rs
  • shank/tests/context_api_working_test.rs
  • shank/tests/instruction_integration_test.rs
  • shank/tests/accounts_macro_test.rs
  • shank/tests/context_method_test.rs
  • shank/tests/expand/optional_accounts.expanded.rs
  • shank/tests/expand/custom_lifetime.expanded.rs
  • shank/tests/context_generation_test.rs
  • shank/tests/context_api_test.rs
  • shank/tests/context_issue_demo.rs
  • shank/tests/context_usage_test.rs
  • shank-idl/tests/fixtures/instructions/single_file/instruction_with_complex_accounts_struct.rs
  • shank/tests/enhanced_accounts_test.rs
  • shank-idl/tests/fixtures/instructions/single_file/instruction_with_simple_accounts_struct.rs
  • shank/tests/expand/no_constraints.expanded.rs
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Format all Rust source files according to rustfmt.toml
Use #[derive(ShankAccount)] to mark account structs; optionally annotate with #[seeds] for PDA generation
Use #[derive(ShankInstruction)] to mark instruction enums; use #[account] attributes on fields as needed
Use #[derive(ShankType)] to mark custom types for IDL inclusion
Use #[derive(ShankBuilder)] to generate instruction builders
Use #[derive(ShankContext)] to generate account context structs
Use #[padding] on fields to mark padding in the generated IDL
Use #[idl_type("TypeName")] to override a field’s IDL type
Use #[idl_name("name")] to rename a field in the IDL while keeping the Rust field name
Use #[skip] on fields to exclude them from the generated IDL

Files:

  • shank/tests/expand_tests.rs
  • shank-macro-impl/src/lib.rs
  • shank/tests/expand/empty_struct.rs
  • shank/src/context.rs
  • shank/src/lib.rs
  • shank-macro/src/accounts.rs
  • shank-idl/tests/generate_expected_idl.rs
  • shank-macro-impl/src/accounts/account_struct.rs
  • shank/tests/expand/empty_struct.expanded.rs
  • shank/tests/expand/basic_accounts.rs
  • shank-idl/src/lib.rs
  • shank-idl/tests/test_shank_accounts_expansion.rs
  • shank-idl/src/accounts_extraction.rs
  • shank/tests/working_context_demo.rs
  • shank-idl/tests/instructions.rs
  • shank/examples/accounts_struct_example.rs
  • shank/tests/expand/complex_constraints.rs
  • shank/examples/context_usage_example.rs
  • shank/tests/basic_context_test.rs
  • shank/tests/simple_context_test.rs
  • shank/tests/expand/custom_lifetime.rs
  • shank/tests/context_method_demo.rs
  • shank/tests/instruction_with_accounts_struct_test.rs
  • shank/tests/expand/basic_accounts.expanded.rs
  • shank-macro-impl/src/accounts/mod.rs
  • shank/tests/final_context_test.rs
  • shank/tests/expand/optional_accounts.rs
  • shank-macro-impl/src/instruction/instruction.rs
  • shank/tests/expand/complex_constraints.expanded.rs
  • shank/tests/idl_generation_test.rs
  • shank-idl/tests/fixtures/instructions/single_file/instruction_with_accounts_struct.rs
  • shank/tests/expand/no_constraints.rs
  • shank/tests/context_runtime_test.rs
  • shank/tests/error_handling_test.rs
  • shank-macro-impl/src/accounts/account_field.rs
  • shank/tests/context_api_working_test.rs
  • shank/tests/instruction_integration_test.rs
  • shank-idl/src/file.rs
  • shank/tests/accounts_macro_test.rs
  • shank/tests/context_method_test.rs
  • shank/tests/expand/optional_accounts.expanded.rs
  • shank/tests/expand/custom_lifetime.expanded.rs
  • shank-idl/src/idl_instruction.rs
  • shank/tests/context_generation_test.rs
  • shank/tests/context_api_test.rs
  • shank/tests/context_issue_demo.rs
  • shank/tests/context_usage_test.rs
  • shank-idl/tests/fixtures/instructions/single_file/instruction_with_complex_accounts_struct.rs
  • shank/examples/anchor_style_example.rs
  • shank/tests/enhanced_accounts_test.rs
  • shank-idl/tests/fixtures/instructions/single_file/instruction_with_simple_accounts_struct.rs
  • shank-macro-impl/src/instruction/account_attrs.rs
  • shank/examples/full_solana_example.rs
  • shank-macro/src/lib.rs
  • shank/tests/expand/no_constraints.expanded.rs
**/Cargo.toml

📄 CodeRabbit inference engine (CLAUDE.md)

Set Rust edition to 2018 in all crates (edition = "2018" in Cargo.toml)

Files:

  • shank/Cargo.toml
🧬 Code graph analysis (49)
shank-macro-impl/src/lib.rs (1)
shank-idl/src/file.rs (1)
  • accounts (99-108)
shank/src/lib.rs (1)
shank/tests/context_api_working_test.rs (1)
  • key (14-16)
shank-macro/src/accounts.rs (1)
shank-macro-impl/src/accounts/mod.rs (1)
  • try_from (21-48)
shank-idl/tests/generate_expected_idl.rs (2)
shank-idl/src/file.rs (2)
  • parse_file (64-97)
  • optional_program_address (51-56)
shank-idl/src/idl.rs (1)
  • try_into_json (64-67)
shank-macro-impl/src/accounts/account_struct.rs (1)
shank-macro-impl/src/accounts/account_field.rs (1)
  • from_field (15-76)
shank/tests/expand/empty_struct.expanded.rs (5)
shank/tests/expand/basic_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/expand/complex_constraints.expanded.rs (1)
  • __shank_accounts (23-85)
shank/tests/expand/custom_lifetime.expanded.rs (1)
  • __shank_accounts (15-41)
shank/tests/expand/no_constraints.expanded.rs (1)
  • __shank_accounts (14-25)
shank/tests/expand/optional_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/expand/basic_accounts.rs (2)
shank/examples/accounts_struct_example.rs (1)
  • main (103-150)
shank/examples/anchor_style_example.rs (2)
  • main (113-127)
  • main (130-132)
shank-idl/tests/test_shank_accounts_expansion.rs (2)
shank-idl/src/lib.rs (2)
  • extract_idl (44-63)
  • default (32-38)
shank-idl/src/file.rs (3)
  • default (39-47)
  • instructions (110-130)
  • accounts (99-108)
shank-idl/src/accounts_extraction.rs (3)
shank-macro-impl/src/parsers/attrs.rs (1)
  • get_derive_attr (74-79)
shank-idl/src/file.rs (1)
  • accounts (99-108)
shank-idl/src/idl_field.rs (1)
  • field (35-39)
shank/tests/working_context_demo.rs (3)
shank/examples/accounts_struct_example.rs (1)
  • main (103-150)
shank/examples/context_usage_example.rs (2)
  • main (95-113)
  • main (116-119)
shank/tests/expand/basic_accounts.expanded.rs (2)
  • main (54-54)
  • __shank_accounts (17-52)
shank-idl/tests/instructions.rs (1)
shank-idl/src/file.rs (2)
  • parse_file (64-97)
  • optional_program_address (51-56)
shank/examples/accounts_struct_example.rs (4)
shank-idl/src/file.rs (1)
  • accounts (99-108)
shank/examples/anchor_style_example.rs (2)
  • main (113-127)
  • main (130-132)
shank/examples/context_usage_example.rs (2)
  • main (95-113)
  • main (116-119)
shank/examples/full_solana_example.rs (2)
  • main (131-142)
  • main (145-159)
shank/tests/expand/complex_constraints.rs (2)
shank/examples/accounts_struct_example.rs (1)
  • main (103-150)
shank/examples/anchor_style_example.rs (2)
  • main (113-127)
  • main (130-132)
shank/examples/context_usage_example.rs (3)
shank/examples/accounts_struct_example.rs (1)
  • main (103-150)
shank/examples/anchor_style_example.rs (2)
  • main (113-127)
  • main (130-132)
shank/examples/full_solana_example.rs (2)
  • main (131-142)
  • main (145-159)
shank/tests/basic_context_test.rs (2)
shank/src/lib.rs (1)
  • key (10-10)
shank/tests/context_api_working_test.rs (1)
  • key (14-16)
shank/tests/simple_context_test.rs (1)
shank/tests/expand/basic_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/expand/custom_lifetime.rs (3)
shank/examples/accounts_struct_example.rs (1)
  • main (103-150)
shank/examples/anchor_style_example.rs (2)
  • main (113-127)
  • main (130-132)
shank/examples/full_solana_example.rs (2)
  • main (131-142)
  • main (145-159)
shank/tests/context_method_demo.rs (1)
shank/tests/expand/basic_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/instruction_with_accounts_struct_test.rs (1)
shank-idl/src/file.rs (1)
  • accounts (99-108)
shank/tests/expand/basic_accounts.expanded.rs (5)
shank/tests/expand/complex_constraints.expanded.rs (1)
  • __shank_accounts (23-85)
shank/tests/expand/custom_lifetime.expanded.rs (1)
  • __shank_accounts (15-41)
shank/tests/expand/empty_struct.expanded.rs (1)
  • __shank_accounts (5-9)
shank/tests/expand/no_constraints.expanded.rs (1)
  • __shank_accounts (14-25)
shank/tests/expand/optional_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank-macro-impl/src/accounts/mod.rs (3)
shank-macro-impl/src/accounts/account_struct.rs (1)
  • from_fields (12-23)
shank/src/lib.rs (1)
  • key (10-10)
shank/tests/context_api_working_test.rs (1)
  • key (14-16)
shank/tests/final_context_test.rs (2)
shank/src/lib.rs (1)
  • key (10-10)
shank/tests/context_api_working_test.rs (1)
  • key (14-16)
shank/tests/expand/optional_accounts.rs (3)
shank/examples/accounts_struct_example.rs (1)
  • main (103-150)
shank/examples/anchor_style_example.rs (2)
  • main (113-127)
  • main (130-132)
shank/examples/full_solana_example.rs (2)
  • main (131-142)
  • main (145-159)
shank-macro-impl/src/instruction/instruction.rs (2)
shank-idl/src/idl_instruction.rs (2)
  • try_from (22-29)
  • try_from (67-165)
shank-macro-impl/src/instruction/account_attrs.rs (3)
  • try_from (193-219)
  • try_from (225-248)
  • attrs (226-230)
shank/tests/expand/complex_constraints.expanded.rs (4)
shank/tests/expand/basic_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/expand/custom_lifetime.expanded.rs (1)
  • __shank_accounts (15-41)
shank/tests/expand/no_constraints.expanded.rs (1)
  • __shank_accounts (14-25)
shank/tests/expand/optional_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/idl_generation_test.rs (6)
shank/tests/expand/basic_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/expand/complex_constraints.expanded.rs (1)
  • __shank_accounts (23-85)
shank/tests/expand/custom_lifetime.expanded.rs (1)
  • __shank_accounts (15-41)
shank/tests/expand/empty_struct.expanded.rs (1)
  • __shank_accounts (5-9)
shank/tests/expand/no_constraints.expanded.rs (1)
  • __shank_accounts (14-25)
shank/tests/expand/optional_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank-idl/tests/fixtures/instructions/single_file/instruction_with_accounts_struct.rs (1)
shank-idl/src/file.rs (1)
  • accounts (99-108)
shank/tests/expand/no_constraints.rs (2)
shank/examples/accounts_struct_example.rs (1)
  • main (103-150)
shank/examples/anchor_style_example.rs (2)
  • main (113-127)
  • main (130-132)
shank/tests/context_runtime_test.rs (6)
shank/tests/expand/basic_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/expand/complex_constraints.expanded.rs (1)
  • __shank_accounts (23-85)
shank/tests/expand/custom_lifetime.expanded.rs (1)
  • __shank_accounts (15-41)
shank/tests/expand/empty_struct.expanded.rs (1)
  • __shank_accounts (5-9)
shank/tests/expand/no_constraints.expanded.rs (1)
  • __shank_accounts (14-25)
shank/tests/expand/optional_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/error_handling_test.rs (7)
shank/tests/expand/basic_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/expand/complex_constraints.expanded.rs (1)
  • __shank_accounts (23-85)
shank/tests/expand/custom_lifetime.expanded.rs (1)
  • __shank_accounts (15-41)
shank/tests/expand/empty_struct.expanded.rs (1)
  • __shank_accounts (5-9)
shank/tests/expand/no_constraints.expanded.rs (1)
  • __shank_accounts (14-25)
shank/tests/expand/optional_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/context_api_test.rs (1)
  • test_different_lifetime_names (83-97)
shank/tests/context_api_working_test.rs (1)
shank/src/lib.rs (1)
  • key (10-10)
shank/tests/instruction_integration_test.rs (6)
shank/tests/expand/basic_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/expand/complex_constraints.expanded.rs (1)
  • __shank_accounts (23-85)
shank/tests/expand/custom_lifetime.expanded.rs (1)
  • __shank_accounts (15-41)
shank/tests/expand/empty_struct.expanded.rs (1)
  • __shank_accounts (5-9)
shank/tests/expand/no_constraints.expanded.rs (1)
  • __shank_accounts (14-25)
shank/tests/expand/optional_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank-idl/src/file.rs (2)
shank-idl/src/accounts_extraction.rs (1)
  • extract_shank_accounts_structs (12-26)
shank-idl/src/idl_instruction.rs (3)
  • try_into_with_accounts (33-43)
  • ix (23-27)
  • ix (37-41)
shank/tests/context_method_test.rs (3)
shank/tests/expand/basic_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/expand/optional_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank-macro-impl/src/parsed_struct/struct_attr.rs (1)
  • len (81-83)
shank/tests/expand/optional_accounts.expanded.rs (5)
shank/tests/expand/basic_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/expand/complex_constraints.expanded.rs (1)
  • __shank_accounts (23-85)
shank/tests/expand/custom_lifetime.expanded.rs (1)
  • __shank_accounts (15-41)
shank/tests/expand/empty_struct.expanded.rs (1)
  • __shank_accounts (5-9)
shank/tests/expand/no_constraints.expanded.rs (1)
  • __shank_accounts (14-25)
shank/tests/expand/custom_lifetime.expanded.rs (3)
shank/tests/expand/basic_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/expand/empty_struct.expanded.rs (1)
  • __shank_accounts (5-9)
shank/tests/expand/no_constraints.expanded.rs (1)
  • __shank_accounts (14-25)
shank-idl/src/idl_instruction.rs (3)
shank-idl/src/file.rs (2)
  • instructions (110-130)
  • accounts (99-108)
shank-macro-impl/src/instruction/account_attrs.rs (2)
  • try_from (193-219)
  • try_from (225-248)
shank-macro-impl/src/instruction/instruction.rs (3)
  • try_from (50-57)
  • try_from (63-76)
  • try_from (101-177)
shank/tests/context_generation_test.rs (6)
shank/tests/expand/basic_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/expand/complex_constraints.expanded.rs (1)
  • __shank_accounts (23-85)
shank/tests/expand/custom_lifetime.expanded.rs (1)
  • __shank_accounts (15-41)
shank/tests/expand/empty_struct.expanded.rs (1)
  • __shank_accounts (5-9)
shank/tests/expand/no_constraints.expanded.rs (1)
  • __shank_accounts (14-25)
shank/tests/expand/optional_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/context_api_test.rs (7)
shank/tests/expand/basic_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/expand/complex_constraints.expanded.rs (1)
  • __shank_accounts (23-85)
shank/tests/expand/custom_lifetime.expanded.rs (1)
  • __shank_accounts (15-41)
shank/tests/expand/empty_struct.expanded.rs (1)
  • __shank_accounts (5-9)
shank/tests/expand/no_constraints.expanded.rs (1)
  • __shank_accounts (14-25)
shank/tests/expand/optional_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/error_handling_test.rs (1)
  • test_different_lifetime_names (175-196)
shank/tests/context_issue_demo.rs (2)
shank/tests/expand/basic_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/expand/optional_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/context_usage_test.rs (4)
shank/src/lib.rs (1)
  • key (10-10)
shank/tests/context_api_working_test.rs (1)
  • key (14-16)
shank/tests/expand/basic_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/expand/complex_constraints.expanded.rs (1)
  • __shank_accounts (23-85)
shank-idl/tests/fixtures/instructions/single_file/instruction_with_complex_accounts_struct.rs (1)
shank-idl/src/file.rs (1)
  • accounts (99-108)
shank/examples/anchor_style_example.rs (4)
shank-idl/src/file.rs (3)
  • accounts (99-108)
  • ctx (147-151)
  • ctx (153-157)
shank/examples/accounts_struct_example.rs (1)
  • main (103-150)
shank/examples/context_usage_example.rs (2)
  • main (95-113)
  • main (116-119)
shank/examples/full_solana_example.rs (2)
  • main (131-142)
  • main (145-159)
shank/tests/enhanced_accounts_test.rs (1)
shank-idl/src/file.rs (1)
  • accounts (99-108)
shank-idl/tests/fixtures/instructions/single_file/instruction_with_simple_accounts_struct.rs (1)
shank-idl/src/file.rs (1)
  • accounts (99-108)
shank-macro-impl/src/instruction/account_attrs.rs (2)
shank-idl/src/idl_instruction.rs (2)
  • try_from (22-29)
  • try_from (67-165)
shank-macro-impl/src/instruction/instruction.rs (3)
  • try_from (50-57)
  • try_from (63-76)
  • try_from (101-177)
shank/examples/full_solana_example.rs (6)
shank-idl/src/file.rs (3)
  • accounts (99-108)
  • ctx (147-151)
  • ctx (153-157)
shank/src/lib.rs (1)
  • key (10-10)
shank/tests/context_api_working_test.rs (2)
  • key (14-16)
  • main (116-118)
shank/examples/accounts_struct_example.rs (1)
  • main (103-150)
shank/examples/anchor_style_example.rs (2)
  • main (113-127)
  • main (130-132)
shank/examples/context_usage_example.rs (2)
  • main (95-113)
  • main (116-119)
shank-macro/src/lib.rs (2)
shank-idl/src/file.rs (1)
  • accounts (99-108)
shank-macro/src/accounts.rs (1)
  • derive_accounts (6-9)
shank/tests/expand/no_constraints.expanded.rs (3)
shank/tests/expand/basic_accounts.expanded.rs (1)
  • __shank_accounts (17-52)
shank/tests/expand/complex_constraints.expanded.rs (1)
  • __shank_accounts (23-85)
shank/tests/expand/empty_struct.expanded.rs (1)
  • __shank_accounts (5-9)
🪛 markdownlint-cli2 (0.18.1)
shank-idl/ACCOUNTS_STRUCT_IDL.md

130-130: Files should end with a single newline character

(MD047, single-trailing-newline)

🔇 Additional comments (68)
shank/Cargo.toml (2)

13-20: Feature-gated Solana dep looks correct.

Optional solana-program dependency and feature mapping via dep:solana-program are set up properly; edition = "2018" complies with guidelines.


13-20: No change required — feature name is consistently hyphenated ("solana-program").
Search results show only cfg(feature = "solana-program") usages; no "solana_program" occurrences found.

shank/tests/context_method_test.rs (1)

29-47: Confirm “optional” flag position (tuple index 5) is stable.

If the tuple shape evolves, this test will silently mis-read fields. Consider asserting the field name too to anchor expectations.

shank/tests/context_method_demo.rs (3)

50-64: Good coverage of IDL flags.

The assertions validate mut, signer, and optional semantics clearly.


93-107: Mirror the same IDL assertions in the no-feature path.

Consistent and clear; keeps behavior aligned across cfgs.


25-48: Unify context() signature in docs/examples to match the macro API

Repo contains mixed examples using:

  • context(&accounts) / context(accounts)
  • context(accounts, program_id)
  • context(program_id, accounts)

Update all comments/examples to the single canonical signature the macro actually generates. Representative locations to fix: shank-macro/src/lib.rs (~lines 345 and 434), EXAMPLE_USAGE.md:34, shank/examples/full_solana_example.rs:87/154, shank/examples/context_usage_example.rs:45, shank/tests/context_usage_test.rs:77/151/225, shank/tests/context_method_demo.rs:35.

shank-idl/tests/fixtures/instructions/single_file/instruction_with_simple_accounts_struct.json (1)

14-17: Confirm expected IDL field casing.

The account "dataAccount" (camelCase) may not match default generation from a Rust field named data_account unless #[idl_name("dataAccount")] is used. Align casing or annotate the Rust source.

shank/tests/expand/empty_struct.rs (1)

3-6: LGTM: empty struct expansion fixture.

Fits macrotest pattern; main() stub is appropriate for expansion.

shank/tests/instruction_with_accounts_struct_test.rs (1)

80-89: No change required — IDL names are camelCase by default. The fixture pair shows Rust field data_account -> IDL "dataAccount" (shank-idl/tests/fixtures/instructions/single_file/instruction_with_simple_accounts_struct.rs and .json); only add #[idl_name("dataAccount")] if you want to override the default conversion.

shank-macro-impl/src/lib.rs (2)

2-2: New accounts module export matches the added derive surface

Good addition. Ensure all internal references use this module path consistently.


17-17: Proc-macro entrypoint wired: ShankAccounts → derive_accounts

Verified — shank-macro/src/lib.rs contains #[proc_macro_derive(ShankAccounts)] and pub fn shank_accounts(...) which calls derive_accounts(...). (shank-macro/src/lib.rs:447–452)

shank-idl/src/lib.rs (1)

8-8: Private accounts_extraction module inclusion

LGTM. Keep it private unless there’s a clear external consumer to avoid API surface creep.

shank/tests/expand/empty_struct.expanded.rs (1)

1-11: Snapshot contents look sane for the empty-accounts case

No issues. Keep in mind snapshots can be toolchain‑sensitive; update fixtures if rustc output changes.

If you hit frequent snapshot churn, consider macrotest filters to strip unstable attrs like #[rustc_box].

shank/tests/expand/no_constraints.rs (1)

10-15: No-constraints derive case: OK

Good fixture for read-only defaults without attributes.

shank/src/lib.rs (1)

4-5: Inspect context module for Solana imports

Please check the output of the above script to confirm whether shank/src/context.rs imports any Solana crates. Once we verify there are no Solana dependencies, we can remove the feature‐gate suggestion and consider this re‐export safe.

shank-macro/src/accounts.rs (1)

6-9: Bridge is correct — proc‑macro maps errors to compile_error()
Confirmed: shank-macro/src/lib.rs — shank_accounts calls derive_accounts(input).unwrap_or_else(to_compile_error).into() and to_compile_error delegates to ParseError::to_compile_error(...).

shank/tests/expand/no_constraints.expanded.rs (1)

12-25: Accounts metadata looks correct for a no-constraints struct.

Indices, mut/signers/optional flags, and descriptions (None) align with expectations.

shank-idl/src/file.rs (1)

110-129: Correctly wires ShankAccounts into instruction IDL conversion.

Early extraction and passing into IdlInstructions::try_into_with_accounts is the right integration point.

Please confirm there’s no double-iteration hazard on ctx.structs() across instructions() and accounts() in large crates (should be fine since each creates a fresh iterator).

shank-idl/tests/fixtures/instructions/single_file/instruction_with_accounts_struct.json (1)

1-94: Fixture aligns with struct-based accounts expansion.

Accounts, args, and discriminants look consistent; optionality encoded via isOptional.

Double-check naming conventions (e.g., systemProgram vs. Rust system_program) match the generator’s casing rules for this fixture comparison.

shank-idl/tests/instructions.rs (3)

193-209: Good coverage: simple accounts struct round‑trip

Test mirrors existing pattern and asserts full IDL equality. Looks solid.


211-226: Good: accounts struct with optional handling

Consistent with the fixture layout and ParseIdlConfig usage.


228-243: Good: complex struct scenario exercised

Nice to see a complex coverage case added.

shank/tests/basic_context_test.rs (1)

1-17: LGTM: minimal mock and derive usage

Local AccountInfo mock and attributes look fine for exercising the macro.

shank/tests/expand/custom_lifetime.rs (1)

10-17: LGTM: lifetime variance covered

Good case ensuring lifetimes other than 'info are accepted.

shank/tests/simple_context_test.rs (2)

25-30: Strengthen assertions to validate optional flag

Since the struct includes #[account(optional)], assert the metadata reflects it.
[suggest_minor_issue]
Apply:

 let accounts = SimpleAccounts::__shank_accounts();
 assert_eq!(accounts.len(), 3);
+// payer, data, optional_account
+assert_eq!(accounts[2].1, "optional_account");
+assert_eq!(accounts[2].5, true); // isOptional

13-23: LGTM: simple struct shape for __shank_accounts()

Attributes and field order suit the metadata checks.

shank-idl/tests/fixtures/instructions/single_file/instruction_with_simple_accounts_struct.rs (1)

4-17: LGTM: fixture focuses on attributes, not types

Appropriate for AST-based IDL extraction; types being non-reference is fine here.

shank/tests/error_handling_test.rs (8)

10-21: LGTM: documents compile-time conflict clearly

Good to keep the conflicting case commented with rationale.


22-49: LGTM: signer vs optional_signer semantics validated

Index mapping matches expanded tests tuple layout.


51-71: LGTM: description capture behavior

Covers presence/absence; concise.


73-98: LGTM: field order preservation

Validates deterministic ordering.


100-172: LGTM: comprehensive flag matrix

Good breadth across combinations including desc.


174-196: LGTM: lifetime-parameter variants

Matches expectations from expanded fixtures.


198-217: LGTM: special field names persisted verbatim

Covers common edge cases.


219-245: LGTM: long description handling

Validates empty and long docs as well.

EXAMPLE_USAGE.md (1)

45-50: Remaining-accounts handling verified — no change required

Confirmed: Context exposes pub remaining_accounts: &'a [U]; the macro builds it as &accounts[#expected_accounts..]; minimum-account validation returns Err(ProgramError::NotEnoughAccountKeys) when too few are provided; tests cover both extra and no-extra cases (shank/src/context.rs, shank-macro-impl/src/accounts/mod.rs, shank/tests/context_api_working_test.rs, shank/tests/context_usage_test.rs).

shank-macro-impl/src/accounts/account_field.rs (2)

60-66: Semantics: should optional_signer imply optional?

Current metadata can mark optional_signer=true with optional=false. Consider setting optional=true when optional_signer is present, or document the intended semantics.


23-58: No action required — workspace uses syn 1.x (syn = 1.0.82)
Cargo.toml entries show syn = 1.0.82, so attr.parse_meta(), attr.path and NestedMeta are compatible; no migration to syn 2 required.

shank-idl/ACCOUNTS_STRUCT_IDL.md (1)

41-46: Incorrect: placeholder-name mismatch claim — no change required

Both the IDL and the emitter already use "_accounts_struct{StructName}"; the "accountsStruct..." variant is not present in the repo. Found at shank-idl/ACCOUNTS_STRUCT_IDL.md:42 and shank-macro-impl/src/instruction/instruction.rs:155.

Likely an incorrect or invalid review comment.

shank-macro-impl/src/instruction/instruction.rs (1)

134-167: Unify placeholder naming and robustly extract struct name

Use the last path segment for the struct name (handles qualified paths) and standardize the placeholder to "_accounts_struct{StructName}" to align snapshots.

File: shank-macro-impl/src/instruction/instruction.rs
Lines: 134-167

-            Err(_) => {
+            Err(_) => {
                 let accounts_source = AccountsSource::try_from(attrs)?;
                 let accounts = match &accounts_source {
                     AccountsSource::Inline(accs) => accs.0.clone(),
                     AccountsSource::Struct(path) => {
-                        // Create placeholder accounts based on the struct name
-                        // The actual resolution will happen during IDL generation
-                        // For now, create a single placeholder account that indicates this uses a struct
-                        vec![InstructionAccount {
-                            ident: ident.clone(),
-                            index: Some(0),
-                            name: format!("__accounts_struct_{}", path.get_ident().unwrap_or(&ident)),
-                            writable: false,
-                            signer: false,
-                            optional_signer: false,
-                            desc: Some(format!("Accounts defined by struct: {}", path.get_ident().unwrap_or(&ident))),
-                            optional: false,
-                        }]
+                        // Placeholder resolved during IDL generation
+                        let struct_name = path.segments.last()
+                            .map(|seg| seg.ident.to_string())
+                            .unwrap_or_else(|| ident.to_string());
+                        vec![InstructionAccount {
+                            ident: ident.clone(),
+                            index: Some(0),
+                            name: format!("__accounts_struct_{}", struct_name),
+                            writable: false,
+                            signer: false,
+                            optional_signer: false,
+                            desc: Some(format!("Accounts defined by struct: {}", struct_name)),
+                            optional: false,
+                        }]
                     }
                 };
                 let strategies: InstructionStrategies = attrs.into();
                 (accounts_source, accounts, strategies)
             }
shank/tests/context_api_test.rs (1)

65-79: Great coverage on optional and mut/signer combos.

These assertions give good signal on tuple ordering and flags for writable/signer/optional.

shank/tests/context_runtime_test.rs (2)

31-63: LGTM: feature-gated stubs make the tests portable.

The cfg blocks keep tests compiling without solana-program; good separation.


153-173: Nice default-constraints sweep; consider asserting desc is None.

You already assert None at index 6; good coverage.

shank-macro/src/lib.rs (2)

246-247: Approve — derive_instruction already accepts #[accounts(StructName)] and wires the extraction path.

Parser and dispatch are implemented: shank-macro-impl/src/instruction/account_attrs.rs (is_accounts_struct_attr, from_accounts_struct_attr, try_from), wiring in shank-macro-impl/src/instruction/instruction.rs (uses to_accounts / AccountsSource), and account struct conversion in shank-macro-impl/src/accounts/account_struct.rs::to_instruction_accounts.


365-453: Approve — ShankAccounts macro emits IDL metadata and context helpers.
gen_impl defines pub fn __shank_accounts() -> Vec<(u32, &'static str, bool, bool, bool, bool, Option)> and a pub fn context(...) -> ::shank::Context<...> (see shank-macro-impl/src/accounts/mod.rs, around lines 61 and 111).

shank-macro-impl/src/accounts/account_struct.rs (1)

25-35: Tuple order verified — no action required.
gen_account_metadata returns (index, name, writable, signer, optional_signer, optional, desc) in that exact order, matching gen_account_list usage.

shank-idl/tests/fixtures/instructions/single_file/instruction_with_complex_accounts_struct.json (1)

41-49: Optional-signer semantics are implemented as-is; the suggested isOptional = optional || optional_signer is incorrect.

shank-idl/src/accounts_extraction.rs:120–124 clears optional when optional_signer is set, and shank-idl/src/idl_instruction.rs:358–362 maps is_signer = signer, is_optional = optional, is_optional_signer = optional_signer.

Likely an incorrect or invalid review comment.

shank/tests/final_context_test.rs (1)

52-53: Confirm Context generic usage matches the crate API.

If Context is defined with lifetimes (e.g., Context<'info, T>), this may need to be Context<TestAccounts<'_>> or similar. Please verify and adjust.

shank-idl/src/idl_instruction.rs (1)

229-253: Incorrect — accounts_map is keyed by the short struct ident; last-segment lookup is correct.

shank-idl/src/accounts_extraction.rs inserts keys with let struct_name = struct_item.ident.to_string(); followed by accounts_map.insert(struct_name, accounts); (around lines 18–21), so resolving via struct_path.segments.last() matches the map keys. No change required.

Likely an incorrect or invalid review comment.

shank/tests/context_api_working_test.rs (2)

20-30: Good: ShankAccounts usage and field shapes look consistent.

Derive, lifetimes, and Option<&AccountInfo> are coherent with the intended API.


96-101: Confirmed: optional-account sentinel is crate::ID.

The generated context/macro explicitly maps optional fields to None when accounts[#idx].key == &crate::ID (see shank-render/src/context/render_context.rs:25 and shank-macro-impl/src/accounts/mod.rs:90-93, comment 108-111). The test's expectation that key == ID yields None is correct.

shank-idl/src/accounts_extraction.rs (1)

126-136: LGTM: InstructionAccount construction.

Fields mapped coherently (index, ident, flags, desc).

shank-idl/tests/fixtures/instructions/single_file/instruction_with_accounts_struct.rs (1)

1-44: LGTM: Struct-based accounts expansion setup.

Accounts structs and enum wiring match the intended flow.

shank/tests/expand/complex_constraints.rs (1)

10-29: LGTM: Constraint coverage looks comprehensive.

Good spread of mut, signer, optional, and combinations.

shank/tests/context_usage_test.rs (1)

25-33: API mismatch: tests call context(accounts, &program_id) and expect Result<...>.

This relies on the macro fix in accounts/mod.rs. Without it, these tests won’t compile. Re-run after applying the macro patch.

Also applies to: 76-83, 150-158, 223-231, 271-275

shank/tests/context_issue_demo.rs (2)

25-28: API mismatch: context() currently returns Context, not Result.

Once macro is fixed to return Result and take program_id, this test will be valid. Track dependency on macro change.


31-62: Ensure context() is cfg-gated off in non-spl builds.

With the proposed macro change, context() won’t be generated here. This test should compile cleanly.

shank/tests/expand/complex_constraints.expanded.rs (1)

21-85: Expanded tuple shape/flags look correct.

Writable/signer/optional_signer/optional flags and descriptions align with attributes.

Confirm expand golden files don’t expect context() to appear; if they do, update fixtures or ensure macro cfg-gates generation.

shank-idl/tests/fixtures/instructions/single_file/instruction_with_complex_accounts_struct.rs (3)

3-23: Struct fields use owned AccountInfo; OK for IDL, but not for context().

This fixture is IDL-only. Ensure tests run without solana-program feature so context() isn’t generated; otherwise owned types will mismatch the macro’s reference-based context assignment.


25-41: Mixed instruction spec looks good.

Combining #[accounts(ComplexAccounts)] with old-style inline accounts exercises the new expansion path well.


43-48: Local AccountInfo mock is fine for IDL.

No changes needed.

shank/examples/context_usage_example.rs (1)

23-36: Docs vs behavior: optional accounts must use Option.

You did this correctly for metadata. Good.

Please confirm the generated ctx.metadata type is Option<&AccountInfo<'_>> as intended across all derives.

shank-macro-impl/src/instruction/account_attrs.rs (1)

65-153: Inline account parser looks solid; nice coverage of aliases and mutual exclusivity.

No functional issues found.

shank/examples/anchor_style_example.rs (1)

112-127: Example text aligns with Option usage.

Messages correctly describe Option<&AccountInfo>. After fixing types above, this stays consistent.

shank/examples/full_solana_example.rs (1)

79-109: Processor paths look consistent with context(program_id, accounts).

Logging keys and optional handling are clear.

Please confirm Pubkey implements Display in your MSRV; if not, switch to {:?}.

Also applies to: 111-129

shank/tests/enhanced_accounts_test.rs (1)

92-99: Mixed old/new styles exercised correctly.

Good coverage ensuring backward compatibility.

Also applies to: 114-123

shank/examples/accounts_struct_example.rs (1)

11-36: LGTM: clear, idiomatic use of ShankAccounts for examples

Field attributes (mut/signer/optional/desc) look correct, and using PhantomData keeps this example buildable without Solana deps. No functional concerns.

Also applies to: 38-51, 53-66

shank/tests/instruction_integration_test.rs (1)

77-79: Note on optional fields in tests

Using #[account(optional)] with a non-Option field is fine here because you’re only asserting IDL metadata via __shank_accounts(). If you later exercise context() runtime generation, these should be Option<AccountInfo<'info>> (or similar) to reflect optionality.

Also applies to: 159-161

Comment on lines +10 to +12
// Program ID - normally created by declare_id! macro
pub const ID: [u8; 32] = [1; 32];

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Program ID type should reflect Solana’s Pubkey/declare_id!

Using [u8; 32] for ID can mislead users. Prefer declare_id! so examples align with real projects.

Apply:

-// Program ID - normally created by declare_id! macro
-pub const ID: [u8; 32] = [1; 32];
+// Program ID - typically created by `declare_id!`, which defines `crate::ID: Pubkey`
+use solana_program::{declare_id, pubkey::Pubkey};
+declare_id!("11111111111111111111111111111111");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Program ID - normally created by declare_id! macro
pub const ID: [u8; 32] = [1; 32];
// Program ID - typically created by `declare_id!`, which defines `crate::ID: Pubkey`
use solana_program::{declare_id, pubkey::Pubkey};
declare_id!("11111111111111111111111111111111");
🤖 Prompt for AI Agents
In EXAMPLE_USAGE.md around lines 10 to 12, replace the raw byte array pub const
ID: [u8; 32] = [1; 32]; with the standard Solana pattern using the declare_id!
macro so examples match real projects: import or reference
solana_program::declare_id! (or the appropriate crate path in the example) and
use declare_id!("ReplaceWithYourProgramIdHere"); ensuring the example shows the
macro usage rather than a [u8; 32] literal and updating any surrounding text to
mention inserting the real program id string.

Comment on lines +31 to +41
```rust
// When optional account is provided:
let accounts = [payer, vault, authority_account, system_program];
let ctx = CreateVaultAccounts::context(&accounts);
assert!(ctx.accounts.optional_authority.is_some());

// When optional account is NOT provided (pass program ID as placeholder):
let accounts = [payer, vault, program_id_placeholder, system_program];
let ctx = CreateVaultAccounts::context(&accounts);
assert!(ctx.accounts.optional_authority.is_none());
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick

Clarify the “program ID placeholder”

Explicitly note that the placeholder must be an AccountInfo whose key equals crate::ID; otherwise readers may try to pass a Pubkey.

Apply:

-// When optional account is NOT provided (pass program ID as placeholder):
-let accounts = [payer, vault, program_id_placeholder, system_program];
+// When optional account is NOT provided (pass an AccountInfo whose key == crate::ID as placeholder):
+// e.g., let program_id_placeholder = AccountInfo { key: &ID.to_bytes(), data: &[], owner: &ID.to_bytes() /* mock */ };
+let accounts = [payer, vault, program_id_placeholder /* AccountInfo with key == crate::ID */, system_program];

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In EXAMPLE_USAGE.md around lines 31 to 41, the comment about passing a "program
ID placeholder" is unclear; update the example text to explicitly state that the
placeholder must be an AccountInfo whose key equals crate::ID (not a Pubkey),
and instruct readers to construct or pass an AccountInfo with key == crate::ID
when they want to omit the optional account so the context check behaves as
shown.

Comment on lines +45 to +49
1. **Ergonomic API**: No need to pass program ID parameter - uses `crate::ID` automatically
2. **Type Safety**: Optional accounts use `Option<&AccountInfo>` types
3. **Runtime Detection**: Checks if `account.key == crate::ID` to determine None/Some
4. **IDL Generation**: Proper `"isOptional": true` flags in generated IDL
5. **Remaining Accounts**: Automatically handles extra accounts beyond the struct definition
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

🧩 Analysis chain

Inconsistent API description: context() with or without program_id

Elsewhere (examples/… and docs), context(program_id, accounts) appears. Here it claims no program_id is needed. Please unify and document the canonical signature and when each form applies.

Would you confirm the intended API and update all docs accordingly?


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "PWD: $(pwd)"
echo "--- repo root listing ---"
ls -la

echo
echo "---- EXAMPLE_USAGE.md (first 200 lines) ----"
if [ -f EXAMPLE_USAGE.md ]; then
  sed -n '1,200p' EXAMPLE_USAGE.md || true
else
  echo "EXAMPLE_USAGE.md not found at repo root"
fi

echo
echo "---- rg: search for 'context(' across repo ----"
rg -n -S -C2 "context\(" || true

echo
echo "---- rg: search for 'context(program_id' ----"
rg -n -S -C2 "context\(.*program_id" || true

echo
echo "---- rg: search for the exact substring 'context(program_id, accounts' ----"
rg -n -F "context(program_id, accounts" || true

echo
echo "---- rg: look for function/definition 'fn context' / 'def context' / 'function context' ----"
rg -n -S -C2 "fn context\b" || true
rg -n -S -C2 "\bdef context\b" || true
rg -n -S -C2 "\bfunction context\b" || true

Length of output: 28416


Unify the context(...) API to a single canonical signature and apply it across the repo.

Repo mixes context(&[AccountInfo]) and context(program_id, accounts) in docs, examples, tests, and generated code — this must be unified.

Files to update:

  • Implementations / generator: shank-macro-impl/src/accounts/mod.rs, shank-render/src/context/render_context.rs
  • Macro docs: shank-macro/src/lib.rs
  • Examples / docs: EXAMPLE_USAGE.md, shank/examples/full_solana_example.rs, shank/examples/context_usage_example.rs, shank/examples/anchor_style_example.rs, shank/examples/accounts_struct_example.rs
  • Tests: shank/tests/* (e.g. context_usage_test.rs, final_context_test.rs, context_issue_demo.rs)

Action required: choose the canonical signature (either accounts-only — using crate::ID as the placeholder for missing optional accounts — or explicit program_id + accounts), update codegen/macros, fix all examples/tests/docs to match, and document the chosen behavior for optional accounts in EXAMPLE_USAGE.md and macro docs.

}
```

This follows Solana's modern optional accounts pattern where missing optional accounts are represented by the program ID. No newline at end of file
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick

Scope the claim about optional-accounts convention

“Solana’s modern optional accounts pattern” is framework-convention, not a protocol rule. Rephrase to avoid overstating.

Apply:

-This follows Solana's modern optional accounts pattern where missing optional accounts are represented by the program ID.
+This follows a common framework convention where missing optional accounts are represented by the program ID.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
This follows Solana's modern optional accounts pattern where missing optional accounts are represented by the program ID.
This follows a common framework convention where missing optional accounts are represented by the program ID.
🤖 Prompt for AI Agents
In EXAMPLE_USAGE.md around line 64, the sentence asserts "Solana’s modern
optional accounts pattern" as a protocol rule; rephrase to scope it as a
framework/convention instead of a protocol requirement. Update the wording to
indicate this is a common framework or developer convention (e.g., "a common
convention in many Solana frameworks") and avoid language that implies it is
mandated by the Solana protocol.

Comment on lines +16 to +24
#[account(mut, signer, desc = "The payer and authority")]
pub payer: AccountInfo<'info>,

#[account(mut, desc = "The vault account to create")]
pub vault: AccountInfo<'info>,

#[account(desc = "System program")]
pub system_program: AccountInfo<'info>,
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick

Use &'info AccountInfo<'info> in examples for consistency

Examples elsewhere (tests/examples) use references. Update this snippet to avoid confusion.

-    pub payer: AccountInfo<'info>,
+    pub payer: &'info AccountInfo<'info>,
-    pub vault: AccountInfo<'info>,
+    pub vault: &'info AccountInfo<'info>,
-    pub system_program: AccountInfo<'info>,
+    pub system_program: &'info AccountInfo<'info>,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#[account(mut, signer, desc = "The payer and authority")]
pub payer: AccountInfo<'info>,
#[account(mut, desc = "The vault account to create")]
pub vault: AccountInfo<'info>,
#[account(desc = "System program")]
pub system_program: AccountInfo<'info>,
}
#[account(mut, signer, desc = "The payer and authority")]
pub payer: &'info AccountInfo<'info>,
#[account(mut, desc = "The vault account to create")]
pub vault: &'info AccountInfo<'info>,
#[account(desc = "System program")]
pub system_program: &'info AccountInfo<'info>,
}
🤖 Prompt for AI Agents
In shank-idl/ACCOUNTS_STRUCT_IDL.md around lines 16 to 24, the account fields
are declared as owned AccountInfo<'info> which is inconsistent with other
examples; update the declarations to use references by changing each field to
&'info AccountInfo<'info> (preserving attributes like mut and signer where
present) so the snippet matches the rest of the codebase and avoids confusion.

pub optional_authority: Option<&'info AccountInfo<'info>>,
}

fn main() {} No newline at end of file
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick

Add trailing newline

Minor formatting polish for editors and linters.

🤖 Prompt for AI Agents
In shank/tests/expand/optional_accounts.rs around line 22, the file is missing a
trailing newline; add a newline at the end of the file so the final line ends
with a newline character to satisfy editors and linters.

Comment on lines +10 to +55
#[test]
fn test_idl_generation_basic_accounts() {
#[derive(ShankAccounts)]
pub struct BasicAccounts<'info> {
#[account(mut, signer, desc = "The payer account")]
pub payer: &'info AccountInfo<'info>,

#[account(mut, desc = "The data account")]
pub data: &'info AccountInfo<'info>,

#[account(desc = "The system program")]
pub system_program: &'info AccountInfo<'info>,
}

let idl = BasicAccounts::__shank_accounts();

// Verify we have 3 accounts
assert_eq!(idl.len(), 3);

// Check payer account
assert_eq!(idl[0].0, 0); // index
assert_eq!(idl[0].1, "payer"); // name
assert_eq!(idl[0].2, true); // mut
assert_eq!(idl[0].3, true); // signer
assert_eq!(idl[0].4, false); // optional_signer (false for regular signer)
assert_eq!(idl[0].5, false); // optional
assert_eq!(idl[0].6, Some("The payer account".to_string())); // description

// Check data account
assert_eq!(idl[1].0, 1);
assert_eq!(idl[1].1, "data");
assert_eq!(idl[1].2, true); // mut
assert_eq!(idl[1].3, false); // not signer
assert_eq!(idl[1].4, false); // optional_signer
assert_eq!(idl[1].5, false); // not optional
assert_eq!(idl[1].6, Some("The data account".to_string()));

// Check system program
assert_eq!(idl[2].0, 2);
assert_eq!(idl[2].1, "system_program");
assert_eq!(idl[2].2, false); // not mut
assert_eq!(idl[2].3, false); // not signer
assert_eq!(idl[2].4, false); // optional_signer
assert_eq!(idl[2].5, false); // not optional
assert_eq!(idl[2].6, Some("The system program".to_string()));
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick

Solid coverage of tuple semantics and flag combinations.

Tests accurately validate indices, names, mut/signers, optional flags, and descriptions across scenarios.

Consider a small helper or a type alias to avoid tuple indexing noise in assertions for readability:

type AccountMeta = (u32, &'static str, bool, bool, bool, bool, Option<String>);

Also applies to: 57-90, 92-160, 162-181, 183-203, 205-212, 214-231, 233-263, 265-290

🤖 Prompt for AI Agents
In shank/tests/idl_generation_test.rs around lines 10 to 55, the assertions use
raw tuple indexing which harms readability; add a local type alias like
AccountMeta = (u32, &'static str, bool, bool, bool, bool, Option<String>) near
the top of the test file and then replace the index-based assertions with either
pattern destructuring (let (idx, name, is_mut, is_signer, opt_signer, opt, desc)
= idl[n]; assert_eq!(idx, ...); ...) or by binding idl[n] to a variable of type
AccountMeta before asserting; apply the same alias/destructuring approach to the
other listed test ranges to reduce tuple-index noise and make assertions
clearer.

Comment on lines +25 to +27
#[account(optional_signer, desc = "Authority on the vault")]
pub authority: AccountInfo<'info>,

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Optional signer field should use Option and standard flags.

Type isn’t optional though attribute says optional_signer. Prefer Option<...> with #[account(optional, signer)] for consistency with other tests and API.

-    #[account(optional_signer, desc = "Authority on the vault")]
-    pub authority: AccountInfo<'info>,
+    #[account(optional, signer, desc = "Authority on the vault")]
+    pub authority: Option<AccountInfo<'info>>,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#[account(optional_signer, desc = "Authority on the vault")]
pub authority: AccountInfo<'info>,
#[account(optional, signer, desc = "Authority on the vault")]
pub authority: Option<AccountInfo<'info>>,
🤖 Prompt for AI Agents
In shank/tests/instruction_with_accounts_struct_test.rs around lines 25 to 27,
the field is marked optional_signer but is typed as AccountInfo<'info>; change
the field type to Option<AccountInfo<'info>> and replace the attribute with the
standard #[account(optional, signer)] form, and update any test setup/uses to
pass Some(account) or None and handle the Option (e.g., unwrap or map) where the
field is accessed.

Comment on lines +32 to +34
fn main() {
println!("Simple context test");
} No newline at end of file
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove main() from integration test

Avoid defining fn main() in tests.

Apply:

-fn main() {
-    println!("Simple context test");
-}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fn main() {
println!("Simple context test");
}
🤖 Prompt for AI Agents
In shank/tests/simple_context_test.rs around lines 32 to 34, the file currently
defines a standalone fn main() in an integration test which should be removed;
replace the top-level main function with one or more #[test] functions (or move
helper code to a module and expose test functions) so the test runner executes
them — delete fn main(), add a #[test] fn simple_context_test() { /* move test
logic here and use assertions instead of println! */ } and ensure the file
compiles as an integration test without a main.

Comment on lines +22 to +35
fn main() {
println!("Working context demo");
println!("The context() method should be available on DemoAccounts");
println!("It should return Context<DemoAccounts> with accounts and remaining_accounts fields");

// For now, let's just test that the IDL generation works
let accounts_idl = DemoAccounts::__shank_accounts();
println!("Generated {} accounts in IDL", accounts_idl.len());

// In a real program, this would work:
// let ctx = DemoAccounts::context(&accounts);
// ctx.accounts.payer.key // access payer
// ctx.remaining_accounts // access any extra accounts
} No newline at end of file
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

main() in an integration test will conflict with the test harness.

Integration tests in tests/ are compiled with a generated main; defining your own main leads to build failure. Convert to a #[test] or move to examples/.

-fn main() {
+#[test]
+fn working_context_demo() {
     println!("Working context demo");
     println!("The context() method should be available on DemoAccounts");
     println!("It should return Context<DemoAccounts> with accounts and remaining_accounts fields");
     
     // For now, let's just test that the IDL generation works
     let accounts_idl = DemoAccounts::__shank_accounts();
     println!("Generated {} accounts in IDL", accounts_idl.len());
     
     // In a real program, this would work:
     // let ctx = DemoAccounts::context(&accounts);
     // ctx.accounts.payer.key // access payer
     // ctx.remaining_accounts // access any extra accounts
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fn main() {
println!("Working context demo");
println!("The context() method should be available on DemoAccounts");
println!("It should return Context<DemoAccounts> with accounts and remaining_accounts fields");
// For now, let's just test that the IDL generation works
let accounts_idl = DemoAccounts::__shank_accounts();
println!("Generated {} accounts in IDL", accounts_idl.len());
// In a real program, this would work:
// let ctx = DemoAccounts::context(&accounts);
// ctx.accounts.payer.key // access payer
// ctx.remaining_accounts // access any extra accounts
}
#[test]
fn working_context_demo() {
println!("Working context demo");
println!("The context() method should be available on DemoAccounts");
println!("It should return Context<DemoAccounts> with accounts and remaining_accounts fields");
// For now, let's just test that the IDL generation works
let accounts_idl = DemoAccounts::__shank_accounts();
println!("Generated {} accounts in IDL", accounts_idl.len());
// In a real program, this would work:
// let ctx = DemoAccounts::context(&accounts);
// ctx.accounts.payer.key // access payer
// ctx.remaining_accounts // access any extra accounts
}
🤖 Prompt for AI Agents
In shank/tests/working_context_demo.rs around lines 22 to 35, the file defines a
main() which conflicts with the test harness for integration tests; replace main
with a test or move the file to examples. To fix: either convert the function to
a #[test] (rename to e.g. fn working_context_demo_test) and change prints to
assertions (e.g. assert!(DemoAccounts::__shank_accounts().len() > 0) or other
meaningful asserts), or move the file out of tests/ into examples/ (keeping
main) so it’s built as a binary example instead of an integration test.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review continued from previous batch...

Comment on lines +11 to +26
/// Extract ShankAccounts structs and their metadata
pub fn extract_shank_accounts_structs<'a>(
structs: impl Iterator<Item = &'a ItemStruct>,
) -> Result<HashMap<String, Vec<InstructionAccount>>> {
let mut accounts_map = HashMap::new();

for struct_item in structs {
if let Some(_attr) = get_derive_attr(&struct_item.attrs, DERIVE_ACCOUNTS_ATTR) {
let struct_name = struct_item.ident.to_string();
let accounts = extract_accounts_from_struct(struct_item)?;
accounts_map.insert(struct_name, accounts);
}
}

Ok(accounts_map)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick

Naming mismatch with guidelines (ShankAccount vs ShankAccounts).

The code targets ShankAccounts; update docs/guidelines to avoid confusion or provide a compatibility note.

🤖 Prompt for AI Agents
In shank-idl/src/accounts_extraction.rs around lines 11 to 26, the doc comment
and naming refer to "ShankAccounts" but project guidelines use "ShankAccount"
(singular) causing mismatch; update the comment and any exposed identifiers to
match the canonical name: change the docstring to "Extract ShankAccount structs
and their metadata" (or add a brief compatibility note if keeping the plural),
and update project docs/guidelines to reflect the chosen singular/plural
convention so names are consistent across code and docs.

Comment on lines +28 to +33
/// Extract individual accounts from a ShankAccounts struct by calling its __shank_accounts method
fn extract_accounts_from_struct(struct_item: &ItemStruct) -> Result<Vec<InstructionAccount>> {
// This is where we need to get the account metadata.
// The challenge is that at parse time, we can't execute the __shank_accounts() method.
// We need to parse the struct fields and their #[account(...)] attributes directly.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick

Fix doc comment to reflect actual behavior.

This function parses field attributes; it does not call __shank_accounts().

-/// Extract individual accounts from a ShankAccounts struct by calling its __shank_accounts method
+/// Extract individual accounts from a ShankAccounts struct by parsing its #[account(...)] field attributes
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// Extract individual accounts from a ShankAccounts struct by calling its __shank_accounts method
fn extract_accounts_from_struct(struct_item: &ItemStruct) -> Result<Vec<InstructionAccount>> {
// This is where we need to get the account metadata.
// The challenge is that at parse time, we can't execute the __shank_accounts() method.
// We need to parse the struct fields and their #[account(...)] attributes directly.
/// Extract individual accounts from a ShankAccounts struct by parsing its #[account(...)] field attributes
fn extract_accounts_from_struct(struct_item: &ItemStruct) -> Result<Vec<InstructionAccount>> {
// This is where we need to get the account metadata.
// The challenge is that at parse time, we can't execute the __shank_accounts() method.
// We need to parse the struct fields and their #[account(...)] attributes directly.
🤖 Prompt for AI Agents
In shank-idl/src/accounts_extraction.rs around lines 28 to 33, the doc comment
incorrectly states that the function extracts accounts by calling
__shank_accounts(); update the comment to accurately describe current behavior:
explain that the function parses the struct fields and reads their
#[account(...)] attributes at parse time (since __shank_accounts() cannot be
executed), and mention it returns a Vec<InstructionAccount> derived from those
attributes. Keep the comment concise and aligned with the implemented parsing
behavior.

Comment on lines +70 to +76
let has_option_type = if let Type::Path(type_path) = &field.ty {
if let Some(segment) = type_path.path.segments.first() {
segment.ident == "Option"
} else {
false
}
} else {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Option detection is fragile (uses first segment).

Fully-qualified types (e.g., std::option::Option) won’t be detected. Use the last path segment.

-    let has_option_type = if let Type::Path(type_path) = &field.ty {
-        if let Some(segment) = type_path.path.segments.first() {
-            segment.ident == "Option"
-        } else {
-            false
-        }
-    } else {
-        false
-    };
+    let has_option_type = matches!(&field.ty, Type::Path(type_path)
+        if type_path.path.segments.last().map_or(false, |seg| seg.ident == "Option"));
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let has_option_type = if let Type::Path(type_path) = &field.ty {
if let Some(segment) = type_path.path.segments.first() {
segment.ident == "Option"
} else {
false
}
} else {
let has_option_type = matches!(&field.ty, Type::Path(type_path)
if type_path.path.segments.last().map_or(false, |seg| seg.ident == "Option"));
🤖 Prompt for AI Agents
In shank-idl/src/accounts_extraction.rs around lines 70 to 76, the current
Option<T> detection inspects the first path segment which misses fully-qualified
paths like std::option::Option; change the logic to inspect the last path
segment instead (e.g., use path.segments.last()) and compare that segment.ident
to "Option", keeping the existing Type::Path check; ensure you handle the Option
being present or not (map/and_then or pattern) so you don’t panic on empty
segments.

Comment on lines +80 to +107
// Parse #[account(...)] attributes
for attr in &field.attrs {
if attr.path.is_ident("account") {
// Use a simple string-based parsing approach for now
// This is a simplified version - in production we'd want more robust parsing
let tokens_str = attr.tokens.to_string();

// Simple parsing of common attributes
if tokens_str.contains("mut") || tokens_str.contains("writable") {
writable = true;
}
if tokens_str.contains("signer") {
signer = true;
}
if tokens_str.contains("optional_signer") {
optional_signer = true;
} else if tokens_str.contains("optional") {
optional = true;
}

// Extract description using simple regex-like approach
if let Some(desc_start) = tokens_str.find("desc = \"") {
let desc_content = &tokens_str[desc_start + 8..];
if let Some(desc_end) = desc_content.find('"') {
desc = Some(desc_content[..desc_end].to_string());
}
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

🧩 Analysis chain

Replace string-based parsing with syn’s nested meta; support alias attributes.

String contains() is brittle (spacing/ordering) and misses aliases (write/writ/w, sign/sig/s, opt/option). Parse tokens robustly and accept aliases to match the macro’s surface.

-    // Parse #[account(...)] attributes
-    for attr in &field.attrs {
-        if attr.path.is_ident("account") {
-            // Use a simple string-based parsing approach for now
-            // This is a simplified version - in production we'd want more robust parsing
-            let tokens_str = attr.tokens.to_string();
-            
-            // Simple parsing of common attributes
-            if tokens_str.contains("mut") || tokens_str.contains("writable") {
-                writable = true;
-            }
-            if tokens_str.contains("signer") {
-                signer = true;
-            }
-            if tokens_str.contains("optional_signer") {
-                optional_signer = true;
-            } else if tokens_str.contains("optional") {
-                optional = true;
-            }
-            
-            // Extract description using simple regex-like approach
-            if let Some(desc_start) = tokens_str.find("desc = \"") {
-                let desc_content = &tokens_str[desc_start + 8..];
-                if let Some(desc_end) = desc_content.find('"') {
-                    desc = Some(desc_content[..desc_end].to_string());
-                }
-            }
-        }
-    }
+    // Parse #[account(...)] attributes robustly and support aliases
+    for attr in &field.attrs {
+        #[allow(deprecated)]
+        if attr.path().is_ident("account") {
+            attr.parse_nested_meta(|meta| {
+                let key = meta.path.segments.last().map(|s| s.ident.to_string()).unwrap_or_default();
+                match key.as_str() {
+                    // writable variants
+                    "mut" | "writable" | "write" | "writ" | "w" => writable = true,
+                    // signer variants
+                    "signer" | "sign" | "sig" | "s" => signer = true,
+                    // optional variants
+                    "optional_signer" => optional_signer = true,
+                    "optional" | "opt" | "option" => optional = true,
+                    // desc = "..."
+                    "desc" => {
+                        let val: shank_macro_impl::syn::LitStr = meta.value()?.parse()?;
+                        desc = Some(val.value());
+                    }
+                    _ => {}
+                }
+                Ok(())
+            })?;
+        }
+    }

🏁 Script executed:

#!/bin/bash
# Ensure syn 2.x is in use (parse_nested_meta API)
rg -n --glob '**/Cargo.toml' -C2 'syn\s*='
# Grep for alias attributes in tests to confirm coverage alignment
rg -n --type=rust -C2 '#\[account\((write|writ|w|sign|sig|s|opt|option)\b'

Length of output: 4788


Use Attribute::parse_nested_meta for robust #[account(...)] parsing with alias support

  • In shank-idl/src/accounts_extraction.rs, replace the tokens_str.contains(...) logic with attr.parse_nested_meta(|meta| { … }), matching on meta.path variants:
    • writable: "mut" | "writable" | "write" | "writ" | "w"
    • signer: "signer" | "sign" | "sig" | "s"
    • optional: "optional_signer"; and "optional" | "opt" | "option"
  • Parse desc via meta.value()?.parse::<LitStr>()? instead of manual string slicing (requires full feature for parse_nested_meta) (docs.rs)
  • Remove brittle, order- and spacing-dependent string parsing entirely.
🤖 Prompt for AI Agents
In shank-idl/src/accounts_extraction.rs around lines 80 to 107, replace the
brittle tokens_str.contains(...) string parsing with syn's
Attribute::parse_nested_meta to robustly parse #[account(...)] entries: call
attr.parse_nested_meta(|meta| { ... }) (requires syn "full" feature), match on
meta.path to set writable when the path is any of
"mut","writable","write","writ","w", set signer when path is
"signer","sign","sig","s", set optional_signer for "optional_signer" and
optional for "optional","opt","option"; for desc extract the literal via
meta.value()?.parse::<LitStr>()? instead of manual slicing; remove the old
string-based checks and ensure error propagation or sensible defaults on parse
failures.

Comment on lines +147 to +156
ensure!(
discriminant < u8::MAX as usize,
anyhow!(
"Instruction variant discriminants have to be <= u8::MAX ({}), \
but the discriminant of variant '{}' is {}",
u8::MAX,
ident,
discriminant
)
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Off-by-one on discriminant bound (excludes 255).

The check uses < u8::MAX but the error text says <=. Use <= to accept 255.

-        ensure!(
-            discriminant < u8::MAX as usize,
+        ensure!(
+            discriminant <= u8::MAX as usize,
             anyhow!(
                 "Instruction variant discriminants have to be <= u8::MAX ({}), \
                     but the discriminant of variant '{}' is {}",
                 u8::MAX,
                 ident,
                 discriminant
             )
         );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ensure!(
discriminant < u8::MAX as usize,
anyhow!(
"Instruction variant discriminants have to be <= u8::MAX ({}), \
but the discriminant of variant '{}' is {}",
u8::MAX,
ident,
discriminant
)
);
ensure!(
discriminant <= u8::MAX as usize,
anyhow!(
"Instruction variant discriminants have to be <= u8::MAX ({}), \
but the discriminant of variant '{}' is {}",
u8::MAX,
ident,
discriminant
)
);
🤖 Prompt for AI Agents
In shank-idl/src/idl_instruction.rs around lines 147 to 156, the discriminant
bound check is off-by-one: it currently uses `< u8::MAX as usize` which rejects
the valid value 255 while the message states "<=". Change the comparison to `<=
u8::MAX as usize` so discriminant 255 is accepted, and keep the error message
as-is; ensure the types remain consistent (usize vs u8::MAX) when making the
comparison.

@@ -0,0 +1,43 @@
use shank::ShankAccounts;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick

Unused import.

Same here; remove unused import.

-use shank::ShankAccounts;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
use shank::ShankAccounts;
🤖 Prompt for AI Agents
In shank/tests/expand/custom_lifetime.expanded.rs around line 1, the statement
"use shank::ShankAccounts;" is an unused import; remove that import line from
the file (or replace it with a used import if intended) and run the build/test
to ensure no other references rely on it.

@@ -0,0 +1,54 @@
use shank::ShankAccounts;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick

Unused import.

use shank::ShankAccounts; isn’t needed in the expanded output; removing avoids warnings.

-use shank::ShankAccounts;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
use shank::ShankAccounts;
🤖 Prompt for AI Agents
In shank/tests/expand/optional_accounts.expanded.rs at lines 1-1, the line `use
shank::ShankAccounts;` is an unused import; remove this use statement from the
expanded file (or the generated source) so the compiler no longer emits an
unused import warning, and re-run the build/generation to confirm no other
references to ShankAccounts remain.

Comment on lines +4 to +8
pub struct AccountInfo<'info> {
pub key: &'info [u8; 32],
pub data: &'info [u8],
pub owner: &'info [u8; 32],
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick

Provide key() like other tests so optional detection can work.

Other tests implement key() on AccountInfo; add the same here for consistency.

 pub struct AccountInfo<'info> {
     pub key: &'info [u8; 32],
     pub data: &'info [u8],
     pub owner: &'info [u8; 32],
 }
+
+impl<'info> AccountInfo<'info> {
+    pub fn key(&self) -> &'info [u8; 32] {
+        self.key
+    }
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pub struct AccountInfo<'info> {
pub key: &'info [u8; 32],
pub data: &'info [u8],
pub owner: &'info [u8; 32],
}
pub struct AccountInfo<'info> {
pub key: &'info [u8; 32],
pub data: &'info [u8],
pub owner: &'info [u8; 32],
}
impl<'info> AccountInfo<'info> {
pub fn key(&self) -> &'info [u8; 32] {
self.key
}
}
🤖 Prompt for AI Agents
In shank/tests/final_context_test.rs around lines 4 to 8, AccountInfo is missing
the key() accessor used by other tests; add an impl block that defines a
key(&self) -> &'info [u8; 32] method that returns self.key so optional account
detection works the same as the other tests.

Comment on lines +61 to +86
#[test]
fn test_minimal_accounts() {
let payer_key = [1u8; 32];
let data_key = [2u8; 32];

let payer = AccountInfo {
key: &payer_key,
data: &[],
owner: &[0; 32],
};

let data = AccountInfo {
key: &data_key,
data: &[],
owner: &[0; 32],
};

// Only provide required accounts (payer and data, no optional)
let accounts = [payer, data];

let ctx: Context<TestAccounts> = TestAccounts::context(&accounts);

assert_eq!(ctx.accounts.payer.key, &payer_key);
assert_eq!(ctx.accounts.data.key, &data_key);
assert!(ctx.accounts.optional_account.is_some()); // Still present at index 2, but would be None if we had proper ID detection
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix optional-account semantics in the minimal test (should be None, not Some).

When only required accounts are supplied, the optional account must be None. Either:

  • pass a third placeholder with key == ID, or
  • allow shorter slices and treat missing tail slots as None.

Apply this to use the ID sentinel and assert None:

@@
-    // Only provide required accounts (payer and data, no optional)
-    let accounts = [payer, data];
+    // Provide a placeholder for the optional account using the sentinel ID
+    let optional_placeholder = AccountInfo {
+        key: &ID,
+        data: &[],
+        owner: &[0; 32],
+    };
+    let accounts = [payer, data, optional_placeholder];
@@
-    assert!(ctx.accounts.optional_account.is_some()); // Still present at index 2, but would be None if we had proper ID detection
+    assert!(ctx.accounts.optional_account.is_none());
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#[test]
fn test_minimal_accounts() {
let payer_key = [1u8; 32];
let data_key = [2u8; 32];
let payer = AccountInfo {
key: &payer_key,
data: &[],
owner: &[0; 32],
};
let data = AccountInfo {
key: &data_key,
data: &[],
owner: &[0; 32],
};
// Only provide required accounts (payer and data, no optional)
let accounts = [payer, data];
let ctx: Context<TestAccounts> = TestAccounts::context(&accounts);
assert_eq!(ctx.accounts.payer.key, &payer_key);
assert_eq!(ctx.accounts.data.key, &data_key);
assert!(ctx.accounts.optional_account.is_some()); // Still present at index 2, but would be None if we had proper ID detection
}
#[test]
fn test_minimal_accounts() {
let payer_key = [1u8; 32];
let data_key = [2u8; 32];
let payer = AccountInfo {
key: &payer_key,
data: &[],
owner: &[0; 32],
};
let data = AccountInfo {
key: &data_key,
data: &[],
owner: &[0; 32],
};
// Provide a placeholder for the optional account using the sentinel ID
let optional_placeholder = AccountInfo {
key: &ID,
data: &[],
owner: &[0; 32],
};
let accounts = [payer, data, optional_placeholder];
let ctx: Context<TestAccounts> = TestAccounts::context(&accounts);
assert_eq!(ctx.accounts.payer.key, &payer_key);
assert_eq!(ctx.accounts.data.key, &data_key);
assert!(ctx.accounts.optional_account.is_none());
}
🤖 Prompt for AI Agents
In shank/tests/final_context_test.rs around lines 61-86, the minimal-accounts
test currently treats the optional account as Some because the accounts array
length provides that slot; to fix, supply the optional-account sentinel ID so
the framework recognizes it as missing and the field becomes None: add a third
AccountInfo with key set to the library's ID sentinel (e.g. the constant ID or
&[0u8;32] if that is the sentinel) and matching placeholder data/owner, then
update the assertion to assert!(ctx.accounts.optional_account.is_none());
alternatively, if the project prefers slice-shortening behavior, adjust
TestAccounts::context to accept shorter slices and map missing tail slots to
None instead (but prefer adding the sentinel in this test).

Comment on lines +88 to +90
fn main() {
println!("Final context test - context method is working!");
} No newline at end of file
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove main() from integration test.

Integration tests are built with the test harness main; providing your own main can conflict.

-fn main() {
-    println!("Final context test - context method is working!");
-}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fn main() {
println!("Final context test - context method is working!");
}
🤖 Prompt for AI Agents
In shank/tests/final_context_test.rs around lines 88 to 90, the test file
defines a standalone main() which conflicts with Rust's integration test
harness; remove the main() function (and its println!) so the file only contains
test functions or helper code, ensuring the test harness supplies the entry
point.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review continued from previous batch...

Comment on lines +13 to +26
#[derive(ShankAccounts)]
pub struct CreateTokenAccounts<'info> {
#[account(mut, signer, desc = "Payer and authority")]
pub payer: AccountInfo<'info>,

#[account(mut, desc = "Token mint to create")]
pub mint: AccountInfo<'info>,

#[account(desc = "System program")]
pub system_program: AccountInfo<'info>,

#[account(desc = "Token program")]
pub token_program: AccountInfo<'info>,
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

🧩 Analysis chain

Fix compile error: visibility qualifiers on inner items (E0449)

Rust forbids pub structs/enums (and the pub methods the macro generates) inside function bodies. Hoist these type/enum definitions to module scope (top of the file) or remove pub and move them out of the test functions. Otherwise, derives will expand to pub impls inside fn and fail to compile.

Apply this pattern (example for the first test; replicate for the rest):

@@
 #[test]
 fn test_accounts_with_instruction_integration() {
-    #[derive(ShankAccounts)]
-    pub struct CreateTokenAccounts<'info> {
-        #[account(mut, signer, desc = "Payer and authority")]
-        pub payer: AccountInfo<'info>,
-        #[account(mut, desc = "Token mint to create")]
-        pub mint: AccountInfo<'info>,
-        #[account(desc = "System program")]
-        pub system_program: AccountInfo<'info>,
-        #[account(desc = "Token program")]
-        pub token_program: AccountInfo<'info>,
-    }
-
-    #[derive(ShankInstruction)]
-    pub enum TokenInstruction {
-        #[accounts(CreateTokenAccounts)]
-        CreateToken { 
-            decimals: u8,
-            supply: u64,
-        },
-    }
+    // Types moved to module scope; test body unchanged
     let accounts_metadata = CreateTokenAccounts::__shank_accounts();
     assert_eq!(accounts_metadata.len(), 4);
@@
 }

Place near the top of this file (module scope):

#[derive(ShankAccounts)]
pub struct CreateTokenAccounts<'info> {
    #[account(mut, signer, desc = "Payer and authority")]
    pub payer: AccountInfo<'info>,
    #[account(mut, desc = "Token mint to create")]
    pub mint: AccountInfo<'info>,
    #[account(desc = "System program")]
    pub system_program: AccountInfo<'info>,
    #[account(desc = "Token program")]
    pub token_program: AccountInfo<'info>,
}

#[derive(ShankInstruction)]
pub enum TokenInstruction {
    #[accounts(CreateTokenAccounts)]
    CreateToken { 
        decimals: u8,
        supply: u64,
    },
}

Repeat this hoisting for:

  • InitializeAccounts, TransferAccounts, CloseAccounts, ProgramInstruction
  • BasicAccounts, CreateAccounts, UpdateAccounts, AdminAccounts, ComplexProgram
  • ComplexDataAccounts, DataInstruction
  • SharedAccounts, SharedInstruction

Also applies to: 29-36, 56-64, 66-80, 82-91, 94-109, 133-136, 139-148, 151-161, 164-174, 178-202, 225-231, 235-251, 262-270, 273-282


🏁 Script executed:

#!/bin/bash
# Verify no inner items with visibility qualifiers remain in this test file
rg -nP '^\s*pub\s+(struct|enum)\s' shank/tests/instruction_integration_test.rs -n -C0
# Expect: only module-scope hits at top-level, none inside #[test] fns.

Length of output: 650


🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE="shank/tests/instruction_integration_test.rs"
echo ">>> Matches (with 6 lines of context):"
rg -n -C6 '^\s*pub\s+(struct|enum)\s' "$FILE" || true

echo
echo ">>> Context blocks around each match (±10 lines):"
rg -n --only-matching '^\s*pub\s+(struct|enum)\s' "$FILE" | cut -d: -f1 | sort -n | uniq | while read -r ln; do
  start=$((ln - 10)); if [ "$start" -lt 1 ]; then start=1; fi
  end=$((ln + 10))
  echo "----- lines $start-$end -----"
  sed -n "${start},${end}p" "$FILE" | nl -v${start} -ba -w3 -s": "
done

Length of output: 6422


Hoist pub structs/enums to module scope — fixes E0449

Confirmed: multiple pub struct / pub enum definitions are declared inside #[test] functions in shank/tests/instruction_integration_test.rs. Move these type definitions to module scope (near the top of the file) so derives expand to module-level pub impls; keep test bodies unchanged.

Hoist the following (file:line):

  • CreateTokenAccounts — line 14
  • TokenInstruction — line 30
  • InitializeAccounts — line 56
  • TransferAccounts — line 67
  • CloseAccounts — line 82
  • ProgramInstruction — line 95
  • BasicAccounts — line 133
  • CreateAccounts — line 140
  • UpdateAccounts — line 152
  • AdminAccounts — line 165
  • ComplexProgram — line 178
  • ComplexDataAccounts — line 225
  • DataInstruction — line 235
  • SharedAccounts — line 262
  • SharedInstruction — line 273
🤖 Prompt for AI Agents
In shank/tests/instruction_integration_test.rs around lines 13 to 273, several
pub struct/enum definitions are declared inside #[test] functions which causes
E0449; hoist the listed types to module scope (near the top of the file) so
their derived pub impls expand at module level: move CreateTokenAccounts (line
14), TokenInstruction (line 30), InitializeAccounts (56), TransferAccounts (67),
CloseAccounts (82), ProgramInstruction (95), BasicAccounts (133), CreateAccounts
(140), UpdateAccounts (152), AdminAccounts (165), ComplexProgram (178),
ComplexDataAccounts (225), DataInstruction (235), SharedAccounts (262), and
SharedInstruction (273) out of test functions into top-level module scope while
leaving the test bodies unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant