diff --git a/starlark/src/docs/markdown.rs b/starlark/src/docs/markdown.rs index b9d2d4bd8..ac13cdaf5 100644 --- a/starlark/src/docs/markdown.rs +++ b/starlark/src/docs/markdown.rs @@ -343,10 +343,10 @@ impl<'a> RenderMarkdown for TypeRenderer<'a> { DocParam::NoArgs => "*".to_owned(), DocParam::OnlyPosBefore => "/".to_owned(), DocParam::Args { typ, name, .. } => { - format!("{}{}", name, raw_type_prefix(": ", typ)) + format!("*{}{}", name, raw_type_prefix(": ", typ)) } DocParam::Kwargs { typ, name, .. } => { - format!("{}{}", name, raw_type_prefix(": ", typ)) + format!("**{}{}", name, raw_type_prefix(": ", typ)) } }); diff --git a/starlark/src/docs/mod.rs b/starlark/src/docs/mod.rs index 2c52c580d..b6cd47eab 100644 --- a/starlark/src/docs/mod.rs +++ b/starlark/src/docs/mod.rs @@ -509,14 +509,21 @@ impl DocFunction { Some(args) => { let entries = Self::parse_params(kind, args); for x in &mut params { - match x { - DocParam::Arg { name, docs, .. } - | DocParam::Args { name, docs, .. } - | DocParam::Kwargs { name, docs, .. } => match entries.get(name) { - Some(raw) => *docs = DocString::from_docstring(kind, raw), - _ => (), - }, - _ => (), + if let Some((docs, raw)) = match x { + DocParam::Arg { name, docs, .. } => { + entries.get(name).map(|raw| (docs, raw)) + } + DocParam::Args { name, docs, .. } => entries + .get(name) + .or_else(|| entries.get(&format!("*{}", name))) + .map(|raw| (docs, raw)), + DocParam::Kwargs { name, docs, .. } => entries + .get(name) + .or_else(|| entries.get(&format!("**{}", name))) + .map(|raw| (docs, raw)), + _ => None, + } { + *docs = DocString::from_docstring(kind, raw); } } } @@ -647,7 +654,7 @@ impl DocParam { Some(indented) } - fn render_as_code(&self) -> String { + pub fn render_as_code(&self) -> String { match self { DocParam::Arg { name, @@ -662,9 +669,13 @@ impl DocParam { }, DocParam::NoArgs => "*".to_owned(), DocParam::OnlyPosBefore => "/".to_owned(), - DocParam::Args { name, typ, .. } | DocParam::Kwargs { name, typ, .. } => match typ { - t if t.is_any() => name.clone(), - typ => format!("{}: {}", name, typ), + DocParam::Args { name, typ, .. } => match typ { + t if t.is_any() => format!("*{}", name), + typ => format!("*{}: {}", name, typ), + }, + DocParam::Kwargs { name, typ, .. } => match typ { + t if t.is_any() => format!("**{}", name), + typ => format!("**{}: {}", name, typ), }, } } diff --git a/starlark_bin/bin/bazel.rs b/starlark_bin/bin/bazel.rs index 07610e800..335391655 100644 --- a/starlark_bin/bin/bazel.rs +++ b/starlark_bin/bin/bazel.rs @@ -141,6 +141,7 @@ pub(crate) fn main( prelude: &[PathBuf], dialect: Dialect, globals: Globals, + eager: bool, ) -> anyhow::Result<()> { if !lsp { return Err(anyhow::anyhow!("Bazel mode only supports `--lsp`")); @@ -154,6 +155,7 @@ pub(crate) fn main( is_interactive, dialect, globals, + eager, )?; ctx.mode = ContextMode::Check; @@ -173,6 +175,7 @@ pub(crate) struct BazelContext { pub(crate) globals: Globals, pub(crate) builtin_docs: HashMap, pub(crate) builtin_symbols: HashMap, + pub(crate) eager: bool, } impl BazelContext { @@ -187,6 +190,7 @@ impl BazelContext { module: bool, dialect: Dialect, globals: Globals, + eager: bool, ) -> anyhow::Result { let prelude: Vec<_> = prelude .iter() @@ -263,6 +267,7 @@ impl BazelContext { }), external_output_base: output_base .map(|output_base| PathBuf::from(output_base).join("external")), + eager, }) } @@ -886,4 +891,8 @@ impl LspContext for BazelContext { Ok(names) } + + fn is_eager(&self) -> bool { + self.eager + } } diff --git a/starlark_bin/bin/eval.rs b/starlark_bin/bin/eval.rs index a4028046f..963b635dd 100644 --- a/starlark_bin/bin/eval.rs +++ b/starlark_bin/bin/eval.rs @@ -70,6 +70,7 @@ pub(crate) struct Context { pub(crate) globals: Globals, pub(crate) builtin_docs: HashMap, pub(crate) builtin_symbols: HashMap, + pub(crate) eager: bool, } /// The outcome of evaluating (checking, parsing or running) given starlark code. @@ -101,6 +102,7 @@ impl Context { module: bool, dialect: Dialect, globals: Globals, + eager: bool, ) -> anyhow::Result { let prelude: Vec<_> = prelude .iter() @@ -143,6 +145,7 @@ impl Context { globals, builtin_docs, builtin_symbols, + eager, }) } @@ -379,4 +382,8 @@ impl LspContext for Context { fn get_environment(&self, _uri: &LspUrl) -> DocModule { DocModule::default() } + + fn is_eager(&self) -> bool { + self.eager + } } diff --git a/starlark_bin/bin/main.rs b/starlark_bin/bin/main.rs index ca35d06a4..5e4243024 100644 --- a/starlark_bin/bin/main.rs +++ b/starlark_bin/bin/main.rs @@ -142,6 +142,9 @@ struct Args { )] files: Vec, + #[arg(long = "eager", help = "In LSP mode, eagerly load all files.")] + eager: bool, + #[arg( long = "bazel", help = "Run in Bazel mode (temporary, will be removed)" @@ -298,6 +301,7 @@ fn main() -> anyhow::Result<()> { &prelude, dialect, globals, + args.eager, )?; return Ok(()); } @@ -313,6 +317,7 @@ fn main() -> anyhow::Result<()> { is_interactive, dialect, globals, + args.eager, )?; if args.lsp { diff --git a/starlark_lsp/src/bind.rs b/starlark_lsp/src/bind.rs index f25690e25..56bd141b2 100644 --- a/starlark_lsp/src/bind.rs +++ b/starlark_lsp/src/bind.rs @@ -20,6 +20,8 @@ use std::collections::HashMap; use starlark::codemap::Pos; use starlark::codemap::Span; use starlark::syntax::AstModule; +use starlark_syntax::syntax::ast::Argument; +use starlark_syntax::syntax::ast::ArgumentP; use starlark_syntax::syntax::ast::AssignIdentP; use starlark_syntax::syntax::ast::AssignP; use starlark_syntax::syntax::ast::AstAssignIdent; @@ -33,6 +35,7 @@ use starlark_syntax::syntax::ast::AstTypeExpr; use starlark_syntax::syntax::ast::Clause; use starlark_syntax::syntax::ast::DefP; use starlark_syntax::syntax::ast::Expr; +use starlark_syntax::syntax::ast::ExprP; use starlark_syntax::syntax::ast::ForClause; use starlark_syntax::syntax::ast::ForP; use starlark_syntax::syntax::ast::IdentP; @@ -43,21 +46,35 @@ use starlark_syntax::syntax::module::AstModuleFields; #[derive(Debug, Clone, Eq, PartialEq)] pub(crate) enum Assigner { /// Obtained from `load`. `name` is the symbol in that file, not necessarily the local name - Load { - path: AstString, - name: AstString, - }, - Argument, // From a function argument - Assign, // From an assignment + Load { path: AstString, name: AstString }, + /// From a function call argument + Argument, + /// From an assignment + Assign, } #[derive(Debug)] pub(crate) enum Bind { - Set(Assigner, AstAssignIdent), // Variable assigned to directly - Get(AstIdent), // Variable that is referenced - GetDotted(GetDotted), // Variable is referenced, but is part of a dotted access - Flow, // Flow control occurs here (if, for etc) - can arrive or leave at this point - Scope(Scope), // Entering a new scope (lambda/def/comprehension) + /// Variable assigned to directly + Set(Assigner, AstAssignIdent), + /// Variable that is referenced + Get(AstIdent), + /// Variable is referenced, but is part of a dotted access + GetDotted(GetDotted), + /// An indirect reference, i.e. a named argument in a function call + IndirectReference(IndirectReference), + /// Flow control occurs here (if, for etc) - can arrive or leave at this point + Flow, + /// Entering a new scope (lambda/def/comprehension) + Scope(Scope), +} + +#[derive(Debug)] +pub(crate) struct IndirectReference { + pub(crate) argument_name: AstString, + // TODO: This could also be a dotted access, another function call, etc. These kinds of + // references are not captured at the moment. + pub(crate) function: AstIdent, } /// A 'get' bind that was part of a dotted member access pattern. @@ -123,7 +140,7 @@ impl Scope { Bind::Scope(scope) => scope.free.iter().for_each(|(k, v)| { free.entry(k.clone()).or_insert(*v); }), - Bind::Flow => {} + Bind::IndirectReference(_) | Bind::Flow => {} } } for x in bound.keys() { @@ -183,10 +200,19 @@ fn dot_access<'a>(lhs: &'a AstExpr, attribute: &'a AstString, res: &mut Vec { - f(name, attributes, res); - // make sure that if someone does a(b).c, 'b' is bound and considered used. + Expr::Call(func_name, parameters) => { + f(func_name, attributes, res); for parameter in parameters { + if let ExprP::Identifier(func_name) = &func_name.node { + if let ArgumentP::Named(arg_name, _) = ¶meter.node { + res.push(Bind::IndirectReference(IndirectReference { + argument_name: arg_name.clone(), + function: func_name.clone(), + })) + } + } + + // make sure that if someone does a(b).c, 'b' is bound and considered used. expr(parameter.expr(), res); } } @@ -221,6 +247,24 @@ fn expr(x: &AstExpr, res: &mut Vec) { expr(&x.0, res); expr(&x.1, res) }), + Expr::Call(func, args) => { + expr(func, res); + for x in args { + if let ExprP::Identifier(function_ident) = &func.node { + match &**x { + Argument::Named(name, value) => { + res.push(Bind::IndirectReference(IndirectReference { + argument_name: name.clone(), + function: function_ident.clone(), + })); + expr(value, res); + } + _ => expr(x.expr(), res), + } + } + expr(x.expr(), res) + } + } // Uninteresting - just recurse _ => x.visit_expr(|x| expr(x, res)), diff --git a/starlark_lsp/src/completion.rs b/starlark_lsp/src/completion.rs index da2e0fc16..5acc69fda 100644 --- a/starlark_lsp/src/completion.rs +++ b/starlark_lsp/src/completion.rs @@ -31,7 +31,6 @@ use lsp_types::TextEdit; use starlark::codemap::ResolvedSpan; use starlark::docs::markdown::render_doc_item; use starlark::docs::markdown::render_doc_param; -use starlark::docs::DocItem; use starlark::docs::DocMember; use starlark::docs::DocParam; use starlark_syntax::codemap::ResolvedPos; @@ -42,7 +41,6 @@ use crate::definition::Definition; use crate::definition::DottedDefinition; use crate::definition::IdentifierDefinition; use crate::definition::LspModule; -use crate::exported::SymbolKind as ExportedSymbolKind; use crate::server::Backend; use crate::server::LspContext; use crate::server::LspUrl; @@ -96,17 +94,14 @@ impl Backend { ( key, CompletionItem { - kind: Some(match value.kind { - SymbolKind::Method => CompletionItemKind::METHOD, - SymbolKind::Variable => CompletionItemKind::VARIABLE, - }), + kind: Some(value.kind.into()), detail: value.detail, documentation: value .doc .map(|doc| { Documentation::MarkupContent(MarkupContent { kind: MarkupKind::Markdown, - value: render_doc_item(&value.name, &doc), + value: render_doc_item(&value.name, &doc.to_doc_item()), }) }) .or_else(|| { @@ -258,11 +253,11 @@ impl Backend { ) .remove(name) .and_then(|symbol| match symbol.kind { - SymbolKind::Method => symbol.doc, - SymbolKind::Variable => None, + SymbolKind::Method { .. } => symbol.doc, + SymbolKind::Constant | SymbolKind::Variable => None, }) .and_then(|docs| match docs { - DocItem::Function(doc_function) => Some( + DocMember::Function(doc_function) => Some( doc_function .params .into_iter() @@ -286,12 +281,12 @@ impl Backend { self.get_ast_or_load_from_disk(&load_uri)? .and_then(|ast| ast.find_exported_symbol(name)) .and_then(|symbol| match symbol.kind { - ExportedSymbolKind::Any => None, - ExportedSymbolKind::Function { argument_names } => Some( - argument_names + SymbolKind::Constant | SymbolKind::Variable => None, + SymbolKind::Method { arguments } => Some( + arguments .into_iter() - .map(|name| CompletionItem { - label: name, + .map(|arg| CompletionItem { + label: arg.name, kind: Some(CompletionItemKind::PROPERTY), ..Default::default() }) @@ -330,6 +325,8 @@ impl Backend { } // None of these can be functions, so can't have any parameters. IdentifierDefinition::LoadPath { .. } + | IdentifierDefinition::LocationWithParameterReference { .. } + | IdentifierDefinition::LoadedLocationWithParameterReference { .. } | IdentifierDefinition::StringLiteral { .. } | IdentifierDefinition::NotFound => None, }) diff --git a/starlark_lsp/src/definition.rs b/starlark_lsp/src/definition.rs index 1d19c3aba..875967200 100644 --- a/starlark_lsp/src/definition.rs +++ b/starlark_lsp/src/definition.rs @@ -42,13 +42,14 @@ use starlark_syntax::syntax::uniplate::Visit; use crate::bind::scope; use crate::bind::Assigner; use crate::bind::Bind; +use crate::bind::IndirectReference; use crate::bind::Scope; use crate::exported::AstModuleExportedSymbols; -use crate::exported::Symbol; use crate::loaded::AstModuleLoadedSymbols; use crate::loaded::LoadedSymbol; +use crate::symbols::Symbol; -/// The location of a definition for a given identifier. See [`AstModule::find_definition_at_location`]. +/// The location of a definition for a given identifier. See [`LspModule::find_definition_at_location`]. #[derive(Debug, Clone, Eq, PartialEq)] pub(crate) enum IdentifierDefinition { /// The definition was found at this location in the current file. @@ -57,6 +58,12 @@ pub(crate) enum IdentifierDefinition { destination: ResolvedSpan, name: String, }, + LocationWithParameterReference { + source: ResolvedSpan, + destination: ResolvedSpan, + name: String, + parameter: String, + }, /// The symbol was loaded from another file. "destination" is the position within the /// "load()" statement, but additionally, the path in that load statement, and the /// name of the symbol within that file are provided so that additional lookups can @@ -67,6 +74,13 @@ pub(crate) enum IdentifierDefinition { path: String, name: String, }, + LoadedLocationWithParameterReference { + source: ResolvedSpan, + destination: ResolvedSpan, + path: String, + name: String, + parameter: String, + }, /// The symbol is the path component of a `load` statement. This is the raw string /// that is in the AST, and needs to be properly resolved to a path to be useful. LoadPath { source: ResolvedSpan, path: String }, @@ -88,7 +102,9 @@ impl IdentifierDefinition { fn source(&self) -> Option { match self { IdentifierDefinition::Location { source, .. } + | IdentifierDefinition::LocationWithParameterReference { source, .. } | IdentifierDefinition::LoadedLocation { source, .. } + | IdentifierDefinition::LoadedLocationWithParameterReference { source, .. } | IdentifierDefinition::LoadPath { source, .. } | IdentifierDefinition::StringLiteral { source, .. } | IdentifierDefinition::Unresolved { source, .. } => Some(*source), @@ -124,17 +140,27 @@ enum TempIdentifierDefinition<'a> { source: Span, destination: Span, name: &'a str, + /// In the case that the symbol is a parameter to a function, this is the name of the + /// parameter that the symbol is being used as. + parameter_name: Option<&'a str>, }, LoadedLocation { source: Span, destination: Span, path: &'a str, name: &'a str, + /// In the case that the symbol is a parameter to a function, this is the name of the + /// parameter that the symbol is being used as. + parameter_name: Option<&'a str>, }, /// The definition was not found in the current scope but the name of an identifier /// was found at the given position. This should be checked in outer scopes /// to attempt to find the definition. - Name { source: Span, name: &'a str }, + Name { + source: Span, + name: &'a str, + parameter_name: Option<&'a str>, + }, /// None of the accesses matched the position that was provided. NotFound, } @@ -275,13 +301,14 @@ impl LspModule { /// Look at the given scope and child scopes to try to find where the identifier /// accessed at Pos is defined. - fn find_definition_in_scope<'a>(scope: &'a Scope, pos: Pos) -> TempDefinition<'a> { + fn find_definition_in_scope(scope: &Scope, pos: Pos) -> TempDefinition { /// Look for a name in the given scope, with a given source, and return the right /// type of [`TempIdentifierDefinition`] based on whether / how the variable is bound. fn resolve_get_in_scope<'a>( scope: &'a Scope, name: &'a str, source: Span, + parameter_name: Option<&'a str>, ) -> TempIdentifierDefinition<'a> { match scope.bound.get(name) { Some((Assigner::Load { path, name }, span)) => { @@ -290,31 +317,51 @@ impl LspModule { destination: *span, path, name: name.as_str(), + parameter_name, } } Some((_, span)) => TempIdentifierDefinition::Location { source, destination: *span, name, + parameter_name, }, // We know the symbol name, but it might only be available in // an outer scope. - None => TempIdentifierDefinition::Name { source, name }, + None => TempIdentifierDefinition::Name { + source, + name, + parameter_name, + }, } } for bind in &scope.inner { match bind { Bind::Get(g) if g.span.contains(pos) => { - return resolve_get_in_scope(scope, g.node.ident.as_str(), g.span).into(); + return resolve_get_in_scope(scope, g.node.ident.as_str(), g.span, None).into(); + } + Bind::IndirectReference(IndirectReference { + argument_name, + function, + }) if argument_name.span.contains(pos) => { + return resolve_get_in_scope( + scope, + function.node.ident.as_str(), + argument_name.span, + Some(argument_name.node.as_str()), + ) + .into(); } Bind::Scope(inner_scope) => { match Self::find_definition_in_scope(inner_scope, pos) { TempDefinition::Identifier(TempIdentifierDefinition::Name { source, name, + parameter_name, }) => { - return resolve_get_in_scope(scope, name, source).into(); + return resolve_get_in_scope(scope, name, source, parameter_name) + .into(); } TempDefinition::Identifier(TempIdentifierDefinition::NotFound) => {} TempDefinition::Dotted(TempDottedDefinition { @@ -323,6 +370,7 @@ impl LspModule { TempIdentifierDefinition::Name { source: root_source, name: root_name, + parameter_name: None, }, variable, attributes, @@ -333,6 +381,7 @@ impl LspModule { scope, root_name, root_source, + None, ), variable, attributes, @@ -356,6 +405,7 @@ impl LspModule { scope, root_identifier.node.ident.as_str(), root_identifier.span, + None, ); // If someone clicks on the "root" identifier, just treat it as a "get" match idx { @@ -372,15 +422,15 @@ impl LspModule { } } } - // For everything else, just ignore it. Note that the `Get` is ignored - // because we already checked the pos above. - Bind::Set(_, _) | Bind::Flow | Bind::Get(_) => {} + // For everything else, just ignore it. Note that the `Get` and `IndirectReference` + // are ignored because we already checked the pos above. + Bind::Set(_, _) | Bind::Flow | Bind::Get(_) | Bind::IndirectReference(_) => {} } } TempIdentifierDefinition::NotFound.into() } - /// Converts a `TempIdentifierDefinition` to an `IdentifierDefinition`, resolving spans + /// Converts a [`TempIdentifierDefinition`] to an [`IdentifierDefinition`], resolving spans /// against the current AST, owning strings, etc. fn get_definition_location( &self, @@ -393,28 +443,58 @@ impl LspModule { source, destination, name, - } => IdentifierDefinition::Location { - source: self.ast.codemap().resolve_span(source), - destination: self.ast.codemap().resolve_span(destination), - name: name.to_owned(), + parameter_name, + } => match parameter_name { + Some(argument_name) => IdentifierDefinition::LocationWithParameterReference { + source: self.ast.codemap().resolve_span(source), + destination: self.ast.codemap().resolve_span(destination), + name: name.to_owned(), + parameter: argument_name.to_owned(), + }, + None => IdentifierDefinition::Location { + source: self.ast.codemap().resolve_span(source), + destination: self.ast.codemap().resolve_span(destination), + name: name.to_owned(), + }, }, - TempIdentifierDefinition::Name { source, name } => match scope.bound.get(name) { + TempIdentifierDefinition::Name { + source, + name, + parameter_name, + } => match scope.bound.get(name) { None => IdentifierDefinition::Unresolved { source: self.ast.codemap().resolve_span(source), name: name.to_owned(), }, - Some((Assigner::Load { path, name }, span)) => { - IdentifierDefinition::LoadedLocation { + Some((Assigner::Load { path, name }, span)) => match parameter_name { + Some(parameter_name) => { + IdentifierDefinition::LoadedLocationWithParameterReference { + source: self.ast.codemap().resolve_span(source), + destination: self.ast.codemap().resolve_span(*span), + path: path.node.clone(), + name: name.node.clone(), + parameter: parameter_name.to_owned(), + } + } + None => IdentifierDefinition::LoadedLocation { source: self.ast.codemap().resolve_span(source), destination: self.ast.codemap().resolve_span(*span), path: path.node.clone(), name: name.node.clone(), - } - } - Some((_, span)) => IdentifierDefinition::Location { - source: self.ast.codemap().resolve_span(source), - destination: self.ast.codemap().resolve_span(*span), - name: name.to_owned(), + }, + }, + Some((_, span)) => match parameter_name { + Some(parameter_name) => IdentifierDefinition::LocationWithParameterReference { + source: self.ast.codemap().resolve_span(source), + destination: self.ast.codemap().resolve_span(*span), + name: name.to_owned(), + parameter: parameter_name.to_owned(), + }, + None => IdentifierDefinition::Location { + source: self.ast.codemap().resolve_span(source), + destination: self.ast.codemap().resolve_span(*span), + name: name.to_owned(), + }, }, }, // If we could not find the symbol, see if the current position is within @@ -425,11 +505,23 @@ impl LspModule { destination, path, name, - } => IdentifierDefinition::LoadedLocation { - source: self.ast.codemap().resolve_span(source), - destination: self.ast.codemap().resolve_span(destination), - path: path.to_owned(), - name: name.to_owned(), + parameter_name, + } => match parameter_name { + Some(parameter_name) => { + IdentifierDefinition::LoadedLocationWithParameterReference { + source: self.ast.codemap().resolve_span(source), + destination: self.ast.codemap().resolve_span(destination), + path: path.to_owned(), + name: name.to_owned(), + parameter: parameter_name.to_owned(), + } + } + None => IdentifierDefinition::LoadedLocation { + source: self.ast.codemap().resolve_span(source), + destination: self.ast.codemap().resolve_span(destination), + path: path.to_owned(), + name: name.to_owned(), + }, }, } } @@ -455,7 +547,8 @@ impl LspModule { /// Attempt to find the location in this module where an exported symbol is defined. pub(crate) fn find_exported_symbol_span(&self, name: &str) -> Option { self.find_exported_symbol(name) - .map(|symbol| symbol.span.resolve_span()) + .and_then(|symbol| symbol.span) + .map(|span| self.ast.codemap().resolve_span(span)) } /// Attempt to find the location in this module where a member of a struct (named `name`) @@ -619,7 +712,7 @@ pub(crate) mod helpers { use super::*; - /// Result of parsing a starlark fixture that has range markers in it. See `FixtureWithRanges::from_fixture` + /// Result of parsing a starlark fixture that has range markers in it. See [`FixtureWithRanges::from_fixture`] #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct FixtureWithRanges { filename: String, diff --git a/starlark_lsp/src/docs.rs b/starlark_lsp/src/docs.rs index 17d2b1fe2..b3871cb10 100644 --- a/starlark_lsp/src/docs.rs +++ b/starlark_lsp/src/docs.rs @@ -38,15 +38,28 @@ pub(crate) fn get_doc_item_for_def(def: &DefP

) -> Option Some(DocParam::Arg { + ParameterP::Normal(p, _) => Some(DocParam::Arg { name: p.ident.to_owned(), docs: None, typ: Ty::any(), default_value: None, }), + ParameterP::Args(p, _) => Some(DocParam::Args { + name: p.ident.to_owned(), + docs: None, + typ: Ty::any(), + }), + ParameterP::KwArgs(p, _) => Some(DocParam::Kwargs { + name: p.ident.to_owned(), + docs: None, + typ: Ty::any(), + }), + ParameterP::WithDefaultValue(p, _, _) => Some(DocParam::Arg { + name: p.ident.to_owned(), + docs: None, + typ: Ty::any(), + default_value: Some("_".to_owned()), + }), _ => None, }) .collect(); diff --git a/starlark_lsp/src/exported.rs b/starlark_lsp/src/exported.rs index c1eacc5e2..815db39cf 100644 --- a/starlark_lsp/src/exported.rs +++ b/starlark_lsp/src/exported.rs @@ -16,78 +16,30 @@ */ use lsp_types::CompletionItem; -use lsp_types::CompletionItemKind; use lsp_types::Documentation; use lsp_types::MarkupContent; use lsp_types::MarkupKind; -use starlark::codemap::FileSpan; use starlark::collections::SmallMap; use starlark::docs::markdown::render_doc_item; -use starlark::docs::DocItem; +use starlark::docs::DocMember; use starlark::syntax::AstModule; use starlark_syntax::syntax::ast::AstAssignIdent; -use starlark_syntax::syntax::ast::Expr; use starlark_syntax::syntax::ast::Stmt; use starlark_syntax::syntax::module::AstModuleFields; use starlark_syntax::syntax::top_level_stmts::top_level_stmts; use crate::docs::get_doc_item_for_assign; use crate::docs::get_doc_item_for_def; - -/// The type of an exported symbol. -/// If unknown, will use `Any`. -#[derive(Debug, PartialEq, Eq, Clone, Hash)] -pub(crate) enum SymbolKind { - /// Any kind of symbol. - Any, - /// The symbol represents something that can be called, for example - /// a `def` or a variable assigned to a `lambda`. - Function { argument_names: Vec }, -} - -impl SymbolKind { - pub(crate) fn from_expr(x: &Expr) -> Self { - match x { - Expr::Lambda(lambda) => Self::Function { - argument_names: lambda - .params - .iter() - .filter_map(|param| param.split().0.map(|name| name.to_string())) - .collect(), - }, - _ => Self::Any, - } - } -} - -impl From for CompletionItemKind { - fn from(value: SymbolKind) -> Self { - match value { - SymbolKind::Any => CompletionItemKind::CONSTANT, - SymbolKind::Function { .. } => CompletionItemKind::FUNCTION, - } - } -} - -/// A symbol. Returned from [`AstModule::exported_symbols`]. -#[derive(Debug, PartialEq, Clone)] -pub(crate) struct Symbol { - /// The name of the symbol. - pub(crate) name: String, - /// The location of its definition. - pub(crate) span: FileSpan, - /// The type of symbol it represents. - pub(crate) kind: SymbolKind, - /// The documentation for this symbol. - pub(crate) docs: Option, -} +use crate::symbols::MethodSymbolArgument; +use crate::symbols::Symbol; +use crate::symbols::SymbolKind; impl From for CompletionItem { fn from(value: Symbol) -> Self { - let documentation = value.docs.map(|docs| { + let documentation = value.doc.map(|doc| { Documentation::MarkupContent(MarkupContent { kind: MarkupKind::Markdown, - value: render_doc_item(&value.name, &docs), + value: render_doc_item(&value.name, &doc.to_doc_item()), }) }); Self { @@ -112,18 +64,19 @@ impl AstModuleExportedSymbols for AstModule { let mut result: SmallMap<&str, _> = SmallMap::new(); fn add<'a>( - me: &AstModule, result: &mut SmallMap<&'a str, Symbol>, name: &'a AstAssignIdent, kind: SymbolKind, - resolve_docs: impl FnOnce() -> Option, + resolve_docs: impl FnOnce() -> Option, ) { if !name.ident.starts_with('_') { result.entry(&name.ident).or_insert(Symbol { name: name.ident.clone(), - span: me.file_span(name.span), + detail: None, + span: Some(name.span), kind, - docs: resolve_docs(), + doc: resolve_docs(), + param: None, }); } } @@ -134,35 +87,44 @@ impl AstModuleExportedSymbols for AstModule { Stmt::Assign(assign) => { assign.lhs.visit_lvalue(|name| { let kind = SymbolKind::from_expr(&assign.rhs); - add(self, &mut result, name, kind, || { + add(&mut result, name, kind, || { last_node .and_then(|last| get_doc_item_for_assign(last, &assign.lhs)) - .map(DocItem::Property) + .map(DocMember::Property) }); }); } Stmt::AssignModify(dest, _, _) => { dest.visit_lvalue(|name| { - add(self, &mut result, name, SymbolKind::Any, || { + add(&mut result, name, SymbolKind::Variable, || { last_node .and_then(|last| get_doc_item_for_assign(last, dest)) - .map(DocItem::Property) + .map(DocMember::Property) }); }); } Stmt::Def(def) => { + let doc_item = get_doc_item_for_def(def); add( - self, &mut result, &def.name, - SymbolKind::Function { - argument_names: def + SymbolKind::Method { + arguments: def .params .iter() - .filter_map(|param| param.split().0.map(|name| name.to_string())) + .filter_map(|param| { + MethodSymbolArgument::from_ast_parameter( + param, + doc_item.as_ref().and_then(|doc| { + param.node.ident().and_then(|ident| { + doc.find_param_with_name(&ident.ident) + }) + }), + ) + }) .collect(), }, - || get_doc_item_for_def(def).map(DocItem::Function), + || doc_item.map(DocMember::Function), ); } _ => {} @@ -197,7 +159,11 @@ d = 2 ); let res = modu.exported_symbols(); assert_eq!( - res.map(|symbol| format!("{} {}", symbol.span, symbol.name)), + res.map(|symbol| format!( + "{} {}", + modu.file_span(symbol.span.expect("span should be set")), + symbol.name + )), &["X:3:5-6 b", "X:4:1-2 d"] ); } diff --git a/starlark_lsp/src/lib.rs b/starlark_lsp/src/lib.rs index 496a94f93..14b438154 100644 --- a/starlark_lsp/src/lib.rs +++ b/starlark_lsp/src/lib.rs @@ -30,6 +30,7 @@ mod exported; pub(crate) mod inspect; pub(crate) mod loaded; pub mod server; +pub(crate) mod signature; mod symbols; #[cfg(all(test, not(windows)))] mod test; diff --git a/starlark_lsp/src/server.rs b/starlark_lsp/src/server.rs index faac435fd..4e23887f2 100644 --- a/starlark_lsp/src/server.rs +++ b/starlark_lsp/src/server.rs @@ -39,22 +39,31 @@ use lsp_server::Response; use lsp_server::ResponseError; use lsp_types::notification::DidChangeTextDocument; use lsp_types::notification::DidCloseTextDocument; +use lsp_types::notification::DidCreateFiles; +use lsp_types::notification::DidDeleteFiles; use lsp_types::notification::DidOpenTextDocument; +use lsp_types::notification::DidRenameFiles; use lsp_types::notification::LogMessage; use lsp_types::notification::PublishDiagnostics; use lsp_types::request::Completion; +use lsp_types::request::DocumentSymbolRequest; use lsp_types::request::GotoDefinition; use lsp_types::request::HoverRequest; +use lsp_types::request::SignatureHelpRequest; +use lsp_types::request::WorkspaceSymbolRequest; use lsp_types::CompletionItem; use lsp_types::CompletionItemKind; use lsp_types::CompletionOptions; use lsp_types::CompletionParams; use lsp_types::CompletionResponse; use lsp_types::DefinitionOptions; +use lsp_types::DeleteFilesParams; use lsp_types::Diagnostic; use lsp_types::DidChangeTextDocumentParams; use lsp_types::DidCloseTextDocumentParams; use lsp_types::DidOpenTextDocumentParams; +use lsp_types::DocumentSymbolParams; +use lsp_types::DocumentSymbolResponse; use lsp_types::Documentation; use lsp_types::GotoDefinitionParams; use lsp_types::GotoDefinitionResponse; @@ -64,6 +73,7 @@ use lsp_types::HoverParams; use lsp_types::HoverProviderCapability; use lsp_types::InitializeParams; use lsp_types::LanguageString; +use lsp_types::Location; use lsp_types::LocationLink; use lsp_types::LogMessageParams; use lsp_types::MarkedString; @@ -71,16 +81,25 @@ use lsp_types::MarkupContent; use lsp_types::MarkupKind; use lsp_types::MessageType; use lsp_types::OneOf; +use lsp_types::ParameterInformation; +use lsp_types::ParameterLabel; use lsp_types::Position; use lsp_types::PublishDiagnosticsParams; use lsp_types::Range; +use lsp_types::RenameFilesParams; use lsp_types::ServerCapabilities; +use lsp_types::SignatureHelp; +use lsp_types::SignatureHelpOptions; +use lsp_types::SignatureHelpParams; use lsp_types::TextDocumentSyncCapability; use lsp_types::TextDocumentSyncKind; use lsp_types::TextEdit; use lsp_types::Url; use lsp_types::WorkDoneProgressOptions; use lsp_types::WorkspaceFolder; +use lsp_types::WorkspaceSymbol; +use lsp_types::WorkspaceSymbolParams; +use lsp_types::WorkspaceSymbolResponse; use serde::de::DeserializeOwned; use serde::Deserialize; use serde::Deserializer; @@ -93,6 +112,7 @@ use starlark::docs::markdown::render_doc_member; use starlark::docs::markdown::render_doc_param; use starlark::docs::DocMember; use starlark::docs::DocModule; +use starlark::docs::DocParam; use starlark::syntax::AstModule; use starlark_syntax::codemap::ResolvedPos; use starlark_syntax::syntax::ast::AstPayload; @@ -107,7 +127,12 @@ use crate::definition::IdentifierDefinition; use crate::definition::LspModule; use crate::inspect::AstModuleInspect; use crate::inspect::AutocompleteType; +use crate::signature; +use crate::symbols; use crate::symbols::find_symbols_at_location; +use crate::symbols::MethodSymbolArgument; +use crate::symbols::Symbol; +use crate::symbols::SymbolKind; /// The request to get the file contents for a starlark: URI struct StarlarkFileContentsRequest {} @@ -367,6 +392,11 @@ pub trait LspContext { let _unused = (document_uri, kind, current_value, workspace_root); Ok(Vec::new()) } + + /// Should the LSP eagerly load all files in the workspace? Otherwise, only files that are + /// opened will be parsed. This can be useful for large workspaces, but eager loading + /// adds support for workspace symbols, etc. + fn is_eager(&self) -> bool; } /// Errors when [`LspContext::resolve_load()`] cannot resolve a given path. @@ -395,7 +425,7 @@ pub(crate) struct Backend { /// The logic implementations of stuff impl Backend { - fn server_capabilities(settings: LspServerSettings) -> ServerCapabilities { + fn server_capabilities(context: &T, settings: LspServerSettings) -> ServerCapabilities { let definition_provider = settings.enable_goto_definition.then_some({ OneOf::Right(DefinitionOptions { work_done_progress_options: WorkDoneProgressOptions { @@ -408,6 +438,19 @@ impl Backend { definition_provider, completion_provider: Some(CompletionOptions::default()), hover_provider: Some(HoverProviderCapability::Simple(true)), + document_symbol_provider: Some(OneOf::Left(true)), + signature_help_provider: Some(SignatureHelpOptions { + trigger_characters: Some(vec!["(".to_string(), ",".to_string()]), + retrigger_characters: Some(vec![",".to_string()]), + work_done_progress_options: WorkDoneProgressOptions { + work_done_progress: None, + }, + }), + workspace_symbol_provider: if context.is_eager() { + Some(OneOf::Left(true)) + } else { + None + }, ..ServerCapabilities::default() } } @@ -431,9 +474,8 @@ impl Backend { Ok(module) } - fn validate(&self, uri: Url, version: Option, text: String) -> anyhow::Result<()> { - let uri = uri.try_into()?; - let eval_result = self.context.parse_file_with_contents(&uri, text); + fn validate(&self, uri: &LspUrl, version: Option, text: String) -> anyhow::Result<()> { + let eval_result = self.context.parse_file_with_contents(uri, text); if let Some(ast) = eval_result.ast { let module = Arc::new(LspModule::new(ast)); let mut last_valid_parse = self.last_valid_parse.write().unwrap(); @@ -444,8 +486,9 @@ impl Backend { } fn did_open(&self, params: DidOpenTextDocumentParams) -> anyhow::Result<()> { + let uri = params.text_document.uri.try_into()?; self.validate( - params.text_document.uri, + &uri, Some(params.text_document.version as i64), params.text_document.text, ) @@ -454,14 +497,16 @@ impl Backend { fn did_change(&self, params: DidChangeTextDocumentParams) -> anyhow::Result<()> { // We asked for Sync full, so can just grab all the text from params let change = params.content_changes.into_iter().next().unwrap(); - self.validate( - params.text_document.uri, - Some(params.text_document.version as i64), - change.text, - ) + let uri = params.text_document.uri.try_into()?; + self.validate(&uri, Some(params.text_document.version as i64), change.text) } fn did_close(&self, params: DidCloseTextDocumentParams) -> anyhow::Result<()> { + // If in eager mode, don't remove the file from the `last_valid_parse`. + if self.context.is_eager() { + return Ok(()); + } + { let mut last_valid_parse = self.last_valid_parse.write().unwrap(); last_valid_parse.remove(¶ms.text_document.uri.clone().try_into()?); @@ -470,6 +515,32 @@ impl Backend { Ok(()) } + fn did_rename(&self, params: RenameFilesParams) -> anyhow::Result<()> { + let mut last_valid_parse = self.last_valid_parse.write().unwrap(); + for file in params.files { + let old_uri = Url::parse(&file.old_uri).unwrap(); + let old_uri = old_uri.try_into()?; + let new_uri = Url::parse(&file.new_uri).unwrap(); + let new_uri = new_uri.try_into()?; + + if let Some(ast) = last_valid_parse.remove(&old_uri) { + last_valid_parse.insert(new_uri, ast); + } + } + Ok(()) + } + + fn did_delete(&self, params: DeleteFilesParams) -> anyhow::Result<()> { + let mut last_valid_parse = self.last_valid_parse.write().unwrap(); + for file in params.files { + let uri = Url::parse(&file.uri).unwrap(); + let uri = uri.try_into()?; + last_valid_parse.remove(&uri); + self.publish_diagnostics(uri.try_into()?, Vec::new(), None); + } + Ok(()) + } + /// Go to the definition of the symbol at the current cursor if that definition is in /// the same file. /// @@ -506,6 +577,29 @@ impl Backend { self.send_response(new_response(id, self.hover_info(params, initialize_params))); } + fn signature_help( + &self, + id: RequestId, + params: SignatureHelpParams, + initialize_params: &InitializeParams, + ) { + self.send_response(new_response( + id, + self.signature_help_info(params, initialize_params), + )); + } + + /// Offer an overview of symbols in the current document. + fn document_symbols(&self, id: RequestId, params: DocumentSymbolParams) { + self.send_response(new_response(id, self.get_document_symbols(params))); + } + + /// Offer an overview of all symbols in the current workspace. + /// Only supported in eager mode. + fn workspace_symbols(&self, id: RequestId, params: WorkspaceSymbolParams) { + self.send_response(new_response(id, self.get_workspace_symbols(params))); + } + /// Get the file contents of a starlark: URI. fn get_starlark_file_contents(&self, id: RequestId, params: StarlarkFileContentsParams) { let response: anyhow::Result<_> = match params.uri { @@ -557,6 +651,7 @@ impl Backend { definition: IdentifierDefinition, source: ResolvedSpan, member: Option<&str>, + document: &LspModule, uri: &LspUrl, workspace_root: Option<&Path>, ) -> anyhow::Result> { @@ -565,6 +660,41 @@ impl Backend { destination: target, .. } => Self::location_link(source, uri, target)?, + IdentifierDefinition::LocationWithParameterReference { + destination: target, + name, + parameter, + .. + } => { + // TODO: This seems very inefficient. Once the document starts + // holding the `Scope` including AST nodes, this indirection + // should be removed. + let location = find_symbols_at_location( + document.ast.codemap(), + document.ast.statement(), + ResolvedPos { + line: target.begin.line, + column: target.begin.column, + }, + ) + .remove(&name) + .and_then(|symbol| match symbol.kind { + SymbolKind::Method { arguments } => arguments + .iter() + .find(|arg| arg.name == parameter) + .or_else(|| arguments.iter().find(|arg| arg.is_kwargs)) + .and_then(|arg| arg.span), + SymbolKind::Constant | SymbolKind::Variable => symbol.span, + }); + + Self::location_link( + source, + uri, + location + .map(|span| document.ast.codemap().resolve_span(span)) + .unwrap_or(target), + )? + } IdentifierDefinition::LoadedLocation { destination: location, path, @@ -585,6 +715,37 @@ impl Backend { } } } + IdentifierDefinition::LoadedLocationWithParameterReference { + destination: location, + path, + name, + parameter, + .. + } => { + assert!(member.is_none()); + + let load_uri = self.resolve_load_path(&path, uri, workspace_root)?; + let Some(ast) = self.get_ast_or_load_from_disk(&load_uri)? else { + return Self::location_link(source, uri, location); + }; + + let resolved_symbol = ast.find_exported_symbol(&name); + let resolved_location: Option = resolved_symbol + .and_then(|symbol| match symbol.kind { + SymbolKind::Method { arguments } => Some(arguments), + _ => None, + }) + .and_then(|arguments| arguments.into_iter().find(|arg| arg.name == parameter)) + .and_then(|arg| arg.span) + .map(|span| ast.ast.codemap().resolve_span(span)); + + match resolved_location { + None => Self::location_link(source, uri, location)?, + Some(resolved_location) => { + Self::location_link(source, &load_uri, resolved_location)? + } + } + } IdentifierDefinition::NotFound => None, IdentifierDefinition::LoadPath { path, .. } => { match self.resolve_load_path(&path, uri, workspace_root) { @@ -676,6 +837,7 @@ impl Backend { definition, source, None, + &ast, &uri, workspace_root.as_deref(), )?, @@ -705,6 +867,7 @@ impl Backend { .expect("to have at least one component") .as_str(), ), + &ast, &uri, workspace_root.as_deref(), )?, @@ -1058,57 +1221,74 @@ impl Backend { document_uri: &LspUrl, workspace_root: Option<&Path>, ) -> anyhow::Result> { + let symbol = self.get_symbol_for_identifier_definition( + &identifier_definition, + document, + document_uri, + workspace_root, + )?; + Ok(match identifier_definition { - IdentifierDefinition::Location { - destination, - name, - source, - } => { - // TODO: This seems very inefficient. Once the document starts - // holding the `Scope` including AST nodes, this indirection - // should be removed. - find_symbols_at_location( - document.ast.codemap(), - document.ast.statement(), - ResolvedPos { - line: destination.begin.line, - column: destination.begin.column, - }, - ) - .remove(&name) - .and_then(|symbol| { - symbol - .doc - .map(|docs| Hover { + IdentifierDefinition::Location { source, .. } => symbol.and_then(|symbol| { + symbol + .doc + .map(|docs| Hover { + contents: HoverContents::Array(vec![MarkedString::String( + render_doc_item(&symbol.name, &docs.to_doc_item()), + )]), + range: Some(source.into()), + }) + .or_else(|| { + symbol.param.map(|docs| Hover { contents: HoverContents::Array(vec![MarkedString::String( - render_doc_item(&symbol.name, &docs), + render_doc_param(&docs), )]), range: Some(source.into()), }) - .or_else(|| { - symbol.param.map(|docs| Hover { - contents: HoverContents::Array(vec![MarkedString::String( - render_doc_param(&docs), - )]), - range: Some(source.into()), - }) - }) - }) + }) + }), + IdentifierDefinition::LocationWithParameterReference { + source, parameter, .. } - IdentifierDefinition::LoadedLocation { - path, name, source, .. - } => { - // Symbol loaded from another file. Find the file and get the definition - // from there, hopefully including the docs. - let load_uri = self.resolve_load_path(&path, document_uri, workspace_root)?; - self.get_ast_or_load_from_disk(&load_uri)?.and_then(|ast| { - ast.find_exported_symbol(&name).and_then(|symbol| { - symbol.docs.map(|docs| Hover { + | IdentifierDefinition::LoadedLocationWithParameterReference { + source, + parameter, + .. + } => symbol.and_then(|symbol| match symbol.kind { + SymbolKind::Method { arguments } => arguments + .iter() + .find(|arg| arg.name == parameter) + .or_else(|| arguments.iter().find(|arg| arg.is_kwargs)) + .and_then(|arg| { + Some(Hover { contents: HoverContents::Array(vec![MarkedString::String( - render_doc_item(&symbol.name, &docs), + render_doc_param(arg.doc.as_ref()?), )]), range: Some(source.into()), }) + }), + _ => None, + }), + IdentifierDefinition::LoadedLocation { source, .. } => { + // Symbol loaded from another file. Find the file and get the definition + // from there, hopefully including the docs. + symbol.and_then(|symbol| { + symbol.doc.map(|docs| Hover { + contents: HoverContents::Array(vec![MarkedString::String( + render_doc_item(&symbol.name, &docs.to_doc_item()), + )]), + range: Some(source.into()), + }) + }) + } + IdentifierDefinition::Unresolved { source, .. } => { + // Try to resolve as a global symbol. + symbol.and_then(|symbol| { + symbol.doc.map(|doc| Hover { + contents: HoverContents::Array(vec![MarkedString::String( + render_doc_member(&symbol.name, &doc), + )]), + range: Some(source.into()), }) }) } @@ -1148,24 +1328,309 @@ impl Backend { _ => None, } } - IdentifierDefinition::Unresolved { source, name } => { + IdentifierDefinition::LoadPath { .. } | IdentifierDefinition::NotFound => None, + }) + } + + fn get_symbol_for_identifier_definition( + &self, + identifier_definition: &IdentifierDefinition, + document: &LspModule, + document_uri: &LspUrl, + workspace_root: Option<&Path>, + ) -> anyhow::Result> { + match identifier_definition { + IdentifierDefinition::Location { + destination, name, .. + } + | IdentifierDefinition::LocationWithParameterReference { + destination, name, .. + } => { + // TODO: This seems very inefficient. Once the document starts + // holding the `Scope` including AST nodes, this indirection + // should be removed. + Ok(find_symbols_at_location( + document.ast.codemap(), + document.ast.statement(), + ResolvedPos { + line: destination.begin.line, + column: destination.begin.column, + }, + ) + .remove(name)) + } + IdentifierDefinition::LoadedLocation { path, name, .. } + | IdentifierDefinition::LoadedLocationWithParameterReference { path, name, .. } => { + // Symbol loaded from another file. Find the file and get the definition + // from there, hopefully including the docs. + let load_uri = self.resolve_load_path(path, document_uri, workspace_root)?; + Ok(self + .get_ast_or_load_from_disk(&load_uri)? + .and_then(|ast| ast.find_exported_symbol(name))) + } + IdentifierDefinition::Unresolved { name, .. } => { // Try to resolve as a global symbol. - self.context + Ok(self + .context .get_environment(document_uri) .members .into_iter() - .find(|symbol| symbol.0 == name) - .map(|symbol| Hover { - contents: HoverContents::Array(vec![MarkedString::String( - render_doc_member(&symbol.0, &symbol.1), - )]), - range: Some(source.into()), - }) + .find_map(|(symbol_name, doc)| { + if &symbol_name == name { + Some(Symbol { + name: symbol_name, + kind: match &doc { + DocMember::Property(_) => SymbolKind::Constant, + DocMember::Function(doc) => SymbolKind::Method { + arguments: doc + .params + .iter() + .filter_map(|param| match param { + DocParam::Arg { name, .. } => { + Some(MethodSymbolArgument { + name: name.clone(), + doc: Some(param.clone()), + span: None, + is_kwargs: false, + }) + } + DocParam::Args { name, .. } => { + Some(MethodSymbolArgument { + name: name.clone(), + doc: Some(param.clone()), + span: None, + is_kwargs: false, + }) + } + DocParam::Kwargs { name, .. } => { + Some(MethodSymbolArgument { + name: name.clone(), + doc: Some(param.clone()), + span: None, + is_kwargs: true, + }) + } + DocParam::NoArgs | DocParam::OnlyPosBefore => None, + }) + .collect(), + }, + }, + doc: Some(doc), + param: None, + span: None, + detail: None, + }) + } else { + None + } + })) } - IdentifierDefinition::LoadPath { .. } | IdentifierDefinition::NotFound => None, + IdentifierDefinition::StringLiteral { .. } + | IdentifierDefinition::LoadPath { .. } + | IdentifierDefinition::NotFound => Ok(None), + } + } + + fn signature_help_info( + &self, + params: SignatureHelpParams, + initialize_params: &InitializeParams, + ) -> anyhow::Result { + let uri = params + .text_document_position_params + .text_document + .uri + .try_into()?; + let line = params.text_document_position_params.position.line; + let character = params.text_document_position_params.position.character; + let workspace_root = + Self::get_workspace_root(initialize_params.workspace_folders.as_ref(), &uri); + + Ok(match self.get_ast(&uri) { + Some(ast) => { + let calls = signature::function_signatures(&ast, line, character)?; + let mut signatures = Vec::new(); + let mut active_signature = None; + let mut active_parameter = None; + + for call in calls { + // Look up the function definition + let definition = ast.find_definition_at_location( + call.function_span.begin.line as u32, + call.function_span.begin.column as u32, + ); + let identifier_definition = match definition { + Definition::Identifier(i) => i, + Definition::Dotted(DottedDefinition { + root_definition_location, + .. + }) => root_definition_location, + }; + if let Some(symbol) = self.get_symbol_for_identifier_definition( + &identifier_definition, + &ast, + &uri, + workspace_root.as_deref(), + )? { + match symbol.kind { + SymbolKind::Method { arguments } => { + let value = lsp_types::SignatureInformation { + documentation: symbol.doc.as_ref().map(|doc| { + Documentation::MarkupContent(MarkupContent { + kind: MarkupKind::Markdown, + value: render_doc_member(&symbol.name, doc), + }) + }), + label: format!( + "{}({})", + symbol.name, + symbol.doc + .as_ref() + .and_then(|doc| match doc { + DocMember::Property(_) => None, + DocMember::Function(func_doc) => Some(func_doc), + }).map(|doc| { + doc.params.iter().map(|param| param.render_as_code()).collect::>().join(", ") + }) + .unwrap_or_else(|| arguments.iter().map(|arg| &arg.name).join(", ")), + ), + active_parameter: match call.current_argument { + signature::CallArgument::Positional(i) => Some(i as u32), + signature::CallArgument::Named(name) => arguments + .iter() + .enumerate() + .find_map(|(i, arg)| { + if arg.name == name { + Some(i as u32) + } else { + None + } + }), + signature::CallArgument::None => None, + }, + parameters: Some( + arguments + .into_iter() + .map(|arg| ParameterInformation { + documentation: symbol.doc.as_ref().and_then(|doc| { + match doc { + DocMember::Property(_) => todo!(), + DocMember::Function(func) => func.params.iter().find_map(|param| { + let param_name = match param { + DocParam::Arg { name , .. } => name, + DocParam::Args { name, .. } => name, + DocParam::Kwargs { name, .. } => name, + DocParam::NoArgs | DocParam::OnlyPosBefore => return None, + }; + + if param_name == &arg.name { + Some(Documentation::MarkupContent(MarkupContent { + kind: MarkupKind::Markdown, + value: render_doc_param(param), + })) + } else { + None + } + }), + } + }), + label: ParameterLabel::Simple(arg.name), + }) + .collect(), + ), + }; + active_parameter = value.active_parameter; + signatures.push(value); + + active_signature = Some(signatures.len() as u32 - 1); + } + SymbolKind::Constant | SymbolKind::Variable => { + // That's weird, innit? + } + } + } + } + + SignatureHelp { + signatures, + active_signature, + active_parameter, + } + } + None => SignatureHelp { + signatures: vec![], + active_signature: None, + active_parameter: None, + }, }) } + fn get_document_symbols( + &self, + params: DocumentSymbolParams, + ) -> anyhow::Result { + let uri = params.text_document.uri.try_into()?; + + let document = match self.get_ast(&uri) { + Some(document) => document, + None => return Ok(DocumentSymbolResponse::Nested(vec![])), + }; + + let result = symbols::get_document_symbols( + document.ast.codemap(), + document.ast.statement(), + symbols::DocumentSymbolMode::IncludeLoads, + ); + + Ok(result.into()) + } + + fn get_workspace_symbols( + &self, + params: WorkspaceSymbolParams, + ) -> anyhow::Result { + let query = ¶ms.query.to_lowercase(); + + let symbols = self + .last_valid_parse + .read() + .unwrap() + .iter() + .flat_map(|(uri, document)| { + let uri: Url = uri.try_into().unwrap(); + + symbols::get_document_symbols( + document.ast.codemap(), + document.ast.statement(), + symbols::DocumentSymbolMode::OmitLoads, + ) + .into_iter() + .filter_map(move |symbol| { + if !query.is_empty() && !symbol.name.to_lowercase().contains(query) { + return None; + } + + let location = Location { + uri: uri.clone(), + range: symbol.range, + }; + + Some(WorkspaceSymbol { + name: symbol.name, + kind: symbol.kind, + location: OneOf::Left(location), + container_name: None, + tags: None, + // TODO? + data: None, + }) + }) + }) + .collect(); + + Ok(WorkspaceSymbolResponse::Nested(symbols)) + } + fn get_workspace_root( workspace_roots: Option<&Vec>, target: &LspUrl, @@ -1210,6 +1675,7 @@ impl Backend { fn main_loop(&self, initialize_params: InitializeParams) -> anyhow::Result<()> { self.log_message(MessageType::INFO, "Starlark server initialised"); + self.preload_workspace(&initialize_params); for msg in &self.connection.receiver { match msg { Message::Request(req) => { @@ -1223,6 +1689,12 @@ impl Backend { self.completion(req.id, params, &initialize_params); } else if let Some(params) = as_request::(&req) { self.hover(req.id, params, &initialize_params); + } else if let Some(params) = as_request::(&req) { + self.signature_help(req.id, params, &initialize_params); + } else if let Some(params) = as_request::(&req) { + self.document_symbols(req.id, params); + } else if let Some(params) = as_request::(&req) { + self.workspace_symbols(req.id, params); } else if self.connection.handle_shutdown(&req)? { return Ok(()); } @@ -1235,6 +1707,20 @@ impl Backend { self.did_change(params)?; } else if let Some(params) = as_notification::(&x) { self.did_close(params)?; + } else if let Some(params) = as_notification::(&x) { + if self.context.is_eager() { + for file in params.files { + let url = Url::parse(&file.uri).unwrap(); + let path = url.to_file_path().ok(); + if let Some(path) = path { + self.preload_file_if_relevant(&LspUrl::File(path))?; + } + } + } + } else if let Some(params) = as_notification::(&x) { + self.did_delete(params)?; + } else if let Some(params) = as_notification::(&x) { + self.did_rename(params)?; } } Message::Response(_) => { @@ -1244,6 +1730,90 @@ impl Backend { } Ok(()) } + + fn preload_workspace(&self, initialize_params: &InitializeParams) { + if !self.context.is_eager() { + return; + } + + // Figure out the roots to crawl from + let workspace_roots: Vec<_> = + if let Some(workspace_folders) = initialize_params.workspace_folders.as_ref() { + workspace_folders + .iter() + .filter_map(|folder| folder.uri.to_file_path().ok()) + .collect() + } else if let Some(root_uri) = initialize_params.root_uri.as_ref() { + root_uri.to_file_path().ok().into_iter().collect() + } else { + #[allow(deprecated)] + initialize_params + .root_path + .as_ref() + .map(PathBuf::from) + .into_iter() + .collect() + }; + + for root in workspace_roots { + if let Err(e) = self.preload_directory(&root) { + self.log_message( + MessageType::WARNING, + &format!("Error preloading workspace: {:#}", e), + ); + } + } + } + + fn preload_directory(&self, dir: &Path) -> anyhow::Result<()> { + for entry in std::fs::read_dir(dir)? { + let entry = entry?; + let file_type = entry.file_type()?; + if file_type.is_file() { + self.preload_file_if_relevant(&LspUrl::File(entry.path()))?; + } else if file_type.is_dir() { + self.preload_directory(&entry.path())?; + } + } + + Ok(()) + } + + fn preload_file_if_relevant(&self, file: &LspUrl) -> anyhow::Result<()> { + if !self.context.is_eager() { + return Ok(()); + } + + // Check if it's a Starlark-ish file. + match file { + LspUrl::File(path) => { + let ext = path.extension().and_then(|ext| ext.to_str()); + let file_name = path.file_name().and_then(|name| name.to_str()); + let is_starlark = matches!( + (ext, file_name), + (Some("star"), _) + | (Some("bzl"), _) + | (Some("bzlmod"), _) + | (Some("bazel"), _) + | (_, Some("BUILD")) + | (_, Some("WORKSPACE")) + ); + + if !is_starlark { + return Ok(()); + } + } + LspUrl::Starlark(_) | LspUrl::Other(_) => return Ok(()), + } + + let contents = self.context.get_load_contents(file)?; + if let Some(contents) = contents { + // NOTE: This also inserts the AST into the cache. + self.validate(file, None, contents)?; + } + + Ok(()) + } } /// Instantiate an LSP server that reads on stdin, and writes to stdout @@ -1274,11 +1844,11 @@ pub fn server_with_connection( .as_ref() .and_then(|opts| serde_json::from_value(opts.clone()).ok()) .unwrap_or_default(); - let capabilities_payload = Backend::::server_capabilities(server_settings); + let capabilities_payload = Backend::server_capabilities(&context, server_settings); let server_capabilities = serde_json::to_value(capabilities_payload).unwrap(); let initialize_data = serde_json::json!({ - "capabilities": server_capabilities, + "capabilities": server_capabilities, }); connection.initialize_finish(init_request_id, initialize_data)?; diff --git a/starlark_lsp/src/signature.rs b/starlark_lsp/src/signature.rs new file mode 100644 index 000000000..5e63477d2 --- /dev/null +++ b/starlark_lsp/src/signature.rs @@ -0,0 +1,105 @@ +/* + * Copyright 2019 The Starlark in Rust Authors. + * Copyright (c) Facebook, Inc. and its affiliates. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use starlark::codemap::CodeMap; +use starlark::codemap::ResolvedPos; +use starlark::codemap::ResolvedSpan; +use starlark_syntax::syntax::ast::ArgumentP; +use starlark_syntax::syntax::ast::AstExprP; +use starlark_syntax::syntax::ast::AstPayload; +use starlark_syntax::syntax::ast::ExprP; +use starlark_syntax::syntax::module::AstModuleFields; + +use crate::definition::LspModule; + +#[derive(Debug)] +pub(crate) struct Call { + pub(crate) function_span: ResolvedSpan, + pub(crate) current_argument: CallArgument, +} + +#[derive(Debug)] +pub(crate) enum CallArgument { + Positional(usize), + Named(String), + None, +} + +pub(crate) fn function_signatures( + ast: &LspModule, + line: u32, + character: u32, +) -> anyhow::Result> { + let pos = ResolvedPos { + line: line as usize, + column: character as usize, + }; + + let mut calls = Vec::new(); + ast.ast + .statement() + .visit_expr(|expr| visit_expr(expr, &mut calls, ast.ast.codemap(), pos)); + + Ok(calls) +} + +fn visit_expr( + expr: &AstExprP

, + calls: &mut Vec, + codemap: &CodeMap, + pos: ResolvedPos, +) { + let expr_span = codemap.resolve_span(expr.span); + if !expr_span.contains(pos) { + return; + } + + if let ExprP::Call(target, args) = &expr.node { + if let ExprP::Identifier(ident) = &target.node { + let current_argument = args + .iter() + .enumerate() + .find_map(|(i, arg)| { + let arg_span = codemap.resolve_span(arg.span); + if !arg_span.contains(pos) { + return None; + } + + match &arg.node { + ArgumentP::Positional(_) => Some(CallArgument::Positional(i)), + ArgumentP::Named(name, _) => Some(CallArgument::Named(name.node.clone())), + ArgumentP::Args(_) => todo!(), + ArgumentP::KwArgs(_) => todo!(), + } + }) + .unwrap_or({ + if args.is_empty() { + CallArgument::None + } else { + CallArgument::Positional(args.len()) + } + }); + + calls.push(Call { + function_span: codemap.resolve_span(ident.span), + current_argument, + }); + } + } + + expr.visit_expr(|expr| visit_expr(expr, calls, codemap, pos)); +} diff --git a/starlark_lsp/src/symbols.rs b/starlark_lsp/src/symbols.rs index 0f63052c9..f219176cc 100644 --- a/starlark_lsp/src/symbols.rs +++ b/starlark_lsp/src/symbols.rs @@ -18,12 +18,23 @@ //! Find which symbols are in scope at a particular point. use std::collections::HashMap; +use std::ops::Deref; +use lsp_types::CompletionItemKind; +use lsp_types::DocumentSymbol; +use lsp_types::SymbolKind as LspSymbolKind; use starlark::codemap::CodeMap; -use starlark::docs::DocItem; +use starlark::codemap::Span; +use starlark::docs::DocFunction; +use starlark::docs::DocMember; use starlark::docs::DocParam; use starlark_syntax::codemap::ResolvedPos; +use starlark_syntax::syntax::ast::ArgumentP; use starlark_syntax::syntax::ast::AssignP; +use starlark_syntax::syntax::ast::AssignTargetP; +use starlark_syntax::syntax::ast::AstAssignIdentP; +use starlark_syntax::syntax::ast::AstExprP; +use starlark_syntax::syntax::ast::AstLiteral; use starlark_syntax::syntax::ast::AstPayload; use starlark_syntax::syntax::ast::AstStmtP; use starlark_syntax::syntax::ast::ExprP; @@ -36,16 +47,82 @@ use crate::docs::get_doc_item_for_def; #[derive(Debug, PartialEq)] pub(crate) enum SymbolKind { - Method, + Constant, + Method { + arguments: Vec, + }, Variable, } +#[derive(Debug, PartialEq)] +pub(crate) struct MethodSymbolArgument { + pub(crate) name: String, + pub(crate) span: Option, + pub(crate) doc: Option, + pub(crate) is_kwargs: bool, +} + +impl MethodSymbolArgument { + pub(crate) fn from_ast_parameter( + param: &ParameterP

, + doc: Option<&DocParam>, + ) -> Option { + match param { + ParameterP::Normal(p, _) | ParameterP::WithDefaultValue(p, _, _) => Some(Self { + name: p.ident.clone(), + span: Some(p.span), + doc: doc.cloned(), + is_kwargs: false, + }), + ParameterP::Args(p, _) => Some(Self { + name: p.ident.clone(), + span: Some(p.span), + doc: doc.cloned(), + is_kwargs: false, + }), + ParameterP::KwArgs(p, _) => Some(Self { + name: p.ident.clone(), + span: Some(p.span), + doc: doc.cloned(), + is_kwargs: true, + }), + ParameterP::NoArgs => None, + } + } +} + +impl From for CompletionItemKind { + fn from(value: SymbolKind) -> Self { + match value { + SymbolKind::Method { .. } => CompletionItemKind::FUNCTION, + SymbolKind::Variable => CompletionItemKind::VARIABLE, + SymbolKind::Constant => CompletionItemKind::CONSTANT, + } + } +} + +impl SymbolKind { + pub(crate) fn from_expr(x: &AstExprP

) -> Self { + match &x.node { + ExprP::Lambda(lambda) => Self::Method { + arguments: lambda + .params + .iter() + .filter_map(|param| MethodSymbolArgument::from_ast_parameter(¶m.node, None)) + .collect(), + }, + _ => Self::Variable, + } + } +} + #[derive(Debug, PartialEq)] pub(crate) struct Symbol { pub(crate) name: String, + pub(crate) span: Option, pub(crate) detail: Option, pub(crate) kind: SymbolKind, - pub(crate) doc: Option, + pub(crate) doc: Option, pub(crate) param: Option, } @@ -66,8 +143,11 @@ pub(crate) fn find_symbols_at_location( StmtP::Assign(AssignP { lhs, ty: _, rhs }) => lhs.visit_lvalue(|x| { symbols.entry(x.ident.clone()).or_insert_with(|| Symbol { name: x.ident.clone(), - kind: (match rhs.node { - ExprP::Lambda(_) => SymbolKind::Method, + span: Some(x.span), + kind: (match &rhs.node { + ExprP::Lambda(lambda) => SymbolKind::Method { + arguments: argument_names(&lambda.params, None), + }, _ => SymbolKind::Variable, }), detail: None, @@ -78,8 +158,11 @@ pub(crate) fn find_symbols_at_location( StmtP::AssignModify(dest, _, source) => dest.visit_lvalue(|x| { symbols.entry(x.ident.clone()).or_insert_with(|| Symbol { name: x.ident.clone(), - kind: (match source.node { - ExprP::Lambda(_) => SymbolKind::Method, + span: Some(x.span), + kind: (match &source.node { + ExprP::Lambda(lambda) => SymbolKind::Method { + arguments: argument_names(&lambda.params, None), + }, _ => SymbolKind::Variable, }), detail: None, @@ -91,6 +174,7 @@ pub(crate) fn find_symbols_at_location( var.visit_lvalue(|x| { symbols.entry(x.ident.clone()).or_insert_with(|| Symbol { name: x.ident.clone(), + span: Some(x.span), kind: SymbolKind::Variable, detail: None, doc: None, @@ -106,9 +190,12 @@ pub(crate) fn find_symbols_at_location( .entry(def.name.ident.clone()) .or_insert_with(|| Symbol { name: def.name.ident.clone(), - kind: SymbolKind::Method, + span: Some(def.name.span), + kind: SymbolKind::Method { + arguments: argument_names(&def.params, doc.as_ref()), + }, detail: None, - doc: doc.clone().map(DocItem::Function), + doc: doc.clone().map(DocMember::Function), param: None, }); @@ -123,6 +210,7 @@ pub(crate) fn find_symbols_at_location( p.ident.clone(), Symbol { name: p.ident.clone(), + span: Some(p.span), kind: SymbolKind::Variable, detail: None, doc: None, @@ -143,9 +231,10 @@ pub(crate) fn find_symbols_at_location( local.ident.clone(), Symbol { name: local.ident.clone(), + span: Some(local.span), detail: Some(format!("Loaded from {}", load.module.node)), // TODO: This should be dynamic based on the actual loaded value. - kind: SymbolKind::Method, + kind: SymbolKind::Method { arguments: vec![] }, // TODO: Pull from the original file. doc: None, param: None, @@ -161,16 +250,281 @@ pub(crate) fn find_symbols_at_location( symbols } +fn argument_names( + params: &[starlark::codemap::Spanned>], + doc: Option<&DocFunction>, +) -> Vec { + params + .iter() + .filter_map(|param| { + MethodSymbolArgument::from_ast_parameter( + ¶m.node, + doc.and_then(|doc| { + param + .ident() + .and_then(|name| doc.find_param_with_name(&name.node.ident)) + }), + ) + }) + .collect() +} + +#[derive(Copy, Clone, Debug, PartialEq)] +pub enum DocumentSymbolMode { + IncludeLoads, + OmitLoads, +} + +pub fn get_document_symbols( + codemap: &CodeMap, + ast: &AstStmtP

