Resolve non-source file labels (#205)

README.md

@@ -90,7 +90,7 @@ then you'll get autocomplete suggestions for the attributes on `ctx`, like `ctx.
     - [x] Function definitions
     - [x] Struct fields
     - [x] Provider fields
-    - [x] Labels in `load` statements
+    - [x] Labels and targets
     - [ ] Rule attributes
 - Type inference
     - [x] Basic type inference

crates/starpls/src/document.rs

@@ -11,7 +11,7 @@ use starpls_bazel::{
     label::{PartialParse, RepoKind},
     APIContext, Label, ParseError,
 };
-use starpls_common::{Dialect, FileId, LoadItemCandidate, LoadItemCandidateKind};
+use starpls_common::{Dialect, FileId, LoadItemCandidate, LoadItemCandidateKind, ResolvedPath};
 use starpls_ide::FileLoader;
 use std::{
     collections::HashMap,
@@ -253,7 +253,6 @@ impl DefaultFileLoader {
                 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() {
@@ -264,7 +263,16 @@ impl DefaultFileLoader {
                         },
                         PathBuf::new(),
                     ),
-                    None => return Ok(None),
+                    None => {
+                        bail!(
+                            "Could not resolve repository \"{}{}\" from current repository mapping",
+                            match label.kind() {
+                                RepoKind::Canonical => "@@",
+                                _ => "@",
+                            },
+                            label.repo()
+                        )
+                    }
                 }
             }
             RepoKind::Canonical | RepoKind::Apparent => {
@@ -298,6 +306,46 @@ impl DefaultFileLoader {
             canonical_repo: canonical_repo_res,
         }))
     }
