feat: improve performance by delaying computation of fixes for diagnostics

This commit is contained in:
Aleksey Kladov 2021-04-13 11:48:12 +03:00
parent 04b5fcfdb2
commit 06a633ff42
7 changed files with 74 additions and 40 deletions

View file

@ -84,6 +84,7 @@ pub struct DiagnosticsConfig {
pub(crate) fn diagnostics(
db: &RootDatabase,
config: &DiagnosticsConfig,
resolve: bool,
file_id: FileId,
) -> Vec<Diagnostic> {
let _p = profile::span("diagnostics");
@ -107,25 +108,25 @@ pub(crate) fn diagnostics(
let res = RefCell::new(res);
let sink_builder = DiagnosticSinkBuilder::new()
.on::<hir::diagnostics::UnresolvedModule, _>(|d| {
res.borrow_mut().push(diagnostic_with_fix(d, &sema));
res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve));
})
.on::<hir::diagnostics::MissingFields, _>(|d| {
res.borrow_mut().push(diagnostic_with_fix(d, &sema));
res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve));
})
.on::<hir::diagnostics::MissingOkOrSomeInTailExpr, _>(|d| {
res.borrow_mut().push(diagnostic_with_fix(d, &sema));
res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve));
})
.on::<hir::diagnostics::NoSuchField, _>(|d| {
res.borrow_mut().push(diagnostic_with_fix(d, &sema));
res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve));
})
.on::<hir::diagnostics::RemoveThisSemicolon, _>(|d| {
res.borrow_mut().push(diagnostic_with_fix(d, &sema));
res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve));
})
.on::<hir::diagnostics::IncorrectCase, _>(|d| {
res.borrow_mut().push(warning_with_fix(d, &sema));
res.borrow_mut().push(warning_with_fix(d, &sema, resolve));
})
.on::<hir::diagnostics::ReplaceFilterMapNextWithFindMap, _>(|d| {
res.borrow_mut().push(warning_with_fix(d, &sema));
res.borrow_mut().push(warning_with_fix(d, &sema, resolve));
})
.on::<hir::diagnostics::InactiveCode, _>(|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: DiagnosticWithFix>(d: &D, sema: &Semantics<RootDatabase>) -> Diagnostic {
fn diagnostic_with_fix<D: DiagnosticWithFix>(
d: &D,
sema: &Semantics<RootDatabase>,
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: DiagnosticWithFix>(d: &D, sema: &Semantics<RootDatabase>) -> Diagnostic {
fn warning_with_fix<D: DiagnosticWithFix>(
d: &D,
sema: &Semantics<RootDatabase>,
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::<Vec<_>>();
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);

View file

@ -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<RootDatabase>) -> Option<Assist>;
/// `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<RootDatabase>, _resolve: bool) -> Option<Assist>;
}
impl DiagnosticWithFix for UnresolvedModule {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Assist> {
fn fix(&self, sema: &Semantics<RootDatabase>, _resolve: bool) -> Option<Assist> {
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<RootDatabase>) -> Option<Assist> {
fn fix(&self, sema: &Semantics<RootDatabase>, _resolve: bool) -> Option<Assist> {
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<RootDatabase>) -> Option<Assist> {
fn fix(&self, sema: &Semantics<RootDatabase>, _resolve: bool) -> Option<Assist> {
// 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<RootDatabase>) -> Option<Assist> {
fn fix(&self, sema: &Semantics<RootDatabase>, _resolve: bool) -> Option<Assist> {
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<RootDatabase>) -> Option<Assist> {
fn fix(&self, sema: &Semantics<RootDatabase>, _resolve: bool) -> Option<Assist> {
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<RootDatabase>) -> Option<Assist> {
fn fix(&self, sema: &Semantics<RootDatabase>, resolve: bool) -> Option<Assist> {
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<RootDatabase>) -> Option<Assist> {
fn fix(&self, sema: &Semantics<RootDatabase>, _resolve: bool) -> Option<Assist> {
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())?;

View file

@ -50,7 +50,7 @@ impl Diagnostic for UnlinkedFile {
}
impl DiagnosticWithFix for UnlinkedFile {
fn fix(&self, sema: &hir::Semantics<RootDatabase>) -> Option<Assist> {
fn fix(&self, sema: &hir::Semantics<RootDatabase>, _resolve: bool) -> Option<Assist> {
// If there's an existing module that could add a `mod` item to include the unlinked file,
// suggest that as a fix.

View file

@ -526,9 +526,10 @@ impl Analysis {
pub fn diagnostics(
&self,
config: &DiagnosticsConfig,
resolve: bool,
file_id: FileId,
) -> Cancelable<Vec<Diagnostic>> {
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

View file

@ -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;

View file

@ -1182,7 +1182,7 @@ pub(crate) fn publish_diagnostics(
let diagnostics: Vec<Diagnostic> = 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),

View file

@ -168,6 +168,7 @@ impl Server {
self.send_notification(r)
}
#[track_caller]
pub(crate) fn request<R>(&self, params: R::Params, expected_resp: Value)
where
R: lsp_types::request::Request,