editor: Don't skip unnecessary errors and warnings in GoToDiagnostic

Go to next/previous diagnostic skipped every diagnostic tagged as
unnecessary, while the editor only drops the underline for unnecessary
diagnostics below warning severity. An error that is also tagged as
unnecessary, like an unused variable in Go, was shown with a red
squiggle but F8 jumped right past it.

Only skip unnecessary diagnostics that are faded out without an
underline, matching how they are rendered.

Closes #64790

crates/diagnostics/src/diagnostics_tests.rs

@@ -1985,6 +1985,80 @@ async fn go_to_diagnostic_with_severity(cx: &mut TestAppContext) {
     cx.assert_editor_state(indoc! {"error ˇwarning info hint"});
 }
 
+#[gpui::test]
+async fn go_to_diagnostic_with_unnecessary_code(cx: &mut TestAppContext) {
+    init_test(cx);
+
+    let mut cx = EditorTestContext::new(cx).await;
+    let lsp_store =
+        cx.update_editor(|editor, _, cx| editor.project().unwrap().read(cx).lsp_store());
+
+    cx.set_state(indoc! {"ˇstart error warning hint"});
+
+    cx.update(|_, cx| {
+        lsp_store.update(cx, |lsp_store, cx| {
+            lsp_store
+                .update_diagnostics(
+                    LanguageServerId(0),
+                    lsp::PublishDiagnosticsParams {
+                        uri: lsp::Uri::from_file_path(path!("/root/file")).unwrap(),
+                        version: None,
+                        diagnostics: vec![
+                            lsp::Diagnostic {
+                                range: lsp::Range::new(
+                                    lsp::Position::new(0, 6),
+                                    lsp::Position::new(0, 11),
+                                ),
+                                severity: Some(lsp::DiagnosticSeverity::ERROR),
+                                tags: Some(vec![lsp::DiagnosticTag::UNNECESSARY]),
+                                ..Default::default()
+                            },
+                            lsp::Diagnostic {
+                                range: lsp::Range::new(
+                                    lsp::Position::new(0, 12),
+                                    lsp::Position::new(0, 19),
+                                ),
+                                severity: Some(lsp::DiagnosticSeverity::WARNING),
+                                tags: Some(vec![lsp::DiagnosticTag::UNNECESSARY]),
+                                ..Default::default()
+                            },
+                            lsp::Diagnostic {
+                                range: lsp::Range::new(
+                                    lsp::Position::new(0, 20),
+                                    lsp::Position::new(0, 24),
+                                ),
+                                severity: Some(lsp::DiagnosticSeverity::HINT),
+                                tags: Some(vec![lsp::DiagnosticTag::UNNECESSARY]),
+                                ..Default::default()
+                            },
+                        ],
+                    },
+                    None,
+                    DiagnosticSourceKind::Pushed,
+                    &[],
+                    cx,
+                )
+                .unwrap()
+        });
+    });
+    cx.run_until_parked();
+
+    // Unnecessary errors and warnings are underlined like any other diagnostic, so they're
+    // navigable, while unnecessary hints are only faded out and get skipped.
+    cx.update_editor(|editor, window, cx| {
+        editor.go_to_diagnostic(&GoToDiagnostic::default(), window, cx);
+    });
+    cx.assert_editor_state(indoc! {"start ˇerror warning hint"});
+    cx.update_editor(|editor, window, cx| {
+        editor.go_to_diagnostic(&GoToDiagnostic::default(), window, cx);
+    });
+    cx.assert_editor_state(indoc! {"start error ˇwarning hint"});
+    cx.update_editor(|editor, window, cx| {
+        editor.go_to_diagnostic(&GoToDiagnostic::default(), window, cx);
+    });
+    cx.assert_editor_state(indoc! {"start ˇerror warning hint"});
+}
+
 #[gpui::test]
 async fn test_buffer_diagnostics(cx: &mut TestAppContext) {
     init_test(cx);

crates/editor/src/diagnostics.rs

@@ -104,7 +104,7 @@ impl Editor {
             .filter(move |entry| entry.range.start <= cursor)
             .filter(move |entry| severity.matches(entry.diagnostic.severity))
             .filter(|entry| entry.range.start != entry.range.end)
-            .filter(|entry| !entry.diagnostic.is_unnecessary)
+            .filter(|entry| !Self::is_faded_unnecessary_diagnostic(&entry.diagnostic))
     }
 
     fn diagnostics_after_cursor<'a>(
@@ -117,7 +117,15 @@ impl Editor {
             .filter(move |entry| entry.range.start >= cursor)
             .filter(move |entry| severity.matches(entry.diagnostic.severity))
             .filter(|entry| entry.range.start != entry.range.end)
-            .filter(|entry| !entry.diagnostic.is_unnecessary)
+            .filter(|entry| !Self::is_faded_unnecessary_diagnostic(&entry.diagnostic))
+    }
+
+    /// Unnecessary code below warning severity is only faded out, without an
+    /// underline (see `DisplaySnapshot::diagnostic_underline_style`), so there
+    /// is nothing visible to jump to. More severe ones are still underlined and
+    /// should stay navigable.
+    fn is_faded_unnecessary_diagnostic(diagnostic: &language::Diagnostic) -> bool {
+        diagnostic.is_unnecessary && diagnostic.severity > lsp::DiagnosticSeverity::WARNING
     }
 
     /// Attempts to expand the diagnostic at the current cursor position,