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/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_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/symbols.rs b/crates/ark/src/lsp/symbols.rs index 5ecb76391..5557f457a 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -265,16 +265,90 @@ fn collect_call( let Some(callee) = node.child_by_field_name("function") else { return Ok(()); }; - if !callee.is_identifier() { + + 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(node, contents, symbols), + _ => {}, // fallthrough + } + } + + collect_call_arguments(node, contents, symbols)?; + + Ok(()) +} + +fn collect_call_arguments( + node: &Node, + contents: &Rope, + symbols: &mut Vec, +) -> anyhow::Result<()> { + let Some(arguments) = node.child_by_field_name("arguments") else { return Ok(()); + }; + + let mut cursor = node.walk(); + for arg in arguments.children(&mut cursor) { + if arg.kind() != "argument" { + continue; + } + + 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(&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(&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(&body, contents, 0, symbols)?; + }; + } } - let fun_symbol = contents.node_slice(&callee)?.to_string(); + Ok(()) +} - match fun_symbol.as_str() { - "test_that" => collect_call_test_that(node, contents, symbols)?, - _ => {}, +fn collect_method( + 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(&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(()) } @@ -328,32 +402,36 @@ fn collect_assignment( 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); } - // otherwise, just index as generic object + // Otherwise, collect as generic object 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 symbol = new_symbol(name, SymbolKind::VARIABLE, Range { start, end }); + // Now recurse into RHS + let mut children = Vec::new(); + collect_symbols(&rhs, contents, 0, &mut children)?; + + let symbol = new_symbol_node(name, SymbolKind::VARIABLE, Range { start, end }, children); symbols.push(symbol); Ok(()) @@ -681,6 +759,78 @@ test_that('foo', { 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' + ) +) " )); }