Skip to content

Commit

Permalink
Use symbols relevant to file type
Browse files Browse the repository at this point in the history
  • Loading branch information
@hauserx
hauserx committed Jan 15, 2025
1 parent df5dac2 commit 4c0ac86
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 88 deletions.
93 changes: 60 additions & 33 deletions src/bazel.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,7 @@ pub(crate) struct BazelContext<Client> {

fn is_workspace_file(uri: &LspUrl) -> bool {
match uri {
LspUrl::File(path) => path
.file_name()
.map(|name| name == "WORKSPACE" || name == "WORKSPACE.bazel")
.unwrap_or(false),
LspUrl::File(path) => FileType::from_path(path) == FileType::Workspace,
LspUrl::Starlark(_) => false,
LspUrl::Other(_) => false,
}
Expand Down Expand Up @@ -438,7 +435,7 @@ impl<Client: BazelClient> BazelContext<Client> {
/// Returns protos for bazel globals (like int, str, dir; but also e.g. cc_library, alias,
/// test_suite etc.).
// TODO: Consider caching this
fn get_bazel_globals(&self, uri: &LspUrl) -> (builtin::BuildLanguage, builtin::Builtins) {
fn get_bazel_globals(&self, uri: &LspUrl) -> (builtin::BuildLanguage, Vec<builtin::Value>) {
let language_proto = self.get_build_language_proto(uri);

let language_proto = language_proto
Expand All @@ -450,17 +447,31 @@ impl<Client: BazelClient> BazelContext<Client> {
// TODO: builtins are also dependent on bazel version, but there is no way to obtain those,
// see https://github.com/bazel-contrib/vscode-bazel/issues/1.
let builtins_proto = include_bytes!(env!("BUILTIN_PB"));
let builtins = builtin::Builtins::decode(&builtins_proto[..]).unwrap();
let file_type = FileType::from_lsp_url(uri);
let globals = builtin::Builtins::decode(&builtins_proto[..])
.unwrap()
.global
.into_iter()
// TODO: For unkonwn file types (like BUILD.in), should we use only global api context;
// guess what's the file type, or just use symbols from all file contexts?
.filter(|global| builtin::api_context_applicable_for_file_type(global.api_context(), file_type))
.collect();

(language, builtins)
(language, globals)
}

fn try_get_environment(&self, uri: &LspUrl) -> anyhow::Result<DocModule> {
let file_type = FileType::from_lsp_url(uri);
let (language, builtins) = self.get_bazel_globals(uri);
let (language, globals) = self.get_bazel_globals(uri);

// TODO: Replace fetching builtins with executing bazel info starlark-environments-proto.
// Commented out for now, as current implementation adds not-relevant symbols.
//language
//let members: SmallMap<_, _> = builtin::build_language_to_doc_members(&language)
// .chain(builtin::builtins_to_doc_members(&globals))
// .map(|(name, member)| (name, DocItem::Member(member)))
// .collect();

let members: SmallMap<_, _> = builtin::build_language_to_doc_members(&language)
.chain(builtin::builtins_to_doc_members(&builtins, file_type))
let members: SmallMap<_, _> = builtin::builtins_to_doc_members(&globals)
.map(|(name, member)| (name, DocItem::Member(member)))
.collect();

Expand All @@ -471,19 +482,13 @@ impl<Client: BazelClient> BazelContext<Client> {
}

fn get_bazel_globals_names(&self, uri: &LspUrl) -> HashSet<String> {
let (language, builtins) = self.get_bazel_globals(uri);

language
.rule
.iter()
.map(|rule| rule.name.clone())
.chain(builtins.global.iter().map(|global| global.name.clone()))
.chain(
builtin::MISSING_GLOBALS
.iter()
.map(|missing| missing.to_string()),
)
.collect()
let (language, globals) = self.get_bazel_globals(uri);
// .rule
// .iter()
// .map(|rule| rule.name.clone())
// .chain(globals.iter().map(|global| global.name.clone()))
// .collect()
globals.iter().map(|global| global.name.clone()).collect()
}
}

Expand Down Expand Up @@ -1094,11 +1099,14 @@ mod tests {

let module = context.get_environment(&LspUrl::File(PathBuf::from("/foo/bar.bzl")));

assert!(module_contains(&module, "cc_library"));
assert!(module_contains(&module, "rule"));
assert!(!module_contains(&module, "cc_library"));

let module = context.get_environment(&LspUrl::File(PathBuf::from("/foo/bar/BUILD")));

assert!(module_contains(&module, "cc_library"));
assert!(module_contains(&module, "select"));
assert!(!module_contains(&module, "rule"));

Ok(())
}
Expand Down Expand Up @@ -1320,25 +1328,44 @@ mod tests {
let fixture = TestFixture::new("simple")?;
let context = fixture.context()?;

let result = context.parse_file_with_contents(
&LspUrl::File(PathBuf::from("/foo.bzl")),
"
let content = "
test_suite(name='my_test_suite');
module(name='my_module');
unknown_global_function(42);
a=int(7);
register_toolchains([':my_toolchain']);
"
.to_string(),
);
.to_string();

let result = context.parse_file_with_contents(
&LspUrl::File(PathBuf::from("/BUILD")),
content.clone());

assert_eq!(1, result.diagnostics.len());
assert_eq!(2, result.diagnostics.len());
assert_eq!(
"Use of undefined variable `module`",
result.diagnostics[0].message
);
assert_eq!(
"Use of undefined variable `unknown_global_function`",
result.diagnostics[1].message
);

let result = context.parse_file_with_contents(
&LspUrl::File(PathBuf::from("/MODULE.bazel")),
content);

assert_eq!(2, result.diagnostics.len());
assert_eq!(
"Use of undefined variable `test_suite`",
result.diagnostics[0].message
);
assert_eq!(
"Use of undefined variable `unknown_global_function`",
result.diagnostics[1].message
);

Ok(())
}
Expand Down
67 changes: 17 additions & 50 deletions src/builtin.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,45 +10,6 @@ use starlark::{

use crate::file_type::FileType;

/// Names of globals missing in builtins reported by bazel.
/// See e.g. https://github.com/bazel-contrib/vscode-bazel/issues/1#issuecomment-2036369868
pub static MISSING_GLOBALS: &'static [&'static str] = &[
// All values from https://bazel.build/rules/lib/globals/workspace
"bind",
"register_execution_platforms",
"register_toolchains",
"workspace",
// Values from https://bazel.build/rules/lib/globals/module
"archive_override",
"bazel_dep",
"git_override",
"include",
"inject_repo",
"local_path_override",
"module",
"multiple_version_override",
"override_repo",
"register_execution_platforms",
"register_toolchains",
"single_version_override",
"use_extension",
"use_repo",
"use_repo_rule",
// Missing values from https://bazel.build/rules/lib/globals/build
"package",
"repo_name",
// Missing values from https://bazel.build/rules/lib/globals/bzl
"exec_transition",
"module_extension",
"repository_rule",
"tag_class",
// Marked as not documented on https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/packages/BuildGlobals.java
"licenses",
"environment_group",
// Removed in https://github.com/bazelbuild/bazel/commit/5ade9da5de25bc93d0ec79faea8f08a54e5b9a68
"distribs",
];

static HTML_CONVERTER: LazyLock<htmd::HtmlToMarkdown> = LazyLock::new(|| {
HtmlToMarkdown::builder()
.add_handler(vec!["pre"], |element: Element| {
Expand Down Expand Up @@ -123,19 +84,25 @@ pub fn rule_to_doc_member(rule: &RuleDefinition) -> DocMember {
})
}

pub fn api_context_applicable_for_file_type(
api_context: ApiContext,
file_type: FileType) -> bool {
match api_context {
ApiContext::All => true,
ApiContext::Bzl => file_type == FileType::Library,
ApiContext::Build => file_type == FileType::Build,
ApiContext::Module => file_type == FileType::Module,
ApiContext::Repo => file_type == FileType::Repo,
ApiContext::Vendor => file_type == FileType::Vendor,
ApiContext::Workspace => file_type == FileType::Workspace,
}
}

pub fn builtins_to_doc_members<'a>(
builtins: &'a Builtins,
file_type: FileType,
globals: &'a Vec<Value>,
) -> impl Iterator<Item = (String, DocMember)> + 'a {
builtins.global.iter().flat_map(move |global| {
if global.api_context == ApiContext::All as i32
|| (global.api_context == ApiContext::Bzl as i32 && file_type == FileType::Library)
|| (global.api_context == ApiContext::Build as i32 && file_type == FileType::Build)
{
Some((global.name.clone(), value_to_doc_member(global)))
} else {
None
}
globals.iter().flat_map(move |global| {
Some((global.name.clone(), value_to_doc_member(global)))
})
}

Expand Down
22 changes: 17 additions & 5 deletions src/file_type.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@ use starlark_lsp::server::LspUrl;
pub enum FileType {
Build,
Library,
Module,
Repo,
Vendor,
Workspace,
Unknown,
}

impl FileType {
pub const BUILD_FILE_NAMES: [&'static str; 2] = ["BUILD", "BUILD.bazel"];
const LIBRARY_EXTENSIONS: [&'static str; 1] = ["bzl"];

pub fn from_lsp_url(url: &LspUrl) -> Self {
if let LspUrl::File(path) = url {
Expand All @@ -23,14 +26,23 @@ impl FileType {

pub fn from_path<P: AsRef<Path>>(path: P) -> Self {
if let Some(file_name) = path.as_ref().file_name() {
if Self::BUILD_FILE_NAMES.iter().any(|name| *name == file_name) {
return Self::Build;
match file_name.to_string_lossy().as_ref() {
"BUILD" | "BUILD.bazel" => return Self::Build,
"MODULE.bazel" => return Self::Module,
"REPO.bazel" => return Self::Repo,
"VENDOR.bazel" => return Self::Vendor,
"WORKSPACE" | "WORKSPACE.bazel" => return Self::Workspace,
_ => (),
}
}

if let Some(extension) = path.as_ref().extension() {
if Self::LIBRARY_EXTENSIONS.iter().any(|ext| *ext == extension) {
return Self::Library;
match extension.to_string_lossy().as_ref() {
"bzl" => return Self::Library,
// It's common for repos to contain files like ext_dep.BUILD, those files are used
// as BUILD files for external repositories.
"BUILD" => return Self::Build,
_ => (),
}
}

Expand Down

0 comments on commit 4c0ac86

Please sign in to comment.