Skip to content

Commit

Permalink
Upd missing_owner_check to not report local variables, duplicate warn…
Browse files Browse the repository at this point in the history
…ings when to_account_info is called on Anchor's AccountInfo.
  • Loading branch information
Vara Prasad Bandaru committed Jan 30, 2024
1 parent 3c7044b commit d34ddca
Showing 1 changed file with 23 additions and 1 deletion.
24 changes: 23 additions & 1 deletion lints/missing_owner_check/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use clippy_utils::{
use if_chain::if_chain;
use rustc_hir::{
intravisit::{walk_expr, FnKind, Visitor},
Body, Expr, ExprKind, FnDecl, HirId,
Body, Expr, ExprKind, FnDecl, HirId, QPath
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
Expand Down Expand Up @@ -107,6 +107,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for AccountUses<'cx, 'tcx> {
if !is_call_to_clone(self.cx, expr);
let ty = self.cx.typeck_results().expr_ty(expr);
if match_type(self.cx, ty, &paths::SOLANA_PROGRAM_ACCOUNT_INFO);
if !is_expr_local_variable(expr);
if !is_safe_to_account_info(self.cx, expr);
let mut spanless_eq = SpanlessEq::new(self.cx);
if !self.uses.iter().any(|e| spanless_eq.eq_expr(e, expr));
Expand All @@ -118,6 +119,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for AccountUses<'cx, 'tcx> {
}
}

// s3v3ru5: the following check removes duplicate warnings where lint would report both `x` and `x.clone()` expressions.
/// Return true if the expr is a method call to clone else false
fn is_call_to_clone<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
if_chain! {
Expand All @@ -132,6 +134,24 @@ fn is_call_to_clone<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> boo
}
}

// s3v3ru5: if a local variable is of type AccountInfo, the rhs of the let statement assigning to variable
// will be of type AccountInfo. The lint would check that expression and there is no need for checking the
// local variable as well.
// This removes the false positives of following pattern:
// `let x = {Account, Program, ...verified structs}.to_account_info()`,
// the lint reports uses of `x`. Having this check would remove such false positives.
fn is_expr_local_variable<'tcx>(expr: &'tcx Expr<'tcx>) -> bool {
if_chain! {
if let ExprKind::Path(QPath::Resolved(None, path)) = expr.kind;
if path.segments.len() == 1;
then {
true
} else {
false
}
}
}

// smoelius: See: https://github.com/crytic/solana-lints/issues/31
fn is_safe_to_account_info<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
if_chain! {
Expand All @@ -157,6 +177,8 @@ fn is_safe_to_account_info<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>)
&paths::ANCHOR_LANG_ACCOUNT_LOADER,
&paths::ANCHOR_LANG_SIGNER,
&paths::ANCHOR_LANG_SYSVAR,
// s3v3ru5: The following line will remove duplicate warnings where lint reports both `x` and `x.to_account_info()` when x is of type Anchor's AccountInfo.
&paths::SOLANA_PROGRAM_ACCOUNT_INFO,
],
)
.is_some();
Expand Down

0 comments on commit d34ddca

Please sign in to comment.