Skip to content

Commit

Permalink
Auto merge of #16647 - Young-Flash:fix_replace_filter_map_next_with_f…
Browse files Browse the repository at this point in the history
…ind_map, r=Veykril

fix: replace_filter_map_next_with_find_map shouldn't work for dyn trait

close #16596
  • Loading branch information
bors committed Feb 26, 2024
2 parents e1983d2 + 7a58a23 commit 8929853
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 9 deletions.
30 changes: 21 additions & 9 deletions crates/hir-ty/src/diagnostics/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use tracing::debug;
use triomphe::Arc;
use typed_arena::Arena;

use crate::Interner;
use crate::{
db::HirDatabase,
diagnostics::match_check::{
Expand Down Expand Up @@ -149,17 +150,18 @@ impl ExprValidator {
None => return,
};

if filter_map_next_checker
.get_or_insert_with(|| {
FilterMapNextChecker::new(&self.owner.resolver(db.upcast()), db)
})
.check(call_id, receiver, &callee)
.is_some()
{
let checker = filter_map_next_checker.get_or_insert_with(|| {
FilterMapNextChecker::new(&self.owner.resolver(db.upcast()), db)
});

if checker.check(call_id, receiver, &callee).is_some() {
self.diagnostics.push(BodyValidationDiagnostic::ReplaceFilterMapNextWithFindMap {
method_call_expr: call_id,
});
}

let receiver_ty = self.infer[*receiver].clone();
checker.prev_receiver_ty = Some(receiver_ty);
}
}

Expand Down Expand Up @@ -393,6 +395,7 @@ struct FilterMapNextChecker {
filter_map_function_id: Option<hir_def::FunctionId>,
next_function_id: Option<hir_def::FunctionId>,
prev_filter_map_expr_id: Option<ExprId>,
prev_receiver_ty: Option<chalk_ir::Ty<Interner>>,
}

impl FilterMapNextChecker {
Expand All @@ -417,7 +420,12 @@ impl FilterMapNextChecker {
),
None => (None, None),
};
Self { filter_map_function_id, next_function_id, prev_filter_map_expr_id: None }
Self {
filter_map_function_id,
next_function_id,
prev_filter_map_expr_id: None,
prev_receiver_ty: None,
}
}

// check for instances of .filter_map(..).next()
Expand All @@ -434,7 +442,11 @@ impl FilterMapNextChecker {

if *function_id == self.next_function_id? {
if let Some(prev_filter_map_expr_id) = self.prev_filter_map_expr_id {
if *receiver_expr_id == prev_filter_map_expr_id {
let is_dyn_trait = self
.prev_receiver_ty
.as_ref()
.map_or(false, |it| it.strip_references().dyn_trait().is_some());
if *receiver_expr_id == prev_filter_map_expr_id && !is_dyn_trait {
return Some(());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,21 @@ fn foo() {
);
}

#[test]
fn replace_filter_map_next_dont_work_for_not_sized_issues_16596() {
check_diagnostics(
r#"
//- minicore: iterators
fn foo() {
let mut j = [0].into_iter();
let i: &mut dyn Iterator<Item = i32> = &mut j;
let dummy_fn = |v| (v > 0).then_some(v + 1);
let _res = i.filter_map(dummy_fn).next();
}
"#,
);
}

#[test]
fn replace_filter_map_next_with_find_map_no_diagnostic_without_next() {
check_diagnostics(
Expand Down

0 comments on commit 8929853

Please sign in to comment.