From e633471545eaba4259b274c6130267fa55586e4f Mon Sep 17 00:00:00 2001 From: withered-magic Date: Mon, 15 Apr 2024 15:36:16 -0700 Subject: [PATCH] support label goto definition --- crates/starpls/src/document.rs | 142 +++++++++++++--------- crates/starpls/src/handlers/requests.rs | 34 +++--- crates/starpls_ide/src/goto_definition.rs | 19 +-- crates/starpls_ide/src/lib.rs | 24 +++- 4 files changed, 135 insertions(+), 84 deletions(-) diff --git a/crates/starpls/src/document.rs b/crates/starpls/src/document.rs index 65f54ae..05c9a27 100644 --- a/crates/starpls/src/document.rs +++ b/crates/starpls/src/document.rs @@ -218,9 +218,90 @@ impl DefaultFileLoader { assert!(from_path.pop()); from_path } + + fn resolve_bazel_label(&self, label: &Label, from: FileId) -> anyhow::Result> { + let repo_kind = label.kind(); + let (root, package) = match &repo_kind { + RepoKind::Apparent if self.bzlmod_enabled => { + let from_path = self.interner.lookup_by_file_id(from); + let from_repo = if from_path.starts_with(&self.workspace) { + "" + } else { + let from_path = from_path.strip_prefix(&self.external_output_base)?; + match from_path + .components() + .next() + .as_ref() + .and_then(|component| component.as_os_str().to_str()) + { + Some(from_repo) => from_repo, + None => return Ok(None), + } + }; + + let canonical_repo = self + .bazel_client + .resolve_repo_from_mapping(label.repo(), from_repo)?; + + match canonical_repo { + Some(canonical_repo) => ( + if canonical_repo.is_empty() { + self.workspace.clone() + } else { + self.external_output_base.join(canonical_repo) + }, + PathBuf::new(), + ), + None => return Ok(None), + } + } + RepoKind::Canonical | RepoKind::Apparent => { + (self.external_output_base.join(label.repo()), PathBuf::new()) + } + RepoKind::Current => { + // Find the Bazel workspace root. + let from_path = self.interner.lookup_by_file_id(from); + match starpls_bazel::resolve_workspace(&from_path)? { + Some(root) => root, + None => { + bail!("not in a Bazel workspace") + } + } + } + }; + + // Loading targets using a relative label causes them to be resolved from the closest package to the importing file. + let resolved_path = if label.is_relative() { + package + } else { + root.join(label.package()) + } + .join(label.target()); + + Ok(Some(resolved_path)) + } } impl FileLoader for DefaultFileLoader { + fn resolve_path( + &self, + path: &str, + dialect: Dialect, + from: FileId, + ) -> anyhow::Result> { + if dialect != Dialect::Bazel { + return Ok(None); + } + + // Parse the load path as a Bazel label. + let label = match Label::parse(path) { + Result::Ok(label) => label, + Err(err) => return Err(anyhow!("error parsing label: {}", err.err)), + }; + + self.resolve_bazel_label(&label, from) + } + fn load_file( &self, path: &str, @@ -252,63 +333,10 @@ impl FileLoader for DefaultFileLoader { let resolved_path = match self.read_cache_result(&repo_kind, path, from) { Some(path) => path, None => { - let (root, package) = - match &repo_kind { - RepoKind::Apparent if self.bzlmod_enabled => { - let from_path = self.interner.lookup_by_file_id(from); - let from_repo = - if from_path.starts_with(&self.workspace) { - "" - } else { - let from_path = from_path - .strip_prefix(&self.external_output_base)?; - match from_path.components().next().as_ref().and_then( - |component| component.as_os_str().to_str(), - ) { - Some(from_repo) => from_repo, - None => return Ok(None), - } - }; - - let canonical_repo = self - .bazel_client - .resolve_repo_from_mapping(label.repo(), from_repo)?; - - match canonical_repo { - Some(canonical_repo) => ( - if canonical_repo.is_empty() { - self.workspace.clone() - } else { - self.external_output_base.join(canonical_repo) - }, - PathBuf::new(), - ), - None => return Ok(None), - } - } - RepoKind::Canonical | RepoKind::Apparent => { - (self.external_output_base.join(label.repo()), PathBuf::new()) - } - RepoKind::Current => { - // Find the Bazel workspace root. - let from_path = self.interner.lookup_by_file_id(from); - match starpls_bazel::resolve_workspace(&from_path)? { - Some(root) => root, - None => { - bail!("not in a Bazel workspace") - } - } - } - }; - - // Loading targets using a relative label causes them to be resolved from the closest package to the importing file. - let resolved_path = if label.is_relative() { - package - } else { - root.join(label.package()) - } - .join(label.target()); - + let resolved_path = match self.resolve_bazel_label(&label, from)? { + Some(resolved_path) => resolved_path, + None => return Ok(None), + }; self.record_cache_result(&repo_kind, path, from, resolved_path.clone()); resolved_path } diff --git a/crates/starpls/src/handlers/requests.rs b/crates/starpls/src/handlers/requests.rs index d1920de..0583368 100644 --- a/crates/starpls/src/handlers/requests.rs +++ b/crates/starpls/src/handlers/requests.rs @@ -56,21 +56,25 @@ pub(crate) fn goto_definition( }; let to_lsp_location = |location: Location| -> Option { - let line_index = snapshot - .analysis_snapshot - .line_index(location.file_id) - .ok()??; - let range = convert::lsp_range_from_text_range(location.range, line_index); - Some(lsp_types::Location { - uri: lsp_types::Url::from_file_path( - snapshot - .document_manager - .read() - .lookup_by_file_id(location.file_id), - ) - .ok()?, - range: range?, - }) + let location = match location { + Location::Local { file_id, range } => { + let line_index = snapshot.analysis_snapshot.line_index(file_id).ok()??; + let range = convert::lsp_range_from_text_range(range, line_index); + lsp_types::Location { + uri: lsp_types::Url::from_file_path( + snapshot.document_manager.read().lookup_by_file_id(file_id), + ) + .ok()?, + range: range?, + } + } + Location::External { path } => lsp_types::Location { + uri: lsp_types::Url::from_file_path(path).ok()?, + range: Default::default(), + }, + }; + + Some(location) }; Ok(Some( diff --git a/crates/starpls_ide/src/goto_definition.rs b/crates/starpls_ide/src/goto_definition.rs index 7890fe3..6ae7bf7 100644 --- a/crates/starpls_ide/src/goto_definition.rs +++ b/crates/starpls_ide/src/goto_definition.rs @@ -32,12 +32,12 @@ pub(crate) fn goto_definition( .flat_map(|def| match def { ScopeDef::LoadItem(load_item) => { let def = scope_def_for_load_item(db, &sema, file, &load_item)?; - Some(Location { + Some(Location::Local { file_id: def.file.id(db), range: def.value.syntax_node_ptr(db, def.file)?.text_range(), }) } - _ => def.syntax_node_ptr(db, file).map(|ptr| Location { + _ => def.syntax_node_ptr(db, file).map(|ptr| Location::Local { file_id, range: ptr.text_range(), }), @@ -62,7 +62,7 @@ pub(crate) fn goto_definition( ast::Argument::Keyword(kwarg) => { let name = kwarg.name()?; (name.name()?.text() == token.text()).then(|| { - vec![Location { + vec![Location::Local { file_id: struct_call_expr.file.id(db), range: name.syntax().text_range(), }] @@ -87,7 +87,7 @@ pub(crate) fn goto_definition( ast::LiteralKind::String(s) if s.value().as_deref() == Some(token.text()) => { - Some(vec![Location { + Some(vec![Location::Local { file_id: provider_fields.file.id(db), range: syntax.text_range(), }]) @@ -101,7 +101,7 @@ pub(crate) fn goto_definition( if let Some(load_module) = ast::LoadModule::cast(parent.clone()) { let load_stmt = ast::LoadStmt::cast(load_module.syntax().parent()?)?; let file = sema.resolve_load_stmt(file, &load_stmt)?; - return Some(vec![Location { + return Some(vec![Location::Local { file_id: file.id(db), range: TextRange::new(TextSize::new(0), TextSize::new(1)), }]); @@ -110,7 +110,7 @@ pub(crate) fn goto_definition( if let Some(load_item) = ast::LoadItem::cast(parent) { let load_item = sema.resolve_load_item(file, &load_item)?; let def = scope_def_for_load_item(db, &sema, file, &load_item)?; - return Some(vec![Location { + return Some(vec![Location::Local { file_id: def.file.id(db), range: def.value.syntax_node_ptr(db, def.file)?.text_range(), }]); @@ -139,7 +139,7 @@ fn scope_def_for_load_item( #[cfg(test)] mod tests { - use crate::{AnalysisSnapshot, FilePosition}; + use crate::{AnalysisSnapshot, FilePosition, Location}; use starpls_test_util::parse_fixture; fn check_goto_definition(fixture: &str) { @@ -150,7 +150,10 @@ mod tests { .unwrap() .unwrap() .into_iter() - .map(|loc| loc.range) + .map(|loc| match loc { + Location::Local { range, .. } => range, + _ => panic!("expected local location"), + }) .collect::>(); assert_eq!(expected, actual); } diff --git a/crates/starpls_ide/src/lib.rs b/crates/starpls_ide/src/lib.rs index 5f1b5fb..85327a1 100644 --- a/crates/starpls_ide/src/lib.rs +++ b/crates/starpls_ide/src/lib.rs @@ -6,7 +6,7 @@ use starpls_common::{Db, Diagnostic, Dialect, File, FileId, LoadItemCandidate}; use starpls_hir::{BuiltinDefs, Db as _, ExprId, GlobalCtxt, LoadItemId, LoadStmt, ParamId, Ty}; use starpls_syntax::{LineIndex, TextRange, TextSize}; use starpls_test_util::builtins_with_catch_all_functions; -use std::{fmt::Debug, panic, sync::Arc}; +use std::{fmt::Debug, panic, path::PathBuf, sync::Arc}; pub use crate::{ completions::{CompletionItem, CompletionItemKind, CompletionMode}, @@ -336,9 +336,9 @@ impl AnalysisSnapshot { impl panic::RefUnwindSafe for AnalysisSnapshot {} #[derive(Clone, Debug, PartialEq, Eq)] -pub struct Location { - pub file_id: FileId, - pub range: TextRange, +pub enum Location { + Local { file_id: FileId, range: TextRange }, + External { path: PathBuf }, } #[derive(Clone, Debug, PartialEq, Eq)] @@ -349,6 +349,13 @@ pub struct FilePosition { /// A trait for loading a path and listing its exported symbols. pub trait FileLoader: Send + Sync + 'static { + fn resolve_path( + &self, + path: &str, + dialect: Dialect, + from: FileId, + ) -> anyhow::Result>; + /// Open the Starlark file corresponding to the given `path` and of the given `Dialect`. fn load_file( &self, @@ -399,4 +406,13 @@ impl FileLoader for SimpleFileLoader { ) -> anyhow::Result>> { Ok(None) } + + fn resolve_path( + &self, + _path: &str, + _dialect: Dialect, + _from: FileId, + ) -> anyhow::Result> { + Ok(None) + } }