Merge #9253

9253: internal: refactor missing or or some diagnostic r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <[email protected]>

9 files changed, 357 insertions, 151 deletions

diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs
index a2e792b3b..e4bd90c3f 100644
--- a/crates/ide/src/diagnostics/fixes.rs
+++ b/crates/ide/src/diagnostics/fixes.rs
@@ -1,10 +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 create_field;
-mod remove_semicolon;
-mod replace_with_find_map;
-mod wrap_tail_expr;
 
 use hir::{diagnostics::Diagnostic, Semantics};
 use ide_assists::AssistResolveStrategy;
diff --git a/crates/ide/src/diagnostics/fixes/create_field.rs b/crates/ide/src/diagnostics/fixes/create_field.rs
deleted file mode 100644
index 8b1378917..000000000
--- a/crates/ide/src/diagnostics/fixes/create_field.rs
+++ /dev/null
@@ -1 +0,0 @@
-
diff --git a/crates/ide/src/diagnostics/fixes/remove_semicolon.rs b/crates/ide/src/diagnostics/fixes/remove_semicolon.rs
deleted file mode 100644
index f1724d479..000000000
--- a/crates/ide/src/diagnostics/fixes/remove_semicolon.rs
+++ /dev/null
@@ -1,41 +0,0 @@
-use hir::{db::AstDatabase, diagnostics::RemoveThisSemicolon, Semantics};
-use ide_assists::{Assist, AssistResolveStrategy};
-use ide_db::{source_change::SourceChange, RootDatabase};
-use syntax::{ast, AstNode};
-use text_edit::TextEdit;
-
-use crate::diagnostics::{fix, DiagnosticWithFixes};
-
-impl DiagnosticWithFixes for RemoveThisSemicolon {
-    fn fixes(
-        &self,
-        sema: &Semantics<RootDatabase>,
-        _resolve: &AssistResolveStrategy,
-    ) -> Option<Vec<Assist>> {
-        let root = sema.db.parse_or_expand(self.file)?;
-
-        let semicolon = self
-            .expr
-            .to_node(&root)
-            .syntax()
-            .parent()
-            .and_then(ast::ExprStmt::cast)
-            .and_then(|expr| expr.semicolon_token())?
-            .text_range();
-
-        let edit = TextEdit::delete(semicolon);
-        let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit);
-
-        Some(vec![fix("remove_semicolon", "Remove this semicolon", source_change, semicolon)])
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use crate::diagnostics::tests::check_fix;
-
-    #[test]
-    fn remove_semicolon() {
-        check_fix(r#"fn f() -> i32 { 92$0; }"#, r#"fn f() -> i32 { 92 }"#);
-    }
-}
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<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 });
-}
-"#,
-        )
-    }
-}
diff --git a/crates/ide/src/diagnostics/missing_fields.rs b/crates/ide/src/diagnostics/missing_fields.rs
index 95cd64956..d01f05041 100644
--- a/crates/ide/src/diagnostics/missing_fields.rs
+++ b/crates/ide/src/diagnostics/missing_fields.rs
@@ -94,6 +94,77 @@ fn baz(s: S) {
     }
 
     #[test]
