Skip to content

Commit

Permalink
Add more context to MultipleMatchingProjectDependencies diagnostic
Browse files Browse the repository at this point in the history
  • Loading branch information
SpontanCombust committed May 14, 2024
1 parent b31e15c commit e3fe1b6
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 17 deletions.
11 changes: 7 additions & 4 deletions crates/diagnostics/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ pub enum DiagnosticKind {
ProjectDependencyPathNotFound(PathBuf),
ProjectDependencyNameNotFound(String),
ProjectDependencyNameNotFoundAtPath(String),
MultipleMatchingProjectDependencies(String),
MultipleMatchingProjectDependencies {
content_name: String,
matching_paths: Vec<AbsPath>
},

// syntax analysis
MissingSyntax(String),
Expand Down Expand Up @@ -87,7 +90,7 @@ impl DiagnosticKind {
| ProjectDependencyPathNotFound(_)
| ProjectDependencyNameNotFound(_)
| ProjectDependencyNameNotFoundAtPath(_)
| MultipleMatchingProjectDependencies(_) => DiagnosticDomain::ProjectSystem,
| MultipleMatchingProjectDependencies { .. } => DiagnosticDomain::ProjectSystem,
MissingSyntax(_)
| InvalidSyntax => DiagnosticDomain::SyntaxAnalysis,
SymbolNameTaken { .. }
Expand All @@ -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,
Expand All @@ -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::<Vec<_>>()),

MissingSyntax(s) => format!("Syntax error: expected {}", s),
InvalidSyntax => "Syntax error: unexpected syntax".into(),
Expand Down
4 changes: 2 additions & 2 deletions crates/lsp/src/tasks/content_indexing_tasks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
27 changes: 16 additions & 11 deletions crates/project/src/content_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<AbsPath>
}
}

Expand Down Expand Up @@ -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(),
Expand All @@ -336,17 +337,17 @@ 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
});
}
}
}
}

/// 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<GraphNode>) -> Result<usize, usize> {
/// 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<GraphNode>) -> Result<usize, Vec<AbsPath>> {
let mut candidates = Vec::new();
for (i, n) in self.nodes.iter().enumerate() {
if n.in_repository && n.content.content_name() == name {
Expand All @@ -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 {
Expand All @@ -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())
}
}
}
Expand Down

0 comments on commit e3fe1b6

Please sign in to comment.