internal: refactor find_map diagnostic

This commit is contained in:
Aleksey Kladov 2021-06-13 20:32:54 +03:00
parent 24262f9ff6
commit de1fc70ccd
6 changed files with 192 additions and 193 deletions

View File

@ -41,6 +41,7 @@ diagnostics![
MissingUnsafe,
NoSuchField,
RemoveThisSemicolon,
ReplaceFilterMapNextWithFindMap,
UnimplementedBuiltinMacro,
UnresolvedExternCrate,
UnresolvedImport,
@ -121,9 +122,6 @@ pub struct MissingFields {
pub missed_fields: Vec<Name>,
}
// 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<ast::Expr>,
}
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<SyntaxNodePtr> {
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<AstPtr<ast::Expr>>,

View File

@ -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 } => {

View File

@ -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::<hir::diagnostics::IncorrectCase, _>(|d| {
res.borrow_mut().push(warning_with_fix(d, &sema, resolve));
})
.on::<hir::diagnostics::ReplaceFilterMapNextWithFindMap, _>(|d| {
res.borrow_mut().push(warning_with_fix(d, &sema, resolve));
})
.on::<UnlinkedFile, _>(|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<T> { Some(T), None }
}
pub mod iter {
pub trait Iterator {
type Item;
fn filter_map<B, F>(self, f: F) -> FilterMap where F: FnMut(Self::Item) -> Option<B> { FilterMap }
fn next(&mut self) -> Option<Self::Item>;
}
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(

View File

@ -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;

View File

@ -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<RootDatabase>,
_resolve: &AssistResolveStrategy,
) -> Option<Vec<Assist>> {
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<T> { Some(T), None }
}
pub mod iter {
pub trait Iterator {
type Item;
fn filter_map<B, F>(self, f: F) -> FilterMap where F: FnMut(Self::Item) -> Option<B> { FilterMap }
fn next(&mut self) -> Option<Self::Item>;
}
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 });
}
"#,
)
}
}

View File

@ -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<Vec<Assist>> {
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<T> { Some(T), None }
}
pub mod iter {
pub trait Iterator {
type Item;
fn filter_map<B, F>(self, f: F) -> FilterMap where F: FnMut(Self::Item) -> Option<B> { FilterMap }
fn next(&mut self) -> Option<Self::Item>;
}
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<T> { Some(T), None }
}
pub mod iter {
pub trait Iterator {
type Item;
fn filter_map<B, F>(self, f: F) -> FilterMap where F: FnMut(Self::Item) -> Option<B> { FilterMap }
fn next(&mut self) -> Option<Self::Item>;
}
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 });
}
"#,
)
}
}