Skip to content

Commit

Permalink
BE: Extract find_call_in_function callback arguments into a "context"…
Browse files Browse the repository at this point in the history
… type

Summary: The list of args may grow and change, this will allow for greater flexibility during development and avoid the necessity of using a lot of _args.

Reviewed By: alanz

Differential Revision: D48468510

fbshipit-source-id: 6d2f16d069fecc5481d3f9c0857862152006d91a
  • Loading branch information
ir-regular authored and facebook-github-bot committed Aug 22, 2023
1 parent 83f75a6 commit 0916de8
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 13 deletions.
27 changes: 18 additions & 9 deletions crates/ide/src/codemod_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,15 +287,17 @@ impl MFA {
}
}

pub struct CheckCallCtx<'a, T> {
pub mfa: &'a FunctionMatch,
pub target: &'a CallTarget<ExprId>,
pub t: &'a T,
pub args: &'a [ExprId],
pub def_fb: &'a InFunctionBody<&'a FunctionDef>,
}

/// Check a specific call instance, and return the contents of a
/// diagnostic if needed.
pub type CheckCall<'a, T> = &'a dyn Fn(
&FunctionMatch,
&T,
&CallTarget<ExprId>,
&[ExprId],
&InFunctionBody<&FunctionDef>,
) -> Option<String>;
pub type CheckCall<'a, T> = &'a dyn Fn(CheckCallCtx<T>) -> Option<String>;

pub(crate) fn find_call_in_function<T>(
diags: &mut Vec<Diagnostic>,
Expand All @@ -321,7 +323,14 @@ pub(crate) fn find_call_in_function<T>(
&mut |acc, _, ctx| {
if let Expr::Call { target, args } = ctx.expr {
if let Some((mfa, t)) = matcher.get_match(&target, &args, sema, &def_fb.body()) {
if let Some(match_descr) = check_call(mfa, t, &target, &args, &def_fb) {
let context = CheckCallCtx {
mfa,
t,
target: &target,
args: &args,
def_fb: &def_fb,
};
if let Some(match_descr) = check_call(context) {
// Got one.
let call_expr_id = if let Some(expr_id) = ctx.in_macro {
expr_id
Expand Down Expand Up @@ -401,7 +410,7 @@ mod tests {
sema,
def,
&mfas,
&move |_mfa, _, _target, _args, _def_fb| Some("Diagnostic Message".to_string()),
&move |_ctx| Some("Diagnostic Message".to_string()),
move |_sema, def_fb, __target, _args, extra_info, range| {
let diag = Diagnostic::new(
DiagnosticCode::AdHoc("test".to_string()),
Expand Down
5 changes: 4 additions & 1 deletion crates/ide/src/diagnostics/application_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use lazy_static::lazy_static;

use super::Diagnostic;
use crate::codemod_helpers::find_call_in_function;
use crate::codemod_helpers::CheckCallCtx;
use crate::codemod_helpers::FunctionMatch;
// @fb-only: use crate::diagnostics;
use crate::diagnostics::DiagnosticCode;
Expand Down Expand Up @@ -100,7 +101,9 @@ pub(crate) fn process_badmatches(
sema,
def,
&mfas,
&move |_mfa, action, _target, args, def_fb| match action {
&move |CheckCallCtx {
t, args, def_fb, ..
}: CheckCallCtx<'_, &BadEnvCallAction>| match t {
BadEnvCallAction::AppArg(arg_index) => {
let arg = args.get(*arg_index)?;
check_valid_application(sema, def_fb, arg, def)
Expand Down
2 changes: 1 addition & 1 deletion crates/ide/src/diagnostics/cross_node_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ pub(crate) fn process_badmatches(
sema,
def,
&mfas,
&move |_mfa, _, _target, _args, _def_fb| {
&move |_ctx| {
Some(r#"Production code must not use cross node eval (e.g. `rpc:call()`)"#.to_string())
},
move |_sema, def_fb, _target, _args, extra_info, range| {
Expand Down
5 changes: 3 additions & 2 deletions crates/ide/src/diagnostics/replace_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use super::Severity;
use crate::codemod_helpers::find_call_in_function;
use crate::codemod_helpers::statement_range;
use crate::codemod_helpers::CheckCall;
use crate::codemod_helpers::CheckCallCtx;
use crate::codemod_helpers::FunctionMatch;
use crate::codemod_helpers::MFA;
use crate::diagnostics::DiagnosticCode;
Expand All @@ -50,7 +51,7 @@ pub fn replace_call_site(
) {
replace_call_site_if_args_match(
mfa,
&|_mfa, _, _target, _args, _def_fb| Some("".to_string()),
&|_ctx| Some("".to_string()),
replacement,
diagnostic_builder,
acc,
Expand Down Expand Up @@ -381,7 +382,7 @@ mod tests {
name: "fire_bombs".into(),
arity: 2,
}),
&|_, _, _target, args, def_fb| match &def_fb[args[1]] {
&|CheckCallCtx { args, def_fb, .. }: CheckCallCtx<()>| match def_fb[args[1]] {
Expr::Literal(Literal::Integer(42)) => Some("with 42".to_string()),
_ => None,
},
Expand Down

0 comments on commit 0916de8

Please sign in to comment.