From f96778cc47dc5f80d6599956f936318b7234d28c Mon Sep 17 00:00:00 2001 From: Alan Zimmerman Date: Wed, 27 Mar 2024 07:28:36 -0700 Subject: [PATCH] rework combining diagnostics: make combination symmetric Summary: Also process syntax errors per function from the Erlang Service when combining them as related info for undefined function diagnostics Reviewed By: michalmuskala Differential Revision: D54493062 fbshipit-source-id: 06ad0f835c4187363cba797f8390a4a3af47e775 --- Cargo.lock | 1 - crates/ide/Cargo.toml | 1 - crates/ide/src/diagnostics.rs | 273 +++++++++++++---------- crates/ide/src/diagnostics_collection.rs | 26 +-- crates/ide/src/tests.rs | 2 +- crates/ide_db/src/diagnostic_code.rs | 1 + 6 files changed, 173 insertions(+), 131 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fb9deb8f25..372261f5f9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -510,7 +510,6 @@ dependencies = [ "log", "parking_lot 0.12.1", "profile", - "range-set", "regex", "serde", "smallvec", diff --git a/crates/ide/Cargo.toml b/crates/ide/Cargo.toml index 2de40d3c42..262eb0c41e 100644 --- a/crates/ide/Cargo.toml +++ b/crates/ide/Cargo.toml @@ -22,7 +22,6 @@ lazy_static.workspace = true log.workspace = true parking_lot.workspace = true profile.workspace = true -range-set.workspace = true regex.workspace = true serde.workspace = true smallvec.workspace = true diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 58c8c45914..262e239c76 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -9,7 +9,6 @@ use std::collections::BTreeSet; use std::fmt; -use std::ops::RangeInclusive; use std::sync::Arc; use elp_eqwalizer::EqwalizerDiagnostic; @@ -59,7 +58,6 @@ use hir::InFile; use hir::Semantic; use itertools::Itertools; use lazy_static::lazy_static; -use range_set::RangeSet; use regex::Regex; use text_edit::TextEdit; @@ -112,10 +110,6 @@ pub struct Diagnostic { pub related_info: Option>, pub code: DiagnosticCode, pub code_doc_uri: Option, - // Used to combine syntax errors with erlang_service ones. If we - // have a syntax error in the range, we filter out erlang_service - // ones in the same range. We set it to the range of the enclosing form. - pub form_range: Option, } pub fn group_label_ignore() -> GroupLabel { @@ -134,7 +128,6 @@ impl Diagnostic { fixes: None, related_info: None, code_doc_uri: code.as_uri(), - form_range: None, } } @@ -184,11 +177,6 @@ impl Diagnostic { } } - pub(crate) fn with_form_range(mut self, form_range: Option) -> Diagnostic { - self.form_range = form_range; - self - } - pub(crate) fn experimental(self) -> Diagnostic { self.add_categories([Category::Experimental]) } @@ -360,14 +348,20 @@ impl<'a> DiagnosticsConfig<'a> { } } -pub type Labeled = FxHashMap, Vec>; +pub type Labeled = FxHashMap, Vec>; -pub type TextRangeSet = RangeSet<[RangeInclusive; 1]>; +/// Label for a given diagnostic, giving an MFA as a string if the +/// diagnostic is in a FunDecl, or the range of the enclosing form +/// otherwise. +#[derive(Debug, Clone, Hash, PartialEq, Eq)] +pub enum DiagnosticLabel { + MFA(Label), + Range(TextRange), +} #[derive(Debug, Clone)] pub struct LabeledDiagnostics { pub normal: Vec, - pub syntax_error_form_ranges: TextRangeSet, /// Syntax error diagnostics labeled by the name/arity of the function enclosing them pub labeled_syntax_errors: Labeled, /// "Undefined XXX" diagnostics labeled by the name/arity of XXX. @@ -377,7 +371,6 @@ pub struct LabeledDiagnostics { impl Default for LabeledDiagnostics { fn default() -> Self { Self { - syntax_error_form_ranges: RangeSet::from_elements(vec![]), normal: Default::default(), labeled_syntax_errors: Default::default(), labeled_undefined_errors: Default::default(), @@ -388,7 +381,6 @@ impl Default for LabeledDiagnostics { impl LabeledDiagnostics { pub fn new(diagnostics: Vec) -> LabeledDiagnostics { LabeledDiagnostics { - syntax_error_form_ranges: RangeSet::from_elements(vec![]), normal: diagnostics, labeled_syntax_errors: FxHashMap::default(), labeled_undefined_errors: FxHashMap::default(), @@ -417,11 +409,6 @@ impl LabeledDiagnostics { } } -/// Convert a `TextRange` into a form suitable for a `TextRangeSet` -fn to_range(range: &TextRange) -> RangeInclusive { - RangeInclusive::new(range.start().into(), range.end().into()) -} - pub fn eqwalizer_to_diagnostic( sema: &Semantic, file_id: FileId, @@ -454,7 +441,6 @@ pub fn eqwalizer_to_diagnostic( fixes: None, related_info: None, code_doc_uri: Some(d.uri.clone()), - form_range: None, }; add_eqwalizer_assists(sema, file_id, d, &mut diagnostic); diagnostic @@ -477,7 +463,7 @@ pub fn native_diagnostics( let mut res = Vec::new(); - let (labeled_syntax_errors, syntax_error_form_ranges) = if report_diagnostics { + let labeled_syntax_errors = if report_diagnostics { let sema = Semantic::new(db); if file_kind.is_module() { @@ -511,12 +497,11 @@ pub fn native_diagnostics( ), }; Diagnostic::error(code, widen_range(err.range()), message) - .with_form_range(get_form_range(&parse.syntax_node(), err.range())) }); let source_file = db.parse(file_id).tree(); - group_syntax_errors(&source_file, parse_diagnostics) + label_syntax_errors(&source_file, parse_diagnostics) } else { - (FxHashMap::default(), RangeSet::from_elements(vec![])) + FxHashMap::default() }; let metadata = db.elp_metadata(file_id); // TODO: can we ever disable DiagnosticCode::SyntaxError? @@ -529,49 +514,44 @@ pub fn native_diagnostics( }); LabeledDiagnostics { - syntax_error_form_ranges, normal: res, labeled_syntax_errors, labeled_undefined_errors: FxHashMap::default(), } } -/// Get the range of the form enclosing the passed-in range, if there -/// is one. -fn get_form_range(syntax: &SyntaxNode, range: TextRange) -> Option { - algo::find_node_at_offset::(syntax, range.start()).map(|n| n.syntax().text_range()) -} - -fn group_syntax_errors( +fn label_syntax_errors( source_file: &SourceFile, diagnostics: impl Iterator, -) -> (Labeled, TextRangeSet) { +) -> Labeled { let mut map: Labeled = FxHashMap::default(); - let mut ranges = RangeSet::from_elements(vec![]); diagnostics.for_each(|d| { - if let Some(range) = d.form_range { - ranges.insert_range(to_range(&range)); - } - map.entry(function_label(&d, source_file)) + map.entry(diagnostic_label(&d, source_file)) .or_default() .push(d); }); - (map, ranges) + map } -fn function_label(d: &Diagnostic, source_file: &SourceFile) -> Option