diff options
author | Phil Ellison <[email protected]> | 2020-12-30 15:46:05 +0000 |
---|---|---|
committer | Phil Ellison <[email protected]> | 2021-01-23 07:40:25 +0000 |
commit | 1ff860b93c972e0f8d3a8ee582c255fa59e9b284 (patch) | |
tree | 0533465ecb7329b6185c9259fb0f9c2b5ab0f7b0 /crates/hir_ty/src | |
parent | 1316422a7c2ef26e9da78fa23f170407b1cb39bb (diff) |
Implement fix, add tests
Diffstat (limited to 'crates/hir_ty/src')
-rw-r--r-- | crates/hir_ty/src/diagnostics.rs | 56 | ||||
-rw-r--r-- | crates/hir_ty/src/diagnostics/expr.rs | 27 |
2 files changed, 55 insertions, 28 deletions
diff --git a/crates/hir_ty/src/diagnostics.rs b/crates/hir_ty/src/diagnostics.rs index b4cf81c10..3d7663f6a 100644 --- a/crates/hir_ty/src/diagnostics.rs +++ b/crates/hir_ty/src/diagnostics.rs | |||
@@ -392,7 +392,7 @@ impl Diagnostic for IncorrectCase { | |||
392 | #[derive(Debug)] | 392 | #[derive(Debug)] |
393 | pub struct ReplaceFilterMapNextWithFindMap { | 393 | pub struct ReplaceFilterMapNextWithFindMap { |
394 | pub file: HirFileId, | 394 | pub file: HirFileId, |
395 | pub filter_map_expr: AstPtr<ast::Expr>, | 395 | /// This expression is the whole method chain up to and including `.filter_map(..).next()`. |
396 | pub next_expr: AstPtr<ast::Expr>, | 396 | pub next_expr: AstPtr<ast::Expr>, |
397 | } | 397 | } |
398 | 398 | ||
@@ -404,7 +404,7 @@ impl Diagnostic for ReplaceFilterMapNextWithFindMap { | |||
404 | "replace filter_map(..).next() with find_map(..)".to_string() | 404 | "replace filter_map(..).next() with find_map(..)".to_string() |
405 | } | 405 | } |
406 | fn display_source(&self) -> InFile<SyntaxNodePtr> { | 406 | fn display_source(&self) -> InFile<SyntaxNodePtr> { |
407 | InFile { file_id: self.file, value: self.filter_map_expr.clone().into() } | 407 | InFile { file_id: self.file, value: self.next_expr.clone().into() } |
408 | } | 408 | } |
409 | fn as_any(&self) -> &(dyn Any + Send + 'static) { | 409 | fn as_any(&self) -> &(dyn Any + Send + 'static) { |
410 | self | 410 | self |
@@ -671,15 +671,55 @@ fn foo() { break; } | |||
671 | } | 671 | } |
672 | 672 | ||
673 | #[test] | 673 | #[test] |
674 | fn replace_missing_filter_next_with_find_map() { | 674 | fn replace_filter_next_with_find_map() { |
675 | check_diagnostics( | 675 | check_diagnostics( |
676 | r#" | 676 | r#" |
677 | fn foo() { | 677 | fn foo() { |
678 | let m = [1, 2, 3] | 678 | let m = [1, 2, 3].iter().filter_map(|x| if *x == 2 { Some (4) } else { None }).next(); |
679 | .iter() | 679 | //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ replace filter_map(..).next() with find_map(..) |
680 | .filter_map(|x| if *x == 2 { Some (4) } else { None }) | 680 | } |
681 | .next(); | 681 | "#, |
682 | //^^^ Replace .filter_map(..).next() with .find_map(..) | 682 | ); |
683 | } | ||
684 | |||
685 | #[test] | ||
686 | fn replace_filter_next_with_find_map_no_diagnostic_without_next() { | ||
687 | check_diagnostics( | ||
688 | r#" | ||
689 | fn foo() { | ||
690 | let m = [1, 2, 3] | ||
691 | .iter() | ||
692 | .filter_map(|x| if *x == 2 { Some (4) } else { None }) | ||
693 | .len(); | ||
694 | } | ||
695 | "#, | ||
696 | ); | ||
697 | } | ||
698 | |||
699 | #[test] | ||
700 | fn replace_filter_next_with_find_map_no_diagnostic_with_intervening_methods() { | ||
701 | check_diagnostics( | ||
702 | r#" | ||
703 | fn foo() { | ||
704 | let m = [1, 2, 3] | ||
705 | .iter() | ||
706 | .filter_map(|x| if *x == 2 { Some (4) } else { None }) | ||
707 | .map(|x| x + 2) | ||
708 | .len(); | ||
709 | } | ||
710 | "#, | ||
711 | ); | ||
712 | } | ||
713 | |||
714 | #[test] | ||
715 | fn replace_filter_next_with_find_map_no_diagnostic_if_not_in_chain() { | ||
716 | check_diagnostics( | ||
717 | r#" | ||
718 | fn foo() { | ||
719 | let m = [1, 2, 3] | ||
720 | .iter() | ||
721 | .filter_map(|x| if *x == 2 { Some (4) } else { None }); | ||
722 | let n = m.next(); | ||
683 | } | 723 | } |
684 | "#, | 724 | "#, |
685 | ); | 725 | ); |
diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs index 170d23178..b87557ff5 100644 --- a/crates/hir_ty/src/diagnostics/expr.rs +++ b/crates/hir_ty/src/diagnostics/expr.rs | |||
@@ -41,16 +41,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> { | |||
41 | ExprValidator { owner, infer, sink } | 41 | ExprValidator { owner, infer, sink } |
42 | } | 42 | } |
43 | 43 | ||
44 | fn bar() { | ||
45 | // LOOK FOR THIS | ||
46 | let m = [1, 2, 3] | ||
47 | .iter() | ||
48 | .filter_map(|x| if *x == 2 { Some(4) } else { None }) | ||
49 | .next(); | ||
50 | } | ||
51 | |||
52 | pub(super) fn validate_body(&mut self, db: &dyn HirDatabase) { | 44 | pub(super) fn validate_body(&mut self, db: &dyn HirDatabase) { |
53 | // DO NOT MERGE: just getting something working for now | ||
54 | self.check_for_filter_map_next(db); | 45 | self.check_for_filter_map_next(db); |
55 | 46 | ||
56 | let body = db.body(self.owner.into()); | 47 | let body = db.body(self.owner.into()); |
@@ -169,24 +160,20 @@ impl<'a, 'b> ExprValidator<'a, 'b> { | |||
169 | 160 | ||
170 | for (id, expr) in body.exprs.iter() { | 161 | for (id, expr) in body.exprs.iter() { |
171 | if let Expr::MethodCall { receiver, method_name, args, .. } = expr { | 162 | if let Expr::MethodCall { receiver, method_name, args, .. } = expr { |
172 | let method_name_hack_do_not_merge = format!("{}", method_name); | 163 | let method_name = format!("{}", method_name); |
173 | 164 | ||
174 | if method_name_hack_do_not_merge == "filter_map" && args.len() == 1 { | 165 | if method_name == "filter_map" && args.len() == 1 { |
175 | prev = Some((id, args[0])); | 166 | prev = Some(id); |
176 | continue; | 167 | continue; |
177 | } | 168 | } |
178 | 169 | ||
179 | if method_name_hack_do_not_merge == "next" { | 170 | if method_name == "next" { |
180 | if let Some((filter_map_id, filter_map_args)) = prev { | 171 | if let Some(filter_map_id) = prev { |
181 | if *receiver == filter_map_id { | 172 | if *receiver == filter_map_id { |
182 | let (_, source_map) = db.body_with_source_map(self.owner.into()); | 173 | let (_, source_map) = db.body_with_source_map(self.owner.into()); |
183 | if let (Ok(filter_map_source_ptr), Ok(next_source_ptr)) = ( | 174 | if let Ok(next_source_ptr) = source_map.expr_syntax(id) { |
184 | source_map.expr_syntax(filter_map_id), | ||
185 | source_map.expr_syntax(id), | ||
186 | ) { | ||
187 | self.sink.push(ReplaceFilterMapNextWithFindMap { | 175 | self.sink.push(ReplaceFilterMapNextWithFindMap { |
188 | file: filter_map_source_ptr.file_id, | 176 | file: next_source_ptr.file_id, |
189 | filter_map_expr: filter_map_source_ptr.value, | ||
190 | next_expr: next_source_ptr.value, | 177 | next_expr: next_source_ptr.value, |
191 | }); | 178 | }); |
192 | } | 179 | } |