From de1fc70ccd3bf7a0850e036a12cf866a80d46458 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 20:32:54 +0300 Subject: internal: refactor find_map diagnostic --- crates/hir/src/diagnostics.rs | 19 +-- crates/hir/src/lib.rs | 11 +- crates/ide/src/diagnostics.rs | 88 +--------- crates/ide/src/diagnostics/fixes.rs | 1 - .../src/diagnostics/fixes/replace_with_find_map.rs | 84 ---------- .../replace_filter_map_next_with_find_map.rs | 182 +++++++++++++++++++++ 6 files changed, 192 insertions(+), 193 deletions(-) delete mode 100644 crates/ide/src/diagnostics/fixes/replace_with_find_map.rs create mode 100644 crates/ide/src/diagnostics/replace_filter_map_next_with_find_map.rs diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 9afee0b90..c294a803b 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -41,6 +41,7 @@ diagnostics![ MissingUnsafe, NoSuchField, RemoveThisSemicolon, + ReplaceFilterMapNextWithFindMap, UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedImport, @@ -121,9 +122,6 @@ pub struct MissingFields { pub missed_fields: Vec, } -// Diagnostic: replace-filter-map-next-with-find-map -// -// This diagnostic is triggered when `.filter_map(..).next()` is used, rather than the more concise `.find_map(..)`. #[derive(Debug)] pub struct ReplaceFilterMapNextWithFindMap { pub file: HirFileId, @@ -131,21 +129,6 @@ pub struct ReplaceFilterMapNextWithFindMap { pub next_expr: AstPtr, } -impl Diagnostic for ReplaceFilterMapNextWithFindMap { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("replace-filter-map-next-with-find-map") - } - fn message(&self) -> String { - "replace filter_map(..).next() with find_map(..)".to_string() - } - fn display_source(&self) -> InFile { - InFile { file_id: self.file, value: self.next_expr.clone().into() } - } - fn as_any(&self) -> &(dyn Any + Send + 'static) { - self - } -} - #[derive(Debug)] pub struct MismatchedArgCount { pub call_expr: InFile>, diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index aaab5336a..b2731b62f 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1168,10 +1168,13 @@ impl Function { } BodyValidationDiagnostic::ReplaceFilterMapNextWithFindMap { method_call_expr } => { if let Ok(next_source_ptr) = source_map.expr_syntax(method_call_expr) { - sink.push(ReplaceFilterMapNextWithFindMap { - file: next_source_ptr.file_id, - next_expr: next_source_ptr.value, - }); + acc.push( + ReplaceFilterMapNextWithFindMap { + file: next_source_ptr.file_id, + next_expr: next_source_ptr.value, + } + .into(), + ); } } BodyValidationDiagnostic::MismatchedArgCount { call_expr, expected, found } => { diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index e3974e1ae..ec5318594 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -13,6 +13,7 @@ mod missing_ok_or_some_in_tail_expr; mod missing_unsafe; mod no_such_field; mod remove_this_semicolon; +mod replace_filter_map_next_with_find_map; mod unimplemented_builtin_macro; mod unresolved_extern_crate; mod unresolved_import; @@ -167,9 +168,6 @@ pub(crate) fn diagnostics( .on::(|d| { res.borrow_mut().push(warning_with_fix(d, &sema, resolve)); }) - .on::(|d| { - res.borrow_mut().push(warning_with_fix(d, &sema, resolve)); - }) .on::(|d| { // Limit diagnostic to the first few characters in the file. This matches how VS Code // renders it with the full span, but on other editors, and is less invasive. @@ -225,6 +223,7 @@ pub(crate) fn diagnostics( AnyDiagnostic::MissingUnsafe(d) => missing_unsafe::missing_unsafe(&ctx, &d), AnyDiagnostic::NoSuchField(d) => no_such_field::no_such_field(&ctx, &d), AnyDiagnostic::RemoveThisSemicolon(d) => remove_this_semicolon::remove_this_semicolon(&ctx, &d), + AnyDiagnostic::ReplaceFilterMapNextWithFindMap(d) => replace_filter_map_next_with_find_map::replace_filter_map_next_with_find_map(&ctx, &d), AnyDiagnostic::UnimplementedBuiltinMacro(d) => unimplemented_builtin_macro::unimplemented_builtin_macro(&ctx, &d), AnyDiagnostic::UnresolvedExternCrate(d) => unresolved_extern_crate::unresolved_extern_crate(&ctx, &d), AnyDiagnostic::UnresolvedImport(d) => unresolved_import::unresolved_import(&ctx, &d), @@ -672,89 +671,6 @@ mod foo; ); } - // Register the required standard library types to make the tests work - fn add_filter_map_with_find_next_boilerplate(body: &str) -> String { - let prefix = r#" - //- /main.rs crate:main deps:core - use core::iter::Iterator; - use core::option::Option::{self, Some, None}; - "#; - let suffix = r#" - //- /core/lib.rs crate:core - pub mod option { - pub enum Option { Some(T), None } - } - pub mod iter { - pub trait Iterator { - type Item; - fn filter_map(self, f: F) -> FilterMap where F: FnMut(Self::Item) -> Option { FilterMap } - fn next(&mut self) -> Option; - } - pub struct FilterMap {} - impl Iterator for FilterMap { - type Item = i32; - fn next(&mut self) -> i32 { 7 } - } - } - "#; - format!("{}{}{}", prefix, body, suffix) - } - - #[test] - fn replace_filter_map_next_with_find_map2() { - check_diagnostics(&add_filter_map_with_find_next_boilerplate( - r#" - fn foo() { - let m = [1, 2, 3].iter().filter_map(|x| if *x == 2 { Some (4) } else { None }).next(); - //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ replace filter_map(..).next() with find_map(..) - } - "#, - )); - } - - #[test] - fn replace_filter_map_next_with_find_map_no_diagnostic_without_next() { - check_diagnostics(&add_filter_map_with_find_next_boilerplate( - r#" - fn foo() { - let m = [1, 2, 3] - .iter() - .filter_map(|x| if *x == 2 { Some (4) } else { None }) - .len(); - } - "#, - )); - } - - #[test] - fn replace_filter_map_next_with_find_map_no_diagnostic_with_intervening_methods() { - check_diagnostics(&add_filter_map_with_find_next_boilerplate( - r#" - fn foo() { - let m = [1, 2, 3] - .iter() - .filter_map(|x| if *x == 2 { Some (4) } else { None }) - .map(|x| x + 2) - .len(); - } - "#, - )); - } - - #[test] - fn replace_filter_map_next_with_find_map_no_diagnostic_if_not_in_chain() { - check_diagnostics(&add_filter_map_with_find_next_boilerplate( - r#" - fn foo() { - let m = [1, 2, 3] - .iter() - .filter_map(|x| if *x == 2 { Some (4) } else { None }); - let n = m.next(); - } - "#, - )); - } - #[test] fn missing_record_pat_field_no_diagnostic_if_not_exhaustive() { check_diagnostics( diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs index 350575b3a..e4bd90c3f 100644 --- a/crates/ide/src/diagnostics/fixes.rs +++ b/crates/ide/src/diagnostics/fixes.rs @@ -1,7 +1,6 @@ //! Provides a way to attach fixes to the diagnostics. //! The same module also has all curret custom fixes for the diagnostics implemented. mod change_case; -mod replace_with_find_map; use hir::{diagnostics::Diagnostic, Semantics}; use ide_assists::AssistResolveStrategy; diff --git a/crates/ide/src/diagnostics/fixes/replace_with_find_map.rs b/crates/ide/src/diagnostics/fixes/replace_with_find_map.rs deleted file mode 100644 index 444bf563b..000000000 --- a/crates/ide/src/diagnostics/fixes/replace_with_find_map.rs +++ /dev/null @@ -1,84 +0,0 @@ -use hir::{db::AstDatabase, diagnostics::ReplaceFilterMapNextWithFindMap, Semantics}; -use ide_assists::{Assist, AssistResolveStrategy}; -use ide_db::{source_change::SourceChange, RootDatabase}; -use syntax::{ - ast::{self, ArgListOwner}, - AstNode, TextRange, -}; -use text_edit::TextEdit; - -use crate::diagnostics::{fix, DiagnosticWithFixes}; - -impl DiagnosticWithFixes for ReplaceFilterMapNextWithFindMap { - fn fixes( - &self, - sema: &Semantics, - _resolve: &AssistResolveStrategy, - ) -> Option> { - let root = sema.db.parse_or_expand(self.file)?; - let next_expr = self.next_expr.to_node(&root); - let next_call = ast::MethodCallExpr::cast(next_expr.syntax().clone())?; - - let filter_map_call = ast::MethodCallExpr::cast(next_call.receiver()?.syntax().clone())?; - let filter_map_name_range = filter_map_call.name_ref()?.ident_token()?.text_range(); - let filter_map_args = filter_map_call.arg_list()?; - - let range_to_replace = - TextRange::new(filter_map_name_range.start(), next_expr.syntax().text_range().end()); - let replacement = format!("find_map{}", filter_map_args.syntax().text()); - let trigger_range = next_expr.syntax().text_range(); - - let edit = TextEdit::replace(range_to_replace, replacement); - - let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit); - - Some(vec![fix( - "replace_with_find_map", - "Replace filter_map(..).next() with find_map()", - source_change, - trigger_range, - )]) - } -} - -#[cfg(test)] -mod tests { - use crate::diagnostics::tests::check_fix; - - #[test] - fn replace_with_wind_map() { - check_fix( - r#" -//- /main.rs crate:main deps:core -use core::iter::Iterator; -use core::option::Option::{self, Some, None}; -fn foo() { - let m = [1, 2, 3].iter().$0filter_map(|x| if *x == 2 { Some (4) } else { None }).next(); -} -//- /core/lib.rs crate:core -pub mod option { - pub enum Option { Some(T), None } -} -pub mod iter { - pub trait Iterator { - type Item; - fn filter_map(self, f: F) -> FilterMap where F: FnMut(Self::Item) -> Option { FilterMap } - fn next(&mut self) -> Option; - } - pub struct FilterMap {} - impl Iterator for FilterMap { - type Item = i32; - fn next(&mut self) -> i32 { 7 } - } -} -"#, - r#" -use core::iter::Iterator; -use core::option::Option::{self, Some, None}; -fn foo() { - let m = [1, 2, 3].iter().find_map(|x| if *x == 2 { Some (4) } else { None }); -} -"#, - ) - } -} diff --git a/crates/ide/src/diagnostics/replace_filter_map_next_with_find_map.rs b/crates/ide/src/diagnostics/replace_filter_map_next_with_find_map.rs new file mode 100644 index 000000000..f3b011495 --- /dev/null +++ b/crates/ide/src/diagnostics/replace_filter_map_next_with_find_map.rs @@ -0,0 +1,182 @@ +use hir::{db::AstDatabase, InFile}; +use ide_db::source_change::SourceChange; +use syntax::{ + ast::{self, ArgListOwner}, + AstNode, TextRange, +}; +use text_edit::TextEdit; + +use crate::{ + diagnostics::{fix, Diagnostic, DiagnosticsContext}, + Assist, Severity, +}; + +// Diagnostic: replace-filter-map-next-with-find-map +// +// This diagnostic is triggered when `.filter_map(..).next()` is used, rather than the more concise `.find_map(..)`. +pub(super) fn replace_filter_map_next_with_find_map( + ctx: &DiagnosticsContext<'_>, + d: &hir::ReplaceFilterMapNextWithFindMap, +) -> Diagnostic { + Diagnostic::new( + "replace-filter-map-next-with-find-map", + "replace filter_map(..).next() with find_map(..)", + ctx.sema.diagnostics_display_range(InFile::new(d.file, d.next_expr.clone().into())).range, + ) + .severity(Severity::WeakWarning) + .with_fixes(fixes(ctx, d)) +} + +fn fixes( + ctx: &DiagnosticsContext<'_>, + d: &hir::ReplaceFilterMapNextWithFindMap, +) -> Option> { + let root = ctx.sema.db.parse_or_expand(d.file)?; + let next_expr = d.next_expr.to_node(&root); + let next_call = ast::MethodCallExpr::cast(next_expr.syntax().clone())?; + + let filter_map_call = ast::MethodCallExpr::cast(next_call.receiver()?.syntax().clone())?; + let filter_map_name_range = filter_map_call.name_ref()?.ident_token()?.text_range(); + let filter_map_args = filter_map_call.arg_list()?; + + let range_to_replace = + TextRange::new(filter_map_name_range.start(), next_expr.syntax().text_range().end()); + let replacement = format!("find_map{}", filter_map_args.syntax().text()); + let trigger_range = next_expr.syntax().text_range(); + + let edit = TextEdit::replace(range_to_replace, replacement); + + let source_change = SourceChange::from_text_edit(d.file.original_file(ctx.sema.db), edit); + + Some(vec![fix( + "replace_with_find_map", + "Replace filter_map(..).next() with find_map()", + source_change, + trigger_range, + )]) +} + +#[cfg(test)] +mod tests { + use crate::diagnostics::tests::check_fix; + + // Register the required standard library types to make the tests work + #[track_caller] + fn check_diagnostics(ra_fixture: &str) { + let prefix = r#" +//- /main.rs crate:main deps:core +use core::iter::Iterator; +use core::option::Option::{self, Some, None}; +"#; + let suffix = r#" +//- /core/lib.rs crate:core +pub mod option { + pub enum Option { Some(T), None } +} +pub mod iter { + pub trait Iterator { + type Item; + fn filter_map(self, f: F) -> FilterMap where F: FnMut(Self::Item) -> Option { FilterMap } + fn next(&mut self) -> Option; + } + pub struct FilterMap {} + impl Iterator for FilterMap { + type Item = i32; + fn next(&mut self) -> i32 { 7 } + } +} +"#; + crate::diagnostics::tests::check_diagnostics(&format!("{}{}{}", prefix, ra_fixture, suffix)) + } + + #[test] + fn replace_filter_map_next_with_find_map2() { + check_diagnostics( + r#" + fn foo() { + let m = [1, 2, 3].iter().filter_map(|x| if *x == 2 { Some (4) } else { None }).next(); + } //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ replace filter_map(..).next() with find_map(..) +"#, + ); + } + + #[test] + fn replace_filter_map_next_with_find_map_no_diagnostic_without_next() { + check_diagnostics( + r#" +fn foo() { + let m = [1, 2, 3] + .iter() + .filter_map(|x| if *x == 2 { Some (4) } else { None }) + .len(); +} +"#, + ); + } + + #[test] + fn replace_filter_map_next_with_find_map_no_diagnostic_with_intervening_methods() { + check_diagnostics( + r#" +fn foo() { + let m = [1, 2, 3] + .iter() + .filter_map(|x| if *x == 2 { Some (4) } else { None }) + .map(|x| x + 2) + .len(); +} +"#, + ); + } + + #[test] + fn replace_filter_map_next_with_find_map_no_diagnostic_if_not_in_chain() { + check_diagnostics( + r#" +fn foo() { + let m = [1, 2, 3] + .iter() + .filter_map(|x| if *x == 2 { Some (4) } else { None }); + let n = m.next(); +} +"#, + ); + } + + #[test] + fn replace_with_wind_map() { + check_fix( + r#" +//- /main.rs crate:main deps:core +use core::iter::Iterator; +use core::option::Option::{self, Some, None}; +fn foo() { + let m = [1, 2, 3].iter().$0filter_map(|x| if *x == 2 { Some (4) } else { None }).next(); +} +//- /core/lib.rs crate:core +pub mod option { + pub enum Option { Some(T), None } +} +pub mod iter { + pub trait Iterator { + type Item; + fn filter_map(self, f: F) -> FilterMap where F: FnMut(Self::Item) -> Option { FilterMap } + fn next(&mut self) -> Option; + } + pub struct FilterMap {} + impl Iterator for FilterMap { + type Item = i32; + fn next(&mut self) -> i32 { 7 } + } +} +"#, + r#" +use core::iter::Iterator; +use core::option::Option::{self, Some, None}; +fn foo() { + let m = [1, 2, 3].iter().find_map(|x| if *x == 2 { Some (4) } else { None }); +} +"#, + ) + } +} -- cgit v1.2.3