From 06a633ff421b764428bb946ced914e59532fe13f Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 13 Apr 2021 11:48:12 +0300 Subject: feat: improve performance by delaying computation of fixes for diagnostics --- crates/ide/src/diagnostics.rs | 60 ++++++++++++++-------- crates/ide/src/diagnostics/fixes.rs | 38 +++++++++----- crates/ide/src/diagnostics/unlinked_file.rs | 2 +- crates/ide/src/lib.rs | 8 +-- crates/rust-analyzer/src/cli/diagnostics.rs | 3 +- crates/rust-analyzer/src/handlers.rs | 2 +- .../rust-analyzer/tests/rust-analyzer/support.rs | 1 + 7 files changed, 74 insertions(+), 40 deletions(-) diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 4f0b4a62e..9a883acb9 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -84,6 +84,7 @@ pub struct DiagnosticsConfig { pub(crate) fn diagnostics( db: &RootDatabase, config: &DiagnosticsConfig, + resolve: bool, file_id: FileId, ) -> Vec { let _p = profile::span("diagnostics"); @@ -107,25 +108,25 @@ pub(crate) fn diagnostics( let res = RefCell::new(res); let sink_builder = DiagnosticSinkBuilder::new() .on::(|d| { - res.borrow_mut().push(diagnostic_with_fix(d, &sema)); + res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve)); }) .on::(|d| { - res.borrow_mut().push(diagnostic_with_fix(d, &sema)); + res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve)); }) .on::(|d| { - res.borrow_mut().push(diagnostic_with_fix(d, &sema)); + res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve)); }) .on::(|d| { - res.borrow_mut().push(diagnostic_with_fix(d, &sema)); + res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve)); }) .on::(|d| { - res.borrow_mut().push(diagnostic_with_fix(d, &sema)); + res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve)); }) .on::(|d| { - res.borrow_mut().push(warning_with_fix(d, &sema)); + res.borrow_mut().push(warning_with_fix(d, &sema, resolve)); }) .on::(|d| { - res.borrow_mut().push(warning_with_fix(d, &sema)); + res.borrow_mut().push(warning_with_fix(d, &sema, resolve)); }) .on::(|d| { // If there's inactive code somewhere in a macro, don't propagate to the call-site. @@ -152,7 +153,7 @@ pub(crate) fn diagnostics( // Override severity and mark as unused. res.borrow_mut().push( Diagnostic::hint(range, d.message()) - .with_fix(d.fix(&sema)) + .with_fix(d.fix(&sema, resolve)) .with_code(Some(d.code())), ); }) @@ -208,15 +209,23 @@ pub(crate) fn diagnostics( res.into_inner() } -fn diagnostic_with_fix(d: &D, sema: &Semantics) -> Diagnostic { +fn diagnostic_with_fix( + d: &D, + sema: &Semantics, + resolve: bool, +) -> Diagnostic { Diagnostic::error(sema.diagnostics_display_range(d.display_source()).range, d.message()) - .with_fix(d.fix(&sema)) + .with_fix(d.fix(&sema, resolve)) .with_code(Some(d.code())) } -fn warning_with_fix(d: &D, sema: &Semantics) -> Diagnostic { +fn warning_with_fix( + d: &D, + sema: &Semantics, + resolve: bool, +) -> Diagnostic { Diagnostic::hint(sema.diagnostics_display_range(d.display_source()).range, d.message()) - .with_fix(d.fix(&sema)) + .with_fix(d.fix(&sema, resolve)) .with_code(Some(d.code())) } @@ -271,13 +280,19 @@ fn text_edit_for_remove_unnecessary_braces_with_self_in_use_statement( } fn fix(id: &'static str, label: &str, source_change: SourceChange, target: TextRange) -> Assist { + let mut res = unresolved_fix(id, label, target); + res.source_change = Some(source_change); + res +} + +fn unresolved_fix(id: &'static str, label: &str, target: TextRange) -> Assist { assert!(!id.contains(' ')); Assist { id: AssistId(id, AssistKind::QuickFix), label: Label::new(label), group: None, target, - source_change: Some(source_change), + source_change: None, } } @@ -299,7 +314,7 @@ mod tests { let (analysis, file_position) = fixture::position(ra_fixture_before); let diagnostic = analysis - .diagnostics(&DiagnosticsConfig::default(), file_position.file_id) + .diagnostics(&DiagnosticsConfig::default(), true, file_position.file_id) .unwrap() .pop() .unwrap(); @@ -328,7 +343,7 @@ mod tests { fn check_no_fix(ra_fixture: &str) { let (analysis, file_position) = fixture::position(ra_fixture); let diagnostic = analysis - .diagnostics(&DiagnosticsConfig::default(), file_position.file_id) + .diagnostics(&DiagnosticsConfig::default(), true, file_position.file_id) .unwrap() .pop() .unwrap(); @@ -342,7 +357,7 @@ mod tests { let diagnostics = files .into_iter() .flat_map(|file_id| { - analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap() + analysis.diagnostics(&DiagnosticsConfig::default(), true, file_id).unwrap() }) .collect::>(); assert_eq!(diagnostics.len(), 0, "unexpected diagnostics:\n{:#?}", diagnostics); @@ -350,7 +365,8 @@ mod tests { fn check_expect(ra_fixture: &str, expect: Expect) { let (analysis, file_id) = fixture::file(ra_fixture); - let diagnostics = analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap(); + let diagnostics = + analysis.diagnostics(&DiagnosticsConfig::default(), true, file_id).unwrap(); expect.assert_debug_eq(&diagnostics) } @@ -895,10 +911,11 @@ struct Foo { let (analysis, file_id) = fixture::file(r#"mod foo;"#); - let diagnostics = analysis.diagnostics(&config, file_id).unwrap(); + let diagnostics = analysis.diagnostics(&config, true, file_id).unwrap(); assert!(diagnostics.is_empty()); - let diagnostics = analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap(); + let diagnostics = + analysis.diagnostics(&DiagnosticsConfig::default(), true, file_id).unwrap(); assert!(!diagnostics.is_empty()); } @@ -1004,8 +1021,9 @@ impl TestStruct { let expected = r#"fn foo() {}"#; let (analysis, file_position) = fixture::position(input); - let diagnostics = - analysis.diagnostics(&DiagnosticsConfig::default(), file_position.file_id).unwrap(); + let diagnostics = analysis + .diagnostics(&DiagnosticsConfig::default(), true, file_position.file_id) + .unwrap(); assert_eq!(diagnostics.len(), 1); check_fix(input, expected); diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs index 69cf5288c..7be8b3459 100644 --- a/crates/ide/src/diagnostics/fixes.rs +++ b/crates/ide/src/diagnostics/fixes.rs @@ -20,17 +20,26 @@ use syntax::{ }; use text_edit::TextEdit; -use crate::{diagnostics::fix, references::rename::rename_with_semantics, Assist, FilePosition}; +use crate::{ + diagnostics::{fix, unresolved_fix}, + references::rename::rename_with_semantics, + Assist, FilePosition, +}; /// A [Diagnostic] that potentially has a fix available. /// /// [Diagnostic]: hir::diagnostics::Diagnostic pub(crate) trait DiagnosticWithFix: Diagnostic { - fn fix(&self, sema: &Semantics) -> Option; + /// `resolve` determines if the diagnostic should fill in the `edit` field + /// of the assist. + /// + /// If `resolve` is false, the edit will be computed later, on demand, and + /// can be omitted. + fn fix(&self, sema: &Semantics, _resolve: bool) -> Option; } impl DiagnosticWithFix for UnresolvedModule { - fn fix(&self, sema: &Semantics) -> Option { + fn fix(&self, sema: &Semantics, _resolve: bool) -> Option { let root = sema.db.parse_or_expand(self.file)?; let unresolved_module = self.decl.to_node(&root); Some(fix( @@ -50,7 +59,7 @@ impl DiagnosticWithFix for UnresolvedModule { } impl DiagnosticWithFix for NoSuchField { - fn fix(&self, sema: &Semantics) -> Option { + fn fix(&self, sema: &Semantics, _resolve: bool) -> Option { let root = sema.db.parse_or_expand(self.file)?; missing_record_expr_field_fix( &sema, @@ -61,7 +70,7 @@ impl DiagnosticWithFix for NoSuchField { } impl DiagnosticWithFix for MissingFields { - fn fix(&self, sema: &Semantics) -> Option { + fn fix(&self, sema: &Semantics, _resolve: bool) -> Option { // Note that although we could add a diagnostics to // fill the missing tuple field, e.g : // `struct A(usize);` @@ -97,7 +106,7 @@ impl DiagnosticWithFix for MissingFields { } impl DiagnosticWithFix for MissingOkOrSomeInTailExpr { - fn fix(&self, sema: &Semantics) -> Option { + fn fix(&self, sema: &Semantics, _resolve: bool) -> Option { 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(); @@ -110,7 +119,7 @@ impl DiagnosticWithFix for MissingOkOrSomeInTailExpr { } impl DiagnosticWithFix for RemoveThisSemicolon { - fn fix(&self, sema: &Semantics) -> Option { + fn fix(&self, sema: &Semantics, _resolve: bool) -> Option { let root = sema.db.parse_or_expand(self.file)?; let semicolon = self @@ -130,7 +139,7 @@ impl DiagnosticWithFix for RemoveThisSemicolon { } impl DiagnosticWithFix for IncorrectCase { - fn fix(&self, sema: &Semantics) -> Option { + fn fix(&self, sema: &Semantics, resolve: bool) -> Option { let root = sema.db.parse_or_expand(self.file)?; let name_node = self.ident.to_node(&root); @@ -138,16 +147,19 @@ impl DiagnosticWithFix for IncorrectCase { let frange = name_node.original_file_range(sema.db); let file_position = FilePosition { file_id: frange.file_id, offset: frange.range.start() }; - let rename_changes = - rename_with_semantics(sema, file_position, &self.suggested_text).ok()?; - let label = format!("Rename to {}", self.suggested_text); - Some(fix("change_case", &label, rename_changes, frange.range)) + let mut res = unresolved_fix("change_case", &label, frange.range); + if resolve { + let source_change = rename_with_semantics(sema, file_position, &self.suggested_text); + res.source_change = Some(source_change.ok().unwrap_or_default()); + } + + Some(res) } } impl DiagnosticWithFix for ReplaceFilterMapNextWithFindMap { - fn fix(&self, sema: &Semantics) -> Option { + fn fix(&self, sema: &Semantics, _resolve: bool) -> Option { 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())?; diff --git a/crates/ide/src/diagnostics/unlinked_file.rs b/crates/ide/src/diagnostics/unlinked_file.rs index 5482b7287..7d39f4fbe 100644 --- a/crates/ide/src/diagnostics/unlinked_file.rs +++ b/crates/ide/src/diagnostics/unlinked_file.rs @@ -50,7 +50,7 @@ impl Diagnostic for UnlinkedFile { } impl DiagnosticWithFix for UnlinkedFile { - fn fix(&self, sema: &hir::Semantics) -> Option { + fn fix(&self, sema: &hir::Semantics, _resolve: bool) -> Option { // If there's an existing module that could add a `mod` item to include the unlinked file, // suggest that as a fix. diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index bbc0d5eec..d481be09d 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -526,9 +526,10 @@ impl Analysis { pub fn diagnostics( &self, config: &DiagnosticsConfig, + resolve: bool, file_id: FileId, ) -> Cancelable> { - self.with_db(|db| diagnostics::diagnostics(db, config, file_id)) + self.with_db(|db| diagnostics::diagnostics(db, config, resolve, file_id)) } /// Convenience function to return assists + quick fixes for diagnostics @@ -550,9 +551,10 @@ impl Analysis { if include_fixes { res.extend( - diagnostics::diagnostics(db, diagnostics_config, frange.file_id) + diagnostics::diagnostics(db, diagnostics_config, resolve, frange.file_id) .into_iter() - .filter_map(|it| it.fix), + .filter_map(|it| it.fix) + .filter(|it| it.target.intersect(frange.range).is_some()), ); } res diff --git a/crates/rust-analyzer/src/cli/diagnostics.rs b/crates/rust-analyzer/src/cli/diagnostics.rs index 0085d0e4d..74f784338 100644 --- a/crates/rust-analyzer/src/cli/diagnostics.rs +++ b/crates/rust-analyzer/src/cli/diagnostics.rs @@ -57,7 +57,8 @@ pub fn diagnostics( let crate_name = module.krate().display_name(db).as_deref().unwrap_or("unknown").to_string(); println!("processing crate: {}, module: {}", crate_name, _vfs.file_path(file_id)); - for diagnostic in analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap() + for diagnostic in + analysis.diagnostics(&DiagnosticsConfig::default(), false, file_id).unwrap() { if matches!(diagnostic.severity, Severity::Error) { found_error = true; diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 85cc8602e..4f0c9d23c 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -1182,7 +1182,7 @@ pub(crate) fn publish_diagnostics( let diagnostics: Vec = snap .analysis - .diagnostics(&snap.config.diagnostics(), file_id)? + .diagnostics(&snap.config.diagnostics(), false, file_id)? .into_iter() .map(|d| Diagnostic { range: to_proto::range(&line_index, d.range), diff --git a/crates/rust-analyzer/tests/rust-analyzer/support.rs b/crates/rust-analyzer/tests/rust-analyzer/support.rs index 5e388c0f0..75e677762 100644 --- a/crates/rust-analyzer/tests/rust-analyzer/support.rs +++ b/crates/rust-analyzer/tests/rust-analyzer/support.rs @@ -168,6 +168,7 @@ impl Server { self.send_notification(r) } + #[track_caller] pub(crate) fn request(&self, params: R::Params, expected_resp: Value) where R: lsp_types::request::Request, -- cgit v1.2.3