diff options
author | Emil Lauridsen <[email protected]> | 2020-01-31 18:23:25 +0000 |
---|---|---|
committer | Emil Lauridsen <[email protected]> | 2020-02-03 10:34:24 +0000 |
commit | 790788d5f4013d8d92f110bc12a581d18cf4b6ae (patch) | |
tree | 311e11529c7546b7a09486d5c161039d8bd8f975 | |
parent | 52456c44901c8c38c8bcb742ebe305484af8f36f (diff) |
Rework how we send diagnostics to client.
The previous way of sending from the thread pool suffered from stale
diagnostics due to being canceled before we could clear the old ones.
The key change is moving to sending diagnostics from the main loop
thread, but doing all the hard work in the thread pool. This should
provide the best of both worlds, with little to no of the downsides.
This should hopefully fix a lot of issues, but we'll need testing in
each individual issue to be sure.
-rw-r--r-- | crates/ra_cargo_watch/src/conv.rs | 66 | ||||
-rw-r--r-- | crates/ra_cargo_watch/src/lib.rs | 110 | ||||
-rw-r--r-- | crates/ra_lsp_server/src/diagnostics.rs | 85 | ||||
-rw-r--r-- | crates/ra_lsp_server/src/lib.rs | 1 | ||||
-rw-r--r-- | crates/ra_lsp_server/src/main_loop.rs | 76 | ||||
-rw-r--r-- | crates/ra_lsp_server/src/main_loop/handlers.rs | 37 | ||||
-rw-r--r-- | crates/ra_lsp_server/src/world.rs | 11 |
7 files changed, 178 insertions, 208 deletions
diff --git a/crates/ra_cargo_watch/src/conv.rs b/crates/ra_cargo_watch/src/conv.rs index 8fba400ae..506370535 100644 --- a/crates/ra_cargo_watch/src/conv.rs +++ b/crates/ra_cargo_watch/src/conv.rs | |||
@@ -1,12 +1,11 @@ | |||
1 | //! This module provides the functionality needed to convert diagnostics from | 1 | //! This module provides the functionality needed to convert diagnostics from |
2 | //! `cargo check` json format to the LSP diagnostic format. | 2 | //! `cargo check` json format to the LSP diagnostic format. |
3 | use cargo_metadata::diagnostic::{ | 3 | use cargo_metadata::diagnostic::{ |
4 | Applicability, Diagnostic as RustDiagnostic, DiagnosticLevel, DiagnosticSpan, | 4 | Diagnostic as RustDiagnostic, DiagnosticLevel, DiagnosticSpan, DiagnosticSpanMacroExpansion, |
5 | DiagnosticSpanMacroExpansion, | ||
6 | }; | 5 | }; |
7 | use lsp_types::{ | 6 | use lsp_types::{ |
8 | Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag, Location, | 7 | CodeAction, Diagnostic, DiagnosticRelatedInformation, DiagnosticSeverity, DiagnosticTag, |
9 | NumberOrString, Position, Range, Url, | 8 | Location, NumberOrString, Position, Range, TextEdit, Url, WorkspaceEdit, |
10 | }; | 9 | }; |
11 | use std::{ | 10 | use std::{ |
12 | fmt::Write, | 11 | fmt::Write, |
@@ -117,38 +116,9 @@ fn is_deprecated(rd: &RustDiagnostic) -> bool { | |||
117 | } | 116 | } |
118 | } | 117 | } |
119 | 118 | ||
120 | #[derive(Clone, Debug)] | ||
121 | pub struct SuggestedFix { | ||
122 | pub title: String, | ||
123 | pub location: Location, | ||
124 | pub replacement: String, | ||
125 | pub applicability: Applicability, | ||
126 | pub diagnostics: Vec<Diagnostic>, | ||
127 | } | ||
128 | |||
129 | impl std::cmp::PartialEq<SuggestedFix> for SuggestedFix { | ||
130 | fn eq(&self, other: &SuggestedFix) -> bool { | ||
131 | if self.title == other.title | ||
132 | && self.location == other.location | ||
133 | && self.replacement == other.replacement | ||
134 | { | ||
135 | // Applicability doesn't impl PartialEq... | ||
136 | match (&self.applicability, &other.applicability) { | ||
137 | (Applicability::MachineApplicable, Applicability::MachineApplicable) => true, | ||
138 | (Applicability::HasPlaceholders, Applicability::HasPlaceholders) => true, | ||
139 | (Applicability::MaybeIncorrect, Applicability::MaybeIncorrect) => true, | ||
140 | (Applicability::Unspecified, Applicability::Unspecified) => true, | ||
141 | _ => false, | ||
142 | } | ||
143 | } else { | ||
144 | false | ||
145 | } | ||
146 | } | ||
147 | } | ||
148 | |||
149 | enum MappedRustChildDiagnostic { | 119 | enum MappedRustChildDiagnostic { |
150 | Related(DiagnosticRelatedInformation), | 120 | Related(DiagnosticRelatedInformation), |
151 | SuggestedFix(SuggestedFix), | 121 | SuggestedFix(CodeAction), |
152 | MessageLine(String), | 122 | MessageLine(String), |
153 | } | 123 | } |
154 | 124 | ||
@@ -176,12 +146,20 @@ fn map_rust_child_diagnostic( | |||
176 | rd.message.clone() | 146 | rd.message.clone() |
177 | }; | 147 | }; |
178 | 148 | ||
179 | MappedRustChildDiagnostic::SuggestedFix(SuggestedFix { | 149 | let edit = { |
150 | let edits = vec![TextEdit::new(location.range, suggested_replacement.clone())]; | ||
151 | let mut edit_map = std::collections::HashMap::new(); | ||
152 | edit_map.insert(location.uri, edits); | ||
153 | WorkspaceEdit::new(edit_map) | ||
154 | }; | ||
155 | |||
156 | MappedRustChildDiagnostic::SuggestedFix(CodeAction { | ||
180 | title, | 157 | title, |
181 | location, | 158 | kind: Some("quickfix".to_string()), |
182 | replacement: suggested_replacement.clone(), | 159 | diagnostics: None, |
183 | applicability: span.suggestion_applicability.clone().unwrap_or(Applicability::Unknown), | 160 | edit: Some(edit), |
184 | diagnostics: vec![], | 161 | command: None, |
162 | is_preferred: None, | ||
185 | }) | 163 | }) |
186 | } else { | 164 | } else { |
187 | MappedRustChildDiagnostic::Related(DiagnosticRelatedInformation { | 165 | MappedRustChildDiagnostic::Related(DiagnosticRelatedInformation { |
@@ -195,7 +173,7 @@ fn map_rust_child_diagnostic( | |||
195 | pub(crate) struct MappedRustDiagnostic { | 173 | pub(crate) struct MappedRustDiagnostic { |
196 | pub location: Location, | 174 | pub location: Location, |
197 | pub diagnostic: Diagnostic, | 175 | pub diagnostic: Diagnostic, |
198 | pub suggested_fixes: Vec<SuggestedFix>, | 176 | pub fixes: Vec<CodeAction>, |
199 | } | 177 | } |
200 | 178 | ||
201 | /// Converts a Rust root diagnostic to LSP form | 179 | /// Converts a Rust root diagnostic to LSP form |
@@ -250,15 +228,13 @@ pub(crate) fn map_rust_diagnostic_to_lsp( | |||
250 | } | 228 | } |
251 | } | 229 | } |
252 | 230 | ||
253 | let mut suggested_fixes = vec![]; | 231 | let mut fixes = vec![]; |
254 | let mut message = rd.message.clone(); | 232 | let mut message = rd.message.clone(); |
255 | for child in &rd.children { | 233 | for child in &rd.children { |
256 | let child = map_rust_child_diagnostic(&child, workspace_root); | 234 | let child = map_rust_child_diagnostic(&child, workspace_root); |
257 | match child { | 235 | match child { |
258 | MappedRustChildDiagnostic::Related(related) => related_information.push(related), | 236 | MappedRustChildDiagnostic::Related(related) => related_information.push(related), |
259 | MappedRustChildDiagnostic::SuggestedFix(suggested_fix) => { | 237 | MappedRustChildDiagnostic::SuggestedFix(code_action) => fixes.push(code_action.into()), |
260 | suggested_fixes.push(suggested_fix) | ||
261 | } | ||
262 | MappedRustChildDiagnostic::MessageLine(message_line) => { | 238 | MappedRustChildDiagnostic::MessageLine(message_line) => { |
263 | write!(&mut message, "\n{}", message_line).unwrap(); | 239 | write!(&mut message, "\n{}", message_line).unwrap(); |
264 | 240 | ||
@@ -295,7 +271,7 @@ pub(crate) fn map_rust_diagnostic_to_lsp( | |||
295 | tags: if !tags.is_empty() { Some(tags) } else { None }, | 271 | tags: if !tags.is_empty() { Some(tags) } else { None }, |
296 | }; | 272 | }; |
297 | 273 | ||
298 | Some(MappedRustDiagnostic { location, diagnostic, suggested_fixes }) | 274 | Some(MappedRustDiagnostic { location, diagnostic, fixes }) |
299 | } | 275 | } |
300 | 276 | ||
301 | /// Returns a `Url` object from a given path, will lowercase drive letters if present. | 277 | /// Returns a `Url` object from a given path, will lowercase drive letters if present. |
diff --git a/crates/ra_cargo_watch/src/lib.rs b/crates/ra_cargo_watch/src/lib.rs index a718a5e52..f07c34549 100644 --- a/crates/ra_cargo_watch/src/lib.rs +++ b/crates/ra_cargo_watch/src/lib.rs | |||
@@ -4,22 +4,20 @@ | |||
4 | use cargo_metadata::Message; | 4 | use cargo_metadata::Message; |
5 | use crossbeam_channel::{never, select, unbounded, Receiver, RecvError, Sender}; | 5 | use crossbeam_channel::{never, select, unbounded, Receiver, RecvError, Sender}; |
6 | use lsp_types::{ | 6 | use lsp_types::{ |
7 | Diagnostic, Url, WorkDoneProgress, WorkDoneProgressBegin, WorkDoneProgressEnd, | 7 | CodeAction, CodeActionOrCommand, Diagnostic, Url, WorkDoneProgress, WorkDoneProgressBegin, |
8 | WorkDoneProgressReport, | 8 | WorkDoneProgressEnd, WorkDoneProgressReport, |
9 | }; | 9 | }; |
10 | use std::{ | 10 | use std::{ |
11 | collections::HashMap, | ||
12 | io::{BufRead, BufReader}, | 11 | io::{BufRead, BufReader}, |
13 | path::PathBuf, | 12 | path::PathBuf, |
14 | process::{Command, Stdio}, | 13 | process::{Command, Stdio}, |
15 | sync::Arc, | ||
16 | thread::JoinHandle, | 14 | thread::JoinHandle, |
17 | time::Instant, | 15 | time::Instant, |
18 | }; | 16 | }; |
19 | 17 | ||
20 | mod conv; | 18 | mod conv; |
21 | 19 | ||
22 | use crate::conv::{map_rust_diagnostic_to_lsp, MappedRustDiagnostic, SuggestedFix}; | 20 | use crate::conv::{map_rust_diagnostic_to_lsp, MappedRustDiagnostic}; |
23 | 21 | ||
24 | pub use crate::conv::url_from_path_with_drive_lowercasing; | 22 | pub use crate::conv::url_from_path_with_drive_lowercasing; |
25 | 23 | ||
@@ -38,7 +36,6 @@ pub struct CheckOptions { | |||
38 | #[derive(Debug)] | 36 | #[derive(Debug)] |
39 | pub struct CheckWatcher { | 37 | pub struct CheckWatcher { |
40 | pub task_recv: Receiver<CheckTask>, | 38 | pub task_recv: Receiver<CheckTask>, |
41 | pub state: Arc<CheckState>, | ||
42 | cmd_send: Option<Sender<CheckCommand>>, | 39 | cmd_send: Option<Sender<CheckCommand>>, |
43 | handle: Option<JoinHandle<()>>, | 40 | handle: Option<JoinHandle<()>>, |
44 | } | 41 | } |
@@ -46,7 +43,6 @@ pub struct CheckWatcher { | |||
46 | impl CheckWatcher { | 43 | impl CheckWatcher { |
47 | pub fn new(options: &CheckOptions, workspace_root: PathBuf) -> CheckWatcher { | 44 | pub fn new(options: &CheckOptions, workspace_root: PathBuf) -> CheckWatcher { |
48 | let options = options.clone(); | 45 | let options = options.clone(); |
49 | let state = Arc::new(CheckState::new()); | ||
50 | 46 | ||
51 | let (task_send, task_recv) = unbounded::<CheckTask>(); | 47 | let (task_send, task_recv) = unbounded::<CheckTask>(); |
52 | let (cmd_send, cmd_recv) = unbounded::<CheckCommand>(); | 48 | let (cmd_send, cmd_recv) = unbounded::<CheckCommand>(); |
@@ -54,13 +50,12 @@ impl CheckWatcher { | |||
54 | let mut check = CheckWatcherThread::new(options, workspace_root); | 50 | let mut check = CheckWatcherThread::new(options, workspace_root); |
55 | check.run(&task_send, &cmd_recv); | 51 | check.run(&task_send, &cmd_recv); |
56 | }); | 52 | }); |
57 | CheckWatcher { task_recv, cmd_send: Some(cmd_send), handle: Some(handle), state } | 53 | CheckWatcher { task_recv, cmd_send: Some(cmd_send), handle: Some(handle) } |
58 | } | 54 | } |
59 | 55 | ||
60 | /// Returns a CheckWatcher that doesn't actually do anything | 56 | /// Returns a CheckWatcher that doesn't actually do anything |
61 | pub fn dummy() -> CheckWatcher { | 57 | pub fn dummy() -> CheckWatcher { |
62 | let state = Arc::new(CheckState::new()); | 58 | CheckWatcher { task_recv: never(), cmd_send: None, handle: None } |
63 | CheckWatcher { task_recv: never(), cmd_send: None, handle: None, state } | ||
64 | } | 59 | } |
65 | 60 | ||
66 | /// Schedule a re-start of the cargo check worker. | 61 | /// Schedule a re-start of the cargo check worker. |
@@ -87,84 +82,13 @@ impl std::ops::Drop for CheckWatcher { | |||
87 | } | 82 | } |
88 | } | 83 | } |
89 | 84 | ||
90 | #[derive(Clone, Debug)] | ||
91 | pub struct CheckState { | ||
92 | diagnostic_collection: HashMap<Url, Vec<Diagnostic>>, | ||
93 | suggested_fix_collection: HashMap<Url, Vec<SuggestedFix>>, | ||
94 | } | ||
95 | |||
96 | impl CheckState { | ||
97 | fn new() -> CheckState { | ||
98 | CheckState { | ||
99 | diagnostic_collection: HashMap::new(), | ||
100 | suggested_fix_collection: HashMap::new(), | ||
101 | } | ||
102 | } | ||
103 | |||
104 | /// Clear the cached diagnostics, and schedule updating diagnostics by the | ||
105 | /// server, to clear stale results. | ||
106 | pub fn clear(&mut self) -> Vec<Url> { | ||
107 | let cleared_files: Vec<Url> = self.diagnostic_collection.keys().cloned().collect(); | ||
108 | self.diagnostic_collection.clear(); | ||
109 | self.suggested_fix_collection.clear(); | ||
110 | cleared_files | ||
111 | } | ||
112 | |||
113 | pub fn diagnostics_for(&self, uri: &Url) -> Option<&[Diagnostic]> { | ||
114 | self.diagnostic_collection.get(uri).map(|d| d.as_slice()) | ||
115 | } | ||
116 | |||
117 | pub fn fixes_for(&self, uri: &Url) -> Option<&[SuggestedFix]> { | ||
118 | self.suggested_fix_collection.get(uri).map(|d| d.as_slice()) | ||
119 | } | ||
120 | |||
121 | pub fn add_diagnostic_with_fixes(&mut self, file_uri: Url, diagnostic: DiagnosticWithFixes) { | ||
122 | for fix in diagnostic.suggested_fixes { | ||
123 | self.add_suggested_fix_for_diagnostic(fix, &diagnostic.diagnostic); | ||
124 | } | ||
125 | self.add_diagnostic(file_uri, diagnostic.diagnostic); | ||
126 | } | ||
127 | |||
128 | fn add_diagnostic(&mut self, file_uri: Url, diagnostic: Diagnostic) { | ||
129 | let diagnostics = self.diagnostic_collection.entry(file_uri).or_default(); | ||
130 | |||
131 | // If we're building multiple targets it's possible we've already seen this diagnostic | ||
132 | let is_duplicate = diagnostics.iter().any(|d| are_diagnostics_equal(d, &diagnostic)); | ||
133 | if is_duplicate { | ||
134 | return; | ||
135 | } | ||
136 | |||
137 | diagnostics.push(diagnostic); | ||
138 | } | ||
139 | |||
140 | fn add_suggested_fix_for_diagnostic( | ||
141 | &mut self, | ||
142 | mut suggested_fix: SuggestedFix, | ||
143 | diagnostic: &Diagnostic, | ||
144 | ) { | ||
145 | let file_uri = suggested_fix.location.uri.clone(); | ||
146 | let file_suggestions = self.suggested_fix_collection.entry(file_uri).or_default(); | ||
147 | |||
148 | let existing_suggestion: Option<&mut SuggestedFix> = | ||
149 | file_suggestions.iter_mut().find(|s| s == &&suggested_fix); | ||
150 | if let Some(existing_suggestion) = existing_suggestion { | ||
151 | // The existing suggestion also applies to this new diagnostic | ||
152 | existing_suggestion.diagnostics.push(diagnostic.clone()); | ||
153 | } else { | ||
154 | // We haven't seen this suggestion before | ||
155 | suggested_fix.diagnostics.push(diagnostic.clone()); | ||
156 | file_suggestions.push(suggested_fix); | ||
157 | } | ||
158 | } | ||
159 | } | ||
160 | |||
161 | #[derive(Debug)] | 85 | #[derive(Debug)] |
162 | pub enum CheckTask { | 86 | pub enum CheckTask { |
163 | /// Request a clearing of all cached diagnostics from the check watcher | 87 | /// Request a clearing of all cached diagnostics from the check watcher |
164 | ClearDiagnostics, | 88 | ClearDiagnostics, |
165 | 89 | ||
166 | /// Request adding a diagnostic with fixes included to a file | 90 | /// Request adding a diagnostic with fixes included to a file |
167 | AddDiagnostic(Url, DiagnosticWithFixes), | 91 | AddDiagnostic { url: Url, diagnostic: Diagnostic, fixes: Vec<CodeActionOrCommand> }, |
168 | 92 | ||
169 | /// Request check progress notification to client | 93 | /// Request check progress notification to client |
170 | Status(WorkDoneProgress), | 94 | Status(WorkDoneProgress), |
@@ -279,10 +203,17 @@ impl CheckWatcherThread { | |||
279 | None => return, | 203 | None => return, |
280 | }; | 204 | }; |
281 | 205 | ||
282 | let MappedRustDiagnostic { location, diagnostic, suggested_fixes } = map_result; | 206 | let MappedRustDiagnostic { location, diagnostic, fixes } = map_result; |
207 | let fixes = fixes | ||
208 | .into_iter() | ||
209 | .map(|fix| { | ||
210 | CodeAction { diagnostics: Some(vec![diagnostic.clone()]), ..fix }.into() | ||
211 | }) | ||
212 | .collect(); | ||
283 | 213 | ||
284 | let diagnostic = DiagnosticWithFixes { diagnostic, suggested_fixes }; | 214 | task_send |
285 | task_send.send(CheckTask::AddDiagnostic(location.uri, diagnostic)).unwrap(); | 215 | .send(CheckTask::AddDiagnostic { url: location.uri, diagnostic, fixes }) |
216 | .unwrap(); | ||
286 | } | 217 | } |
287 | 218 | ||
288 | CheckEvent::Msg(Message::BuildScriptExecuted(_msg)) => {} | 219 | CheckEvent::Msg(Message::BuildScriptExecuted(_msg)) => {} |
@@ -294,7 +225,7 @@ impl CheckWatcherThread { | |||
294 | #[derive(Debug)] | 225 | #[derive(Debug)] |
295 | pub struct DiagnosticWithFixes { | 226 | pub struct DiagnosticWithFixes { |
296 | diagnostic: Diagnostic, | 227 | diagnostic: Diagnostic, |
297 | suggested_fixes: Vec<SuggestedFix>, | 228 | fixes: Vec<CodeAction>, |
298 | } | 229 | } |
299 | 230 | ||
300 | /// WatchThread exists to wrap around the communication needed to be able to | 231 | /// WatchThread exists to wrap around the communication needed to be able to |
@@ -429,10 +360,3 @@ impl std::ops::Drop for WatchThread { | |||
429 | } | 360 | } |
430 | } | 361 | } |
431 | } | 362 | } |
432 | |||
433 | fn are_diagnostics_equal(left: &Diagnostic, right: &Diagnostic) -> bool { | ||
434 | left.source == right.source | ||
435 | && left.severity == right.severity | ||
436 | && left.range == right.range | ||
437 | && left.message == right.message | ||
438 | } | ||
diff --git a/crates/ra_lsp_server/src/diagnostics.rs b/crates/ra_lsp_server/src/diagnostics.rs new file mode 100644 index 000000000..ea08bce24 --- /dev/null +++ b/crates/ra_lsp_server/src/diagnostics.rs | |||
@@ -0,0 +1,85 @@ | |||
1 | //! Book keeping for keeping diagnostics easily in sync with the client. | ||
2 | use lsp_types::{CodeActionOrCommand, Diagnostic, Range}; | ||
3 | use ra_ide::FileId; | ||
4 | use std::{collections::HashMap, sync::Arc}; | ||
5 | |||
6 | pub type CheckFixes = Arc<HashMap<FileId, Vec<Fix>>>; | ||
7 | |||
8 | #[derive(Debug, Default, Clone)] | ||
9 | pub struct DiagnosticCollection { | ||
10 | pub native: HashMap<FileId, Vec<Diagnostic>>, | ||
11 | pub check: HashMap<FileId, Vec<Diagnostic>>, | ||
12 | pub check_fixes: CheckFixes, | ||
13 | } | ||
14 | |||
15 | #[derive(Debug, Clone)] | ||
16 | pub struct Fix { | ||
17 | pub range: Range, | ||
18 | pub action: CodeActionOrCommand, | ||
19 | } | ||
20 | |||
21 | #[derive(Debug)] | ||
22 | pub enum DiagnosticTask { | ||
23 | ClearCheck, | ||
24 | AddCheck(FileId, Diagnostic, Vec<CodeActionOrCommand>), | ||
25 | SetNative(FileId, Vec<Diagnostic>), | ||
26 | } | ||
27 | |||
28 | impl DiagnosticCollection { | ||
29 | pub fn clear_check(&mut self) -> Vec<FileId> { | ||
30 | Arc::make_mut(&mut self.check_fixes).clear(); | ||
31 | self.check.drain().map(|(key, _value)| key).collect() | ||
32 | } | ||
33 | |||
34 | pub fn add_check_diagnostic( | ||
35 | &mut self, | ||
36 | file_id: FileId, | ||
37 | diagnostic: Diagnostic, | ||
38 | fixes: Vec<CodeActionOrCommand>, | ||
39 | ) { | ||
40 | let diagnostics = self.check.entry(file_id).or_default(); | ||
41 | for existing_diagnostic in diagnostics.iter() { | ||
42 | if are_diagnostics_equal(&existing_diagnostic, &diagnostic) { | ||
43 | return; | ||
44 | } | ||
45 | } | ||
46 | |||
47 | let check_fixes = Arc::make_mut(&mut self.check_fixes); | ||
48 | check_fixes | ||
49 | .entry(file_id) | ||
50 | .or_default() | ||
51 | .extend(fixes.into_iter().map(|action| Fix { range: diagnostic.range, action })); | ||
52 | diagnostics.push(diagnostic); | ||
53 | } | ||
54 | |||
55 | pub fn set_native_diagnostics(&mut self, file_id: FileId, diagnostics: Vec<Diagnostic>) { | ||
56 | self.native.insert(file_id, diagnostics); | ||
57 | } | ||
58 | |||
59 | pub fn diagnostics_for(&self, file_id: FileId) -> impl Iterator<Item = &Diagnostic> { | ||
60 | let native = self.native.get(&file_id).into_iter().flatten(); | ||
61 | let check = self.check.get(&file_id).into_iter().flatten(); | ||
62 | native.chain(check) | ||
63 | } | ||
64 | |||
65 | pub fn handle_task(&mut self, task: DiagnosticTask) -> Vec<FileId> { | ||
66 | match task { | ||
67 | DiagnosticTask::ClearCheck => self.clear_check(), | ||
68 | DiagnosticTask::AddCheck(file_id, diagnostic, fixes) => { | ||
69 | self.add_check_diagnostic(file_id, diagnostic, fixes); | ||
70 | vec![file_id] | ||
71 | } | ||
72 | DiagnosticTask::SetNative(file_id, diagnostics) => { | ||
73 | self.set_native_diagnostics(file_id, diagnostics); | ||
74 | vec![file_id] | ||
75 | } | ||
76 | } | ||
77 | } | ||
78 | } | ||
79 | |||
80 | fn are_diagnostics_equal(left: &Diagnostic, right: &Diagnostic) -> bool { | ||
81 | left.source == right.source | ||
82 | && left.severity == right.severity | ||
83 | && left.range == right.range | ||
84 | && left.message == right.message | ||
85 | } | ||
diff --git a/crates/ra_lsp_server/src/lib.rs b/crates/ra_lsp_server/src/lib.rs index 2ca149fd5..1208c1343 100644 --- a/crates/ra_lsp_server/src/lib.rs +++ b/crates/ra_lsp_server/src/lib.rs | |||
@@ -29,6 +29,7 @@ mod markdown; | |||
29 | pub mod req; | 29 | pub mod req; |
30 | mod config; | 30 | mod config; |
31 | mod world; | 31 | mod world; |
32 | mod diagnostics; | ||
32 | 33 | ||
33 | pub type Result<T> = std::result::Result<T, Box<dyn std::error::Error + Send + Sync>>; | 34 | pub type Result<T> = std::result::Result<T, Box<dyn std::error::Error + Send + Sync>>; |
34 | pub use crate::{ | 35 | pub use crate::{ |
diff --git a/crates/ra_lsp_server/src/main_loop.rs b/crates/ra_lsp_server/src/main_loop.rs index 508fe08c0..12961ba37 100644 --- a/crates/ra_lsp_server/src/main_loop.rs +++ b/crates/ra_lsp_server/src/main_loop.rs | |||
@@ -17,16 +17,17 @@ use std::{ | |||
17 | use crossbeam_channel::{select, unbounded, RecvError, Sender}; | 17 | use crossbeam_channel::{select, unbounded, RecvError, Sender}; |
18 | use lsp_server::{Connection, ErrorCode, Message, Notification, Request, RequestId, Response}; | 18 | use lsp_server::{Connection, ErrorCode, Message, Notification, Request, RequestId, Response}; |
19 | use lsp_types::{ClientCapabilities, NumberOrString}; | 19 | use lsp_types::{ClientCapabilities, NumberOrString}; |
20 | use ra_cargo_watch::{CheckOptions, CheckTask}; | 20 | use ra_cargo_watch::{url_from_path_with_drive_lowercasing, CheckOptions, CheckTask}; |
21 | use ra_ide::{Canceled, FeatureFlags, FileId, LibraryData, SourceRootId}; | 21 | use ra_ide::{Canceled, FeatureFlags, FileId, LibraryData, SourceRootId}; |
22 | use ra_prof::profile; | 22 | use ra_prof::profile; |
23 | use ra_vfs::{VfsTask, Watch}; | 23 | use ra_vfs::{VfsFile, VfsTask, Watch}; |
24 | use relative_path::RelativePathBuf; | 24 | use relative_path::RelativePathBuf; |
25 | use rustc_hash::FxHashSet; | 25 | use rustc_hash::FxHashSet; |
26 | use serde::{de::DeserializeOwned, Serialize}; | 26 | use serde::{de::DeserializeOwned, Serialize}; |
27 | use threadpool::ThreadPool; | 27 | use threadpool::ThreadPool; |
28 | 28 | ||
29 | use crate::{ | 29 | use crate::{ |
30 | diagnostics::DiagnosticTask, | ||
30 | main_loop::{ | 31 | main_loop::{ |
31 | pending_requests::{PendingRequest, PendingRequests}, | 32 | pending_requests::{PendingRequest, PendingRequests}, |
32 | subscriptions::Subscriptions, | 33 | subscriptions::Subscriptions, |
@@ -254,6 +255,7 @@ pub fn main_loop( | |||
254 | enum Task { | 255 | enum Task { |
255 | Respond(Response), | 256 | Respond(Response), |
256 | Notify(Notification), | 257 | Notify(Notification), |
258 | Diagnostic(DiagnosticTask), | ||
257 | } | 259 | } |
258 | 260 | ||
259 | enum Event { | 261 | enum Event { |
@@ -359,7 +361,7 @@ fn loop_turn( | |||
359 | world_state.maybe_collect_garbage(); | 361 | world_state.maybe_collect_garbage(); |
360 | loop_state.in_flight_libraries -= 1; | 362 | loop_state.in_flight_libraries -= 1; |
361 | } | 363 | } |
362 | Event::CheckWatcher(task) => on_check_task(pool, task, world_state, task_sender)?, | 364 | Event::CheckWatcher(task) => on_check_task(task, world_state, task_sender)?, |
363 | Event::Msg(msg) => match msg { | 365 | Event::Msg(msg) => match msg { |
364 | Message::Request(req) => on_request( | 366 | Message::Request(req) => on_request( |
365 | world_state, | 367 | world_state, |
@@ -464,6 +466,7 @@ fn on_task( | |||
464 | Task::Notify(n) => { | 466 | Task::Notify(n) => { |
465 | msg_sender.send(n.into()).unwrap(); | 467 | msg_sender.send(n.into()).unwrap(); |
466 | } | 468 | } |
469 | Task::Diagnostic(task) => on_diagnostic_task(task, msg_sender, state), | ||
467 | } | 470 | } |
468 | } | 471 | } |
469 | 472 | ||
@@ -621,23 +624,26 @@ fn on_notification( | |||
621 | } | 624 | } |
622 | 625 | ||
623 | fn on_check_task( | 626 | fn on_check_task( |
624 | pool: &ThreadPool, | ||
625 | task: CheckTask, | 627 | task: CheckTask, |
626 | world_state: &mut WorldState, | 628 | world_state: &mut WorldState, |
627 | task_sender: &Sender<Task>, | 629 | task_sender: &Sender<Task>, |
628 | ) -> Result<()> { | 630 | ) -> Result<()> { |
629 | let urls = match task { | 631 | match task { |
630 | CheckTask::ClearDiagnostics => { | 632 | CheckTask::ClearDiagnostics => { |
631 | let state = Arc::get_mut(&mut world_state.check_watcher.state) | 633 | task_sender.send(Task::Diagnostic(DiagnosticTask::ClearCheck))?; |
632 | .expect("couldn't get check watcher state as mutable"); | ||
633 | state.clear() | ||
634 | } | 634 | } |
635 | 635 | ||
636 | CheckTask::AddDiagnostic(url, diagnostic) => { | 636 | CheckTask::AddDiagnostic { url, diagnostic, fixes } => { |
637 | let state = Arc::get_mut(&mut world_state.check_watcher.state) | 637 | let path = url.to_file_path().map_err(|()| format!("invalid uri: {}", url))?; |
638 | .expect("couldn't get check watcher state as mutable"); | 638 | let file_id = world_state |
639 | state.add_diagnostic_with_fixes(url.clone(), diagnostic); | 639 | .vfs |
640 | vec![url] | 640 | .read() |
641 | .path2file(&path) | ||
642 | .map(|it| FileId(it.0)) | ||
643 | .ok_or_else(|| format!("unknown file: {}", path.to_string_lossy()))?; | ||
644 | |||
645 | task_sender | ||
646 | .send(Task::Diagnostic(DiagnosticTask::AddCheck(file_id, diagnostic, fixes)))?; | ||
641 | } | 647 | } |
642 | 648 | ||
643 | CheckTask::Status(progress) => { | 649 | CheckTask::Status(progress) => { |
@@ -647,31 +653,30 @@ fn on_check_task( | |||
647 | }; | 653 | }; |
648 | let not = notification_new::<req::Progress>(params); | 654 | let not = notification_new::<req::Progress>(params); |
649 | task_sender.send(Task::Notify(not)).unwrap(); | 655 | task_sender.send(Task::Notify(not)).unwrap(); |
650 | Vec::new() | ||
651 | } | 656 | } |
652 | }; | 657 | }; |
653 | 658 | ||
654 | let subscriptions = urls | 659 | Ok(()) |
655 | .into_iter() | 660 | } |
656 | .map(|url| { | ||
657 | let path = url.to_file_path().map_err(|()| format!("invalid uri: {}", url))?; | ||
658 | Ok(world_state.vfs.read().path2file(&path).map(|it| FileId(it.0))) | ||
659 | }) | ||
660 | .filter_map(|res| res.transpose()) | ||
661 | .collect::<Result<Vec<_>>>()?; | ||
662 | 661 | ||
663 | // We manually send a diagnostic update when the watcher asks | 662 | fn on_diagnostic_task(task: DiagnosticTask, msg_sender: &Sender<Message>, state: &mut WorldState) { |
664 | // us to, to avoid the issue of having to change the file to | 663 | let subscriptions = state.diagnostics.handle_task(task); |
665 | // receive updated diagnostics. | ||
666 | update_file_notifications_on_threadpool( | ||
667 | pool, | ||
668 | world_state.snapshot(), | ||
669 | false, | ||
670 | task_sender.clone(), | ||
671 | subscriptions, | ||
672 | ); | ||
673 | 664 | ||
674 | Ok(()) | 665 | for file_id in subscriptions { |
666 | let path = state.vfs.read().file2path(VfsFile(file_id.0)); | ||
667 | let uri = match url_from_path_with_drive_lowercasing(&path) { | ||
668 | Ok(uri) => uri, | ||
669 | Err(err) => { | ||
670 | log::error!("Couldn't convert path to url ({}): {:?}", err, path.to_string_lossy()); | ||
671 | continue; | ||
672 | } | ||
673 | }; | ||
674 | |||
675 | let diagnostics = state.diagnostics.diagnostics_for(file_id).cloned().collect(); | ||
676 | let params = req::PublishDiagnosticsParams { uri, diagnostics, version: None }; | ||
677 | let not = notification_new::<req::PublishDiagnostics>(params); | ||
678 | msg_sender.send(not.into()).unwrap(); | ||
679 | } | ||
675 | } | 680 | } |
676 | 681 | ||
677 | struct PoolDispatcher<'a> { | 682 | struct PoolDispatcher<'a> { |
@@ -819,9 +824,8 @@ fn update_file_notifications_on_threadpool( | |||
819 | log::error!("failed to compute diagnostics: {:?}", e); | 824 | log::error!("failed to compute diagnostics: {:?}", e); |
820 | } | 825 | } |
821 | } | 826 | } |
822 | Ok(params) => { | 827 | Ok(task) => { |
823 | let not = notification_new::<req::PublishDiagnostics>(params); | 828 | task_sender.send(Task::Diagnostic(task)).unwrap(); |
824 | task_sender.send(Task::Notify(not)).unwrap(); | ||
825 | } | 829 | } |
826 | } | 830 | } |
827 | } | 831 | } |
diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs index 9aa1e7eea..839bca342 100644 --- a/crates/ra_lsp_server/src/main_loop/handlers.rs +++ b/crates/ra_lsp_server/src/main_loop/handlers.rs | |||
@@ -33,6 +33,7 @@ use crate::{ | |||
33 | to_call_hierarchy_item, to_location, Conv, ConvWith, FoldConvCtx, MapConvWith, TryConvWith, | 33 | to_call_hierarchy_item, to_location, Conv, ConvWith, FoldConvCtx, MapConvWith, TryConvWith, |
34 | TryConvWithToVec, | 34 | TryConvWithToVec, |
35 | }, | 35 | }, |
36 | diagnostics::DiagnosticTask, | ||
36 | req::{self, Decoration, InlayHint, InlayHintsParams, InlayKind}, | 37 | req::{self, Decoration, InlayHint, InlayHintsParams, InlayKind}, |
37 | world::WorldSnapshot, | 38 | world::WorldSnapshot, |
38 | LspError, Result, | 39 | LspError, Result, |
@@ -656,6 +657,7 @@ pub fn handle_code_action( | |||
656 | .filter(|(diag_range, _fix)| diag_range.intersection(&range).is_some()) | 657 | .filter(|(diag_range, _fix)| diag_range.intersection(&range).is_some()) |
657 | .map(|(_range, fix)| fix); | 658 | .map(|(_range, fix)| fix); |
658 | 659 | ||
660 | // TODO: When done, we won't need this, only the one pulling from world.diagnostics | ||
659 | for source_edit in fixes_from_diagnostics { | 661 | for source_edit in fixes_from_diagnostics { |
660 | let title = source_edit.label.clone(); | 662 | let title = source_edit.label.clone(); |
661 | let edit = source_edit.try_conv_with(&world)?; | 663 | let edit = source_edit.try_conv_with(&world)?; |
@@ -676,28 +678,12 @@ pub fn handle_code_action( | |||
676 | res.push(action.into()); | 678 | res.push(action.into()); |
677 | } | 679 | } |
678 | 680 | ||
679 | for fix in world.check_watcher.fixes_for(¶ms.text_document.uri).into_iter().flatten() { | 681 | for fix in world.check_fixes.get(&file_id).into_iter().flatten() { |
680 | let fix_range = fix.location.range.conv_with(&line_index); | 682 | let fix_range = fix.range.conv_with(&line_index); |
681 | if fix_range.intersection(&range).is_none() { | 683 | if fix_range.intersection(&range).is_none() { |
682 | continue; | 684 | continue; |
683 | } | 685 | } |
684 | 686 | res.push(fix.action.clone()); | |
685 | let edit = { | ||
686 | let edits = vec![TextEdit::new(fix.location.range, fix.replacement.clone())]; | ||
687 | let mut edit_map = std::collections::HashMap::new(); | ||
688 | edit_map.insert(fix.location.uri.clone(), edits); | ||
689 | WorkspaceEdit::new(edit_map) | ||
690 | }; | ||
691 | |||
692 | let action = CodeAction { | ||
693 | title: fix.title.clone(), | ||
694 | kind: Some("quickfix".to_string()), | ||
695 | diagnostics: Some(fix.diagnostics.clone()), | ||
696 | edit: Some(edit), | ||
697 | command: None, | ||
698 | is_preferred: None, | ||
699 | }; | ||
700 | res.push(action.into()); | ||
701 | } | 687 | } |
702 | 688 | ||
703 | for assist in world.analysis().assists(FileRange { file_id, range })?.into_iter() { | 689 | for assist in world.analysis().assists(FileRange { file_id, range })?.into_iter() { |
@@ -875,14 +861,10 @@ pub fn handle_document_highlight( | |||
875 | )) | 861 | )) |
876 | } | 862 | } |
877 | 863 | ||
878 | pub fn publish_diagnostics( | 864 | pub fn publish_diagnostics(world: &WorldSnapshot, file_id: FileId) -> Result<DiagnosticTask> { |
879 | world: &WorldSnapshot, | ||
880 | file_id: FileId, | ||
881 | ) -> Result<req::PublishDiagnosticsParams> { | ||
882 | let _p = profile("publish_diagnostics"); | 865 | let _p = profile("publish_diagnostics"); |
883 | let uri = world.file_id_to_uri(file_id)?; | ||
884 | let line_index = world.analysis().file_line_index(file_id)?; | 866 | let line_index = world.analysis().file_line_index(file_id)?; |
885 | let mut diagnostics: Vec<Diagnostic> = world | 867 | let diagnostics: Vec<Diagnostic> = world |
886 | .analysis() | 868 | .analysis() |
887 | .diagnostics(file_id)? | 869 | .diagnostics(file_id)? |
888 | .into_iter() | 870 | .into_iter() |
@@ -896,10 +878,7 @@ pub fn publish_diagnostics( | |||
896 | tags: None, | 878 | tags: None, |
897 | }) | 879 | }) |
898 | .collect(); | 880 | .collect(); |
899 | if let Some(check_diags) = world.check_watcher.diagnostics_for(&uri) { | 881 | Ok(DiagnosticTask::SetNative(file_id, diagnostics)) |
900 | diagnostics.extend(check_diags.iter().cloned()); | ||
901 | } | ||
902 | Ok(req::PublishDiagnosticsParams { uri, diagnostics, version: None }) | ||
903 | } | 882 | } |
904 | 883 | ||
905 | pub fn publish_decorations( | 884 | pub fn publish_decorations( |
diff --git a/crates/ra_lsp_server/src/world.rs b/crates/ra_lsp_server/src/world.rs index 3059ef9ec..1ee02b47c 100644 --- a/crates/ra_lsp_server/src/world.rs +++ b/crates/ra_lsp_server/src/world.rs | |||
@@ -12,9 +12,7 @@ use crossbeam_channel::{unbounded, Receiver}; | |||
12 | use lsp_server::ErrorCode; | 12 | use lsp_server::ErrorCode; |
13 | use lsp_types::Url; | 13 | use lsp_types::Url; |
14 | use parking_lot::RwLock; | 14 | use parking_lot::RwLock; |
15 | use ra_cargo_watch::{ | 15 | use ra_cargo_watch::{url_from_path_with_drive_lowercasing, CheckOptions, CheckWatcher}; |
16 | url_from_path_with_drive_lowercasing, CheckOptions, CheckState, CheckWatcher, | ||
17 | }; | ||
18 | use ra_ide::{ | 16 | use ra_ide::{ |
19 | Analysis, AnalysisChange, AnalysisHost, CrateGraph, FeatureFlags, FileId, LibraryData, | 17 | Analysis, AnalysisChange, AnalysisHost, CrateGraph, FeatureFlags, FileId, LibraryData, |
20 | SourceRootId, | 18 | SourceRootId, |
@@ -25,6 +23,7 @@ use ra_vfs_glob::{Glob, RustPackageFilterBuilder}; | |||
25 | use relative_path::RelativePathBuf; | 23 | use relative_path::RelativePathBuf; |
26 | 24 | ||
27 | use crate::{ | 25 | use crate::{ |
26 | diagnostics::{CheckFixes, DiagnosticCollection}, | ||
28 | main_loop::pending_requests::{CompletedRequest, LatestRequests}, | 27 | main_loop::pending_requests::{CompletedRequest, LatestRequests}, |
29 | LspError, Result, | 28 | LspError, Result, |
30 | }; | 29 | }; |
@@ -55,6 +54,7 @@ pub struct WorldState { | |||
55 | pub task_receiver: Receiver<VfsTask>, | 54 | pub task_receiver: Receiver<VfsTask>, |
56 | pub latest_requests: Arc<RwLock<LatestRequests>>, | 55 | pub latest_requests: Arc<RwLock<LatestRequests>>, |
57 | pub check_watcher: CheckWatcher, | 56 | pub check_watcher: CheckWatcher, |
57 | pub diagnostics: DiagnosticCollection, | ||
58 | } | 58 | } |
59 | 59 | ||
60 | /// An immutable snapshot of the world's state at a point in time. | 60 | /// An immutable snapshot of the world's state at a point in time. |
@@ -63,7 +63,7 @@ pub struct WorldSnapshot { | |||
63 | pub workspaces: Arc<Vec<ProjectWorkspace>>, | 63 | pub workspaces: Arc<Vec<ProjectWorkspace>>, |
64 | pub analysis: Analysis, | 64 | pub analysis: Analysis, |
65 | pub latest_requests: Arc<RwLock<LatestRequests>>, | 65 | pub latest_requests: Arc<RwLock<LatestRequests>>, |
66 | pub check_watcher: CheckState, | 66 | pub check_fixes: CheckFixes, |
67 | vfs: Arc<RwLock<Vfs>>, | 67 | vfs: Arc<RwLock<Vfs>>, |
68 | } | 68 | } |
69 | 69 | ||
@@ -159,6 +159,7 @@ impl WorldState { | |||
159 | task_receiver, | 159 | task_receiver, |
160 | latest_requests: Default::default(), | 160 | latest_requests: Default::default(), |
161 | check_watcher, | 161 | check_watcher, |
162 | diagnostics: Default::default(), | ||
162 | } | 163 | } |
163 | } | 164 | } |
164 | 165 | ||
@@ -220,7 +221,7 @@ impl WorldState { | |||
220 | analysis: self.analysis_host.analysis(), | 221 | analysis: self.analysis_host.analysis(), |
221 | vfs: Arc::clone(&self.vfs), | 222 | vfs: Arc::clone(&self.vfs), |
222 | latest_requests: Arc::clone(&self.latest_requests), | 223 | latest_requests: Arc::clone(&self.latest_requests), |
223 | check_watcher: (*self.check_watcher.state).clone(), | 224 | check_fixes: Arc::clone(&self.diagnostics.check_fixes), |
224 | } | 225 | } |
225 | } | 226 | } |
226 | 227 | ||