From 9de1f2ff2e9f10fc2d2d8e45234c4ebc6ac4cc78 Mon Sep 17 00:00:00 2001 From: Grzegorz Lukasik Date: Mon, 2 Dec 2024 15:03:32 +0100 Subject: [PATCH 1/3] chore: remove get_registered_starlark_docs This follows change from https://github.com/facebook/starlark-rust/commit/9a7b00eecd723c026fb8dc4ffdc9a5729c6ce384 which removed it in starlark_bin Basically get_registered_starlark_docs returned only some random symbols built in into the binary itself, namely: list, StackFrame, dict, string Needed to upgrade starlark-rust, will follow-up with changes needed by Rewrite of DocParams in https://github.com/facebook/starlark-rust/commit/723a272cafb3d0cb1a844906625f790978f125e3 --- src/bazel.rs | 34 ++-------------------------------- 1 file changed, 2 insertions(+), 32 deletions(-) diff --git a/src/bazel.rs b/src/bazel.rs index 6812e69..069c5a6 100644 --- a/src/bazel.rs +++ b/src/bazel.rs @@ -36,9 +36,6 @@ use prost::Message; use starlark::analysis::find_call_name::AstModuleFindCallName; use starlark::analysis::AstModuleLint; use starlark::collections::SmallMap; -use starlark::docs::get_registered_starlark_docs; -use starlark::docs::render_docs_as_code; -use starlark::docs::Doc; use starlark::docs::DocItem; use starlark::docs::DocModule; use starlark::errors::EvalMessage; @@ -132,8 +129,6 @@ struct FilesystemCompletionOptions { pub(crate) struct BazelContext { workspaces: RefCell>>, query_output_base: Option, - pub(crate) builtin_docs: HashMap, - pub(crate) builtin_symbols: HashMap, pub(crate) client: Client, } @@ -150,38 +145,13 @@ fn is_workspace_file(uri: &LspUrl) -> bool { impl BazelContext { pub(crate) fn new(client: Client, query_output_base: Option) -> anyhow::Result { - let mut builtins: HashMap> = HashMap::new(); - let mut builtin_symbols: HashMap = HashMap::new(); - for doc in get_registered_starlark_docs() { - let uri = Self::url_for_doc(&doc); - builtin_symbols.insert(doc.id.name.clone(), uri.clone()); - builtins.entry(uri).or_default().push(doc); - } - let builtin_docs = builtins - .into_iter() - .map(|(u, ds)| (u, render_docs_as_code(&ds))) - .collect(); - Ok(Self { workspaces: RefCell::new(HashMap::new()), query_output_base, - builtin_docs, - builtin_symbols, client, }) } - fn url_for_doc(doc: &Doc) -> LspUrl { - let url = match &doc.item { - DocItem::Module(_) => Url::parse("starlark:/native/builtins.bzl").unwrap(), - DocItem::Type(_) => { - Url::parse(&format!("starlark:/native/builtins/{}.bzl", doc.id.name)).unwrap() - } - DocItem::Member(_) => Url::parse("starlark:/native/builtins.bzl").unwrap(), - }; - LspUrl::try_from(url).unwrap() - } - fn lint_module(&self, uri: &LspUrl, ast: &AstModule) -> Vec { let globals = self.get_bazel_globals_names(uri); @@ -676,7 +646,7 @@ impl LspContext for BazelContext { }, false => Err(ContextError::NotAbsolute(uri.clone()).into()), }, - LspUrl::Starlark(_) => Ok(self.builtin_docs.get(uri).cloned()), + LspUrl::Starlark(_) => Ok(None), _ => Err(ContextError::WrongScheme("file://".to_owned(), uri.clone()).into()), } } @@ -690,7 +660,7 @@ impl LspContext for BazelContext { _current_file: &LspUrl, symbol: &str, ) -> anyhow::Result> { - Ok(self.builtin_symbols.get(symbol).cloned()) + Ok(None) } fn get_string_completion_options( From f7e34916b764d34e426b485751644a16b7021b66 Mon Sep 17 00:00:00 2001 From: Grzegorz Lukasik Date: Tue, 3 Dec 2024 17:38:37 +0100 Subject: [PATCH 2/3] chore: cargo update starlark starlark_lsp starlark_syntax --- Cargo.lock | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5004ae9..06843b8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -51,7 +51,7 @@ dependencies = [ [[package]] name = "allocative" version = "0.3.3" -source = "git+https://github.com/facebook/starlark-rust.git?branch=main#abaceca2b5b43c7cc3152182d7673e64130cde36" +source = "git+https://github.com/facebook/starlark-rust.git?branch=main#760df74a5af3e2ebb903949f74071df9dae62b95" dependencies = [ "allocative_derive", "bumpalo", @@ -63,7 +63,7 @@ dependencies = [ [[package]] name = "allocative_derive" version = "0.3.3" -source = "git+https://github.com/facebook/starlark-rust.git?branch=main#abaceca2b5b43c7cc3152182d7673e64130cde36" +source = "git+https://github.com/facebook/starlark-rust.git?branch=main#760df74a5af3e2ebb903949f74071df9dae62b95" dependencies = [ "proc-macro2", "quote", @@ -401,7 +401,7 @@ dependencies = [ [[package]] name = "cmp_any" version = "0.8.1" -source = "git+https://github.com/facebook/starlark-rust.git?branch=main#abaceca2b5b43c7cc3152182d7673e64130cde36" +source = "git+https://github.com/facebook/starlark-rust.git?branch=main#760df74a5af3e2ebb903949f74071df9dae62b95" [[package]] name = "colorchoice" @@ -523,7 +523,7 @@ dependencies = [ [[package]] name = "display_container" version = "0.9.0" -source = "git+https://github.com/facebook/starlark-rust.git?branch=main#abaceca2b5b43c7cc3152182d7673e64130cde36" +source = "git+https://github.com/facebook/starlark-rust.git?branch=main#760df74a5af3e2ebb903949f74071df9dae62b95" dependencies = [ "either", "indenter", @@ -532,7 +532,7 @@ dependencies = [ [[package]] name = "dupe" version = "0.9.0" -source = "git+https://github.com/facebook/starlark-rust.git?branch=main#abaceca2b5b43c7cc3152182d7673e64130cde36" +source = "git+https://github.com/facebook/starlark-rust.git?branch=main#760df74a5af3e2ebb903949f74071df9dae62b95" dependencies = [ "dupe_derive", ] @@ -540,7 +540,7 @@ dependencies = [ [[package]] name = "dupe_derive" version = "0.9.0" -source = "git+https://github.com/facebook/starlark-rust.git?branch=main#abaceca2b5b43c7cc3152182d7673e64130cde36" +source = "git+https://github.com/facebook/starlark-rust.git?branch=main#760df74a5af3e2ebb903949f74071df9dae62b95" dependencies = [ "proc-macro2", "quote", @@ -911,6 +911,15 @@ dependencies = [ "either", ] +[[package]] +name = "itertools" +version = "0.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "413ee7dfc52ee1a4949ceeb7dbc8a33f2d6c088194d9f922fb8318faf1f01186" +dependencies = [ + "either", +] + [[package]] name = "itoa" version = "1.0.11" @@ -1741,7 +1750,7 @@ checksum = "6980e8d7511241f8acf4aebddbb1ff938df5eebe98691418c4468d0b72a96a67" [[package]] name = "starlark" version = "0.12.0" -source = "git+https://github.com/facebook/starlark-rust.git?branch=main#abaceca2b5b43c7cc3152182d7673e64130cde36" +source = "git+https://github.com/facebook/starlark-rust.git?branch=main#760df74a5af3e2ebb903949f74071df9dae62b95" dependencies = [ "allocative", "anyhow", @@ -1756,7 +1765,7 @@ dependencies = [ "erased-serde", "hashbrown 0.14.3", "inventory", - "itertools 0.10.5", + "itertools 0.13.0", "maplit", "memoffset", "num-bigint", @@ -1780,7 +1789,7 @@ dependencies = [ [[package]] name = "starlark_derive" version = "0.12.0" -source = "git+https://github.com/facebook/starlark-rust.git?branch=main#abaceca2b5b43c7cc3152182d7673e64130cde36" +source = "git+https://github.com/facebook/starlark-rust.git?branch=main#760df74a5af3e2ebb903949f74071df9dae62b95" dependencies = [ "dupe", "proc-macro2", @@ -1791,13 +1800,13 @@ dependencies = [ [[package]] name = "starlark_lsp" version = "0.12.0" -source = "git+https://github.com/facebook/starlark-rust.git?branch=main#abaceca2b5b43c7cc3152182d7673e64130cde36" +source = "git+https://github.com/facebook/starlark-rust.git?branch=main#760df74a5af3e2ebb903949f74071df9dae62b95" dependencies = [ "anyhow", "derivative", "derive_more", "dupe", - "itertools 0.10.5", + "itertools 0.13.0", "lsp-server", "lsp-types", "serde", @@ -1810,7 +1819,7 @@ dependencies = [ [[package]] name = "starlark_map" version = "0.12.0" -source = "git+https://github.com/facebook/starlark-rust.git?branch=main#abaceca2b5b43c7cc3152182d7673e64130cde36" +source = "git+https://github.com/facebook/starlark-rust.git?branch=main#760df74a5af3e2ebb903949f74071df9dae62b95" dependencies = [ "allocative", "dupe", @@ -1823,7 +1832,7 @@ dependencies = [ [[package]] name = "starlark_syntax" version = "0.12.0" -source = "git+https://github.com/facebook/starlark-rust.git?branch=main#abaceca2b5b43c7cc3152182d7673e64130cde36" +source = "git+https://github.com/facebook/starlark-rust.git?branch=main#760df74a5af3e2ebb903949f74071df9dae62b95" dependencies = [ "allocative", "annotate-snippets", From 1e888a82839d7ae663e92f993d17343eaadc8bb3 Mon Sep 17 00:00:00 2001 From: Grzegorz Lukasik Date: Tue, 3 Dec 2024 17:41:01 +0100 Subject: [PATCH 3/3] chore: adapt to starlark-rust rewrite DocParams Rewrite happened in https://github.com/facebook/starlark-rust/commit/723a272cafb3d0cb1a844906625f790978f125e3 --- src/bazel.rs | 62 +++++++++++++++++++++++++---------- src/builtin.rs | 89 +++++++++++++++++++++++++++++++------------------- 2 files changed, 101 insertions(+), 50 deletions(-) diff --git a/src/bazel.rs b/src/bazel.rs index 069c5a6..4b6f913 100644 --- a/src/bazel.rs +++ b/src/bazel.rs @@ -658,7 +658,7 @@ impl LspContext for BazelContext { fn get_url_for_global_symbol( &self, _current_file: &LspUrl, - symbol: &str, + _symbol: &str, ) -> anyhow::Result> { Ok(None) } @@ -782,7 +782,7 @@ mod tests { use lsp_types::CompletionItemKind; use serde_json::json; use starlark::{ - docs::{DocItem, DocMember, DocModule, DocParam, DocString}, + docs::{DocFunction, DocItem, DocMember, DocModule, DocParam, DocString}, typing::Ty, }; use starlark_lsp::{ @@ -1110,21 +1110,23 @@ mod tests { let module = context.get_environment(&LspUrl::File(PathBuf::from("/foo/bar/BUILD"))); - let (_, glob_member) = module - .members - .iter() - .find(|(member, _)| *member == "glob") - .unwrap(); + fn doc_function<'a>(module: &'a DocModule, func_name: &str) -> &'a DocFunction { + let (_, glob_member) = module + .members + .iter() + .find(|(member, _)| *member == func_name) + .unwrap(); - let f = match glob_member { - DocItem::Member(DocMember::Function(f)) => f, - _ => panic!(), - }; + match glob_member { + DocItem::Member(DocMember::Function(f)) => f, + _ => panic!(), + } + } assert_eq!( - *f.params, + doc_function(&module, "glob").params.pos_or_named, vec![ - DocParam::Arg { + DocParam { name: "include".into(), default_value: Some("[]".into()), docs: Some(DocString { @@ -1133,7 +1135,7 @@ mod tests { }), typ: Ty::any(), }, - DocParam::Arg { + DocParam { name: "exclude".into(), default_value: Some("[]".into()), docs: Some(DocString { @@ -1142,7 +1144,7 @@ mod tests { }), typ: Ty::any(), }, - DocParam::Arg { + DocParam { name: "exclude_directories".into(), default_value: Some("1".into()), docs: Some(DocString { @@ -1151,7 +1153,7 @@ mod tests { }), typ: Ty::any(), }, - DocParam::Arg { + DocParam { name: "allow_empty".into(), // TODO: Fix this default_value: Some("unbound".into()), @@ -1164,6 +1166,32 @@ mod tests { ] ); + assert_eq!( + doc_function(&module, "max").params.args, + Some(DocParam { + name: "args".into(), + default_value: None, + docs: Some(DocString { + summary: "The elements to be checked.".into(), + details: None, + }), + typ: Ty::any(), + }) + ); + + assert_eq!( + doc_function(&module, "dict").params.kwargs, + Some(DocParam { + name: "kwargs".into(), + default_value: None, + docs: Some(DocString { + summary: "Dictionary of additional entries.".into(), + details: None, + }), + typ: Ty::any(), + }) + ); + Ok(()) } @@ -1201,7 +1229,7 @@ mod tests { match member { DocMember::Function(function) => { validate_doc_string(function.docs.as_ref()); - for param in &function.params { + for param in &function.params.pos_or_named { validate_doc_string(param.get_doc_string()); } } diff --git a/src/builtin.rs b/src/builtin.rs index b493b68..bcb04b3 100644 --- a/src/builtin.rs +++ b/src/builtin.rs @@ -1,7 +1,7 @@ pub use build_proto::blaze_query::*; pub use builtin_proto::builtin::*; use starlark::{ - docs::{DocFunction, DocMember, DocParam, DocProperty, DocString}, + docs::{DocFunction, DocMember, DocParam, DocParams, DocProperty, DocString}, typing::Ty, }; @@ -9,13 +9,12 @@ 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] = &[ +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", @@ -32,17 +31,14 @@ pub static MISSING_GLOBALS: &'static[&'static str] = &[ "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", @@ -65,19 +61,22 @@ pub fn rule_to_doc_member(rule: &RuleDefinition) -> DocMember { .documentation .as_ref() .and_then(|doc| create_docstring(doc)), - params: rule - .attribute - .iter() - .map(|attribute| DocParam::Arg { - name: attribute.name.clone(), - docs: attribute - .documentation - .as_ref() - .and_then(|doc| create_docstring(doc)), - typ: Ty::any(), - default_value: None, - }) - .collect(), + params: DocParams { + named_only: rule + .attribute + .iter() + .map(|attribute| DocParam { + name: attribute.name.clone(), + docs: attribute + .documentation + .as_ref() + .and_then(|doc| create_docstring(doc)), + typ: Ty::any(), + default_value: None, + }) + .collect(), + ..Default::default() + }, ..Default::default() }) } @@ -102,22 +101,46 @@ fn value_to_doc_member(value: &Value) -> DocMember { let docs = create_docstring(&value.doc); if let Some(callable) = &value.callable { + let mut params = DocParams { + ..Default::default() + }; + + for param in callable.param.iter() { + let name = if param.is_star_arg { + param.name.strip_prefix('*').unwrap_or(¶m.name) + } else if param.is_star_star_arg { + param.name.strip_prefix("**").unwrap_or(¶m.name) + } else { + ¶m.name + } + .to_string(); + + let doc_param = DocParam { + name, + docs: create_docstring(¶m.doc), + typ: Ty::any(), + default_value: if param.is_mandatory { + None + } else { + Some(param.default_value.clone()) + }, + }; + + if param.is_star_arg { + params.args = Some(doc_param); + } else if param.is_star_star_arg { + params.kwargs = Some(doc_param); + } else { + // Most of bazel builtins have positional only parameters, but this information is + // not available in proto, to err on safe side adding all params to positional or + // named. + params.pos_or_named.push(doc_param); + } + } + DocMember::Function(DocFunction { docs, - params: callable - .param - .iter() - .map(|param| DocParam::Arg { - name: param.name.clone(), - docs: create_docstring(¶m.doc), - typ: Ty::any(), - default_value: if param.is_mandatory { - None - } else { - Some(param.default_value.clone()) - }, - }) - .collect(), + params, ..Default::default() }) } else {