diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index f4942f25c..ebb815a6e 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -19,3 +19,6 @@ _below_ the calling function. A general goal is to be able to read linearly from top to bottom with the relevant context and main logic first. The code should be organised like a call stack. Of course that's not always possible, use best judgement to produce the clearest code organization. + + Keep the main logic as unnested as possible. Favour Rust's `let ... else` + syntax to return early or continue a loop in the `else` clause, over `if let`. diff --git a/Cargo.lock b/Cargo.lock index 5664adab1..c51e63d67 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -319,7 +319,6 @@ dependencies = [ "serde", "serde_json", "stdext", - "struct-field-names-as-array", "strum 0.26.2", "strum_macros 0.26.4", "tempfile", @@ -2744,26 +2743,6 @@ version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" -[[package]] -name = "struct-field-names-as-array" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "22ba4bae771f9cc992c4f403636c54d2ef13acde6367583e99d06bb336674dd9" -dependencies = [ - "struct-field-names-as-array-derive", -] - -[[package]] -name = "struct-field-names-as-array-derive" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a2dbf8b57f3ce20e4bb171a11822b283bdfab6c4bb0fe64fa729f045f23a0938" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.32", -] - [[package]] name = "strum" version = "0.24.1" diff --git a/crates/ark/Cargo.toml b/crates/ark/Cargo.toml index d51c0185b..8a1fb4df5 100644 --- a/crates/ark/Cargo.toml +++ b/crates/ark/Cargo.toml @@ -53,7 +53,6 @@ url = "2.4.1" walkdir = "2" yaml-rust = "0.4.5" winsafe = { version = "0.0.19", features = ["kernel"] } -struct-field-names-as-array = "0.3.0" strum = "0.26.2" strum_macros = "0.26.2" futures = "0.3.30" diff --git a/crates/ark/src/lsp/config.rs b/crates/ark/src/lsp/config.rs index d9f51ed90..1a269be6c 100644 --- a/crates/ark/src/lsp/config.rs +++ b/crates/ark/src/lsp/config.rs @@ -1,14 +1,96 @@ use serde::Deserialize; use serde::Serialize; -use struct_field_names_as_array::FieldNamesAsArray; +use serde_json::Value; -use crate::lsp; use crate::lsp::diagnostics::DiagnosticsConfig; +pub struct Setting { + pub key: &'static str, + pub set: fn(&mut LspConfig, Value), +} + +// List of LSP settings for which clients can send `didChangeConfiguration` +// notifications. We register our interest in watching over these settings in +// our `initialized` handler. The `set` methods convert from a json `Value` to +// the expected type, using a default value if the conversion fails. +pub static SETTINGS: &[Setting] = &[ + Setting { + key: "editor.insertSpaces", + set: |cfg, v| { + let default_style = IndentationConfig::default().indent_style; + cfg.document.indent.indent_style = if v + .as_bool() + .unwrap_or_else(|| default_style == IndentStyle::Space) + { + IndentStyle::Space + } else { + IndentStyle::Tab + } + }, + }, + Setting { + key: "editor.indentSize", + set: |cfg, v| { + cfg.document.indent.indent_size = v + .as_u64() + .map(|n| n as usize) + .unwrap_or_else(|| IndentationConfig::default().indent_size) + }, + }, + Setting { + key: "editor.tabSize", + set: |cfg, v| { + cfg.document.indent.tab_width = v + .as_u64() + .map(|n| n as usize) + .unwrap_or_else(|| IndentationConfig::default().tab_width) + }, + }, + Setting { + key: "positron.r.diagnostics.enable", + set: |cfg, v| { + cfg.diagnostics.enable = v + .as_bool() + .unwrap_or_else(|| DiagnosticsConfig::default().enable) + }, + }, + Setting { + key: "positron.r.symbols.includeAssignmentsInBlocks", + set: |cfg, v| { + cfg.symbols.include_assignments_in_blocks = v + .as_bool() + .unwrap_or_else(|| SymbolsConfig::default().include_assignments_in_blocks) + }, + }, + Setting { + key: "positron.r.workspaceSymbols.includeCommentSections", + set: |cfg, v| { + cfg.workspace_symbols.include_comment_sections = v + .as_bool() + .unwrap_or_else(|| WorkspaceSymbolsConfig::default().include_comment_sections) + }, + }, +]; + /// Configuration of the LSP -#[derive(Clone, Debug)] +#[derive(Clone, Default, Debug)] pub(crate) struct LspConfig { pub(crate) diagnostics: DiagnosticsConfig, + pub(crate) symbols: SymbolsConfig, + pub(crate) workspace_symbols: WorkspaceSymbolsConfig, + pub(crate) document: DocumentConfig, +} + +#[derive(Serialize, Deserialize, Clone, Debug)] +pub struct SymbolsConfig { + /// Whether to emit assignments in `{` bloks as document symbols. + pub include_assignments_in_blocks: bool, +} + +#[derive(Serialize, Deserialize, Clone, Debug)] +pub struct WorkspaceSymbolsConfig { + /// Whether to include sections like `# My section ---` in workspace symbols. + pub include_comment_sections: bool, } /// Configuration of a document. @@ -32,14 +114,14 @@ pub struct IndentationConfig { pub tab_width: usize, } -#[derive(Serialize, Deserialize, Clone, Debug)] +#[derive(PartialEq, Serialize, Deserialize, Clone, Debug)] pub enum IndentStyle { Tab, Space, } /// VS Code representation of a document configuration -#[derive(Serialize, Deserialize, FieldNamesAsArray, Clone, Debug)] +#[derive(Serialize, Deserialize, Clone, Debug)] pub(crate) struct VscDocumentConfig { // DEV NOTE: Update `section_from_key()` method after adding a field pub insert_spaces: bool, @@ -47,12 +129,18 @@ pub(crate) struct VscDocumentConfig { pub tab_size: usize, } -#[derive(Serialize, Deserialize, FieldNamesAsArray, Clone, Debug)] +#[derive(Serialize, Deserialize, Clone, Debug)] pub(crate) struct VscDiagnosticsConfig { // DEV NOTE: Update `section_from_key()` method after adding a field pub enable: bool, } +#[derive(Serialize, Deserialize, Clone, Debug)] +pub(crate) struct VscSymbolsConfig { + // DEV NOTE: Update `section_from_key()` method after adding a field + pub include_assignments_in_blocks: bool, +} + #[derive(Serialize, Deserialize, Clone, Debug)] #[serde(untagged)] pub(crate) enum VscIndentSize { @@ -60,76 +148,28 @@ pub(crate) enum VscIndentSize { Size(usize), } -impl Default for LspConfig { +impl Default for SymbolsConfig { fn default() -> Self { Self { - diagnostics: Default::default(), + include_assignments_in_blocks: false, } } } -impl Default for IndentationConfig { +impl Default for WorkspaceSymbolsConfig { fn default() -> Self { Self { - indent_style: IndentStyle::Space, - indent_size: 2, - tab_width: 2, + include_comment_sections: false, } } } -impl VscDocumentConfig { - pub(crate) fn section_from_key(key: &str) -> &str { - match key { - "insert_spaces" => "editor.insertSpaces", - "indent_size" => "editor.indentSize", - "tab_size" => "editor.tabSize", - _ => "unknown", // To be caught via downstream errors - } - } -} - -/// Convert from VS Code representation of a document config to our own -/// representation. Currently one-to-one. -impl From for DocumentConfig { - fn from(x: VscDocumentConfig) -> Self { - let indent_style = indent_style_from_lsp(x.insert_spaces); - - let indent_size = match x.indent_size { - VscIndentSize::Size(size) => size, - VscIndentSize::Alias(var) => { - if var == "tabSize" { - x.tab_size - } else { - lsp::log_warn!("Unknown indent alias {var}, using default"); - 2 - } - }, - }; - - Self { - indent: IndentationConfig { - indent_style, - indent_size, - tab_width: x.tab_size, - }, - } - } -} - -impl VscDiagnosticsConfig { - pub(crate) fn section_from_key(key: &str) -> &str { - match key { - "enable" => "positron.r.diagnostics.enable", - _ => "unknown", // To be caught via downstream errors - } - } -} - -impl From for DiagnosticsConfig { - fn from(value: VscDiagnosticsConfig) -> Self { +impl Default for IndentationConfig { + fn default() -> Self { Self { - enable: value.enable, + indent_style: IndentStyle::Space, + indent_size: 2, + tab_width: 2, } } } diff --git a/crates/ark/src/lsp/handlers.rs b/crates/ark/src/lsp/handlers.rs index d6a6ea118..cc468c5ec 100644 --- a/crates/ark/src/lsp/handlers.rs +++ b/crates/ark/src/lsp/handlers.rs @@ -9,7 +9,6 @@ use anyhow::anyhow; use serde_json::Value; use stdext::unwrap; use stdext::unwrap::IntoResult; -use struct_field_names_as_array::FieldNamesAsArray; use tower_lsp::lsp_types::CodeActionParams; use tower_lsp::lsp_types::CodeActionResponse; use tower_lsp::lsp_types::CompletionItem; @@ -46,8 +45,6 @@ use crate::lsp; use crate::lsp::code_action::code_actions; use crate::lsp::completions::provide_completions; use crate::lsp::completions::resolve_completion; -use crate::lsp::config::VscDiagnosticsConfig; -use crate::lsp::config::VscDocumentConfig; use crate::lsp::definitions::goto_definition; use crate::lsp::document_context::DocumentContext; use crate::lsp::encoding::convert_lsp_range_to_tree_sitter_range; @@ -105,17 +102,16 @@ pub(crate) async fn handle_initialized( // Note that some settings, such as editor indentation properties, may be // changed by extensions or by the user without changing the actual // underlying setting. Unfortunately we don't receive updates in that case. - let mut config_document_regs = collect_regs( - VscDocumentConfig::FIELD_NAMES_AS_ARRAY.to_vec(), - VscDocumentConfig::section_from_key, - ); - let mut config_diagnostics_regs: Vec = collect_regs( - VscDiagnosticsConfig::FIELD_NAMES_AS_ARRAY.to_vec(), - VscDiagnosticsConfig::section_from_key, - ); - - regs.append(&mut config_document_regs); - regs.append(&mut config_diagnostics_regs); + + use crate::lsp::config::SETTINGS; + + for setting in SETTINGS { + regs.push(Registration { + id: uuid::Uuid::new_v4().to_string(), + method: String::from("workspace/didChangeConfiguration"), + register_options: Some(serde_json::json!({ "section": setting.key })), + }); + } } client @@ -125,22 +121,12 @@ pub(crate) async fn handle_initialized( Ok(()) } -fn collect_regs(fields: Vec<&str>, into_section: impl Fn(&str) -> &str) -> Vec { - fields - .into_iter() - .map(|field| Registration { - id: uuid::Uuid::new_v4().to_string(), - method: String::from("workspace/didChangeConfiguration"), - register_options: Some(serde_json::json!({ "section": into_section(field) })), - }) - .collect() -} - #[tracing::instrument(level = "info", skip_all)] pub(crate) fn handle_symbol( params: WorkspaceSymbolParams, + state: &WorldState, ) -> anyhow::Result>> { - symbols::symbols(¶ms) + symbols::symbols(¶ms, state) .map(|res| Some(res)) .or_else(|err| { // Missing doc: Why are we not propagating errors to the frontend? diff --git a/crates/ark/src/lsp/indexer.rs b/crates/ark/src/lsp/indexer.rs index 34f018c7b..dc64e5a79 100644 --- a/crates/ark/src/lsp/indexer.rs +++ b/crates/ark/src/lsp/indexer.rs @@ -121,10 +121,17 @@ fn insert(path: &Path, entry: IndexEntry) -> anyhow::Result<()> { let index = index.entry(path.to_string()).or_default(); - // Retain the first occurrence in the index. In the future we'll track every occurrences and - // their scopes but for now we only track the first definition of an object (in a way, its + // We generally retain only the first occurrence in the index. In the + // future we'll track every occurrences and their scopes but for now we + // only track the first definition of an object (in a way, its // declaration). - if !index.contains_key(&entry.key) { + if let Some(existing_entry) = index.get(&entry.key) { + // Give priority to non-section entries. + if matches!(existing_entry.data, IndexEntryData::Section { .. }) { + index.insert(entry.key.clone(), entry); + } + // Else, ignore. + } else { index.insert(entry.key.clone(), entry); } diff --git a/crates/ark/src/lsp/main_loop.rs b/crates/ark/src/lsp/main_loop.rs index ac5588cea..0ee4be1a3 100644 --- a/crates/ark/src/lsp/main_loop.rs +++ b/crates/ark/src/lsp/main_loop.rs @@ -281,7 +281,7 @@ impl GlobalState { respond(tx, || state_handlers::initialize(params, &mut self.lsp_state, &mut self.world), LspResponse::Initialize)?; }, LspRequest::WorkspaceSymbol(params) => { - respond(tx, || handlers::handle_symbol(params), LspResponse::WorkspaceSymbol)?; + respond(tx, || handlers::handle_symbol(params, &self.world), LspResponse::WorkspaceSymbol)?; }, LspRequest::DocumentSymbol(params) => { respond(tx, || handlers::handle_document_symbol(params, &self.world), LspResponse::DocumentSymbol)?; diff --git a/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_assignment_function_nested.snap b/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_assignment_function_nested.snap new file mode 100644 index 000000000..c12ca2b32 --- /dev/null +++ b/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_assignment_function_nested.snap @@ -0,0 +1,71 @@ +--- +source: crates/ark/src/lsp/symbols.rs +expression: "test_symbol(\"foo <- function() { bar <- function() 1 }\")" +--- +[ + DocumentSymbol { + name: "foo", + detail: Some( + "function()", + ), + kind: Function, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 0, + character: 0, + }, + end: Position { + line: 0, + character: 41, + }, + }, + selection_range: Range { + start: Position { + line: 0, + character: 0, + }, + end: Position { + line: 0, + character: 41, + }, + }, + children: Some( + [ + DocumentSymbol { + name: "bar", + detail: Some( + "function()", + ), + kind: Function, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 0, + character: 20, + }, + end: Position { + line: 0, + character: 39, + }, + }, + selection_range: Range { + start: Position { + line: 0, + character: 20, + }, + end: Position { + line: 0, + character: 39, + }, + }, + children: Some( + [], + ), + }, + ], + ), + }, +] diff --git a/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_call_arguments.snap b/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_call_arguments.snap new file mode 100644 index 000000000..e316696fa --- /dev/null +++ b/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_call_arguments.snap @@ -0,0 +1,69 @@ +--- +source: crates/ark/src/lsp/symbols.rs +expression: "test_symbol(\"\n# section ----\nlocal({\n a <- function() {\n 1\n }\n})\n\")" +--- +[ + DocumentSymbol { + name: "section", + detail: None, + kind: String, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 1, + character: 0, + }, + end: Position { + line: 6, + character: 2, + }, + }, + selection_range: Range { + start: Position { + line: 1, + character: 0, + }, + end: Position { + line: 6, + character: 2, + }, + }, + children: Some( + [ + DocumentSymbol { + name: "a", + detail: Some( + "function()", + ), + kind: Function, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 3, + character: 4, + }, + end: Position { + line: 5, + character: 5, + }, + }, + selection_range: Range { + start: Position { + line: 3, + character: 4, + }, + end: Position { + line: 5, + character: 5, + }, + }, + children: Some( + [], + ), + }, + ], + ), + }, +] diff --git a/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_call_methods.snap b/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_call_methods.snap new file mode 100644 index 000000000..4cb58b084 --- /dev/null +++ b/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_call_methods.snap @@ -0,0 +1,197 @@ +--- +source: crates/ark/src/lsp/symbols.rs +expression: "test_symbol(\"\n# section ----\nlist(\n foo = function() {\n 1\n # nested section ----\n nested <- function() {}\n }, # matched\n function() {\n 2\n # `nested` is a symbol even if the unnamed method is not\n nested <- function () {\n }\n }, # not matched\n bar = function() {\n 3\n }, # matched\n baz = (function() {\n 4\n }) # not matched\n)\n\")" +--- +[ + DocumentSymbol { + name: "section", + detail: None, + kind: String, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 1, + character: 0, + }, + end: Position { + line: 20, + character: 1, + }, + }, + selection_range: Range { + start: Position { + line: 1, + character: 0, + }, + end: Position { + line: 20, + character: 1, + }, + }, + children: Some( + [ + DocumentSymbol { + name: "foo", + detail: Some( + "function()", + ), + kind: Method, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 3, + character: 10, + }, + end: Position { + line: 7, + character: 5, + }, + }, + selection_range: Range { + start: Position { + line: 3, + character: 10, + }, + end: Position { + line: 7, + character: 5, + }, + }, + children: Some( + [ + DocumentSymbol { + name: "nested section", + detail: None, + kind: String, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 5, + character: 8, + }, + end: Position { + line: 6, + character: 31, + }, + }, + selection_range: Range { + start: Position { + line: 5, + character: 8, + }, + end: Position { + line: 6, + character: 31, + }, + }, + children: Some( + [ + DocumentSymbol { + name: "nested", + detail: Some( + "function()", + ), + kind: Function, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 6, + character: 8, + }, + end: Position { + line: 6, + character: 31, + }, + }, + selection_range: Range { + start: Position { + line: 6, + character: 8, + }, + end: Position { + line: 6, + character: 31, + }, + }, + children: Some( + [], + ), + }, + ], + ), + }, + ], + ), + }, + DocumentSymbol { + name: "nested", + detail: Some( + "function()", + ), + kind: Function, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 11, + character: 8, + }, + end: Position { + line: 12, + character: 5, + }, + }, + selection_range: Range { + start: Position { + line: 11, + character: 8, + }, + end: Position { + line: 12, + character: 5, + }, + }, + children: Some( + [], + ), + }, + DocumentSymbol { + name: "bar", + detail: Some( + "function()", + ), + kind: Method, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 14, + character: 10, + }, + end: Position { + line: 16, + character: 5, + }, + }, + selection_range: Range { + start: Position { + line: 14, + character: 10, + }, + end: Position { + line: 16, + character: 5, + }, + }, + children: Some( + [], + ), + }, + ], + ), + }, +] diff --git a/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_call_test_that.snap b/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_call_test_that.snap new file mode 100644 index 000000000..d177e781c --- /dev/null +++ b/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_call_test_that.snap @@ -0,0 +1,222 @@ +--- +source: crates/ark/src/lsp/symbols.rs +expression: "test_symbol(\"\ntest_that_not('foo', {\n 1\n})\n\n# title ----\n\ntest_that('foo', {\n # title1 ----\n 1\n # title2 ----\n foo <- function() {\n 2\n }\n})\n\n# title2 ----\ntest_that('bar', {\n 1\n})\n\")" +--- +[ + DocumentSymbol { + name: "title", + detail: None, + kind: String, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 5, + character: 0, + }, + end: Position { + line: 15, + character: 0, + }, + }, + selection_range: Range { + start: Position { + line: 5, + character: 0, + }, + end: Position { + line: 15, + character: 0, + }, + }, + children: Some( + [ + DocumentSymbol { + name: "Test: foo", + detail: None, + kind: Function, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 7, + character: 0, + }, + end: Position { + line: 14, + character: 2, + }, + }, + selection_range: Range { + start: Position { + line: 7, + character: 0, + }, + end: Position { + line: 14, + character: 2, + }, + }, + children: Some( + [ + DocumentSymbol { + name: "title1", + detail: None, + kind: String, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 8, + character: 2, + }, + end: Position { + line: 9, + character: 3, + }, + }, + selection_range: Range { + start: Position { + line: 8, + character: 2, + }, + end: Position { + line: 9, + character: 3, + }, + }, + children: Some( + [], + ), + }, + DocumentSymbol { + name: "title2", + detail: None, + kind: String, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 10, + character: 2, + }, + end: Position { + line: 13, + character: 3, + }, + }, + selection_range: Range { + start: Position { + line: 10, + character: 2, + }, + end: Position { + line: 13, + character: 3, + }, + }, + children: Some( + [ + DocumentSymbol { + name: "foo", + detail: Some( + "function()", + ), + kind: Function, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 11, + character: 2, + }, + end: Position { + line: 13, + character: 3, + }, + }, + selection_range: Range { + start: Position { + line: 11, + character: 2, + }, + end: Position { + line: 13, + character: 3, + }, + }, + children: Some( + [], + ), + }, + ], + ), + }, + ], + ), + }, + ], + ), + }, + DocumentSymbol { + name: "title2", + detail: None, + kind: String, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 16, + character: 0, + }, + end: Position { + line: 19, + character: 2, + }, + }, + selection_range: Range { + start: Position { + line: 16, + character: 0, + }, + end: Position { + line: 19, + character: 2, + }, + }, + children: Some( + [ + DocumentSymbol { + name: "Test: bar", + detail: None, + kind: Function, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 17, + character: 0, + }, + end: Position { + line: 19, + character: 2, + }, + }, + selection_range: Range { + start: Position { + line: 17, + character: 0, + }, + end: Position { + line: 19, + character: 2, + }, + }, + children: Some( + [], + ), + }, + ], + ), + }, +] diff --git a/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_nested_assignments.snap b/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_nested_assignments.snap new file mode 100644 index 000000000..4cc8ab1c1 --- /dev/null +++ b/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_nested_assignments.snap @@ -0,0 +1,71 @@ +--- +source: crates/ark/src/lsp/symbols.rs +expression: "test_symbol(\"\nlocal({\n inner1 <- 1 # Not a symbol\n})\na <- function() {\n inner2 <- 2 # Not a symbol\n inner3 <- function() 3 # Symbol\n}\n\")" +--- +[ + DocumentSymbol { + name: "a", + detail: Some( + "function()", + ), + kind: Function, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 4, + character: 0, + }, + end: Position { + line: 7, + character: 1, + }, + }, + selection_range: Range { + start: Position { + line: 4, + character: 0, + }, + end: Position { + line: 7, + character: 1, + }, + }, + children: Some( + [ + DocumentSymbol { + name: "inner3", + detail: Some( + "function()", + ), + kind: Function, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 6, + character: 2, + }, + end: Position { + line: 6, + character: 24, + }, + }, + selection_range: Range { + start: Position { + line: 6, + character: 2, + }, + end: Position { + line: 6, + character: 24, + }, + }, + children: Some( + [], + ), + }, + ], + ), + }, +] diff --git a/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_rhs.snap b/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_rhs.snap new file mode 100644 index 000000000..b5e6f1922 --- /dev/null +++ b/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_rhs.snap @@ -0,0 +1,164 @@ +--- +source: crates/ark/src/lsp/symbols.rs +expression: "test_symbol(\"\n# section ----\nclass <- r6::r6class(\n 'class',\n public = list(\n initialize = function() 'initialize',\n foo = function() 'foo'\n ),\n private = list(\n bar = function() 'bar'\n )\n)\n\")" +--- +[ + DocumentSymbol { + name: "section", + detail: None, + kind: String, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 1, + character: 0, + }, + end: Position { + line: 11, + character: 1, + }, + }, + selection_range: Range { + start: Position { + line: 1, + character: 0, + }, + end: Position { + line: 11, + character: 1, + }, + }, + children: Some( + [ + DocumentSymbol { + name: "class", + detail: None, + kind: Variable, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 2, + character: 0, + }, + end: Position { + line: 2, + character: 5, + }, + }, + selection_range: Range { + start: Position { + line: 2, + character: 0, + }, + end: Position { + line: 2, + character: 5, + }, + }, + children: Some( + [ + DocumentSymbol { + name: "initialize", + detail: Some( + "function()", + ), + kind: Method, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 5, + character: 17, + }, + end: Position { + line: 5, + character: 40, + }, + }, + selection_range: Range { + start: Position { + line: 5, + character: 17, + }, + end: Position { + line: 5, + character: 40, + }, + }, + children: Some( + [], + ), + }, + DocumentSymbol { + name: "foo", + detail: Some( + "function()", + ), + kind: Method, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 6, + character: 10, + }, + end: Position { + line: 6, + character: 26, + }, + }, + selection_range: Range { + start: Position { + line: 6, + character: 10, + }, + end: Position { + line: 6, + character: 26, + }, + }, + children: Some( + [], + ), + }, + DocumentSymbol { + name: "bar", + detail: Some( + "function()", + ), + kind: Method, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 9, + character: 10, + }, + end: Position { + line: 9, + character: 26, + }, + }, + selection_range: Range { + start: Position { + line: 9, + character: 10, + }, + end: Position { + line: 9, + character: 26, + }, + }, + children: Some( + [], + ), + }, + ], + ), + }, + ], + ), + }, +] diff --git a/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_rhs_braced_list.snap b/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_rhs_braced_list.snap new file mode 100644 index 000000000..95c351682 --- /dev/null +++ b/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_rhs_braced_list.snap @@ -0,0 +1,69 @@ +--- +source: crates/ark/src/lsp/symbols.rs +expression: "test_symbol(\"\nfoo <- {\n bar <- function() {}\n}\n\")" +--- +[ + DocumentSymbol { + name: "foo", + detail: None, + kind: Variable, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 1, + character: 0, + }, + end: Position { + line: 1, + character: 3, + }, + }, + selection_range: Range { + start: Position { + line: 1, + character: 0, + }, + end: Position { + line: 1, + character: 3, + }, + }, + children: Some( + [ + DocumentSymbol { + name: "bar", + detail: Some( + "function()", + ), + kind: Function, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 2, + character: 4, + }, + end: Position { + line: 2, + character: 24, + }, + }, + selection_range: Range { + start: Position { + line: 2, + character: 4, + }, + end: Position { + line: 2, + character: 24, + }, + }, + children: Some( + [], + ), + }, + ], + ), + }, +] diff --git a/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_rhs_methods.snap b/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_rhs_methods.snap new file mode 100644 index 000000000..b5e6f1922 --- /dev/null +++ b/crates/ark/src/lsp/snapshots/ark__lsp__symbols__tests__symbol_rhs_methods.snap @@ -0,0 +1,164 @@ +--- +source: crates/ark/src/lsp/symbols.rs +expression: "test_symbol(\"\n# section ----\nclass <- r6::r6class(\n 'class',\n public = list(\n initialize = function() 'initialize',\n foo = function() 'foo'\n ),\n private = list(\n bar = function() 'bar'\n )\n)\n\")" +--- +[ + DocumentSymbol { + name: "section", + detail: None, + kind: String, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 1, + character: 0, + }, + end: Position { + line: 11, + character: 1, + }, + }, + selection_range: Range { + start: Position { + line: 1, + character: 0, + }, + end: Position { + line: 11, + character: 1, + }, + }, + children: Some( + [ + DocumentSymbol { + name: "class", + detail: None, + kind: Variable, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 2, + character: 0, + }, + end: Position { + line: 2, + character: 5, + }, + }, + selection_range: Range { + start: Position { + line: 2, + character: 0, + }, + end: Position { + line: 2, + character: 5, + }, + }, + children: Some( + [ + DocumentSymbol { + name: "initialize", + detail: Some( + "function()", + ), + kind: Method, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 5, + character: 17, + }, + end: Position { + line: 5, + character: 40, + }, + }, + selection_range: Range { + start: Position { + line: 5, + character: 17, + }, + end: Position { + line: 5, + character: 40, + }, + }, + children: Some( + [], + ), + }, + DocumentSymbol { + name: "foo", + detail: Some( + "function()", + ), + kind: Method, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 6, + character: 10, + }, + end: Position { + line: 6, + character: 26, + }, + }, + selection_range: Range { + start: Position { + line: 6, + character: 10, + }, + end: Position { + line: 6, + character: 26, + }, + }, + children: Some( + [], + ), + }, + DocumentSymbol { + name: "bar", + detail: Some( + "function()", + ), + kind: Method, + tags: None, + deprecated: None, + range: Range { + start: Position { + line: 9, + character: 10, + }, + end: Position { + line: 9, + character: 26, + }, + }, + selection_range: Range { + start: Position { + line: 9, + character: 10, + }, + end: Position { + line: 9, + character: 26, + }, + }, + children: Some( + [], + ), + }, + ], + ), + }, + ], + ), + }, +] diff --git a/crates/ark/src/lsp/state_handlers.rs b/crates/ark/src/lsp/state_handlers.rs index c5f2a20e0..b748d038d 100644 --- a/crates/ark/src/lsp/state_handlers.rs +++ b/crates/ark/src/lsp/state_handlers.rs @@ -8,11 +8,8 @@ use std::path::Path; use anyhow::anyhow; -use serde_json::Value; -use struct_field_names_as_array::FieldNamesAsArray; use tower_lsp::lsp_types::CompletionOptions; use tower_lsp::lsp_types::CompletionOptionsCompletionItem; -use tower_lsp::lsp_types::ConfigurationItem; use tower_lsp::lsp_types::DidChangeConfigurationParams; use tower_lsp::lsp_types::DidChangeTextDocumentParams; use tower_lsp::lsp_types::DidCloseTextDocumentParams; @@ -42,10 +39,6 @@ use url::Url; use crate::lsp; use crate::lsp::capabilities::Capabilities; use crate::lsp::config::indent_style_from_lsp; -use crate::lsp::config::DocumentConfig; -use crate::lsp::config::VscDiagnosticsConfig; -use crate::lsp::config::VscDocumentConfig; -use crate::lsp::diagnostics::DiagnosticsConfig; use crate::lsp::documents::Document; use crate::lsp::encoding::get_position_encoding_kind; use crate::lsp::indexer; @@ -244,6 +237,9 @@ pub(crate) async fn did_change_configuration( // we should just ignore it. Instead we need to pull the settings again for // all URI of interest. + // Note that the client sends notifications for settings for which we have + // declared interest in. This registration is done in `handle_initialized()`. + update_config(workspace_uris(state), client, state) .instrument(tracing::info_span!("did_change_configuration")) .await @@ -279,97 +275,35 @@ pub(crate) fn did_change_formatting_options( // `insert_final_newline` } +use crate::lsp::config::SETTINGS; + async fn update_config( - uris: Vec, + _uris: Vec, client: &tower_lsp::Client, state: &mut WorldState, ) -> anyhow::Result<()> { - let mut items: Vec = vec![]; - - let diagnostics_keys = VscDiagnosticsConfig::FIELD_NAMES_AS_ARRAY; - let mut diagnostics_items: Vec = diagnostics_keys + // Build the configuration request for global settings + let items: Vec<_> = SETTINGS .iter() - .map(|key| ConfigurationItem { + .map(|mapping| tower_lsp::lsp_types::ConfigurationItem { scope_uri: None, - section: Some(VscDiagnosticsConfig::section_from_key(key).into()), + section: Some(mapping.key.to_string()), }) .collect(); - items.append(&mut diagnostics_items); - - // For document configs we collect all pairs of URIs and config keys of - // interest in a flat vector - let document_keys = VscDocumentConfig::FIELD_NAMES_AS_ARRAY; - let mut document_items: Vec = - itertools::iproduct!(uris.iter(), document_keys.iter()) - .map(|(uri, key)| ConfigurationItem { - scope_uri: Some(uri.clone()), - section: Some(VscDocumentConfig::section_from_key(key).into()), - }) - .collect(); - items.append(&mut document_items); let configs = client.configuration(items).await?; - // We got the config items in a flat vector that's guaranteed to be - // ordered in the same way it was sent in. Be defensive and check that - // we've got the expected number of items before we process them chunk - // by chunk - let n_document_items = document_keys.len(); - let n_diagnostics_items = diagnostics_keys.len(); - let n_items = n_diagnostics_items + (n_document_items * uris.len()); - - if configs.len() != n_items { + if configs.len() != SETTINGS.len() { return Err(anyhow!( "Unexpected number of retrieved configurations: {}/{}", configs.len(), - n_items + SETTINGS.len() )); } - let mut configs = configs.into_iter(); - - // --- Diagnostics - let keys = diagnostics_keys.into_iter(); - let items: Vec = configs.by_ref().take(n_diagnostics_items).collect(); - - // Create a new `serde_json::Value::Object` manually to convert it - // to a `VscDocumentConfig` with `from_value()`. This way serde_json - // can type-check the dynamic JSON value we got from the client. - let mut map = serde_json::Map::new(); - std::iter::zip(keys, items).for_each(|(key, item)| { - map.insert(key.into(), item); - }); - - // Deserialise the VS Code configuration - let config: VscDiagnosticsConfig = serde_json::from_value(serde_json::Value::Object(map))?; - let config: DiagnosticsConfig = config.into(); - - let changed = state.config.diagnostics != config; - state.config.diagnostics = config; - - if changed { - lsp::spawn_diagnostics_refresh_all(state.clone()); - } - - // --- Documents - // For each document, deserialise the vector of JSON values into a typed config - for uri in uris.into_iter() { - let keys = document_keys.into_iter(); - let items: Vec = configs.by_ref().take(n_document_items).collect(); - - let mut map = serde_json::Map::new(); - std::iter::zip(keys, items).for_each(|(key, item)| { - map.insert(key.into(), item); - }); - - // Deserialise the VS Code configuration - let config: VscDocumentConfig = serde_json::from_value(serde_json::Value::Object(map))?; - - // Now convert the VS Code specific type into our own type - let config: DocumentConfig = config.into(); - - // Finally, update the document's config - state.get_document_mut(&uri)?.config = config; + // Apply each config value using its update closure + for (mapping, value) in SETTINGS.iter().zip(configs) { + (mapping.set)(&mut state.config, value); } Ok(()) diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index 26e23ab19..ad70ff049 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -56,7 +56,10 @@ fn new_symbol_node( symbol } -pub fn symbols(params: &WorkspaceSymbolParams) -> anyhow::Result> { +pub(crate) fn symbols( + params: &WorkspaceSymbolParams, + state: &WorldState, +) -> anyhow::Result> { let query = ¶ms.query; let mut info: Vec = Vec::new(); @@ -81,18 +84,21 @@ pub fn symbols(params: &WorkspaceSymbolParams) -> anyhow::Result { - info.push(SymbolInformation { - name: title.to_string(), - kind: SymbolKind::STRING, - location: Location { - uri: Url::from_file_path(path).unwrap(), - range: entry.range, - }, - tags: None, - deprecated: None, - container_name: None, - }); + if state.config.workspace_symbols.include_comment_sections { + info.push(SymbolInformation { + name: title.to_string(), + kind: SymbolKind::STRING, + location: Location { + uri: Url::from_file_path(path).unwrap(), + range: entry.range, + }, + tags: None, + deprecated: None, + container_name: None, + }); + } }, + IndexEntryData::Variable { name } => { info.push(SymbolInformation { name: name.clone(), @@ -122,6 +128,20 @@ struct Section { children: Vec, } +struct CollectContext { + top_level: bool, + include_assignments_in_blocks: bool, +} + +impl CollectContext { + fn new() -> Self { + Self { + top_level: true, + include_assignments_in_blocks: false, + } + } +} + pub(crate) fn document_symbols( state: &WorldState, params: &DocumentSymbolParams, @@ -135,8 +155,11 @@ pub(crate) fn document_symbols( let root_node = ast.root_node(); let mut result = Vec::new(); + let mut ctx = CollectContext::new(); + ctx.include_assignments_in_blocks = state.config.symbols.include_assignments_in_blocks; + // Extract and process all symbols from the AST - if let Err(err) = collect_symbols(&root_node, contents, 0, &mut result) { + if let Err(err) = collect_symbols(&mut ctx, &root_node, contents, 0, &mut result) { log::error!("Failed to collect symbols: {err:?}"); return Ok(Vec::new()); } @@ -146,29 +169,39 @@ pub(crate) fn document_symbols( /// Collect all document symbols from a node recursively fn collect_symbols( + ctx: &mut CollectContext, node: &Node, contents: &Rope, current_level: usize, symbols: &mut Vec, ) -> anyhow::Result<()> { match node.node_type() { - NodeType::Program | NodeType::BracedExpression => { - collect_sections(node, contents, current_level, symbols)?; + NodeType::Program => { + collect_sections(ctx, node, contents, current_level, symbols)?; + }, + + NodeType::BracedExpression => { + ctx.top_level = false; + collect_sections(ctx, node, contents, current_level, symbols)?; + }, + + NodeType::Call => { + collect_call(ctx, node, contents, symbols)?; }, NodeType::BinaryOperator(BinaryOperatorType::LeftAssignment) | NodeType::BinaryOperator(BinaryOperatorType::EqualsAssignment) => { - collect_assignment(node, contents, symbols)?; + collect_assignment(ctx, node, contents, symbols)?; }, // For all other node types, no symbols need to be added _ => {}, } - Ok(()) } fn collect_sections( + ctx: &mut CollectContext, node: &Node, contents: &Rope, current_level: usize, @@ -220,11 +253,11 @@ fn collect_sections( if active_sections.is_empty() { // If no active section, extend current vector of symbols - collect_symbols(&child, contents, current_level, symbols)?; + collect_symbols(ctx, &child, contents, current_level, symbols)?; } else { // Otherwise create new store of symbols for the current section let mut child_symbols = Vec::new(); - collect_symbols(&child, contents, current_level, &mut child_symbols)?; + collect_symbols(ctx, &child, contents, current_level, &mut child_symbols)?; // Nest them inside last section if !child_symbols.is_empty() { @@ -253,43 +286,195 @@ fn collect_sections( Ok(()) } +fn collect_call( + ctx: &mut CollectContext, + node: &Node, + contents: &Rope, + symbols: &mut Vec, +) -> anyhow::Result<()> { + let Some(callee) = node.child_by_field_name("function") else { + return Ok(()); + }; + + if callee.is_identifier() { + let fun_symbol = contents.node_slice(&callee)?.to_string(); + match fun_symbol.as_str() { + "test_that" => return collect_call_test_that(ctx, node, contents, symbols), + _ => {}, // fallthrough + } + } + + collect_call_arguments(ctx, node, contents, symbols)?; + + Ok(()) +} + +fn collect_call_arguments( + ctx: &mut CollectContext, + node: &Node, + contents: &Rope, + symbols: &mut Vec, +) -> anyhow::Result<()> { + let Some(arguments) = node.child_by_field_name("arguments") else { + return Ok(()); + }; + + let mut cursor = arguments.walk(); + for arg in arguments.children(&mut cursor) { + let Some(arg_value) = arg.child_by_field_name("value") else { + continue; + }; + + // Recurse into arguments. They might be a braced list, another call + // that might contain functions, etc. + collect_symbols(ctx, &arg_value, contents, 0, symbols)?; + + if arg_value.kind() == "function_definition" { + if let Some(arg_fun) = arg.child_by_field_name("name") { + // If this is a named function, collect it as a method + collect_method(ctx, &arg_fun, &arg_value, contents, symbols)?; + } else { + // Otherwise, just recurse into the function + let body = arg_value.child_by_field_name("body").into_result()?; + collect_symbols(ctx, &body, contents, 0, symbols)?; + }; + } + } + + Ok(()) +} + +fn collect_method( + ctx: &mut CollectContext, + arg_fun: &Node, + arg_value: &Node, + contents: &Rope, + symbols: &mut Vec, +) -> anyhow::Result<()> { + if !arg_fun.is_identifier_or_string() { + return Ok(()); + } + let arg_name_str = contents.node_slice(&arg_fun)?.to_string(); + + let start = convert_point_to_position(contents, arg_value.start_position()); + let end = convert_point_to_position(contents, arg_value.end_position()); + + let body = arg_value.child_by_field_name("body").into_result()?; + let mut children = vec![]; + collect_symbols(ctx, &body, contents, 0, &mut children)?; + + let mut symbol = new_symbol_node( + arg_name_str, + SymbolKind::METHOD, + Range { start, end }, + children, + ); + + // Don't include whole function as detail as the body often doesn't + // provide useful information and only make the outline more busy (with + // curly braces, newline characters, etc). + symbol.detail = Some(String::from("function()")); + + symbols.push(symbol); + + Ok(()) +} + +// https://github.com/posit-dev/positron/issues/1428 +fn collect_call_test_that( + ctx: &mut CollectContext, + node: &Node, + contents: &Rope, + symbols: &mut Vec, +) -> anyhow::Result<()> { + let Some(arguments) = node.child_by_field_name("arguments") else { + return Ok(()); + }; + + // We don't do any argument matching and just consider the first argument if + // a string. First skip over `(`. + let Some(first_argument) = arguments.child(1).and_then(|n| n.child(0)) else { + return Ok(()); + }; + if !first_argument.is_string() { + return Ok(()); + } + + let Some(string) = first_argument.child_by_field_name("content") else { + return Ok(()); + }; + + // Recurse in arguments. We could skip the first one if we wanted. + let mut children = Vec::new(); + let mut cursor = arguments.walk(); + for child in arguments.children_by_field_name("argument", &mut cursor) { + if let Some(value) = child.child_by_field_name("value") { + collect_symbols(ctx, &value, contents, 0, &mut children)?; + } + } + + let name = contents.node_slice(&string)?.to_string(); + let name = format!("Test: {name}"); + + let start = convert_point_to_position(contents, node.start_position()); + let end = convert_point_to_position(contents, node.end_position()); + + let symbol = new_symbol_node(name, SymbolKind::FUNCTION, Range { start, end }, children); + symbols.push(symbol); + + Ok(()) +} + fn collect_assignment( + ctx: &mut CollectContext, node: &Node, contents: &Rope, symbols: &mut Vec, ) -> anyhow::Result<()> { - // Check for assignment - matches!( - node.node_type(), - NodeType::BinaryOperator(BinaryOperatorType::LeftAssignment) | - NodeType::BinaryOperator(BinaryOperatorType::EqualsAssignment) - ) - .into_result()?; + let (NodeType::BinaryOperator(BinaryOperatorType::LeftAssignment) | + NodeType::BinaryOperator(BinaryOperatorType::EqualsAssignment)) = node.node_type() + else { + return Ok(()); + }; - // check for lhs, rhs - let lhs = node.child_by_field_name("lhs").into_result()?; - let rhs = node.child_by_field_name("rhs").into_result()?; + let (Some(lhs), Some(rhs)) = ( + node.child_by_field_name("lhs"), + node.child_by_field_name("rhs"), + ) else { + return Ok(()); + }; - // check for identifier on lhs, function on rhs + // If a function, collect symbol as function let function = lhs.is_identifier_or_string() && rhs.is_function_definition(); - if function { - return collect_assignment_with_function(node, contents, symbols); + return collect_assignment_with_function(ctx, node, contents, symbols); } - // otherwise, just index as generic object - let name = contents.node_slice(&lhs)?.to_string(); + if ctx.top_level || ctx.include_assignments_in_blocks { + // Collect as generic object, but only if we're at top-level. Assigned + // objects in nested functions and blocks cause the outline to become + // too busy. + let name = contents.node_slice(&lhs)?.to_string(); - let start = convert_point_to_position(contents, lhs.start_position()); - let end = convert_point_to_position(contents, lhs.end_position()); + let start = convert_point_to_position(contents, lhs.start_position()); + let end = convert_point_to_position(contents, lhs.end_position()); - let symbol = new_symbol(name, SymbolKind::VARIABLE, Range { start, end }); - symbols.push(symbol); + // Now recurse into RHS + let mut children = Vec::new(); + collect_symbols(ctx, &rhs, contents, 0, &mut children)?; + + let symbol = new_symbol_node(name, SymbolKind::VARIABLE, Range { start, end }, children); + symbols.push(symbol); + } else { + // Recurse into RHS + collect_symbols(ctx, &rhs, contents, 0, symbols)?; + } Ok(()) } fn collect_assignment_with_function( + ctx: &mut CollectContext, node: &Node, contents: &Rope, symbols: &mut Vec, @@ -321,7 +506,7 @@ fn collect_assignment_with_function( // Process the function body to extract child symbols let mut children = Vec::new(); - collect_symbols(&body, contents, 0, &mut children)?; + collect_symbols(ctx, &body, contents, 0, &mut children)?; let mut symbol = new_symbol_node(name, SymbolKind::FUNCTION, range, children); symbol.detail = Some(detail); @@ -388,7 +573,14 @@ mod tests { let node = doc.ast.root_node(); let mut symbols = Vec::new(); - collect_symbols(&node, &doc.contents, 0, &mut symbols).unwrap(); + collect_symbols( + &mut CollectContext::new(), + &node, + &doc.contents, + 0, + &mut symbols, + ) + .unwrap(); symbols } @@ -466,33 +658,7 @@ mod tests { #[test] fn test_symbol_assignment_function_nested() { - let range = Range { - start: Position { - line: 0, - character: 20, - }, - end: Position { - line: 0, - character: 23, - }, - }; - let bar = new_symbol(String::from("bar"), SymbolKind::VARIABLE, range); - - let range = Range { - start: Position { - line: 0, - character: 0, - }, - end: Position { - line: 0, - character: 30, - }, - }; - let mut foo = new_symbol(String::from("foo"), SymbolKind::FUNCTION, range); - foo.children = Some(vec![bar]); - foo.detail = Some(String::from("function()")); - - assert_eq!(test_symbol("foo <- function() { bar <- 1 }"), vec![foo]); + insta::assert_debug_snapshot!(test_symbol("foo <- function() { bar <- function() 1 }")); } #[test] @@ -510,23 +676,6 @@ foo <- function() { )); } - #[test] - fn test_symbol_braced_list() { - let range = Range { - start: Position { - line: 0, - character: 2, - }, - end: Position { - line: 0, - character: 5, - }, - }; - let foo = new_symbol(String::from("foo"), SymbolKind::VARIABLE, range); - - assert_eq!(test_symbol("{ foo <- 1 }"), vec![foo]); - } - #[test] fn test_symbol_section_ranges_extend() { let symbols = test_symbol( @@ -587,4 +736,120 @@ z <- 3", assert_eq!(section_b.range.start.line, 4); assert_eq!(section_b.range.end.line, 5); // End of function body } + + #[test] + fn test_symbol_call_test_that() { + insta::assert_debug_snapshot!(test_symbol( + " +test_that_not('foo', { + 1 +}) + +# title ---- + +test_that('foo', { + # title1 ---- + 1 + # title2 ---- + foo <- function() { + 2 + } +}) + +# title2 ---- +test_that('bar', { + 1 +}) +" + )); + } + + #[test] + fn test_symbol_call_methods() { + insta::assert_debug_snapshot!(test_symbol( + " +# section ---- +list( + foo = function() { + 1 + # nested section ---- + nested <- function() {} + }, # matched + function() { + 2 + # `nested` is a symbol even if the unnamed method is not + nested <- function () { + } + }, # not matched + bar = function() { + 3 + }, # matched + baz = (function() { + 4 + }) # not matched +) +" + )); + } + + #[test] + fn test_symbol_call_arguments() { + insta::assert_debug_snapshot!(test_symbol( + " +# section ---- +local({ + a <- function() { + 1 + } +}) +" + )); + } + + #[test] + fn test_symbol_rhs_braced_list() { + insta::assert_debug_snapshot!(test_symbol( + " +foo <- { + bar <- function() {} +} +" + )); + } + + #[test] + fn test_symbol_rhs_methods() { + insta::assert_debug_snapshot!(test_symbol( + " +# section ---- +class <- r6::r6class( + 'class', + public = list( + initialize = function() 'initialize', + foo = function() 'foo' + ), + private = list( + bar = function() 'bar' + ) +) +" + )); + } + + #[test] + // Assigned variables in nested contexts are not emitted as symbols + fn test_symbol_nested_assignments() { + insta::assert_debug_snapshot!(test_symbol( + " +local({ + inner1 <- 1 # Not a symbol +}) +a <- function() { + inner2 <- 2 # Not a symbol + inner3 <- function() 3 # Symbol +} +" + )); + assert_eq!(test_symbol("{ foo <- 1 }"), vec![]); + } }