Merge pull request #21738 from Wilfred/custom_check_command_stale_diagnostics

fix: Stale diagnostics when a custom check command is configured

Author: Veykril

crates/rust-analyzer/src/flycheck.rs

@@ -673,27 +673,31 @@ impl FlycheckActor {
                     if self.diagnostics_received == DiagnosticsReceived::NotYet {
                         tracing::trace!(flycheck_id = self.id, "clearing diagnostics");
                         // We finished without receiving any diagnostics.
-                        // Clear everything for good measure
-                        match &self.scope {
-                            FlycheckScope::Workspace => {
-                                self.send(FlycheckMessage::ClearDiagnostics {
-                                    id: self.id,
-                                    kind: ClearDiagnosticsKind::All(ClearScope::Workspace),
-                                });
-                            }
-                            FlycheckScope::Package { package, workspace_deps } => {
-                                for pkg in
-                                    std::iter::once(package).chain(workspace_deps.iter().flatten())
-                                {
-                                    self.send(FlycheckMessage::ClearDiagnostics {
-                                        id: self.id,
-                                        kind: ClearDiagnosticsKind::All(ClearScope::Package(
-                                            pkg.clone(),
-                                        )),
-                                    });
-                                }
-                            }
-                        }
+                        //
+                        // `cargo check` generally outputs something, even if there are no
+                        // warnings/errors, so we always know which package was checked.
+                        //
+                        // ```text
+                        // $ cargo check --message-format=json 2>/dev/null
+                        // {"reason":"compiler-artifact","package_id":"path+file:///Users/wilfred/tmp/scratch#0.1.0",...}
+                        // ```
+                        //
+                        // However, rustc only returns JSON if there are diagnostics present, so a
+                        // build without warnings or errors has an empty output.
+                        //
+                        // ```
+                        // $ rustc --error-format=json bad.rs
+                        // {"$message_type":"diagnostic","message":"mismatched types","...}
+                        //
+                        // $ rustc --error-format=json good.rs
+                        // ```
+                        //
+                        // So if we got zero diagnostics, it was almost certainly a check that
+                        // wasn't specific to a package.
+                        self.send(FlycheckMessage::ClearDiagnostics {
+                            id: self.id,
+                            kind: ClearDiagnosticsKind::All(ClearScope::Workspace),
+                        });
                     } else if res.is_ok() {
                         // We clear diagnostics for packages on
                         // `[CargoCheckMessage::CompilerArtifact]` but there seem to be setups where

crates/rust-analyzer/tests/slow-tests/flycheck.rs

@@ -110,3 +110,45 @@ fn main() {}
         diagnostics.diagnostics,
     );
 }
+
+#[test]
+fn test_flycheck_diagnostics_with_override_command_cleared_after_fix() {
+    if skip_slow_tests() {
+        return;
+    }
+
+    // Start with a program that is lint clean.
+    let server = Project::with_fixture(
+        r#"
+//- /Cargo.toml
+[package]
+name = "foo"
+version = "0.0.0"
+
+//- /src/main.rs
+fn main() {}
+"#,
+    )
+    .with_config(serde_json::json!({
+        "checkOnSave": true,
+        "check": {
+        "overrideCommand": ["rustc", "--error-format=json", "$saved_file"]
+        }
+    }))
+    .server()
+    .wait_until_workspace_is_loaded();
+
+    // Introduce an unused variable.
+    server.write_file_and_save("src/main.rs", "fn main() {\n    let x = 1;\n}\n".to_owned());
+
+    let diags = server.wait_for_diagnostics();
+    assert!(
+        diags.diagnostics.iter().any(|d| d.message.contains("unused variable")),
+        "expected unused variable diagnostic, got: {:?}",
+        diags.diagnostics,
+    );
+
+    // Fix it and verify that diagnostics are cleared.
+    server.write_file_and_save("src/main.rs", "fn main() {\n    let _x = 1;\n}\n".to_owned());
+    server.wait_for_diagnostics_cleared();
+}