, + mode: DocumentSymbolMode, +) -> Vec { + let mut symbols = Vec::new(); + match &ast.node { + StmtP::Expression(expr) => { + if let Some(symbol) = get_document_symbol_for_expr(codemap, None, expr, ast.span) { + symbols.push(symbol); + } + } + StmtP::Assign(assign) => { + if let Some(symbol) = get_document_symbol_for_expr( + codemap, + match &assign.lhs.node { + AssignTargetP::Tuple(_) + | AssignTargetP::Index(_) + | AssignTargetP::Dot(_, _) => None, + AssignTargetP::Identifier(ident) => Some(ident), + }, + &assign.rhs, + ast.span, + ) { + symbols.push(symbol); + } + } + StmtP::Statements(statements) => { + for stmt in statements { + symbols.extend(get_document_symbols(codemap, stmt, mode)); + } + } + StmtP::If(_, body) => { + symbols.extend(get_document_symbols(codemap, body, mode)); + } + StmtP::IfElse(_, bodies) => { + let (if_body, else_body) = bodies.deref(); + symbols.extend(get_document_symbols(codemap, if_body, mode)); + symbols.extend(get_document_symbols(codemap, else_body, mode)); + } + StmtP::For(for_) => { + symbols.extend(get_document_symbols(codemap, &for_.body, mode)); + } + StmtP::Def(def) => { + symbols.push(make_document_symbol( + def.name.ident.clone(), + LspSymbolKind::FUNCTION, + ast.span, + def.name.span, + codemap, + Some( + def.params + .iter() + .filter_map(|param| get_document_symbol_for_parameter(codemap, param)) + .chain(get_document_symbols(codemap, &def.body, mode)) + .collect(), + ), + )); + } + StmtP::Load(load) => { + if mode == DocumentSymbolMode::IncludeLoads { + symbols.push(make_document_symbol( + load.module.node.clone(), + LspSymbolKind::MODULE, + ast.span, + load.module.span, + codemap, + Some( + load.args + .iter() + .map(|loaded_symbol| { + make_document_symbol( + loaded_symbol.local.ident.clone(), + LspSymbolKind::METHOD, + loaded_symbol.span(), + loaded_symbol.local.span, + codemap, + None, + ) + }) + .collect(), + ), + )); + } + } + + // These don't produce any symbols. + StmtP::Break + | StmtP::Continue + | StmtP::Pass + | StmtP::Return(_) + | StmtP::AssignModify(_, _, _) => {} + } + + symbols +} + +fn get_document_symbol_for_parameter( + codemap: &CodeMap, + param: &ParameterP

, +) -> Option { + match param { + ParameterP::NoArgs => None, + ParameterP::Normal(p, _) + | ParameterP::WithDefaultValue(p, _, _) + | ParameterP::Args(p, _) + | ParameterP::KwArgs(p, _) => Some(make_document_symbol( + p.ident.clone(), + LspSymbolKind::VARIABLE, + p.span, + p.span, + codemap, + None, + )), + } +} + +fn get_document_symbol_for_expr( + codemap: &CodeMap, + name: Option<&AstAssignIdentP

>, + expr: &AstExprP

, + outer_range: Span, +) -> Option { + match &expr.node { + ExprP::Call(call, args) => { + if let ExprP::Identifier(func_name) = &call.node { + // Look for a call to `struct`. We'll require passing in a name from the assignment + // expression. The outer range is the range of the entire assignment expression. + if &func_name.node.ident == "struct" { + name.map(|name| { + make_document_symbol( + name.ident.clone(), + LspSymbolKind::STRUCT, + outer_range, + name.span, + codemap, + Some( + args.iter() + .filter_map(|arg| match &arg.node { + ArgumentP::Named(name, _) => Some(make_document_symbol( + name.node.clone(), + LspSymbolKind::FIELD, + arg.span, + name.span, + codemap, + None, + )), + _ => None, + }) + .collect(), + ), + ) + }) + } else { + // Check if this call has a named argument called "name". If so, we'll assume + // that this is a buildable target, and expose it. + args.iter() + .find_map(|arg| match &arg.node { + ArgumentP::Named(name, value) => match (name, &value.node) { + (name, ExprP::Literal(AstLiteral::String(value))) + if &name.node == "name" => + { + Some(value) + } + _ => None, + }, + _ => None, + }) + .map(|target_name| { + make_document_symbol( + target_name.node.clone(), + LspSymbolKind::CONSTANT, + expr.span, + target_name.span, + codemap, + None, + ) + }) + } + } else { + None + } + } + ExprP::Lambda(lambda) => name.map(|name| { + make_document_symbol( + name.ident.clone(), + LspSymbolKind::FUNCTION, + expr.span, + expr.span, + codemap, + Some( + lambda + .params + .iter() + .filter_map(|param| get_document_symbol_for_parameter(codemap, param)) + .chain(get_document_symbol_for_expr( + codemap, + None, + &lambda.body, + lambda.body.span, + )) + .collect(), + ), + ) + }), + + _ => None, + } +} + +fn make_document_symbol( + name: String, + kind: LspSymbolKind, + range: Span, + selection_range: Span, + codemap: &CodeMap, + children: Option>, +) -> DocumentSymbol { + #[allow(deprecated)] + DocumentSymbol { + name, + detail: None, + kind, + tags: None, + deprecated: None, + range: codemap.resolve_span(range).into(), + selection_range: codemap.resolve_span(selection_range).into(), + children, + } +} + #[cfg(test)] mod tests { use std::collections::HashMap; + use lsp_types::DocumentSymbol; + use lsp_types::Position; + use lsp_types::Range; + use lsp_types::SymbolKind as LspSymbolKind; + use starlark::codemap::Pos; + use starlark::codemap::Span; use starlark::syntax::AstModule; use starlark::syntax::Dialect; use starlark_syntax::codemap::ResolvedPos; use starlark_syntax::syntax::module::AstModuleFields; use super::find_symbols_at_location; + use super::get_document_symbols; + use super::DocumentSymbolMode; + use super::MethodSymbolArgument; use super::Symbol; use super::SymbolKind; @@ -201,8 +555,9 @@ my_var = True "exported_a".to_owned(), Symbol { name: "exported_a".to_owned(), + span: Some(Span::new(Pos::new(17), Pos::new(29))), detail: Some("Loaded from foo.star".to_owned()), - kind: SymbolKind::Method, + kind: SymbolKind::Method { arguments: vec![] }, doc: None, param: None, }, @@ -211,8 +566,9 @@ my_var = True "renamed".to_owned(), Symbol { name: "renamed".to_owned(), + span: Some(Span::new(Pos::new(31), Pos::new(38))), detail: Some("Loaded from foo.star".to_owned()), - kind: SymbolKind::Method, + kind: SymbolKind::Method { arguments: vec![] }, doc: None, param: None, }, @@ -221,8 +577,16 @@ my_var = True "method".to_owned(), Symbol { name: "method".to_owned(), + span: Some(Span::new(Pos::new(60), Pos::new(66))), detail: None, - kind: SymbolKind::Method, + kind: SymbolKind::Method { + arguments: vec![MethodSymbolArgument { + name: "param".to_owned(), + span: Some(Span::new(Pos::new(67), Pos::new(72))), + doc: None, + is_kwargs: false, + }] + }, doc: None, param: None, }, @@ -231,6 +595,7 @@ my_var = True "my_var".to_owned(), Symbol { name: "my_var".to_owned(), + span: Some(Span::new(Pos::new(85), Pos::new(91))), detail: None, kind: SymbolKind::Variable, doc: None, @@ -268,8 +633,9 @@ my_var = True "exported_a".to_owned(), Symbol { name: "exported_a".to_owned(), + span: Some(Span::new(Pos::new(17), Pos::new(29))), detail: Some("Loaded from foo.star".to_owned()), - kind: SymbolKind::Method, + kind: SymbolKind::Method { arguments: vec![] }, doc: None, param: None, }, @@ -278,8 +644,9 @@ my_var = True "renamed".to_owned(), Symbol { name: "renamed".to_owned(), + span: Some(Span::new(Pos::new(31), Pos::new(38))), detail: Some("Loaded from foo.star".to_owned()), - kind: SymbolKind::Method, + kind: SymbolKind::Method { arguments: vec![] }, doc: None, param: None, }, @@ -288,8 +655,16 @@ my_var = True "method".to_owned(), Symbol { name: "method".to_owned(), + span: Some(Span::new(Pos::new(60), Pos::new(66))), detail: None, - kind: SymbolKind::Method, + kind: SymbolKind::Method { + arguments: vec![MethodSymbolArgument { + name: "param".to_owned(), + span: Some(Span::new(Pos::new(67), Pos::new(72))), + doc: None, + is_kwargs: false + }], + }, doc: None, param: None, }, @@ -298,16 +673,18 @@ my_var = True "param".to_owned(), Symbol { name: "param".to_owned(), + span: Some(Span::new(Pos::new(67), Pos::new(72))), detail: None, kind: SymbolKind::Variable, doc: None, param: None, - } + }, ), ( "my_var".to_owned(), Symbol { name: "my_var".to_owned(), + span: Some(Span::new(Pos::new(85), Pos::new(91))), detail: None, kind: SymbolKind::Variable, doc: None, @@ -317,4 +694,637 @@ my_var = True ]) ); } + + #[test] + fn document_symbols() { + let ast_module = AstModule::parse( + "t.star", + r#"load("foo.star", "exported_a", renamed = "exported_b") + +def method(param): + foo = struct(field = "value") + bar = lambda x: x + 1 + return lambda y: y + 1 + +baz = struct(field = "value") + +some_rule(name = "qux") + "# + .to_owned(), + &Dialect::Standard, + ) + .unwrap(); + + assert_eq!( + get_document_symbols( + ast_module.codemap(), + ast_module.statement(), + DocumentSymbolMode::IncludeLoads + ), + vec![ + #[allow(deprecated)] + DocumentSymbol { + name: "foo.star".to_owned(), + detail: None, + kind: LspSymbolKind::MODULE, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 0, + character: 0 + }, + end: Position { + line: 0, + character: 54 + } + }, + selection_range: Range { + start: Position { + line: 0, + character: 5 + }, + end: Position { + line: 0, + character: 15 + } + }, + children: Some(vec![ + DocumentSymbol { + name: "exported_a".to_owned(), + detail: None, + kind: LspSymbolKind::METHOD, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 0, + character: 17 + }, + end: Position { + line: 0, + character: 29 + } + }, + selection_range: Range { + start: Position { + line: 0, + character: 17 + }, + end: Position { + line: 0, + character: 29 + } + }, + children: None + }, + DocumentSymbol { + name: "renamed".to_owned(), + detail: None, + kind: LspSymbolKind::METHOD, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 0, + character: 31 + }, + end: Position { + line: 0, + character: 53 + } + }, + selection_range: Range { + start: Position { + line: 0, + character: 31 + }, + end: Position { + line: 0, + character: 38 + } + }, + children: None + } + ]) + }, + #[allow(deprecated)] + DocumentSymbol { + name: "method".to_owned(), + detail: None, + kind: LspSymbolKind::FUNCTION, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 2, + character: 0 + }, + end: Position { + line: 7, + character: 0 + } + }, + selection_range: Range { + start: Position { + line: 2, + character: 4 + }, + end: Position { + line: 2, + character: 10 + } + }, + children: Some(vec![ + DocumentSymbol { + name: "param".to_owned(), + detail: None, + kind: LspSymbolKind::VARIABLE, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 2, + character: 11 + }, + end: Position { + line: 2, + character: 16 + } + }, + selection_range: Range { + start: Position { + line: 2, + character: 11 + }, + end: Position { + line: 2, + character: 16 + } + }, + children: None + }, + DocumentSymbol { + name: "foo".to_owned(), + detail: None, + kind: LspSymbolKind::STRUCT, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 3, + character: 4 + }, + end: Position { + line: 3, + character: 33 + } + }, + selection_range: Range { + start: Position { + line: 3, + character: 4 + }, + end: Position { + line: 3, + character: 7 + } + }, + children: Some(vec![DocumentSymbol { + name: "field".to_owned(), + detail: None, + kind: LspSymbolKind::FIELD, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 3, + character: 17 + }, + end: Position { + line: 3, + character: 32 + } + }, + selection_range: Range { + start: Position { + line: 3, + character: 17 + }, + end: Position { + line: 3, + character: 22 + } + }, + children: None + }]) + }, + DocumentSymbol { + name: "bar".to_owned(), + detail: None, + kind: LspSymbolKind::FUNCTION, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 4, + character: 10 + }, + end: Position { + line: 4, + character: 25 + } + }, + selection_range: Range { + start: Position { + line: 4, + character: 10 + }, + end: Position { + line: 4, + character: 25 + } + }, + children: Some(vec![DocumentSymbol { + name: "x".to_owned(), + detail: None, + kind: LspSymbolKind::VARIABLE, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 4, + character: 17 + }, + end: Position { + line: 4, + character: 18 + } + }, + selection_range: Range { + start: Position { + line: 4, + character: 17 + }, + end: Position { + line: 4, + character: 18 + } + }, + children: None + }]) + } + ]) + }, + #[allow(deprecated)] + DocumentSymbol { + name: "baz".to_owned(), + detail: None, + kind: LspSymbolKind::STRUCT, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 7, + character: 0 + }, + end: Position { + line: 7, + character: 29 + } + }, + selection_range: Range { + start: Position { + line: 7, + character: 0 + }, + end: Position { + line: 7, + character: 3 + } + }, + children: Some(vec![DocumentSymbol { + name: "field".to_owned(), + detail: None, + kind: LspSymbolKind::FIELD, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 7, + character: 13 + }, + end: Position { + line: 7, + character: 28 + } + }, + selection_range: Range { + start: Position { + line: 7, + character: 13 + }, + end: Position { + line: 7, + character: 18 + } + }, + children: None + }]) + }, + #[allow(deprecated)] + DocumentSymbol { + name: "qux".to_owned(), + detail: None, + kind: LspSymbolKind::CONSTANT, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 9, + character: 0 + }, + end: Position { + line: 9, + character: 23 + } + }, + selection_range: Range { + start: Position { + line: 9, + character: 17 + }, + end: Position { + line: 9, + character: 22 + } + }, + children: None + } + ] + ); + + assert_eq!( + get_document_symbols( + ast_module.codemap(), + ast_module.statement(), + DocumentSymbolMode::OmitLoads + ), + vec![ + #[allow(deprecated)] + DocumentSymbol { + name: "method".to_owned(), + detail: None, + kind: LspSymbolKind::FUNCTION, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 2, + character: 0 + }, + end: Position { + line: 7, + character: 0 + } + }, + selection_range: Range { + start: Position { + line: 2, + character: 4 + }, + end: Position { + line: 2, + character: 10 + } + }, + children: Some(vec![ + DocumentSymbol { + name: "param".to_owned(), + detail: None, + kind: LspSymbolKind::VARIABLE, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 2, + character: 11 + }, + end: Position { + line: 2, + character: 16 + } + }, + selection_range: Range { + start: Position { + line: 2, + character: 11 + }, + end: Position { + line: 2, + character: 16 + } + }, + children: None + }, + DocumentSymbol { + name: "foo".to_owned(), + detail: None, + kind: LspSymbolKind::STRUCT, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 3, + character: 4 + }, + end: Position { + line: 3, + character: 33 + } + }, + selection_range: Range { + start: Position { + line: 3, + character: 4 + }, + end: Position { + line: 3, + character: 7 + } + }, + children: Some(vec![DocumentSymbol { + name: "field".to_owned(), + detail: None, + kind: LspSymbolKind::FIELD, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 3, + character: 17 + }, + end: Position { + line: 3, + character: 32 + } + }, + selection_range: Range { + start: Position { + line: 3, + character: 17 + }, + end: Position { + line: 3, + character: 22 + } + }, + children: None + }]) + }, + DocumentSymbol { + name: "bar".to_owned(), + detail: None, + kind: LspSymbolKind::FUNCTION, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 4, + character: 10 + }, + end: Position { + line: 4, + character: 25 + } + }, + selection_range: Range { + start: Position { + line: 4, + character: 10 + }, + end: Position { + line: 4, + character: 25 + } + }, + children: Some(vec![DocumentSymbol { + name: "x".to_owned(), + detail: None, + kind: LspSymbolKind::VARIABLE, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 4, + character: 17 + }, + end: Position { + line: 4, + character: 18 + } + }, + selection_range: Range { + start: Position { + line: 4, + character: 17 + }, + end: Position { + line: 4, + character: 18 + } + }, + children: None + }]) + } + ]) + }, + #[allow(deprecated)] + DocumentSymbol { + name: "baz".to_owned(), + detail: None, + kind: LspSymbolKind::STRUCT, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 7, + character: 0 + }, + end: Position { + line: 7, + character: 29 + } + }, + selection_range: Range { + start: Position { + line: 7, + character: 0 + }, + end: Position { + line: 7, + character: 3 + } + }, + children: Some(vec![DocumentSymbol { + name: "field".to_owned(), + detail: None, + kind: LspSymbolKind::FIELD, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 7, + character: 13 + }, + end: Position { + line: 7, + character: 28 + } + }, + selection_range: Range { + start: Position { + line: 7, + character: 13 + }, + end: Position { + line: 7, + character: 18 + } + }, + children: None + }]) + }, + #[allow(deprecated)] + DocumentSymbol { + name: "qux".to_owned(), + detail: None, + kind: LspSymbolKind::CONSTANT, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 9, + character: 0 + }, + end: Position { + line: 9, + character: 23 + } + }, + selection_range: Range { + start: Position { + line: 9, + character: 17 + }, + end: Position { + line: 9, + character: 22 + } + }, + children: None + } + ] + ); + } } diff --git a/starlark_lsp/src/test.rs b/starlark_lsp/src/test.rs index e58a815de..61c81c2f5 100644 --- a/starlark_lsp/src/test.rs +++ b/starlark_lsp/src/test.rs @@ -300,6 +300,10 @@ impl LspContext for TestServerContext { .collect(), } } + + fn is_eager(&self) -> bool { + false + } } /// A server for use in testing that provides helpers for sending requests, correlating