Skip to content

Commit f98808d

Browse files
oslfmtoslfmt
committed
fix lint messages when no constraints at all to recommend anchor constraints
1 parent be98da5 commit f98808d

File tree

4 files changed

+89
-60
lines changed

4 files changed

+89
-60
lines changed

‎lints/duplicate-mutable-accounts/src/alternate_constraint.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,12 @@ use clippy_utils::{ty::match_type, SpanlessEq};
1313
use if_chain::if_chain;
1414
use solana_lints::paths;
1515

16+
/// Stores the accounts and if-statements (constraints) found in a function body.
1617
pub struct Values<'cx, 'tcx> {
1718
cx: &'cx LateContext<'tcx>,
19+
/// Lists of account expressions, partitioned by the Account type T
1820
pub accounts: HashMap<DefId, Vec<&'tcx Expr<'tcx>>>,
21+
/// List of tuples, where (x, y), where x is the left operand of the if statement and y is the right
1922
pub if_statements: Vec<(&'tcx Expr<'tcx>, &'tcx Expr<'tcx>)>,
2023
}
2124

@@ -34,7 +37,6 @@ impl<'cx, 'tcx> Values<'cx, 'tcx> {
3437
}
3538

3639
/// Checks if there is a valid key constraint for `first_account` and `second_account`.
37-
/// NOTE: currently only considers `first.key() == second.key()` or the symmetric relation as valid constraints.
3840
/// TODO: if == relation used, should return some error in the THEN block
3941
pub fn check_key_constraint(
4042
&self,

‎lints/duplicate-mutable-accounts/src/lib.rs‎

Lines changed: 54 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,14 @@ dylint_linting::impl_late_lint! {
7979

8080
#[derive(Default, Debug)]
8181
struct DuplicateMutableAccounts {
82-
accounts: HashMap<DefId, Vec<(Symbol, Span)>>,
83-
streams: Streams,
82+
/// Lists of Anchor accounts found in structs that derive Anchor `Accounts` trait, partitioned by Anchor account type
83+
anchor_accounts: HashMap<DefId, Vec<(Symbol, Span)>>,
84+
/// List of Anchor `#[account]` macro constraints
85+
anchor_macro_constraints: Streams,
86+
/// List of pairs of Anchor accounts with same types, without any alternate constraint
8487
spans: Vec<(Span, Span)>,
88+
/// Indicates if alternate constraints were used or not
89+
no_alternate_constraints: bool,
8590
}
8691

8792
impl<'tcx> LateLintPass<'tcx> for DuplicateMutableAccounts {
@@ -94,10 +99,10 @@ impl<'tcx> LateLintPass<'tcx> for DuplicateMutableAccounts {
9499
if match_type(cx, middle_ty, &paths::ANCHOR_ACCOUNT);
95100
if let Some(account_id) = get_anchor_account_type_def_id(field);
96101
then {
97-
if let Some(v) = self.accounts.get_mut(&account_id) {
102+
if let Some(v) = self.anchor_accounts.get_mut(&account_id) {
98103
v.push((field.ident.name, field.span));
99104
} else {
100-
self.accounts.insert(account_id, vec![(field.ident.name, field.span)]);
105+
self.anchor_accounts.insert(account_id, vec![(field.ident.name, field.span)]);
101106
}
102107
}
103108
}
@@ -112,7 +117,7 @@ impl<'tcx> LateLintPass<'tcx> for DuplicateMutableAccounts {
112117
if name.as_str() == "account";
113118
if let MacArgs::Delimited(_, _, token_stream) = &attr_item.args;
114119
then {
115-
self.streams.0.push(token_stream.clone());
120+
self.anchor_macro_constraints.0.push(token_stream.clone());
116121
}
117122
}
118123
}
@@ -127,19 +132,19 @@ impl<'tcx> LateLintPass<'tcx> for DuplicateMutableAccounts {
127132
_: HirId,
128133
) {
129134
if !span.from_expansion() {
130-
// get all mutable references to Accounts and if_statements in body
131135
let mut values = Values::new(cx);
132136
values.get_referenced_accounts_and_if_statements(body);
133137

134-
// NOTE: could do this check in check_post_crate if exprs are replaced with HirId, then use
135-
// the HirId to fetch the expr
136138
values.accounts.values().for_each(|exprs| {
137139
if exprs.len() > 1 {
140+
self.no_alternate_constraints = true; // assume no alternate constraints
138141
for current in 0..exprs.len() - 1 {
139142
for next in current + 1..exprs.len() {
140143
if !values.check_key_constraint(exprs[current], exprs[next]) {
141-
// store for later spanning
142144
self.spans.push((exprs[current].span, exprs[next].span));
145+
} else {
146+
// if there is at least one alt constraint, set flag to false
147+
self.no_alternate_constraints = false;
143148
}
144149
}
145150
}
@@ -149,33 +154,55 @@ impl<'tcx> LateLintPass<'tcx> for DuplicateMutableAccounts {
149154
}
150155

151156
fn check_crate_post(&mut self, cx: &LateContext<'tcx>) {
152-
// if collected some anchor macro constraints then perform v1 lint
153-
if self.streams.0.is_empty() {
154-
// TODO: Not a fan of having it span lints for this check when there are no checks whatsoever.
155-
// I'd rather have it span lints to recommended anchor macros, if no checks are found at all
156-
for (first, second) in &self.spans {
157-
span_lint_and_help(
158-
cx,
159-
DUPLICATE_MUTABLE_ACCOUNTS,
160-
*first,
161-
&format!("the expressions on line {:?} and {:?} have identical Account types, yet do not contain a proper key check.", first, second),
162-
Some(*second),
163-
"add a key check to make sure the accounts have different keys, e.g., x.key() != y.key()",
164-
);
157+
// if no anchor constraints, check for alternate constraints
158+
if self.anchor_macro_constraints.0.is_empty() {
159+
// if no alternate constraints either, recommend using anchor constraints
160+
if self.no_alternate_constraints {
161+
for ident_accounts in self.anchor_accounts.values() {
162+
if ident_accounts.len() > 1 {
163+
for current in 0..ident_accounts.len() - 1 {
164+
for next in current + 1..ident_accounts.len() {
165+
let first = ident_accounts[current];
166+
let second = ident_accounts[next];
167+
span_lint_and_help(
168+
cx,
169+
DUPLICATE_MUTABLE_ACCOUNTS,
170+
first.1,
171+
&format!("{} and {} have identical account types but do not have a key check constraint", first.0, second.0),
172+
Some(second.1),
173+
&format!("add an anchor key check constraint: #[account(constraint = {}.key() != {}.key())]", first.0, second.0)
174+
);
175+
}
176+
}
177+
}
178+
}
179+
} else {
180+
// flag lint for missing alternate constraints
181+
for (first, second) in &self.spans {
182+
span_lint_and_help(
183+
cx,
184+
DUPLICATE_MUTABLE_ACCOUNTS,
185+
*first,
186+
&format!("the expressions on line {:?} and {:?} have identical Account types, yet do not contain a proper key check.", first, second),
187+
Some(*second),
188+
"add a key check to make sure the accounts have different keys, e.g., x.key() != y.key()",
189+
);
190+
}
165191
}
166192
} else {
167-
for v in self.accounts.values() {
168-
if v.len() > 1 {
169-
let mut deq = VecDeque::from(v.clone());
193+
// if using anchor constraints, check and flag for missing anchor constraints
194+
for ident_accounts in self.anchor_accounts.values() {
195+
if ident_accounts.len() > 1 {
196+
let mut deq = VecDeque::from(ident_accounts.clone());
170197
for _ in 0..deq.len() - 1 {
171198
let (first, first_span) = deq.pop_front().unwrap();
172199
for (other, other_span) in &deq {
173200
let stream = create_key_check_constraint_tokenstream(first, *other);
174201
let symmetric_stream =
175202
create_key_check_constraint_tokenstream(*other, first);
176203

177-
if !(self.streams.contains(&stream)
178-
|| self.streams.contains(&symmetric_stream))
204+
if !(self.anchor_macro_constraints.contains(&stream)
205+
|| self.anchor_macro_constraints.contains(&symmetric_stream))
179206
{
180207
span_lint_and_help(
181208
cx,

‎lints/duplicate-mutable-accounts/ui/insecure-2/src/lib.stderr‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,39 @@
1-
error: the expressions on line $DIR/lib.rs:15:27: 15:46 (#0) and $DIR/lib.rs:16:27: 16:46 (#0) have identical Account types, yet do not contain a proper key check.
2-
--> $DIR/lib.rs:15:27
1+
error: user_a and user_b have identical account types but do not have a key check constraint
2+
--> $DIR/lib.rs:28:5
33
|
4-
LL | let user_a = &mut ctx.accounts.user_a;
5-
| ^^^^^^^^^^^^^^^^^^^
4+
LL | user_a: Account<'info, User>,
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D duplicate-mutable-accounts` implied by `-D warnings`
8-
help: add a key check to make sure the accounts have different keys, e.g., x.key() != y.key()
9-
--> $DIR/lib.rs:16:27
8+
help: add an anchor key check constraint: #[account(constraint = user_a.key() != user_b.key())]
9+
--> $DIR/lib.rs:29:5
1010
|
11-
LL | let user_b = &mut ctx.accounts.user_b;
12-
| ^^^^^^^^^^^^^^^^^^^
11+
LL | user_b: Account<'info, User>,
12+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1313

14-
error: the expressions on line $DIR/lib.rs:15:27: 15:46 (#0) and $DIR/lib.rs:17:27: 17:46 (#0) have identical Account types, yet do not contain a proper key check.
15-
--> $DIR/lib.rs:15:27
14+
error: user_a and user_c have identical account types but do not have a key check constraint
15+
--> $DIR/lib.rs:28:5
1616
|
17-
LL | let user_a = &mut ctx.accounts.user_a;
18-
| ^^^^^^^^^^^^^^^^^^^
17+
LL | user_a: Account<'info, User>,
18+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1919
|
20-
help: add a key check to make sure the accounts have different keys, e.g., x.key() != y.key()
21-
--> $DIR/lib.rs:17:27
20+
help: add an anchor key check constraint: #[account(constraint = user_a.key() != user_c.key())]
21+
--> $DIR/lib.rs:30:5
2222
|
23-
LL | let user_c = &mut ctx.accounts.user_c;
24-
| ^^^^^^^^^^^^^^^^^^^
23+
LL | user_c: Account<'info, User>,
24+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2525

26-
error: the expressions on line $DIR/lib.rs:16:27: 16:46 (#0) and $DIR/lib.rs:17:27: 17:46 (#0) have identical Account types, yet do not contain a proper key check.
27-
--> $DIR/lib.rs:16:27
26+
error: user_b and user_c have identical account types but do not have a key check constraint
27+
--> $DIR/lib.rs:29:5
2828
|
29-
LL | let user_b = &mut ctx.accounts.user_b;
30-
| ^^^^^^^^^^^^^^^^^^^
29+
LL | user_b: Account<'info, User>,
30+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3131
|
32-
help: add a key check to make sure the accounts have different keys, e.g., x.key() != y.key()
33-
--> $DIR/lib.rs:17:27
32+
help: add an anchor key check constraint: #[account(constraint = user_b.key() != user_c.key())]
33+
--> $DIR/lib.rs:30:5
3434
|
35-
LL | let user_c = &mut ctx.accounts.user_c;
36-
| ^^^^^^^^^^^^^^^^^^^
35+
LL | user_c: Account<'info, User>,
36+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3737

3838
error: aborting due to 3 previous errors
3939

‎lints/duplicate-mutable-accounts/ui/insecure/src/lib.stderr‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,15 @@
1-
error: the expressions on line $DIR/lib.rs:14:27: 14:46 (#0) and $DIR/lib.rs:15:27: 15:46 (#0) have identical Account types, yet do not contain a proper key check.
2-
--> $DIR/lib.rs:14:27
1+
error: user_a and user_b have identical account types but do not have a key check constraint
2+
--> $DIR/lib.rs:25:5
33
|
4-
LL | let user_a = &mut ctx.accounts.user_a;
5-
| ^^^^^^^^^^^^^^^^^^^
4+
LL | user_a: Account<'info, User>,
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D duplicate-mutable-accounts` implied by `-D warnings`
8-
help: add a key check to make sure the accounts have different keys, e.g., x.key() != y.key()
9-
--> $DIR/lib.rs:15:27
8+
help: add an anchor key check constraint: #[account(constraint = user_a.key() != user_b.key())]
9+
--> $DIR/lib.rs:26:5
1010
|
11-
LL | let user_b = &mut ctx.accounts.user_b;
12-
| ^^^^^^^^^^^^^^^^^^^
11+
LL | user_b: Account<'info, User>,
12+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1313

1414
error: aborting due to previous error
1515

0 commit comments

Comments
 (0)