Add signature help provider

starlark/src/docs/mod.rs

@@ -647,7 +647,7 @@ impl DocParam {
         Some(indented)
     }
 
-    fn render_as_code(&self) -> String {
+    pub fn render_as_code(&self) -> String {
         match self {
             DocParam::Arg {
                 name,

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<LspUrl, String>,
     pub(crate) builtin_symbols: HashMap<String, LspUrl>,
+    pub(crate) eager: bool,
 }
 
 impl BazelContext {
@@ -187,6 +190,7 @@ impl BazelContext {
         module: bool,
         dialect: Dialect,
         globals: Globals,
+        eager: bool,
     ) -> anyhow::Result<Self> {
         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
+    }
 }

starlark_bin/bin/eval.rs

@@ -70,6 +70,7 @@ pub(crate) struct Context {
     pub(crate) globals: Globals,
     pub(crate) builtin_docs: HashMap<LspUrl, String>,
     pub(crate) builtin_symbols: HashMap<String, LspUrl>,
+    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<Self> {
         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
+    }
 }

starlark_bin/bin/main.rs

@@ -142,6 +142,9 @@ struct Args {
     )]
     files: Vec<PathBuf>,
 
+    #[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 {

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<T: LspContext> Backend<T> {
             (
                 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<T: LspContext> Backend<T> {
                 )
                 .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,8 +281,8 @@ impl<T: LspContext> Backend<T> {
                 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(
+                        SymbolKind::Constant | SymbolKind::Variable => None,
+                        SymbolKind::Method { argument_names } => Some(
                             argument_names
                                 .into_iter()
                                 .map(|name| CompletionItem {

starlark_lsp/src/definition.rs

@@ -44,9 +44,9 @@ use crate::bind::Assigner;
 use crate::bind::Bind;
 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`].
 #[derive(Debug, Clone, Eq, PartialEq)]
@@ -275,7 +275,7 @@ 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>(
@@ -455,7 +455,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<ResolvedSpan> {
         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`)

starlark_lsp/src/exported.rs

@@ -16,78 +16,29 @@
  */
 
 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<String> },
-}
-
-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<SymbolKind> 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<DocItem>,
-}
+use crate::symbols::Symbol;
+use crate::symbols::SymbolKind;
 
 impl From<Symbol> 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 +63,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<DocItem>,
+            resolve_docs: impl FnOnce() -> Option<DocMember>,
         ) {
             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 +86,34 @@ 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) => {
                     add(
-                        self,
                         &mut result,
                         &def.name,
-                        SymbolKind::Function {
+                        SymbolKind::Method {
                             argument_names: def
                                 .params
                                 .iter()
                                 .filter_map(|param| param.split().0.map(|name| name.to_string()))
                                 .collect(),
                         },
-                        || get_doc_item_for_def(def).map(DocItem::Function),
+                        || get_doc_item_for_def(def).map(DocMember::Function),
                     );
                 }
                 _ => {}
@@ -197,7 +148,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"]
         );
     }

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;

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<Vec<Call>> {
+    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<P: AstPayload>(
+    expr: &AstExprP<P>,
+    calls: &mut Vec<Call>,
+    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));
+}

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