From e3fe1b6d115054fb46fc8bf5390bbf51db073e08 Mon Sep 17 00:00:00 2001 From: SpontanCombust <61706594+SpontanCombust@users.noreply.github.com> Date: Tue, 14 May 2024 07:14:59 +0200 Subject: [PATCH] Add more context to `MultipleMatchingProjectDependencies` diagnostic --- crates/diagnostics/src/lib.rs | 11 +++++--- .../lsp/src/tasks/content_indexing_tasks.rs | 4 +-- crates/project/src/content_graph.rs | 27 +++++++++++-------- 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/crates/diagnostics/src/lib.rs b/crates/diagnostics/src/lib.rs index 8a21999e..1fdf4180 100644 --- a/crates/diagnostics/src/lib.rs +++ b/crates/diagnostics/src/lib.rs @@ -58,7 +58,10 @@ pub enum DiagnosticKind { ProjectDependencyPathNotFound(PathBuf), ProjectDependencyNameNotFound(String), ProjectDependencyNameNotFoundAtPath(String), - MultipleMatchingProjectDependencies(String), + MultipleMatchingProjectDependencies { + content_name: String, + matching_paths: Vec + }, // syntax analysis MissingSyntax(String), @@ -87,7 +90,7 @@ impl DiagnosticKind { | ProjectDependencyPathNotFound(_) | ProjectDependencyNameNotFound(_) | ProjectDependencyNameNotFoundAtPath(_) - | MultipleMatchingProjectDependencies(_) => DiagnosticDomain::ProjectSystem, + | MultipleMatchingProjectDependencies { .. } => DiagnosticDomain::ProjectSystem, MissingSyntax(_) | InvalidSyntax => DiagnosticDomain::SyntaxAnalysis, SymbolNameTaken { .. } @@ -109,7 +112,7 @@ impl DiagnosticKind { ProjectDependencyPathNotFound(_) => lsp::DiagnosticSeverity::ERROR, ProjectDependencyNameNotFound(_) => lsp::DiagnosticSeverity::ERROR, ProjectDependencyNameNotFoundAtPath(_) => lsp::DiagnosticSeverity::ERROR, - MultipleMatchingProjectDependencies(_) => lsp::DiagnosticSeverity::ERROR, + MultipleMatchingProjectDependencies { .. } => lsp::DiagnosticSeverity::ERROR, MissingSyntax(_) => lsp::DiagnosticSeverity::ERROR, InvalidSyntax => lsp::DiagnosticSeverity::ERROR, @@ -132,7 +135,7 @@ impl DiagnosticKind { ProjectDependencyPathNotFound(p) => format!("Dependency could not be found at path \"{}\"", p.display()), ProjectDependencyNameNotFound(n) => format!("Dependency could not be found with name \"{n}\""), ProjectDependencyNameNotFoundAtPath(n) => format!("Dependency with name \"{n}\" could not be found at specified path"), - MultipleMatchingProjectDependencies(n) => format!("Multiple matching contents for dependency with name \"{n}\""), + MultipleMatchingProjectDependencies { content_name: project_name, matching_paths } => format!("Multiple matching contents for dependency with name \"{project_name}\": {:?}", matching_paths.into_iter().map(|p| p.to_string()).collect::>()), MissingSyntax(s) => format!("Syntax error: expected {}", s), InvalidSyntax => "Syntax error: unexpected syntax".into(), diff --git a/crates/lsp/src/tasks/content_indexing_tasks.rs b/crates/lsp/src/tasks/content_indexing_tasks.rs index b890fcc7..adfc77e8 100644 --- a/crates/lsp/src/tasks/content_indexing_tasks.rs +++ b/crates/lsp/src/tasks/content_indexing_tasks.rs @@ -256,10 +256,10 @@ impl Backend { kind: DiagnosticKind::ProjectDependencyNameNotFoundAtPath(content_name) }).await; }, - ContentGraphError::MultipleMatchingDependencies { content_name, manifest_path, manifest_range } => { + ContentGraphError::MultipleMatchingDependencies { content_name, manifest_path, manifest_range, matching_paths } => { self.reporter.push_diagnostic(&manifest_path, Diagnostic { range: manifest_range, - kind: DiagnosticKind::MultipleMatchingProjectDependencies(content_name) + kind: DiagnosticKind::MultipleMatchingProjectDependencies { content_name, matching_paths } }).await; } } diff --git a/crates/project/src/content_graph.rs b/crates/project/src/content_graph.rs index 376b8ae9..04c5600a 100644 --- a/crates/project/src/content_graph.rs +++ b/crates/project/src/content_graph.rs @@ -46,8 +46,9 @@ pub enum ContentGraphError { /// Manifest from which this error originated manifest_path: AbsPath, // Location in the manifest where the name is present - manifest_range: lsp::Range - //TODO list those matching + manifest_range: lsp::Range, + + matching_paths: Vec } } @@ -325,8 +326,8 @@ impl ContentGraph { Ok(dep_idx) => { self.insert_edge(GraphEdge { dependant_idx: node_idx, dependency_idx: dep_idx }); }, - Err(dep_count) => { - if dep_count == 0 { + Err(matching_paths) => { + if matching_paths.is_empty() { self.errors.push(ContentGraphError::DependencyNameNotFound { content_name: dependency_name.to_owned(), manifest_path: manifest_path.to_owned(), @@ -336,7 +337,8 @@ impl ContentGraph { self.errors.push(ContentGraphError::MultipleMatchingDependencies { content_name: dependency_name.to_owned(), manifest_path: manifest_path.to_owned(), - manifest_range: dependency_name_range.to_owned() + manifest_range: dependency_name_range.to_owned(), + matching_paths }); } } @@ -344,9 +346,8 @@ impl ContentGraph { } /// If there is just one repository content with the name returns Ok with the index. - /// Otherwise returns Err with the number of contents encountered with that name. - /// So if it wasn't found returns Err(0) or if more than one with than name was found returns Err(2) for example. - fn get_dependency_node_index_by_name(&mut self, name: &str, repo_nodes: &mut Vec) -> Result { + /// Otherwise returns Err with the Vec of matching content paths. + fn get_dependency_node_index_by_name(&mut self, name: &str, repo_nodes: &mut Vec) -> Result> { let mut candidates = Vec::new(); for (i, n) in self.nodes.iter().enumerate() { if n.in_repository && n.content.content_name() == name { @@ -358,7 +359,9 @@ impl ContentGraph { if candidates_len == 1 { Ok(candidates[0]) } else if candidates_len > 1 { - Err(candidates_len) + Err(candidates.into_iter() + .map(|i| self.nodes[i].content.path().to_owned()) + .collect()) } else { for (i, n) in repo_nodes.iter().enumerate() { if n.content.content_name() == name { @@ -368,13 +371,15 @@ impl ContentGraph { let candidates_len = candidates.len(); if candidates_len == 0 { - Err(0) + Err(Vec::new()) } else if candidates_len == 1 { let target_node = repo_nodes.remove(candidates[0]); let target_node_idx = self.insert_node(target_node); Ok(target_node_idx) } else { - Err(candidates_len) + Err(candidates.into_iter() + .map(|i| repo_nodes[i].content.path().to_owned()) + .collect()) } } }