Skip to content

Commit

Permalink
perf: Segregate syntax and semantic diagnostics
Browse files Browse the repository at this point in the history
  • Loading branch information
@ShoyuVanilla
ShoyuVanilla committed Aug 2, 2024
1 parent aa00ddc commit eec6375
Show file tree
Hide file tree
Showing 9 changed files with 256 additions and 141 deletions.
78 changes: 61 additions & 17 deletions crates/ide-diagnostics/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ use syntax::{
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
pub enum DiagnosticCode {
RustcHardError(&'static str),
SyntaxError,
RustcLint(&'static str),
Clippy(&'static str),
Ra(&'static str, Severity),
Expand All @@ -107,6 +108,9 @@ impl DiagnosticCode {
DiagnosticCode::RustcHardError(e) => {
format!("https://doc.rust-lang.org/stable/error_codes/{e}.html")
}
DiagnosticCode::SyntaxError => {
String::from("https://doc.rust-lang.org/stable/reference/")
}
DiagnosticCode::RustcLint(e) => {
format!("https://doc.rust-lang.org/rustc/?search={e}")
}
Expand All @@ -125,6 +129,7 @@ impl DiagnosticCode {
| DiagnosticCode::RustcLint(r)
| DiagnosticCode::Clippy(r)
| DiagnosticCode::Ra(r, _) => r,
DiagnosticCode::SyntaxError => "syntax-error",
}
}
}
Expand Down Expand Up @@ -154,7 +159,7 @@ impl Diagnostic {
message,
range: range.into(),
severity: match code {
DiagnosticCode::RustcHardError(_) => Severity::Error,
DiagnosticCode::RustcHardError(_) | DiagnosticCode::SyntaxError => Severity::Error,
// FIXME: Rustc lints are not always warning, but the ones that are currently implemented are all warnings.
DiagnosticCode::RustcLint(_) => Severity::Warning,
// FIXME: We can make this configurable, and if the user uses `cargo clippy` on flycheck, we can
Expand Down Expand Up @@ -297,31 +302,54 @@ impl DiagnosticsContext<'_> {
}
}

/// Request diagnostics for the given [`FileId`]. The produced diagnostics may point to other files
/// Request parser level diagnostics for the given [`FileId`].
pub fn syntax_diagnostics(
db: &RootDatabase,
config: &DiagnosticsConfig,
file_id: FileId,
) -> Vec<Diagnostic> {
let _p = tracing::info_span!("syntax_diagnostics").entered();

if config.disabled.contains("syntax-error") {
return Vec::new();
}

let sema = Semantics::new(db);
let file_id = sema
.attach_first_edition(file_id)
.unwrap_or_else(|| EditionedFileId::current_edition(file_id));

// [#3434] Only take first 128 errors to prevent slowing down editor/ide, the number 128 is chosen arbitrarily.
db.parse_errors(file_id)
.as_deref()
.into_iter()
.flatten()
.take(128)
.map(|err| {
Diagnostic::new(
DiagnosticCode::SyntaxError,
format!("Syntax Error: {err}"),
FileRange { file_id: file_id.into(), range: err.range() },
)
})
.collect()
}

/// Request semantic diagnostics for the given [`FileId`]. The produced diagnostics may point to other files
/// due to macros.
pub fn diagnostics(
pub fn semantic_diagnostics(
db: &RootDatabase,
config: &DiagnosticsConfig,
resolve: &AssistResolveStrategy,
file_id: FileId,
) -> Vec<Diagnostic> {
let _p = tracing::info_span!("diagnostics").entered();
let _p = tracing::info_span!("semantic_diagnostics").entered();
let sema = Semantics::new(db);
let file_id = sema
.attach_first_edition(file_id)
.unwrap_or_else(|| EditionedFileId::current_edition(file_id));
let mut res = Vec::new();

// [#34344] Only take first 128 errors to prevent slowing down editor/ide, the number 128 is chosen arbitrarily.
res.extend(db.parse_errors(file_id).as_deref().into_iter().flatten().take(128).map(|err| {
Diagnostic::new(
DiagnosticCode::RustcHardError("syntax-error"),
format!("Syntax Error: {err}"),
FileRange { file_id: file_id.into(), range: err.range() },
)
}));
let parse_errors = res.len();

let parse = sema.parse(file_id);

// FIXME: This iterates the entire file which is a rather expensive operation.
Expand All @@ -341,8 +369,11 @@ pub fn diagnostics(
match module {
// A bunch of parse errors in a file indicate some bigger structural parse changes in the
// file, so we skip semantic diagnostics so we can show these faster.
Some(m) if parse_errors < 16 => m.diagnostics(db, &mut diags, config.style_lints),
Some(_) => (),
Some(m) => {
if !db.parse_errors(file_id).as_deref().is_some_and(|es| es.len() >= 16) {
m.diagnostics(db, &mut diags, config.style_lints);
}
}
None => handlers::unlinked_file::unlinked_file(&ctx, &mut res, file_id.file_id()),
}

Expand All @@ -363,7 +394,7 @@ pub fn diagnostics(
res.extend(d.errors.iter().take(16).map(|err| {
{
Diagnostic::new(
DiagnosticCode::RustcHardError("syntax-error"),
DiagnosticCode::SyntaxError,
format!("Syntax Error in Expansion: {err}"),
ctx.resolve_precise_location(&d.node.clone(), d.precise_location),
)
Expand Down Expand Up @@ -464,6 +495,19 @@ pub fn diagnostics(
res
}

/// Request both syntax and semantic diagnostics for the given [`FileId`].
pub fn full_diagnostics(
db: &RootDatabase,
config: &DiagnosticsConfig,
resolve: &AssistResolveStrategy,
file_id: FileId,
) -> Vec<Diagnostic> {
let mut res = syntax_diagnostics(db, config, file_id);
let sema = semantic_diagnostics(db, config, resolve, file_id);
res.extend(sema);
res
}

// `__RA_EVERY_LINT` is a fake lint group to allow every lint in proc macros

static RUSTC_LINT_GROUPS_DICT: Lazy<FxHashMap<&str, Vec<&str>>> =
Expand Down
146 changes: 77 additions & 69 deletions crates/ide-diagnostics/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,14 @@ fn check_nth_fix_with_config(
let after = trim_indent(ra_fixture_after);

let (db, file_position) = RootDatabase::with_position(ra_fixture_before);
let diagnostic =
super::diagnostics(&db, &config, &AssistResolveStrategy::All, file_position.file_id.into())
.pop()
.expect("no diagnostics");
let diagnostic = super::full_diagnostics(
&db,
&config,
&AssistResolveStrategy::All,
file_position.file_id.into(),
)
.pop()
.expect("no diagnostics");
let fix = &diagnostic
.fixes
.unwrap_or_else(|| panic!("{:?} diagnostic misses fixes", diagnostic.code))[nth];
Expand Down Expand Up @@ -102,37 +106,39 @@ pub(crate) fn check_has_fix(ra_fixture_before: &str, ra_fixture_after: &str) {
let (db, file_position) = RootDatabase::with_position(ra_fixture_before);
let mut conf = DiagnosticsConfig::test_sample();
conf.expr_fill_default = ExprFillDefaultMode::Default;
let fix =
super::diagnostics(&db, &conf, &AssistResolveStrategy::All, file_position.file_id.into())
.into_iter()
.find(|d| {
d.fixes
.as_ref()
.and_then(|fixes| {
fixes.iter().find(|fix| {
if !fix.target.contains_inclusive(file_position.offset) {
return false;
}
let actual = {
let source_change = fix.source_change.as_ref().unwrap();
let file_id =
*source_change.source_file_edits.keys().next().unwrap();
let mut actual = db.file_text(file_id).to_string();
let fix = super::full_diagnostics(
&db,
&conf,
&AssistResolveStrategy::All,
file_position.file_id.into(),
)
.into_iter()
.find(|d| {
d.fixes
.as_ref()
.and_then(|fixes| {
fixes.iter().find(|fix| {
if !fix.target.contains_inclusive(file_position.offset) {
return false;
}
let actual = {
let source_change = fix.source_change.as_ref().unwrap();
let file_id = *source_change.source_file_edits.keys().next().unwrap();
let mut actual = db.file_text(file_id).to_string();

for (edit, snippet_edit) in source_change.source_file_edits.values()
{
edit.apply(&mut actual);
if let Some(snippet_edit) = snippet_edit {
snippet_edit.apply(&mut actual);
}
}
actual
};
after == actual
})
})
.is_some()
});
for (edit, snippet_edit) in source_change.source_file_edits.values() {
edit.apply(&mut actual);
if let Some(snippet_edit) = snippet_edit {
snippet_edit.apply(&mut actual);
}
}
actual
};
after == actual
})
})
.is_some()
});
assert!(fix.is_some(), "no diagnostic with desired fix");
}

Expand All @@ -144,46 +150,48 @@ pub(crate) fn check_has_single_fix(ra_fixture_before: &str, ra_fixture_after: &s
let mut conf = DiagnosticsConfig::test_sample();
conf.expr_fill_default = ExprFillDefaultMode::Default;
let mut n_fixes = 0;
let fix =
super::diagnostics(&db, &conf, &AssistResolveStrategy::All, file_position.file_id.into())
.into_iter()
.find(|d| {
d.fixes
.as_ref()
.and_then(|fixes| {
n_fixes += fixes.len();
fixes.iter().find(|fix| {
if !fix.target.contains_inclusive(file_position.offset) {
return false;
}
let actual = {
let source_change = fix.source_change.as_ref().unwrap();
let file_id =
*source_change.source_file_edits.keys().next().unwrap();
let mut actual = db.file_text(file_id).to_string();
let fix = super::full_diagnostics(
&db,
&conf,
&AssistResolveStrategy::All,
file_position.file_id.into(),
)
.into_iter()
.find(|d| {
d.fixes
.as_ref()
.and_then(|fixes| {
n_fixes += fixes.len();
fixes.iter().find(|fix| {
if !fix.target.contains_inclusive(file_position.offset) {
return false;
}
let actual = {
let source_change = fix.source_change.as_ref().unwrap();
let file_id = *source_change.source_file_edits.keys().next().unwrap();
let mut actual = db.file_text(file_id).to_string();

for (edit, snippet_edit) in source_change.source_file_edits.values()
{
edit.apply(&mut actual);
if let Some(snippet_edit) = snippet_edit {
snippet_edit.apply(&mut actual);
}
}
actual
};
after == actual
})
})
.is_some()
});
for (edit, snippet_edit) in source_change.source_file_edits.values() {
edit.apply(&mut actual);
if let Some(snippet_edit) = snippet_edit {
snippet_edit.apply(&mut actual);
}
}
actual
};
after == actual
})
})
.is_some()
});
assert!(fix.is_some(), "no diagnostic with desired fix");
assert!(n_fixes == 1, "Too many fixes suggested");
}

/// Checks that there's a diagnostic *without* fix at `$0`.
pub(crate) fn check_no_fix(ra_fixture: &str) {
let (db, file_position) = RootDatabase::with_position(ra_fixture);
let diagnostic = super::diagnostics(
let diagnostic = super::full_diagnostics(
&db,
&DiagnosticsConfig::test_sample(),
&AssistResolveStrategy::All,
Expand Down Expand Up @@ -215,7 +223,7 @@ pub(crate) fn check_diagnostics_with_config(config: DiagnosticsConfig, ra_fixtur
.iter()
.copied()
.flat_map(|file_id| {
super::diagnostics(&db, &config, &AssistResolveStrategy::All, file_id.into())
super::full_diagnostics(&db, &config, &AssistResolveStrategy::All, file_id.into())
.into_iter()
.map(|d| {
let mut annotation = String::new();
Expand Down Expand Up @@ -277,10 +285,10 @@ fn test_disabled_diagnostics() {
let (db, file_id) = RootDatabase::with_single_file(r#"mod foo;"#);
let file_id = file_id.into();

let diagnostics = super::diagnostics(&db, &config, &AssistResolveStrategy::All, file_id);
let diagnostics = super::full_diagnostics(&db, &config, &AssistResolveStrategy::All, file_id);
assert!(diagnostics.is_empty());

let diagnostics = super::diagnostics(
let diagnostics = super::full_diagnostics(
&db,
&DiagnosticsConfig::test_sample(),
&AssistResolveStrategy::All,
Expand Down
27 changes: 23 additions & 4 deletions crates/ide/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -672,14 +672,33 @@ impl Analysis {
.unwrap_or_default())
}

/// Computes the set of diagnostics for the given file.
pub fn diagnostics(
/// Computes the set of parser level diagnostics for the given file.
pub fn syntax_diagnostics(
&self,
config: &DiagnosticsConfig,
file_id: FileId,
) -> Cancellable<Vec<Diagnostic>> {
self.with_db(|db| ide_diagnostics::syntax_diagnostics(db, config, file_id))
}

/// Computes the set of semantic diagnostics for the given file.
pub fn semantic_diagnostics(
&self,
config: &DiagnosticsConfig,
resolve: AssistResolveStrategy,
file_id: FileId,
) -> Cancellable<Vec<Diagnostic>> {
self.with_db(|db| ide_diagnostics::semantic_diagnostics(db, config, &resolve, file_id))
}

/// Computes the set of both syntax and semantic diagnostics for the given file.
pub fn full_diagnostics(
&self,
config: &DiagnosticsConfig,
resolve: AssistResolveStrategy,
file_id: FileId,
) -> Cancellable<Vec<Diagnostic>> {
self.with_db(|db| ide_diagnostics::diagnostics(db, config, &resolve, file_id))
self.with_db(|db| ide_diagnostics::full_diagnostics(db, config, &resolve, file_id))
}

/// Convenience function to return assists + quick fixes for diagnostics
Expand All @@ -697,7 +716,7 @@ impl Analysis {

self.with_db(|db| {
let diagnostic_assists = if diagnostics_config.enabled && include_fixes {
ide_diagnostics::diagnostics(db, diagnostics_config, &resolve, frange.file_id)
ide_diagnostics::full_diagnostics(db, diagnostics_config, &resolve, frange.file_id)
.into_iter()
.flat_map(|it| it.fixes.unwrap_or_default())
.filter(|it| it.target.intersect(frange.range).is_some())
Expand Down
2 changes: 1 addition & 1 deletion crates/rust-analyzer/src/cli/analysis_stats.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,7 @@ impl flags::AnalysisStats {
let mut sw = self.stop_watch();

for &file_id in &file_ids {
_ = analysis.diagnostics(
_ = analysis.full_diagnostics(
&DiagnosticsConfig {
enabled: true,
proc_macros_enabled: true,
Expand Down
Loading

0 comments on commit eec6375

Please sign in to comment.