internal: refactor remove this semicolon diagnostics

crates/hir/src/diagnostics.rs

@@ -39,6 +39,7 @@ diagnostics![
     MissingFields,
     MissingUnsafe,
     NoSuchField,
+    RemoveThisSemicolon,
     UnimplementedBuiltinMacro,
     UnresolvedExternCrate,
     UnresolvedImport,
@@ -153,26 +154,7 @@ pub struct MismatchedArgCount {
 
 #[derive(Debug)]
 pub struct RemoveThisSemicolon {
-    pub file: HirFileId,
+    pub expr: InFile<AstPtr<ast::Expr>>,
-    pub expr: AstPtr<ast::Expr>,
-}
-
-impl Diagnostic for RemoveThisSemicolon {
-    fn code(&self) -> DiagnosticCode {
-        DiagnosticCode("remove-this-semicolon")
-    }
-
-    fn message(&self) -> String {
-        "Remove this semicolon".to_string()
-    }
-
-    fn display_source(&self) -> InFile<SyntaxNodePtr> {
-        InFile { file_id: self.file, value: self.expr.clone().into() }
-    }
-
-    fn as_any(&self) -> &(dyn Any + Send + 'static) {
-        self
-    }
 }
 
 // Diagnostic: missing-ok-or-some-in-tail-expr

crates/hir/src/lib.rs

@@ -1184,10 +1184,7 @@ impl Function {
                 }
                 BodyValidationDiagnostic::RemoveThisSemicolon { expr } => {
                     match source_map.expr_syntax(expr) {
-                        Ok(source_ptr) => sink.push(RemoveThisSemicolon {
+                        Ok(expr) => acc.push(RemoveThisSemicolon { expr }.into()),
-                            file: source_ptr.file_id,
-                            expr: source_ptr.value,
-                        }),
                         Err(SyntheticSyntax) => (),
                     }
                 }

crates/ide/src/diagnostics.rs

@@ -11,6 +11,7 @@ mod mismatched_arg_count;
 mod missing_fields;
 mod missing_unsafe;
 mod no_such_field;
+mod remove_this_semicolon;
 mod unimplemented_builtin_macro;
 mod unresolved_extern_crate;
 mod unresolved_import;
@@ -165,9 +166,6 @@ pub(crate) fn diagnostics(
         .on::<hir::diagnostics::MissingOkOrSomeInTailExpr, _>(|d| {
             res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve));
         })
-        .on::<hir::diagnostics::RemoveThisSemicolon, _>(|d| {
-            res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve));
-        })
         .on::<hir::diagnostics::IncorrectCase, _>(|d| {
             res.borrow_mut().push(warning_with_fix(d, &sema, resolve));
         })
@@ -223,10 +221,11 @@ pub(crate) fn diagnostics(
         let d = match diag {
             AnyDiagnostic::BreakOutsideOfLoop(d) => break_outside_of_loop::break_outside_of_loop(&ctx, &d),
             AnyDiagnostic::MacroError(d) => macro_error::macro_error(&ctx, &d),
+            AnyDiagnostic::MismatchedArgCount(d) => mismatched_arg_count::mismatched_arg_count(&ctx, &d),
             AnyDiagnostic::MissingFields(d) => missing_fields::missing_fields(&ctx, &d),
             AnyDiagnostic::MissingUnsafe(d) => missing_unsafe::missing_unsafe(&ctx, &d),
-            AnyDiagnostic::MismatchedArgCount(d) => mismatched_arg_count::mismatched_arg_count(&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::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),
@@ -838,16 +837,6 @@ fn x(a: S) {
         )
     }
 
-    #[test]
-    fn missing_semicolon() {
-        check_diagnostics(
-            r#"
-                fn test() -> i32 { 123; }
-                                 //^^^ Remove this semicolon
-            "#,
-        );
-    }
-
     #[test]
     fn import_extern_crate_clash_with_inner_item() {
         // This is more of a resolver test, but doesn't really work with the hir_def testsuite.

crates/ide/src/diagnostics/fixes.rs

@@ -1,8 +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;
 

crates/ide/src/diagnostics/fixes/create_field.rs

@@ -1 +0,0 @@
-

crates/ide/src/diagnostics/fixes/remove_semicolon.rs

@@ -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 }"#);
-    }
-}

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))

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 }"#);
+    }
+}