+
+    fn maybe_intern_file(
+        &self,
+        path: PathBuf,
+        from: FileId,
+        fetch_repo_on_err: Option<String>,
+    ) -> anyhow::Result<(FileId, Option<String>)> {
+        // If we've already interned this file, then simply return the file id.
+        let (file_id, contents) = match self.interner.lookup_by_path_buf(&path) {
+            Some(file_id) => (file_id, None),
+            None => {
+                let contents = match fs::read_to_string(&path) {
+                    Result::Ok(contents) => contents,
+                    Err(err) => {
+                        if let Some(canonical_repo) = fetch_repo_on_err {
+                            if !self
+                                .external_output_base
+                                .join(&canonical_repo)
+                                .try_exists()
+                                .ok()
+                                .unwrap_or_default()
+                            {
+                                let _ = self.fetch_repo_sender.send(
+                                    Task::FetchExternalRepoRequest(FetchExternalRepoRequest {
+                                        file_id: from,
+                                        repo: canonical_repo,
+                                    }),
+                                );
+                            }
+                        }
+                        return Err(err.into());
+                    }
+                };
+
+                (self.interner.intern_path(path), Some(contents))
+            }
+        };
+
+        Ok((file_id, contents))
+    }
 }
 
 impl FileLoader for DefaultFileLoader {
@@ -306,7 +354,7 @@ impl FileLoader for DefaultFileLoader {
         path: &str,
         dialect: Dialect,
         from: FileId,
-    ) -> anyhow::Result<Option<PathBuf>> {
+    ) -> anyhow::Result<Option<ResolvedPath>> {
         if dialect != Dialect::Bazel {
             return Ok(None);
         }
@@ -317,8 +365,52 @@ impl FileLoader for DefaultFileLoader {
             Err(err) => return Err(anyhow!("error parsing label: {}", err.err)),
         };
 
-        self.resolve_label(&label, from)
-            .map(|res| res.map(|res| res.resolved_path))
+        let resolved_label = match self.resolve_label(&label, from)? {
+            Some(resolved_label) => resolved_label,
+            None => return Ok(None),
+        };
+
+        let res = if fs::metadata(&resolved_label.resolved_path)
+            .ok()
+            .map(|metadata| metadata.is_file())
+            .unwrap_or_default()
+        {
+            ResolvedPath::Source {
+                path: resolved_label.resolved_path,
+            }
+        } else {
+            if label.target().is_empty() {
+                return Ok(None);
+            }
+
+            let parent = match resolved_label.resolved_path.parent() {
+                Some(parent) => parent,
+                None => return Ok(None),
+            };
+            let build_file = match fs::read_dir(parent)
+                .into_iter()
+                .flat_map(|entries| entries.into_iter())
+                .find_map(|entry| match entry.ok()?.file_name().to_str()? {
+                    file_name @ ("BUILD" | "BUILD.bazel") => Some(file_name.to_string()),
+                    _ => None,
+                }) {
+                Some(build_file) => build_file,
+                None => return Ok(None),
+            };
+            let path = parent.join(build_file);
+
+            // If we've already interned this file, then simply return the file id.
+            let (build_file, contents) =
+                self.maybe_intern_file(path, from, resolved_label.canonical_repo)?;
+
+            ResolvedPath::BuildTarget {
+                build_file,
+                target: label.target().to_string(),
+                contents,
+            }
+        };
+
+        Ok(Some(res))
     }
 
     fn load_file(
@@ -367,38 +459,8 @@ impl FileLoader for DefaultFileLoader {
             }
         };
 
-        Ok(Some({
-            // If we've already interned this file, then simply return the file id.
-            if let Some(file_id) = self.interner.lookup_by_path_buf(&path) {
-                (file_id, dialect, api_context, None)
-            } else {
-                let contents = match fs::read_to_string(&path) {
-                    Result::Ok(contents) => contents,
-                    Err(err) => {
-                        if let Some(canonical_repo) = canonical_repo {
-                            if !self
-                                .external_output_base
-                                .join(&canonical_repo)
-                                .try_exists()
-                                .ok()
-                                .unwrap_or_default()
-                            {
-                                let _ = self.fetch_repo_sender.send(
-                                    Task::FetchExternalRepoRequest(FetchExternalRepoRequest {
-                                        file_id: from,
-                                        repo: canonical_repo,
-                                    }),
-                                );
-                            }
-                        }
-
-                        return Err(err.into());
-                    }
-                };
-                let file_id = self.interner.intern_path(path);
-                (file_id, dialect, api_context, Some(contents))
-            }
-        }))
+        let (file_id, contents) = self.maybe_intern_file(path, from, canonical_repo)?;
+        Ok(Some((file_id, dialect, api_context, contents)))
     }
 
     fn list_load_candidates(
@@ -412,7 +474,6 @@ impl FileLoader for DefaultFileLoader {
                 let from_dir = self.dirname(from);
                 let has_trailing_slash = path.ends_with(MAIN_SEPARATOR);
                 let mut path = from_dir.join(path);
-
                 if !has_trailing_slash {
                     if !path.pop() {
                         return Ok(None);

crates/starpls/src/server.rs

@@ -60,6 +60,8 @@ impl Server {
             }
         };
 
+        eprintln!("server: fetching Bazel configuration");
+
         let bazel_client = Arc::new(BazelCLI::default());
         let info = bazel_client.info().unwrap_or_default();
 

crates/starpls_bazel/src/label.rs

@@ -167,11 +167,13 @@ impl<'a, 'b> Parser<'a, 'b> {
             assert_eq!(self.bump(), Some(':'));
         } else if self.pos == 0 {
         } else {
-            self.parse_package()?;
+            let last_slash = self.parse_package()?;
             return if self.label.package_start == self.label.package_end {
                 Err(ParseError::EmptyPackage)
             } else {
-                self.label.target_start = self.label.package_start;
+                self.label.target_start = last_slash
+                    .map(|pos| pos + 1)
+                    .unwrap_or(self.label.package_start);
                 self.label.target_end = self.label.package_end;
                 Ok(())
             };
@@ -219,21 +221,27 @@ impl<'a, 'b> Parser<'a, 'b> {
         Ok(())
     }
 
-    fn parse_package(&mut self) -> Result<(), ParseError> {
-        let (start, end, has_target_only_chars) = match self.parse_package_or_target(true) {
-            Ok(res) => res,
-            Err(_) => return Err(ParseError::InvalidPackage),
-        };
+    fn parse_package(&mut self) -> Result<Option<u32>, ParseError> {
+        let (start, end, last_slash, has_target_only_chars) =
+            match self.parse_package_or_target(true) {
+                Ok(res) => res,
+                Err(_) => return Err(ParseError::InvalidPackage),
+            };
         if has_target_only_chars {
             return Err(ParseError::InvalidPackage);
         }
+        if let Some(last_slash) = last_slash {
+            if last_slash + 1 == end {
+                return Err(ParseError::InvalidPackage);
+            }
+        }
         self.label.package_start = start;
         self.label.package_end = end;
-        Ok(())
+        Ok(last_slash)
     }
 
     fn parse_target(&mut self) -> Result<(), ParseError> {
-        let (start, end, _) = match self.parse_package_or_target(false) {
+        let (start, end, _, _) = match self.parse_package_or_target(false) {
             Ok(res) => res,
             Err(_) => return Err(ParseError::InvalidTarget),
         };
@@ -245,12 +253,20 @@ impl<'a, 'b> Parser<'a, 'b> {
         Ok(())
     }
 
-    fn parse_package_or_target(&mut self, allow_colon: bool) -> Result<(u32, u32, bool), ()> {
+    fn parse_package_or_target(
+        &mut self,
+        allow_colon: bool,
+    ) -> Result<(u32, u32, Option<u32>, bool), ()> {
         let start = self.pos;
         let mut has_target_only_chars = false;
+        let mut last_slash = None;
         while let Some(c) = self.first() {
             match c {
-                'A'..='Z' | 'a'..='z' | '0'..='9' | '/' | '-' | '.' | '@' | '_' => {
+                '/' => {
+                    last_slash = Some(self.pos);
+                    self.bump();
+                }
+                'A'..='Z' | 'a'..='z' | '0'..='9' | '-' | '.' | '@' | '_' => {
                     self.bump();
                 }
                 '!' | '%' | '^' | '"' | '#' | '$' | '&' | '\'' | '(' | ')' | '*' | '+' | ','
@@ -262,7 +278,7 @@ impl<'a, 'b> Parser<'a, 'b> {
                 _ => return Err(()),
             }
         }
-        Ok((start, self.pos, has_target_only_chars))
+        Ok((start, self.pos, last_slash, has_target_only_chars))
     }
 
     fn bump(&mut self) -> Option<char> {
@@ -329,7 +345,15 @@ mod tests {
 
     #[test]
     fn test_implicit_target() {
-        check("//a", Current, false, "", "a", "a")
+        check("//a", Current, false, "", "a", "a");
+        check(
+            "//crates/starpls_bazel",
+            Current,
+            false,
+            "",
+            "crates/starpls_bazel",
+            "starpls_bazel",
+        )
     }
 
     #[test]
@@ -451,4 +475,9 @@ mod tests {
     fn test_missing_package() {
         check_err("@a//", ParseError::EmptyPackage);
     }
+
+    #[test]
+    fn test_invalid_package_ends_with_slash() {
+        check_err("@a//abc/", ParseError::InvalidPackage);
+    }
 }

crates/starpls_common/src/lib.rs

@@ -2,7 +2,7 @@ use starpls_bazel::APIContext;
 use starpls_syntax::{
     line_index as syntax_line_index, parse_module, LineIndex, Module, ParseTree, SyntaxNode,
 };
-use std::fmt::Debug;
+use std::{fmt::Debug, path::PathBuf};
 
 pub use crate::diagnostics::{Diagnostic, Diagnostics, FileRange, Severity};
 
@@ -41,6 +41,17 @@ pub struct LoadItemCandidate {
 #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct FileId(pub u32);
 
+pub enum ResolvedPath {
+    Source {
+        path: PathBuf,
+    },
+    BuildTarget {
+        build_file: FileId,
+        target: String,
+        contents: Option<String>,
+    },
+}
+
 /// The base Salsa database. Supports file-related operations, like getting/setting file contents.
 pub trait Db: salsa::DbWithJar<Jar> {
     /// Creates a `File` in the database. This will overwrite the currently active
@@ -69,6 +80,13 @@ pub trait Db: salsa::DbWithJar<Jar> {
         path: &str,
         from: FileId,
     ) -> anyhow::Result<Option<Vec<LoadItemCandidate>>>;
+
+    fn resolve_path(
+        &self,
+        path: &str,
+        dialect: Dialect,
+        from: FileId,
+    ) -> anyhow::Result<Option<ResolvedPath>>;
 }
 
 #[salsa::input]

crates/starpls_hir/src/test_database.rs

@@ -1,7 +1,7 @@
 use crate::{def::ExprId, BuiltinDefs, Db, Dialect, GlobalCtxt, LoadItemId, ParamId, Ty};
 use dashmap::{mapref::entry::Entry, DashMap};
 use starpls_bazel::{APIContext, Builtins};
-use starpls_common::{File, FileId, LoadItemCandidate};
+use starpls_common::{File, FileId, LoadItemCandidate, ResolvedPath};
 use starpls_test_util::builtins_with_catch_all_functions;
 use std::sync::Arc;
 
@@ -84,6 +84,15 @@ impl starpls_common::Db for TestDatabase {
     ) -> anyhow::Result<Option<Vec<LoadItemCandidate>>> {
         Ok(None)
     }
+
+    fn resolve_path(
+        &self,
+        _path: &str,
+        _dialect: Dialect,
+        _from: FileId,
+    ) -> anyhow::Result<Option<ResolvedPath>> {
+        Ok(None)
+    }
 }
 
 impl crate::Db for TestDatabase {