From bc34622c50ce2103bffc12ed247213ad6819fbd9 Mon Sep 17 00:00:00 2001 From: danieleliad Date: Fri, 12 Aug 2022 08:24:44 +0300 Subject: [PATCH 1/5] added bazel workspace lsp support --- starlark/bin/eval.rs | 195 ++++++++++++++++++++++++++++++++++--- starlark/bin/main.rs | 25 +++++ starlark/src/lsp/server.rs | 35 +++++-- 3 files changed, 234 insertions(+), 21 deletions(-) diff --git a/starlark/bin/eval.rs b/starlark/bin/eval.rs index a227438d8..7ba0e70fa 100644 --- a/starlark/bin/eval.rs +++ b/starlark/bin/eval.rs @@ -25,6 +25,7 @@ use std::path::PathBuf; use gazebo::prelude::*; use itertools::Either; use lsp_types::Diagnostic; +use lsp_types::Range; use lsp_types::Url; use starlark::environment::FrozenModule; use starlark::environment::Globals; @@ -50,6 +51,13 @@ pub(crate) enum ContextMode { Run, } +#[derive(Debug, Clone)] +pub(crate) struct BazelInfo { + pub(crate) workspace_root: PathBuf, + pub(crate) output_base: PathBuf, + pub(crate) execroot: PathBuf, +} + #[derive(Debug)] pub(crate) struct Context { pub(crate) mode: ContextMode, @@ -58,6 +66,7 @@ pub(crate) struct Context { pub(crate) module: Option, pub(crate) builtin_docs: HashMap, pub(crate) builtin_symbols: HashMap, + pub(crate) bazel_info: Option, } /// The outcome of evaluating (checking, parsing or running) given starlark code. @@ -110,6 +119,7 @@ impl Context { module, builtin_docs, builtin_symbols, + bazel_info: None, }) } @@ -241,6 +251,147 @@ impl Context { } } +fn handle_local_bazel_repository( + info: &Option, + path: &str, + path_buf: PathBuf, + current_file_dir: &PathBuf, +) -> Result { + match info { + None => Err(ResolveLoadError::MissingBazelInfo(path_buf)), + Some(info) => { + let malformed_err = ResolveLoadError::PathMalformed(path_buf.clone()); + let mut split_parts = path.trim_start_match("//").split(':'); + let package = split_parts.next().ok_or(malformed_err.clone())?; + let target = split_parts.next().ok_or(malformed_err.clone())?; + match split_parts.next().is_some() { + true => Err(malformed_err.clone()), + false => { + let file_path = PathBuf::from(package).join(target); + let root_path = current_file_dir + .ancestors() + .find(|a| match a.read_dir() { + Ok(mut entries) => entries + .find(|f| match f { + Ok(f) => ["MODULE.bazel", "WORKSPACE", "WORKSPACE.bazel"] + .contains(&f.file_name().to_str().unwrap_or("")), + _ => false, + }) + .is_some(), + _ => false, + }) + .unwrap_or(&info.workspace_root); + Ok(root_path.join(file_path)) + } + } + } + } +} +fn handle_remote_bazel_repository( + info: &Option, + path: &str, + path_buf: PathBuf, +) -> Result { + match info { + None => Err(ResolveLoadError::MissingBazelInfo(path_buf)), + Some(info) => { + let malformed_err = ResolveLoadError::PathMalformed(path_buf.clone()); + let mut split_parts = path.trim_start_match("@").split("//"); + let repository = split_parts.next().ok_or(malformed_err.clone())?; + split_parts = split_parts.next().ok_or(malformed_err.clone())?.split(":"); + let package = split_parts.next().ok_or(malformed_err.clone())?; + let target = split_parts.next().ok_or(malformed_err.clone())?; + match split_parts.next().is_some() { + true => Err(malformed_err.clone()), + false => { + let execroot_dirname = + info.execroot.file_name().ok_or(malformed_err.clone())?; + + if repository == execroot_dirname { + Ok(info.workspace_root.join(package).join(target)) + } else { + Ok(info + .output_base + .join("external") + .join(repository) + .join(package) + .join(target)) + } + } + } + } + } +} + +fn get_relative_file( + current_file_dir: &PathBuf, + path: &str, + pathbuf: PathBuf, +) -> Result { + let malformed_err = ResolveLoadError::MissingCurrentFilePath(pathbuf.clone()); + let mut split_parts = path.split(":"); + let package = split_parts.next().ok_or(malformed_err.clone())?; + let target = split_parts.next().ok_or(malformed_err.clone())?; + match split_parts.next().is_some() { + true => Err(malformed_err.clone()), + false => Ok(current_file_dir.join(package).join(target)), + } +} +fn label_into_file( + bazel_info: &Option, + path: &str, + current_file_path: &PathBuf, +) -> Result { + let current_file_dir = current_file_path.parent(); + let path_buf = PathBuf::from(path); + + if path.starts_with("@") { + handle_remote_bazel_repository(bazel_info, path, path_buf.clone()) + } else if path.starts_with("//") { + handle_local_bazel_repository(bazel_info, path, path_buf.clone(), current_file_path) + } else if path.contains(":") { + match current_file_dir { + Some(dir) => get_relative_file(&dir.to_path_buf(), path, path_buf.clone()), + None => Err(ResolveLoadError::MissingCurrentFilePath(path_buf)), + } + } else { + match (current_file_dir, path_buf.is_absolute()) { + (_, true) => Ok(path_buf), + (Some(current_file_dir), false) => Ok(current_file_dir.join(&path_buf)), + (None, false) => Err(ResolveLoadError::MissingCurrentFilePath(path_buf)), + } + } +} + +fn replace_fake_file_with_build_target(fake_file: PathBuf) -> Option { + fake_file.parent().and_then(|p| { + let build = p.join("BUILD"); + let build_bazel = p.join("BUILD.bazel"); + if build.exists() { + Some(build) + } else if build_bazel.exists() { + Some(build_bazel) + } else { + None + } + }) +} + +fn find_location_in_build_file( + info: Option, + literal: String, + current_file_pathbuf: PathBuf, + ast: &AstModule, +) -> anyhow::Result> { + let resolved_file = label_into_file(&info, literal.as_str(), ¤t_file_pathbuf)?; + let basename = resolved_file.file_name().and_then(|f| f.to_str()).ok_or( + ResolveLoadError::ResolvedDoesNotExist(resolved_file.clone()), + )?; + let resolved_span = ast + .find_function_call_with_name(basename) + .and_then(|r| Some(Range::from(r))); + Ok(resolved_span) +} impl LspContext for Context { fn parse_file_with_contents(&self, uri: &LspUrl, content: String) -> LspEvalResult { match uri { @@ -257,16 +408,23 @@ impl LspContext for Context { } fn resolve_load(&self, path: &str, current_file: &LspUrl) -> anyhow::Result { - let path = PathBuf::from(path); match current_file { LspUrl::File(current_file_path) => { - let current_file_dir = current_file_path.parent(); - let absolute_path = match (current_file_dir, path.is_absolute()) { - (_, true) => Ok(path), - (Some(current_file_dir), false) => Ok(current_file_dir.join(&path)), - (None, false) => Err(ResolveLoadError::MissingCurrentFilePath(path)), + let mut resolved_file = label_into_file(&self.bazel_info, path, current_file_path)?; + resolved_file = match resolved_file.canonicalize() { + Ok(f) => { + if f.exists() { + Ok(f) + } else { + replace_fake_file_with_build_target(resolved_file.clone()) + .ok_or(ResolveLoadError::ResolvedDoesNotExist(resolved_file)) + } + } + _ => replace_fake_file_with_build_target(resolved_file.clone()) + .ok_or(ResolveLoadError::ResolvedDoesNotExist(resolved_file)), }?; - Ok(Url::from_file_path(absolute_path).unwrap().try_into()?) + + Ok(Url::from_file_path(resolved_file).unwrap().try_into()?) } _ => Err( ResolveLoadError::WrongScheme("file://".to_owned(), current_file.clone()).into(), @@ -279,11 +437,26 @@ impl LspContext for Context { literal: &str, current_file: &LspUrl, ) -> anyhow::Result> { + let current_file_pathbuf = current_file.path().to_path_buf(); self.resolve_load(literal, current_file).map(|url| { - Some(StringLiteralResult { - url, - location_finder: None, - }) + let p = url.path(); + // TODO: we can always give literal location finder + // TODO: but if its a file it will always try to resolve the location but won't be able to and an error will be printed + if p.ends_with("BUILD") || p.ends_with("BUILD.bazel") { + let info = self.bazel_info.clone(); + let literal_copy = literal.to_owned(); + Some(StringLiteralResult { + url, + location_finder: Some(box |ast: &AstModule, _url| { + find_location_in_build_file(info, literal_copy, current_file_pathbuf, ast) + }), + }) + } else { + Some(StringLiteralResult { + url, + location_finder: None, + }) + } }) } diff --git a/starlark/bin/main.rs b/starlark/bin/main.rs index 04936d7e8..bb94b52ea 100644 --- a/starlark/bin/main.rs +++ b/starlark/bin/main.rs @@ -30,9 +30,11 @@ use std::ffi::OsStr; use std::fmt; use std::fmt::Display; use std::path::PathBuf; +use std::process::Command; use std::sync::Arc; use anyhow::anyhow; +use eval::BazelInfo; use eval::Context; use gazebo::prelude::*; use itertools::Either; @@ -214,6 +216,28 @@ fn interactive(ctx: &Context) -> anyhow::Result<()> { } } +fn get_bazel_info_path(path_type: &str) -> anyhow::Result { + let output = Command::new("bazel").arg("info").arg(path_type).output()?; + + if !output.status.success() { + return Err(anyhow!("Failed running command bazel info")); + } + let s = std::str::from_utf8(output.stdout.as_slice())?; + Ok(PathBuf::from(s.trim())) +} + +fn get_bazel_info() -> Option { + let workspace_root = get_bazel_info_path("workspace").ok()?; + let output_base = get_bazel_info_path("output_base").ok()?; + let execroot = get_bazel_info_path("execution_root").ok()?; + + Some(BazelInfo { + workspace_root, + output_base, + execroot, + }) +} + fn main() -> anyhow::Result<()> { gazebo::terminate_on_panic(); @@ -242,6 +266,7 @@ fn main() -> anyhow::Result<()> { if args.lsp { ctx.mode = ContextMode::Check; + ctx.bazel_info = get_bazel_info(); lsp::server::stdio_server(ctx)?; } else if is_interactive { interactive(&ctx)?; diff --git a/starlark/src/lsp/server.rs b/starlark/src/lsp/server.rs index 30993be5f..c54282585 100644 --- a/starlark/src/lsp/server.rs +++ b/starlark/src/lsp/server.rs @@ -292,8 +292,14 @@ pub trait LspContext { } /// Errors when [`LspContext::resolve_load()`] cannot resolve a given path. -#[derive(thiserror::Error, Debug)] +#[derive(thiserror::Error, Debug, Clone)] pub enum ResolveLoadError { + /// Attempted to resolve a load but the path was malformed + #[error("path `{}` provided, but was malformed", .0.display())] + PathMalformed(PathBuf), + /// Attempted to resolve a bazel dependent load but no bazel info could be found + #[error("path `{}` provided, but bazel info could not be determined", .0.display())] + MissingBazelInfo(PathBuf), /// Attempted to resolve a relative path, but no current_file_path was provided, /// so it is not known what to resolve the path against. #[error("Relative path `{}` provided, but current_file_path could not be determined", .0.display())] @@ -301,6 +307,9 @@ pub enum ResolveLoadError { /// The scheme provided was not correct or supported. #[error("Url `{}` was expected to be of type `{}`", .1, .0)] WrongScheme(String, LspUrl), + /// Resolved Loaded file does not exist. + #[error("Resolved file `{}` did not exist", .0.display())] + ResolvedDoesNotExist(PathBuf), } /// Errors when loading contents of a starlark program. @@ -494,8 +503,7 @@ impl Backend { }) => { // If there's an error loading the file to parse it, at least // try to get to the file. - let target_range = self - .get_ast_or_load_from_disk(&url) + self.get_ast_or_load_from_disk(&url) .and_then(|ast| match ast { Some(module) => location_finder(&module.ast, &url), None => Ok(None), @@ -504,13 +512,20 @@ impl Backend { eprintln!("Error jumping to definition: {:#}", e); }) .unwrap_or_default() - .unwrap_or_default(); - Some(LocationLink { - origin_selection_range: Some(source.into()), - target_uri: url.try_into()?, - target_range, - target_selection_range: target_range, - }) + .and_then(|target_range| { + Some(LocationLink { + origin_selection_range: Some(source.into()), + target_uri: url.clone().try_into().ok()?, + target_range, + target_selection_range: target_range, + }) + }) + .or(Some(LocationLink { + origin_selection_range: Some(source.into()), + target_uri: url.try_into()?, + target_range: Range::default(), + target_selection_range: Range::default(), + })) } Some(StringLiteralResult { url, From 12bc1021f47ceaba1986bc26b2e7be5a07b89d11 Mon Sep 17 00:00:00 2001 From: danieleliad Date: Mon, 22 Aug 2022 02:37:12 +0300 Subject: [PATCH 2/5] refactored starlark bazel lsp mode --- bazel/Cargo.toml | 8 ++ bazel/src/bazel_info.rs | 84 ++++++++++++++++ bazel/src/info.txt | 23 +++++ bazel/src/label.rs | 118 +++++++++++++++++++++++ bazel/src/lib.rs | 2 + starlark/Cargo.toml | 1 + starlark/bin/eval.rs | 190 ++++++++----------------------------- starlark/bin/main.rs | 45 ++++----- starlark/src/lsp/server.rs | 3 - 9 files changed, 296 insertions(+), 178 deletions(-) create mode 100644 bazel/Cargo.toml create mode 100644 bazel/src/bazel_info.rs create mode 100644 bazel/src/info.txt create mode 100644 bazel/src/label.rs create mode 100644 bazel/src/lib.rs diff --git a/bazel/Cargo.toml b/bazel/Cargo.toml new file mode 100644 index 000000000..275b2984d --- /dev/null +++ b/bazel/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "bazel" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] diff --git a/bazel/src/bazel_info.rs b/bazel/src/bazel_info.rs new file mode 100644 index 000000000..13ac31c0c --- /dev/null +++ b/bazel/src/bazel_info.rs @@ -0,0 +1,84 @@ +use std::fmt::Debug; +use std::path::PathBuf; +use std::process::Command; + +#[derive(Debug, Clone, PartialEq)] +pub struct BazelInfo { + pub workspace_root: PathBuf, + pub output_base: PathBuf, + pub execroot: PathBuf, +} + +fn parse_bazel_info(info: &str) -> Option { + let mut workspace_root: Option = None; + let mut output_base: Option = None; + let mut execroot: Option = None; + info.split('\n').for_each(|l| { + let split: Vec<&str> = l.splitn(2, ':').collect(); + if split.len() != 2 { + return; + } + let first = split.get(0); + let next = split.get(1); + match (first, next) { + (Some(key), Some(value)) => match key { + &"execution_root" => execroot = Some(value.trim().into()), + &"output_base" => output_base = Some(value.trim().into()), + &"workspace" => workspace_root = Some(value.trim().into()), + _ => {} + }, + _ => {} + } + }); + match (execroot, output_base, workspace_root) { + (Some(execroot), Some(output_base), Some(workspace_root)) => Some(BazelInfo { + workspace_root, + execroot, + output_base, + }), + _ => { + eprintln!( + "Couldn't find workspace_root, execroot or output_base in output:\n`{}`", + info + ); + None + } + } +} + +pub fn get_bazel_info(workspace_dir: Option<&str>) -> Option { + let mut raw_command = Command::new("bazel"); + let mut command = raw_command.arg("info"); + command = match workspace_dir { + Some(d) => command.current_dir(d), + None => command, + }; + + let output = command.output().ok()?; + + if !output.status.success() { + return None; + } + + let s = std::str::from_utf8(output.stdout.as_slice()).ok()?; + + parse_bazel_info(s) +} + +#[cfg(test)] +mod tests { + use super::parse_bazel_info; + use super::BazelInfo; + + #[test] + fn parses_info() { + assert_eq!( + parse_bazel_info(include_str!("info.txt")), + Some(BazelInfo { + workspace_root: "/home/user/dev/bazel/bazel".into(), + execroot: "/home/user/.cache/bazel/_bazel_user/726bdc44ca84ffc53f631c27e313c4cf/execroot/io_bazel".into(), + output_base: "/home/user/.cache/bazel/_bazel_user/726bdc44ca84ffc53f631c27e313c4cf".into() + }) + ) + } +} diff --git a/bazel/src/info.txt b/bazel/src/info.txt new file mode 100644 index 000000000..6eb2ad9ac --- /dev/null +++ b/bazel/src/info.txt @@ -0,0 +1,23 @@ +bazel-bin: /home/user/.cache/bazel/_bazel_user/726bdc44ca84ffc53f631c27e313c4cf/execroot/io_bazel/bazel-out/k8-fastbuild/bin +bazel-genfiles: /home/user/.cache/bazel/_bazel_user/726bdc44ca84ffc53f631c27e313c4cf/execroot/io_bazel/bazel-out/k8-fastbuild/bin +bazel-testlogs: /home/user/.cache/bazel/_bazel_user/726bdc44ca84ffc53f631c27e313c4cf/execroot/io_bazel/bazel-out/k8-fastbuild/testlogs +character-encoding: file.encoding = ISO-8859-1, defaultCharset = ISO-8859-1 +command_log: /home/user/.cache/bazel/_bazel_user/726bdc44ca84ffc53f631c27e313c4cf/command.log +committed-heap-size: 418MB +execution_root: /home/user/.cache/bazel/_bazel_user/726bdc44ca84ffc53f631c27e313c4cf/execroot/io_bazel +gc-count: 11 +gc-time: 42ms +install_base: /home/user/.cache/bazel/_bazel_user/install/41b71f1bb3ce13f20cfeeb31a9357113 +java-home: /home/user/.cache/bazel/_bazel_user/install/41b71f1bb3ce13f20cfeeb31a9357113/embedded_tools/jdk +java-runtime: OpenJDK Runtime Environment (build 11.0.6+10-LTS) by Azul Systems, Inc. +java-vm: OpenJDK 64-Bit Server VM (build 11.0.6+10-LTS, mixed mode) by Azul Systems, Inc. +max-heap-size: 4175MB +output_base: /home/user/.cache/bazel/_bazel_user/726bdc44ca84ffc53f631c27e313c4cf +output_path: /home/user/.cache/bazel/_bazel_user/726bdc44ca84ffc53f631c27e313c4cf/execroot/io_bazel/bazel-out +package_path: %workspace% +release: release 5.2.0 +repository_cache: /home/user/.cache/bazel/_bazel_user/cache/repos/v1 +server_log: /home/user/.cache/bazel/_bazel_user/726bdc44ca84ffc53f631c27e313c4cf/java.log.home.user.log.java.20220821-211440.206313 +server_pid: 206313 +used-heap-size: 266MB +workspace: /home/user/dev/bazel/bazel diff --git a/bazel/src/label.rs b/bazel/src/label.rs new file mode 100644 index 000000000..c41f756e9 --- /dev/null +++ b/bazel/src/label.rs @@ -0,0 +1,118 @@ + +use std::path::PathBuf; + +use crate::bazel_info::BazelInfo; + +pub struct ExternalLabel { + repository: String, + package: String, + target: String, +} +pub struct LocalLabel { + package: String, + target: String, +} + +pub struct RelativeLabel { + sub_package: String, + target: String, +} + +pub enum Label { + External(ExternalLabel), + Local(LocalLabel), + Relative(RelativeLabel), +} + +impl Label { + fn split_package_target(label: &str) -> Option<(&str, &str)> { + let mut split_parts = label.split(":"); + let package = split_parts.next()?; + let target = split_parts.next()?; + Some((package, target)) + } + + fn split_repository_package_target(label: &str) -> Option<(&str, &str, &str)> { + let mut split_parts = label.split("//"); + let repository = split_parts.next()?; + let package_target = split_parts.next()?; + let (package, target) = Self::split_package_target(package_target)?; + Some((repository, package, target)) + } + + pub fn replace_fake_file_with_build_target(fake_file: PathBuf) -> Option { + if fake_file.exists() { + return Some(fake_file); + } + fake_file.parent().and_then(|p| { + let build = p.join("BUILD"); + let build_bazel = p.join("BUILD.bazel"); + if build.exists() { + Some(build) + } else if build_bazel.exists() { + Some(build_bazel) + } else { + None + } + }) + } + + pub fn resolve( + self, + bazel_info: &BazelInfo, + current_file_dir: Option, + ) -> Option { + // TODO: support nested workspaces either by getting info again or at the start getting the info for all the workspaces in the directory + match self { + Label::External(l) => { + let execroot_dirname = bazel_info.execroot.file_name()?; + + if l.repository == execroot_dirname.to_str()? { + Some(bazel_info.workspace_root.join(l.package).join(l.target)) + } else { + Some( + bazel_info + .output_base + .join("external") + .join(l.repository) + .join(l.package) + .join(l.target), + ) + } + } + Label::Local(l) => Some(bazel_info.workspace_root.join(l.package).join(l.target)), + Label::Relative(l) => { + current_file_dir.and_then(|d| Some(d.join(l.sub_package).join(l.target))) + } + } + } + + pub fn new(label: &str) -> Option { + if !label.contains(":") { + return None; + } + + if label.starts_with("@") { + let (repository, package, target) = + Self::split_repository_package_target(label.trim_start_matches('@'))?; + return Some(Label::External(ExternalLabel { + repository: repository.to_owned(), + package: package.to_owned(), + target: target.to_owned(), + })); + } + if label.starts_with("//") { + let (package, target) = Self::split_package_target(label.trim_start_matches("//"))?; + return Some(Label::Local(LocalLabel { + package: package.to_owned(), + target: target.to_owned(), + })); + } + + let (package, target) = Self::split_package_target(label)?; + Some(Label::Relative(RelativeLabel { + sub_package: package.to_owned(), + target: target.to_owned(), + })) + } +} diff --git a/bazel/src/lib.rs b/bazel/src/lib.rs new file mode 100644 index 000000000..396837f13 --- /dev/null +++ b/bazel/src/lib.rs @@ -0,0 +1,2 @@ +pub mod bazel_info; +pub mod label; diff --git a/starlark/Cargo.toml b/starlark/Cargo.toml index b68cbd0be..d3015a56c 100644 --- a/starlark/Cargo.toml +++ b/starlark/Cargo.toml @@ -33,6 +33,7 @@ either = "1.6.1" static_assertions = "1.1.0" memoffset = "0.6.4" thiserror = "1.0.30" +bazel = { version = "0.1.0", path = "../bazel" } starlark_derive = { version = "0.9.0-pre", path = "../starlark_derive" } starlark_map = { version = "0.9.0-pre", path = "../starlark_map" } gazebo.version = "0.8.0" diff --git a/starlark/bin/eval.rs b/starlark/bin/eval.rs index 7ba0e70fa..9014ba378 100644 --- a/starlark/bin/eval.rs +++ b/starlark/bin/eval.rs @@ -22,6 +22,8 @@ use std::iter; use std::path::Path; use std::path::PathBuf; +use bazel::bazel_info::BazelInfo; +use bazel::label::Label; use gazebo::prelude::*; use itertools::Either; use lsp_types::Diagnostic; @@ -51,13 +53,6 @@ pub(crate) enum ContextMode { Run, } -#[derive(Debug, Clone)] -pub(crate) struct BazelInfo { - pub(crate) workspace_root: PathBuf, - pub(crate) output_base: PathBuf, - pub(crate) execroot: PathBuf, -} - #[derive(Debug)] pub(crate) struct Context { pub(crate) mode: ContextMode, @@ -251,147 +246,56 @@ impl Context { } } -fn handle_local_bazel_repository( - info: &Option, - path: &str, - path_buf: PathBuf, - current_file_dir: &PathBuf, -) -> Result { - match info { - None => Err(ResolveLoadError::MissingBazelInfo(path_buf)), - Some(info) => { - let malformed_err = ResolveLoadError::PathMalformed(path_buf.clone()); - let mut split_parts = path.trim_start_match("//").split(':'); - let package = split_parts.next().ok_or(malformed_err.clone())?; - let target = split_parts.next().ok_or(malformed_err.clone())?; - match split_parts.next().is_some() { - true => Err(malformed_err.clone()), - false => { - let file_path = PathBuf::from(package).join(target); - let root_path = current_file_dir - .ancestors() - .find(|a| match a.read_dir() { - Ok(mut entries) => entries - .find(|f| match f { - Ok(f) => ["MODULE.bazel", "WORKSPACE", "WORKSPACE.bazel"] - .contains(&f.file_name().to_str().unwrap_or("")), - _ => false, - }) - .is_some(), - _ => false, - }) - .unwrap_or(&info.workspace_root); - Ok(root_path.join(file_path)) - } - } - } - } -} -fn handle_remote_bazel_repository( +fn find_location_in_build_file( info: &Option, - path: &str, - path_buf: PathBuf, -) -> Result { - match info { - None => Err(ResolveLoadError::MissingBazelInfo(path_buf)), - Some(info) => { - let malformed_err = ResolveLoadError::PathMalformed(path_buf.clone()); - let mut split_parts = path.trim_start_match("@").split("//"); - let repository = split_parts.next().ok_or(malformed_err.clone())?; - split_parts = split_parts.next().ok_or(malformed_err.clone())?.split(":"); - let package = split_parts.next().ok_or(malformed_err.clone())?; - let target = split_parts.next().ok_or(malformed_err.clone())?; - match split_parts.next().is_some() { - true => Err(malformed_err.clone()), - false => { - let execroot_dirname = - info.execroot.file_name().ok_or(malformed_err.clone())?; - - if repository == execroot_dirname { - Ok(info.workspace_root.join(package).join(target)) - } else { - Ok(info - .output_base - .join("external") - .join(repository) - .join(package) - .join(target)) - } - } - } - } - } + literal: String, + current_file_pathbuf: PathBuf, + ast: &AstModule, +) -> anyhow::Result> { + let resolved_file = label_into_file(info, literal.as_str(), ¤t_file_pathbuf, false)?; + let basename = resolved_file.file_name().and_then(|f| f.to_str()).ok_or( + ResolveLoadError::ResolvedDoesNotExist(resolved_file.clone()), + )?; + let resolved_span = ast + .find_function_call_with_name(basename) + .and_then(|r| Some(Range::from(r))); + Ok(resolved_span) } -fn get_relative_file( - current_file_dir: &PathBuf, - path: &str, - pathbuf: PathBuf, -) -> Result { - let malformed_err = ResolveLoadError::MissingCurrentFilePath(pathbuf.clone()); - let mut split_parts = path.split(":"); - let package = split_parts.next().ok_or(malformed_err.clone())?; - let target = split_parts.next().ok_or(malformed_err.clone())?; - match split_parts.next().is_some() { - true => Err(malformed_err.clone()), - false => Ok(current_file_dir.join(package).join(target)), - } -} fn label_into_file( bazel_info: &Option, path: &str, current_file_path: &PathBuf, + replace_build_file: bool, ) -> Result { - let current_file_dir = current_file_path.parent(); + let current_file_dir = current_file_path + .parent() + .and_then(|x| Some(PathBuf::from(x))); let path_buf = PathBuf::from(path); + let label = Label::new(path); + + // TODO: not really malformed should we propogate error from label.resolve or just create a new error: CouldntFind Bazel Label + match (bazel_info, label) { + (Some(info), Some(label)) => label + .resolve(info, current_file_dir) + .and_then(|x| { + if replace_build_file { + Label::replace_fake_file_with_build_target(x) + } else { + Some(x) + } + }) + .and_then(|x| Some(x.canonicalize().unwrap_or(x))) + .ok_or(ResolveLoadError::PathMalformed(path_buf.clone())), - if path.starts_with("@") { - handle_remote_bazel_repository(bazel_info, path, path_buf.clone()) - } else if path.starts_with("//") { - handle_local_bazel_repository(bazel_info, path, path_buf.clone(), current_file_path) - } else if path.contains(":") { - match current_file_dir { - Some(dir) => get_relative_file(&dir.to_path_buf(), path, path_buf.clone()), - None => Err(ResolveLoadError::MissingCurrentFilePath(path_buf)), - } - } else { - match (current_file_dir, path_buf.is_absolute()) { + _ => match (current_file_dir, path_buf.is_absolute()) { (_, true) => Ok(path_buf), (Some(current_file_dir), false) => Ok(current_file_dir.join(&path_buf)), (None, false) => Err(ResolveLoadError::MissingCurrentFilePath(path_buf)), - } + }, } } -fn replace_fake_file_with_build_target(fake_file: PathBuf) -> Option { - fake_file.parent().and_then(|p| { - let build = p.join("BUILD"); - let build_bazel = p.join("BUILD.bazel"); - if build.exists() { - Some(build) - } else if build_bazel.exists() { - Some(build_bazel) - } else { - None - } - }) -} - -fn find_location_in_build_file( - info: Option, - literal: String, - current_file_pathbuf: PathBuf, - ast: &AstModule, -) -> anyhow::Result> { - let resolved_file = label_into_file(&info, literal.as_str(), ¤t_file_pathbuf)?; - let basename = resolved_file.file_name().and_then(|f| f.to_str()).ok_or( - ResolveLoadError::ResolvedDoesNotExist(resolved_file.clone()), - )?; - let resolved_span = ast - .find_function_call_with_name(basename) - .and_then(|r| Some(Range::from(r))); - Ok(resolved_span) -} impl LspContext for Context { fn parse_file_with_contents(&self, uri: &LspUrl, content: String) -> LspEvalResult { match uri { @@ -410,20 +314,8 @@ impl LspContext for Context { fn resolve_load(&self, path: &str, current_file: &LspUrl) -> anyhow::Result { match current_file { LspUrl::File(current_file_path) => { - let mut resolved_file = label_into_file(&self.bazel_info, path, current_file_path)?; - resolved_file = match resolved_file.canonicalize() { - Ok(f) => { - if f.exists() { - Ok(f) - } else { - replace_fake_file_with_build_target(resolved_file.clone()) - .ok_or(ResolveLoadError::ResolvedDoesNotExist(resolved_file)) - } - } - _ => replace_fake_file_with_build_target(resolved_file.clone()) - .ok_or(ResolveLoadError::ResolvedDoesNotExist(resolved_file)), - }?; - + let resolved_file = + label_into_file(&self.bazel_info, path, current_file_path, true)?; Ok(Url::from_file_path(resolved_file).unwrap().try_into()?) } _ => Err( @@ -442,13 +334,13 @@ impl LspContext for Context { let p = url.path(); // TODO: we can always give literal location finder // TODO: but if its a file it will always try to resolve the location but won't be able to and an error will be printed - if p.ends_with("BUILD") || p.ends_with("BUILD.bazel") { + if self.bazel_info.is_some() && p.ends_with("BUILD") || p.ends_with("BUILD.bazel") { let info = self.bazel_info.clone(); let literal_copy = literal.to_owned(); Some(StringLiteralResult { url, - location_finder: Some(box |ast: &AstModule, _url| { - find_location_in_build_file(info, literal_copy, current_file_pathbuf, ast) + location_finder: Some(box move |ast: &AstModule, _url| { + find_location_in_build_file(&info, literal_copy, current_file_pathbuf, ast) }), }) } else { diff --git a/starlark/bin/main.rs b/starlark/bin/main.rs index bb94b52ea..9a2ec8415 100644 --- a/starlark/bin/main.rs +++ b/starlark/bin/main.rs @@ -30,11 +30,10 @@ use std::ffi::OsStr; use std::fmt; use std::fmt::Display; use std::path::PathBuf; -use std::process::Command; use std::sync::Arc; use anyhow::anyhow; -use eval::BazelInfo; +use bazel::bazel_info::get_bazel_info; use eval::Context; use gazebo::prelude::*; use itertools::Either; @@ -48,7 +47,6 @@ use walkdir::WalkDir; use crate::eval::ContextMode; use crate::types::LintMessage; - mod dap; mod eval; mod types; @@ -74,6 +72,20 @@ struct Args { )] lsp: bool, + #[structopt( + long = "bazel", + help = "Configures the LSP server to work with bazel.", + conflicts_with_all = &[ + "interactive", + "dap", + "check", + "json", + "evaluate", + "files", + ], + )] + bazel: bool, + #[structopt( long = "dap", help = "Start a DAP server.", @@ -216,28 +228,6 @@ fn interactive(ctx: &Context) -> anyhow::Result<()> { } } -fn get_bazel_info_path(path_type: &str) -> anyhow::Result { - let output = Command::new("bazel").arg("info").arg(path_type).output()?; - - if !output.status.success() { - return Err(anyhow!("Failed running command bazel info")); - } - let s = std::str::from_utf8(output.stdout.as_slice())?; - Ok(PathBuf::from(s.trim())) -} - -fn get_bazel_info() -> Option { - let workspace_root = get_bazel_info_path("workspace").ok()?; - let output_base = get_bazel_info_path("output_base").ok()?; - let execroot = get_bazel_info_path("execution_root").ok()?; - - Some(BazelInfo { - workspace_root, - output_base, - execroot, - }) -} - fn main() -> anyhow::Result<()> { gazebo::terminate_on_panic(); @@ -266,7 +256,10 @@ fn main() -> anyhow::Result<()> { if args.lsp { ctx.mode = ContextMode::Check; - ctx.bazel_info = get_bazel_info(); + // TODO: workspace dir? + if args.bazel { + ctx.bazel_info = get_bazel_info(None); + } lsp::server::stdio_server(ctx)?; } else if is_interactive { interactive(&ctx)?; diff --git a/starlark/src/lsp/server.rs b/starlark/src/lsp/server.rs index c54282585..8742d111c 100644 --- a/starlark/src/lsp/server.rs +++ b/starlark/src/lsp/server.rs @@ -297,9 +297,6 @@ pub enum ResolveLoadError { /// Attempted to resolve a load but the path was malformed #[error("path `{}` provided, but was malformed", .0.display())] PathMalformed(PathBuf), - /// Attempted to resolve a bazel dependent load but no bazel info could be found - #[error("path `{}` provided, but bazel info could not be determined", .0.display())] - MissingBazelInfo(PathBuf), /// Attempted to resolve a relative path, but no current_file_path was provided, /// so it is not known what to resolve the path against. #[error("Relative path `{}` provided, but current_file_path could not be determined", .0.display())] From 0f8c57c83813ca0f2435d6472b657a88f83c5a30 Mon Sep 17 00:00:00 2001 From: danieleliad Date: Mon, 22 Aug 2022 19:17:06 +0300 Subject: [PATCH 3/5] fixed cr --- bazel/src/label.rs | 159 +++++++++++++++++++++++++++++++++---- starlark/bin/eval.rs | 2 +- starlark/src/lsp/server.rs | 20 +++-- 3 files changed, 159 insertions(+), 22 deletions(-) diff --git a/bazel/src/label.rs b/bazel/src/label.rs index c41f756e9..ac0e33c79 100644 --- a/bazel/src/label.rs +++ b/bazel/src/label.rs @@ -1,23 +1,27 @@ - use std::path::PathBuf; use crate::bazel_info::BazelInfo; +#[derive(PartialEq, Debug)] pub struct ExternalLabel { repository: String, package: String, target: String, } + +#[derive(PartialEq, Debug)] pub struct LocalLabel { package: String, target: String, } +#[derive(PartialEq, Debug)] pub struct RelativeLabel { sub_package: String, target: String, } +#[derive(PartialEq, Debug)] pub enum Label { External(ExternalLabel), Local(LocalLabel), @@ -27,17 +31,32 @@ pub enum Label { impl Label { fn split_package_target(label: &str) -> Option<(&str, &str)> { let mut split_parts = label.split(":"); - let package = split_parts.next()?; - let target = split_parts.next()?; - Some((package, target)) + let package = split_parts.next(); + let target = split_parts.next(); + // Support //foo short for //foo:foo + match (package, target) { + // to avoid // being a valid label + // but //:baz being a valid target + (Some(""), None) => None, + (Some(package), Some(target)) => Some((package, target)), + (Some(package), None) => Some((package, package)), + _ => None, + } } fn split_repository_package_target(label: &str) -> Option<(&str, &str, &str)> { let mut split_parts = label.split("//"); let repository = split_parts.next()?; - let package_target = split_parts.next()?; - let (package, target) = Self::split_package_target(package_target)?; - Some((repository, package, target)) + match split_parts.next() { + // empty package-target could be caused by @foo// or // which we don't want to support so return None + Some("") => None, + Some(package_target) => { + let (package, target) = Self::split_package_target(package_target)?; + Some((repository, package, target)) + } + // @foo shorthand for @foo//:foo + None => Some((repository, "", repository)), + } } pub fn replace_fake_file_with_build_target(fake_file: PathBuf) -> Option { @@ -47,10 +66,10 @@ impl Label { fake_file.parent().and_then(|p| { let build = p.join("BUILD"); let build_bazel = p.join("BUILD.bazel"); - if build.exists() { - Some(build) - } else if build_bazel.exists() { + if build_bazel.exists() { Some(build_bazel) + } else if build.exists() { + Some(build) } else { None } @@ -88,7 +107,8 @@ impl Label { } pub fn new(label: &str) -> Option { - if !label.contains(":") { + // to avoid the "" label being valid + if label.len() == 0 { return None; } @@ -109,10 +129,121 @@ impl Label { })); } - let (package, target) = Self::split_package_target(label)?; - Some(Label::Relative(RelativeLabel { + if label.contains(":") { + let (package, target) = Self::split_package_target(label)?; + Some(Label::Relative(RelativeLabel { + sub_package: package.to_owned(), + target: target.to_owned(), + })) + } else { + // To support foo being shorthand for :foo + Some(Label::Relative(RelativeLabel { + sub_package: "".into(), + target: label.to_owned(), + })) + } + } +} + +#[cfg(test)] +mod tests { + use super::ExternalLabel; + use super::Label; + use super::LocalLabel; + use super::RelativeLabel; + + fn external_label(repository: &str, package: &str, target: &str) -> Label { + Label::External(ExternalLabel { + repository: repository.to_owned(), + package: package.to_owned(), + target: target.to_owned(), + }) + } + + fn local_label(package: &str, target: &str) -> Label { + Label::Local(LocalLabel { + package: package.to_owned(), + target: target.to_owned(), + }) + } + fn relative_label(package: &str, target: &str) -> Label { + Label::Relative(RelativeLabel { sub_package: package.to_owned(), target: target.to_owned(), - })) + }) + } + + #[test] + fn external() { + assert_eq!( + Label::new("@foo//bar:baz"), + Some(external_label("foo", "bar", "baz")) + ) + } + + #[test] + fn external_no_target() { + assert_eq!( + Label::new("@foo//bar"), + Some(external_label("foo", "bar", "bar")) + ) + } + + #[test] + fn external_no_package() { + assert_eq!(Label::new("@foo"), Some(external_label("foo", "", "foo"))) + } + + #[test] + fn external_empty_package_with_target() { + assert_eq!( + Label::new("@foo//:baz"), + Some(external_label("foo", "", "baz")) + ) + } + + #[test] + fn external_empty_package_invalid() { + assert_eq!(Label::new("@foo//"), None) + } + + #[test] + fn local() { + assert_eq!(Label::new("//bar:baz"), Some(local_label("bar", "baz"))) + } + + #[test] + fn local_no_target() { + assert_eq!(Label::new("//bar"), Some(local_label("bar", "bar"))) + } + + #[test] + fn local_no_package() { + assert_eq!(Label::new("//:baz"), Some(local_label("", "baz"))) + } + + #[test] + fn local_no_package_invalid() { + assert_eq!(Label::new("//"), None) + } + + #[test] + fn relative() { + assert_eq!(Label::new("bar:baz"), Some(relative_label("bar", "baz"))) + } + + #[test] + fn relative_no_target() { + assert_eq!(Label::new("baz"), Some(relative_label("", "baz"))) + } + + #[test] + fn relative_only_target() { + assert_eq!(Label::new(":baz"), Some(relative_label("", "baz"))) + } + + #[test] + fn empty() { + assert_eq!(Label::new(""), None) } } diff --git a/starlark/bin/eval.rs b/starlark/bin/eval.rs index 9014ba378..c8b308740 100644 --- a/starlark/bin/eval.rs +++ b/starlark/bin/eval.rs @@ -274,7 +274,7 @@ fn label_into_file( let path_buf = PathBuf::from(path); let label = Label::new(path); - // TODO: not really malformed should we propogate error from label.resolve or just create a new error: CouldntFind Bazel Label + // TODO: not really malformed should we propogate error from label.resolve or just create a new error: Couldnt Find Bazel Label match (bazel_info, label) { (Some(info), Some(label)) => label .resolve(info, current_file_dir) diff --git a/starlark/src/lsp/server.rs b/starlark/src/lsp/server.rs index 8742d111c..044c2d10c 100644 --- a/starlark/src/lsp/server.rs +++ b/starlark/src/lsp/server.rs @@ -500,29 +500,35 @@ impl Backend { }) => { // If there's an error loading the file to parse it, at least // try to get to the file. - self.get_ast_or_load_from_disk(&url) + let location_result = self + .get_ast_or_load_from_disk(&url) .and_then(|ast| match ast { Some(module) => location_finder(&module.ast, &url), None => Ok(None), }) .inspect_err(|e| { eprintln!("Error jumping to definition: {:#}", e); - }) - .unwrap_or_default() - .and_then(|target_range| { + }); + match location_result { + // if the location result was successful + // only return a location link if a location was found + Ok(location) => location.and_then(|target_range| { Some(LocationLink { origin_selection_range: Some(source.into()), target_uri: url.clone().try_into().ok()?, target_range, target_selection_range: target_range, }) - }) - .or(Some(LocationLink { + }), + // if the location result was an error + // try to at least go to the file + _ => Some(LocationLink { origin_selection_range: Some(source.into()), target_uri: url.try_into()?, target_range: Range::default(), target_selection_range: Range::default(), - })) + }), + } } Some(StringLiteralResult { url, From ab85119fc1c88e87ca5917e02c05135df02cc858 Mon Sep 17 00:00:00 2001 From: danieleliad Date: Tue, 23 Aug 2022 02:32:40 +0300 Subject: [PATCH 4/5] finding repository. and supporting @repository//:defs.bzl loading //package:target we now load // as the workspace the file is in right now --- bazel/src/label.rs | 61 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 9 deletions(-) diff --git a/bazel/src/label.rs b/bazel/src/label.rs index ac0e33c79..eb1342b61 100644 --- a/bazel/src/label.rs +++ b/bazel/src/label.rs @@ -76,6 +76,33 @@ impl Label { }) } + fn is_file_root_of_workspace(file: PathBuf) -> bool { + match file.file_name().and_then(|name| name.to_str()) { + Some(name) => name.starts_with("WORKSPACE") || name == "MODULE.bazel", + _ => false, + } + } + + fn resolve_local( + bazel_info: &BazelInfo, + current_file_dir: PathBuf, + l: LocalLabel, + ) -> Option { + for a in current_file_dir.ancestors() { + for file in a.read_dir().ok()? { + match file { + Ok(file) => { + if Self::is_file_root_of_workspace(file.path()) { + return Some(a.to_path_buf().join(l.package).join(l.target)); + } + } + _ => {} + } + } + } + Some(bazel_info.workspace_root.join(l.package).join(l.target)) + } + pub fn resolve( self, bazel_info: &BazelInfo, @@ -89,17 +116,33 @@ impl Label { if l.repository == execroot_dirname.to_str()? { Some(bazel_info.workspace_root.join(l.package).join(l.target)) } else { - Some( - bazel_info - .output_base - .join("external") - .join(l.repository) - .join(l.package) - .join(l.target), - ) + let external_directory = bazel_info.output_base.join("external"); + let repositories: Vec = external_directory + .read_dir() + .ok()? + .filter_map(|e| { + e.ok() + .and_then(|f| Some(f.file_name().to_str().unwrap_or("").to_owned())) + .filter(|name| { + let repository_with_version = l.repository.clone() + "."; + name == l.repository.as_str() + || name.starts_with(repository_with_version.as_str()) + }) + }) + .collect(); + + match repositories.len() { + 1 => repositories.get(0).and_then(|repo| { + Some(external_directory.join(repo).join(l.package).join(l.target)) + }), + _ => None, + } } } - Label::Local(l) => Some(bazel_info.workspace_root.join(l.package).join(l.target)), + Label::Local(l) => { + current_file_dir.and_then(|dir| Self::resolve_local(bazel_info, dir, l)) + } + Label::Relative(l) => { current_file_dir.and_then(|d| Some(d.join(l.sub_package).join(l.target))) } From a30ccfe5dcee98228fcde66648fdd0ca0fd18726 Mon Sep 17 00:00:00 2001 From: danieleliad Date: Tue, 23 Aug 2022 02:54:16 +0300 Subject: [PATCH 5/5] picking the first repository that 'matches' the repository name or repository name. --- bazel/src/label.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/bazel/src/label.rs b/bazel/src/label.rs index eb1342b61..cb5fc1db4 100644 --- a/bazel/src/label.rs +++ b/bazel/src/label.rs @@ -117,7 +117,7 @@ impl Label { Some(bazel_info.workspace_root.join(l.package).join(l.target)) } else { let external_directory = bazel_info.output_base.join("external"); - let repositories: Vec = external_directory + let mut repositories: Vec = external_directory .read_dir() .ok()? .filter_map(|e| { @@ -131,12 +131,11 @@ impl Label { }) .collect(); - match repositories.len() { - 1 => repositories.get(0).and_then(|repo| { - Some(external_directory.join(repo).join(l.package).join(l.target)) - }), - _ => None, - } + // TODO: how to pick just one + repositories.sort(); + repositories.get(0).and_then(|repo| { + Some(external_directory.join(repo).join(l.package).join(l.target)) + }) } } Label::Local(l) => {