+    fn missing_record_pat_field_no_diagnostic_if_not_exhaustive() {
+        check_diagnostics(
+            r"
+struct S { foo: i32, bar: () }
+fn baz(s: S) -> i32 {
+    match s {
+        S { foo, .. } => foo,
+    }
+}
+",
+        )
+    }
+
+    #[test]
+    fn missing_record_pat_field_box() {
+        check_diagnostics(
+            r"
+struct S { s: Box<u32> }
+fn x(a: S) {
+    let S { box s } = a;
+}
+",
+        )
+    }
+
+    #[test]
+    fn missing_record_pat_field_ref() {
+        check_diagnostics(
+            r"
+struct S { s: u32 }
+fn x(a: S) {
+    let S { ref s } = a;
+}
+",
+        )
+    }
+
+    #[test]
+    fn range_mapping_out_of_macros() {
+        // FIXME: this is very wrong, but somewhat tricky to fix.
+        check_fix(
+            r#"
+fn some() {}
+fn items() {}
+fn here() {}
+
+macro_rules! id { ($($tt:tt)*) => { $($tt)*}; }
+
+fn main() {
+    let _x = id![Foo { a: $042 }];
+}
+
+pub struct Foo { pub a: i32, pub b: i32 }
+"#,
+            r#"
+fn some(, b: () ) {}
+fn items() {}
+fn here() {}
+
+macro_rules! id { ($($tt:tt)*) => { $($tt)*}; }
+
+fn main() {
+    let _x = id![Foo { a: 42 }];
+}
+
+pub struct Foo { pub a: i32, pub b: i32 }
+"#,
+        );
+    }
+
+    #[test]
     fn test_fill_struct_fields_empty() {
         check_fix(
             r#"
diff --git a/crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs b/crates/ide/src/diagnostics/missing_ok_or_some_in_tail_expr.rs
index dcb21e037..06005d156 100644
--- a/crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs
+++ b/crates/ide/src/diagnostics/missing_ok_or_some_in_tail_expr.rs
@@ -1,26 +1,45 @@
-use hir::{db::AstDatabase, diagnostics::MissingOkOrSomeInTailExpr, Semantics};
-use ide_assists::{Assist, AssistResolveStrategy};
-use ide_db::{source_change::SourceChange, RootDatabase};
+use hir::db::AstDatabase;
+use ide_assists::Assist;
+use ide_db::source_change::SourceChange;
 use syntax::AstNode;
 use text_edit::TextEdit;
 
-use crate::diagnostics::{fix, DiagnosticWithFixes};
-
-impl DiagnosticWithFixes for MissingOkOrSomeInTailExpr {
-    fn fixes(
-        &self,
-        sema: &Semantics<RootDatabase>,
-        _resolve: &AssistResolveStrategy,
-    ) -> Option<Vec<Assist>> {
-        let root = sema.db.parse_or_expand(self.file)?;
-        let tail_expr = self.expr.to_node(&root);
-        let tail_expr_range = tail_expr.syntax().text_range();
-        let replacement = format!("{}({})", self.required, tail_expr.syntax());
-        let edit = TextEdit::replace(tail_expr_range, replacement);
-        let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit);
-        let name = if self.required == "Ok" { "Wrap with Ok" } else { "Wrap with Some" };
-        Some(vec![fix("wrap_tail_expr", name, source_change, tail_expr_range)])
-    }
+use crate::diagnostics::{fix, Diagnostic, DiagnosticsContext};
+
+// Diagnostic: missing-ok-or-some-in-tail-expr
+//
+// This diagnostic is triggered if a block that should return `Result` returns a value not wrapped in `Ok`,
+// or if a block that should return `Option` returns a value not wrapped in `Some`.
+//
+// Example:
+//
+// ```rust
+// fn foo() -> Result<u8, ()> {
+//     10
+// }
+// ```
+pub(super) fn missing_ok_or_some_in_tail_expr(
+    ctx: &DiagnosticsContext<'_>,
+    d: &hir::MissingOkOrSomeInTailExpr,
+) -> Diagnostic {
+    Diagnostic::new(
+        "missing-ok-or-some-in-tail-expr",
+        format!("wrap return expression in {}", d.required),
+        ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range,
+    )
+    .with_fixes(fixes(ctx, d))
+}
+
+fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingOkOrSomeInTailExpr) -> Option<Vec<Assist>> {
+    let root = ctx.sema.db.parse_or_expand(d.expr.file_id)?;
+    let tail_expr = d.expr.value.to_node(&root);
+    let tail_expr_range = tail_expr.syntax().text_range();
+    let replacement = format!("{}({})", d.required, tail_expr.syntax());
+    let edit = TextEdit::replace(tail_expr_range, replacement);
+    let source_change =
+        SourceChange::from_text_edit(d.expr.file_id.original_file(ctx.sema.db), edit);
+    let name = if d.required == "Ok" { "Wrap with Ok" } else { "Wrap with Some" };
+    Some(vec![fix("wrap_tail_expr", name, source_change, tail_expr_range)])
 }
 
 #[cfg(test)]
diff --git a/crates/ide/src/diagnostics/no_such_field.rs b/crates/ide/src/diagnostics/no_such_field.rs
index 61962de28..edc63c246 100644
--- a/crates/ide/src/diagnostics/no_such_field.rs
+++ b/crates/ide/src/diagnostics/no_such_field.rs
@@ -17,7 +17,7 @@ use crate::{
 pub(super) fn no_such_field(ctx: &DiagnosticsContext<'_>, d: &hir::NoSuchField) -> Diagnostic {
     Diagnostic::new(
         "no-such-field",
-        "no such field".to_string(),
+        "no such field",
         ctx.sema.diagnostics_display_range(d.field.clone().map(|it| it.into())).range,
     )
     .with_fixes(fixes(ctx, d))
diff --git a/crates/ide/src/diagnostics/remove_this_semicolon.rs b/crates/ide/src/diagnostics/remove_this_semicolon.rs
new file mode 100644
index 000000000..814cb0f8c
--- /dev/null
+++ b/crates/ide/src/diagnostics/remove_this_semicolon.rs
@@ -0,0 +1,64 @@
+use hir::db::AstDatabase;
+use ide_db::source_change::SourceChange;
+use syntax::{ast, AstNode};
+use text_edit::TextEdit;
+
+use crate::{
+    diagnostics::{fix, Diagnostic, DiagnosticsContext},
+    Assist,
+};
+
+// Diagnostic: remove-this-semicolon
+//
+// This diagnostic is triggered when there's an erroneous `;` at the end of the block.
+pub(super) fn remove_this_semicolon(
+    ctx: &DiagnosticsContext<'_>,
+    d: &hir::RemoveThisSemicolon,
+) -> Diagnostic {
+    Diagnostic::new(
+        "remove-this-semicolon",
+        "remove this semicolon",
+        ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range,
+    )
+    .with_fixes(fixes(ctx, d))
+}
+
+fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::RemoveThisSemicolon) -> Option<Vec<Assist>> {
+    let root = ctx.sema.db.parse_or_expand(d.expr.file_id)?;
+
+    let semicolon = d
+        .expr
+        .value
+        .to_node(&root)
+        .syntax()
+        .parent()
+        .and_then(ast::ExprStmt::cast)
+        .and_then(|expr| expr.semicolon_token())?
+        .text_range();
+
+    let edit = TextEdit::delete(semicolon);
+    let source_change =
+        SourceChange::from_text_edit(d.expr.file_id.original_file(ctx.sema.db), edit);
+
+    Some(vec![fix("remove_semicolon", "Remove this semicolon", source_change, semicolon)])
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::diagnostics::tests::{check_diagnostics, check_fix};
+
+    #[test]
+    fn missing_semicolon() {
+        check_diagnostics(
+            r#"
+fn test() -> i32 { 123; }
+                 //^^^ remove this semicolon
+"#,
+        );
+    }
+
+    #[test]
+    fn remove_semicolon() {
+        check_fix(r#"fn f() -> i32 { 92$0; }"#, r#"fn f() -> i32 { 92 }"#);
+    }
+}
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<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 });
+}
+"#,
+        )
+    }
+}