From 9bdd1c434a4ca063aad3d0961e3b3b1278da02ff Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 21 Oct 2024 09:49:50 +0200 Subject: [PATCH 01/10] Remove unused `symbols` argument --- crates/ark/src/lsp/symbols.rs | 35 ++++++++++------------------------- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index 0d5d6645f..14ad8f6f0 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -80,8 +80,6 @@ pub(crate) fn document_symbols( state: &WorldState, params: &DocumentSymbolParams, ) -> anyhow::Result> { - let mut symbols: Vec = Vec::new(); - let uri = ¶ms.text_document.uri; let document = state.documents.get(uri).into_result()?; let ast = &document.ast; @@ -104,10 +102,10 @@ pub(crate) fn document_symbols( selection_range: Range { start, end }, }; - // index from the root - index_node(&node, &contents, &mut root, &mut symbols)?; + // Index from the root + index_node(&node, &contents, &mut root)?; - // return the children we found + // Return the children we found Ok(root.children.unwrap_or_default()) } @@ -136,12 +134,7 @@ fn parse_comment_as_section(comment: &str) -> Option<(usize, String)> { None } -fn index_node( - node: &Node, - contents: &Rope, - parent: &mut DocumentSymbol, - symbols: &mut Vec, -) -> Result { +fn index_node(node: &Node, contents: &Rope, parent: &mut DocumentSymbol) -> Result { // Check if the node is a comment and matches the markdown-style comment patterns if node.node_type() == NodeType::Comment { let comment_text = contents.node_slice(&node)?.to_string(); @@ -176,7 +169,7 @@ fn index_node( NodeType::BinaryOperator(BinaryOperatorType::LeftAssignment) | NodeType::BinaryOperator(BinaryOperatorType::EqualsAssignment) ) { - match index_assignment(node, contents, parent, symbols) { + match index_assignment(node, contents, parent) { Ok(handled) => { if handled { return Ok(true); @@ -190,7 +183,7 @@ fn index_node( let mut cursor = node.walk(); for child in node.children(&mut cursor) { if is_indexable(&child) { - let result = index_node(&child, contents, parent, symbols); + let result = index_node(&child, contents, parent); if let Err(error) = result { error!("{:?}", error); } @@ -200,12 +193,7 @@ fn index_node( Ok(true) } -fn index_assignment( - node: &Node, - contents: &Rope, - parent: &mut DocumentSymbol, - symbols: &mut Vec, -) -> Result { +fn index_assignment(node: &Node, contents: &Rope, parent: &mut DocumentSymbol) -> Result { // check for assignment matches!( node.node_type(), @@ -222,7 +210,7 @@ fn index_assignment( let function = lhs.is_identifier_or_string() && rhs.is_function_definition(); if function { - return index_assignment_with_function(node, contents, parent, symbols); + return index_assignment_with_function(node, contents, parent); } // otherwise, just index as generic object @@ -252,7 +240,6 @@ fn index_assignment_with_function( node: &Node, contents: &Rope, parent: &mut DocumentSymbol, - symbols: &mut Vec, ) -> Result { // check for lhs, rhs let lhs = node.child_by_field_name("lhs").into_result()?; @@ -295,7 +282,7 @@ fn index_assignment_with_function( // recurse into this node let parent = parent.children.as_mut().unwrap().last_mut().unwrap(); - index_node(&rhs, contents, parent, symbols)?; + index_node(&rhs, contents, parent)?; Ok(true) } @@ -308,8 +295,6 @@ mod tests { use crate::lsp::documents::Document; fn test_symbol(code: &str) -> Vec { - let mut symbols: Vec = Vec::new(); - let doc = Document::new(code, None); let node = doc.ast.root_node(); @@ -327,7 +312,7 @@ mod tests { selection_range: Range { start, end }, }; - index_node(&node, &doc.contents, &mut root, &mut symbols).unwrap(); + index_node(&node, &doc.contents, &mut root).unwrap(); root.children.unwrap_or_default() } From 57d67765f68826cb60885ce240d00ec7e0a85cb3 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 21 Oct 2024 09:58:36 +0200 Subject: [PATCH 02/10] Return unit type --- crates/ark/src/lsp/symbols.rs | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index 14ad8f6f0..33e796a6b 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -9,7 +9,6 @@ use std::result::Result::Ok; -use anyhow::*; use log::*; use ropey::Rope; use stdext::unwrap::IntoResult; @@ -134,7 +133,7 @@ fn parse_comment_as_section(comment: &str) -> Option<(usize, String)> { None } -fn index_node(node: &Node, contents: &Rope, parent: &mut DocumentSymbol) -> Result { +fn index_node(node: &Node, contents: &Rope, parent: &mut DocumentSymbol) -> anyhow::Result<()> { // Check if the node is a comment and matches the markdown-style comment patterns if node.node_type() == NodeType::Comment { let comment_text = contents.node_slice(&node)?.to_string(); @@ -160,7 +159,7 @@ fn index_node(node: &Node, contents: &Rope, parent: &mut DocumentSymbol) -> Resu parent.children.as_mut().unwrap().push(symbol); // Return early to avoid further processing - return Ok(true); + return Ok(()); } } @@ -170,12 +169,10 @@ fn index_node(node: &Node, contents: &Rope, parent: &mut DocumentSymbol) -> Resu NodeType::BinaryOperator(BinaryOperatorType::EqualsAssignment) ) { match index_assignment(node, contents, parent) { - Ok(handled) => { - if handled { - return Ok(true); - } + Ok(()) => { + return Ok(()); }, - Err(error) => error!("{:?}", error), + Err(err) => error!("{err:?}"), } } @@ -190,10 +187,14 @@ fn index_node(node: &Node, contents: &Rope, parent: &mut DocumentSymbol) -> Resu } } - Ok(true) + Ok(()) } -fn index_assignment(node: &Node, contents: &Rope, parent: &mut DocumentSymbol) -> Result { +fn index_assignment( + node: &Node, + contents: &Rope, + parent: &mut DocumentSymbol, +) -> anyhow::Result<()> { // check for assignment matches!( node.node_type(), @@ -233,14 +234,14 @@ fn index_assignment(node: &Node, contents: &Rope, parent: &mut DocumentSymbol) - // add this symbol to the parent node parent.children.as_mut().unwrap().push(symbol); - Ok(true) + Ok(()) } fn index_assignment_with_function( node: &Node, contents: &Rope, parent: &mut DocumentSymbol, -) -> Result { +) -> anyhow::Result<()> { // check for lhs, rhs let lhs = node.child_by_field_name("lhs").into_result()?; let rhs = node.child_by_field_name("rhs").into_result()?; @@ -284,7 +285,7 @@ fn index_assignment_with_function( let parent = parent.children.as_mut().unwrap().last_mut().unwrap(); index_node(&rhs, contents, parent)?; - Ok(true) + Ok(()) } #[cfg(test)] From 76bfea4d14865e6f2161e52fc138603fc077c242 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 21 Oct 2024 10:17:16 +0200 Subject: [PATCH 03/10] Extract node ctor and document children access safety --- crates/ark/src/lsp/symbols.rs | 94 ++++++++++++++--------------------- 1 file changed, 37 insertions(+), 57 deletions(-) diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index 33e796a6b..242ac4d9e 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -32,6 +32,25 @@ use crate::treesitter::BinaryOperatorType; use crate::treesitter::NodeType; use crate::treesitter::NodeTypeExt; +fn new_symbol( + name: String, + kind: SymbolKind, + detail: Option, + range: Range, +) -> DocumentSymbol { + DocumentSymbol { + name, + kind, + detail, + // Safety: We assume `children` can't be `None` + children: Some(Vec::new()), + deprecated: None, + tags: None, + range, + selection_range: range, + } +} + pub fn symbols(params: &WorkspaceSymbolParams) -> anyhow::Result> { let query = ¶ms.query; let mut info: Vec = Vec::new(); @@ -89,17 +108,9 @@ pub(crate) fn document_symbols( let start = convert_point_to_position(contents, node.start_position()); let end = convert_point_to_position(contents, node.end_position()); - // construct a root symbol, so we always have something to append to - let mut root = DocumentSymbol { - name: "".to_string(), - kind: SymbolKind::NULL, - children: Some(Vec::new()), - deprecated: None, - tags: None, - detail: None, - range: Range { start, end }, - selection_range: Range { start, end }, - }; + // Construct a root symbol, so we always have something to append to + let range = Range { start, end }; + let mut root = new_symbol("".to_string(), SymbolKind::NULL, None, range); // Index from the root index_node(&node, &contents, &mut root)?; @@ -117,6 +128,12 @@ fn is_indexable(node: &Node) -> bool { true } +fn push_child(node: &mut DocumentSymbol, child: DocumentSymbol) { + // Safety: The LSP protocol wraps the list of children in an option but we + // always set it to an empty vector. + node.children.as_mut().unwrap().push(child); +} + // Function to parse a comment and return the section level and title fn parse_comment_as_section(comment: &str) -> Option<(usize, String)> { // Match lines starting with one or more '#' followed by some non-empty content and must end with 4 or more '-', '#', or `=` @@ -144,19 +161,8 @@ fn index_node(node: &Node, contents: &Rope, parent: &mut DocumentSymbol) -> anyh let start = convert_point_to_position(contents, node.start_position()); let end = convert_point_to_position(contents, node.end_position()); - let symbol = DocumentSymbol { - name: title, // Use the title without the trailing '####' or '----' - kind: SymbolKind::STRING, // Treat it as a string section - detail: None, // No need to display level details - children: Some(Vec::new()), // Prepare for child symbols if any - deprecated: None, - tags: None, - range: Range { start, end }, - selection_range: Range { start, end }, - }; - - // Add the symbol to the parent node - parent.children.as_mut().unwrap().push(symbol); + let symbol = new_symbol(title, SymbolKind::STRING, None, Range { start, end }); + push_child(parent, symbol); // Return early to avoid further processing return Ok(()); @@ -220,19 +226,8 @@ fn index_assignment( let start = convert_point_to_position(contents, lhs.start_position()); let end = convert_point_to_position(contents, lhs.end_position()); - let symbol = DocumentSymbol { - name, - kind: SymbolKind::VARIABLE, - detail: None, - children: Some(Vec::new()), - deprecated: None, - tags: None, - range: Range::new(start, end), - selection_range: Range::new(start, end), - }; - - // add this symbol to the parent node - parent.children.as_mut().unwrap().push(symbol); + let symbol = new_symbol(name, SymbolKind::VARIABLE, None, Range { start, end }); + push_child(parent, symbol); Ok(()) } @@ -260,28 +255,13 @@ fn index_assignment_with_function( let name = contents.node_slice(&lhs)?.to_string(); let detail = format!("function({})", arguments.join(", ")); - // build the document symbol - let symbol = DocumentSymbol { - name, - kind: SymbolKind::FUNCTION, - detail: Some(detail), - children: Some(Vec::new()), - deprecated: None, - tags: None, - range: Range { - start: convert_point_to_position(contents, lhs.start_position()), - end: convert_point_to_position(contents, rhs.end_position()), - }, - selection_range: Range { - start: convert_point_to_position(contents, lhs.start_position()), - end: convert_point_to_position(contents, lhs.end_position()), - }, + let range = Range { + start: convert_point_to_position(contents, lhs.start_position()), + end: convert_point_to_position(contents, rhs.end_position()), }; + let symbol = new_symbol(name, SymbolKind::FUNCTION, Some(detail), range); + push_child(parent, symbol); - // add this symbol to the parent node - parent.children.as_mut().unwrap().push(symbol); - - // recurse into this node let parent = parent.children.as_mut().unwrap().last_mut().unwrap(); index_node(&rhs, contents, parent)?; From aad04600dfb32764a867e9483d4560fcfe618edf Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 21 Oct 2024 10:29:33 +0200 Subject: [PATCH 04/10] Move parent while recursing --- crates/ark/src/lsp/symbols.rs | 67 ++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index 242ac4d9e..be3136bd4 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -9,7 +9,6 @@ use std::result::Result::Ok; -use log::*; use ropey::Rope; use stdext::unwrap::IntoResult; use tower_lsp::lsp_types::DocumentSymbol; @@ -110,13 +109,20 @@ pub(crate) fn document_symbols( // Construct a root symbol, so we always have something to append to let range = Range { start, end }; - let mut root = new_symbol("".to_string(), SymbolKind::NULL, None, range); + let root = new_symbol("".to_string(), SymbolKind::NULL, None, range); // Index from the root - index_node(&node, &contents, &mut root)?; + let root = match index_node(root, &node, &contents) { + Ok(root) => root, + Err(err) => { + log::error!("Error indexing node: {err:?}"); + return Ok(Vec::new()); + }, + }; - // Return the children we found - Ok(root.children.unwrap_or_default()) + // Return the children we found. Safety: We always set the children to an + // empty vector. + Ok(root.children.unwrap()) } fn is_indexable(node: &Node) -> bool { @@ -150,7 +156,11 @@ fn parse_comment_as_section(comment: &str) -> Option<(usize, String)> { None } -fn index_node(node: &Node, contents: &Rope, parent: &mut DocumentSymbol) -> anyhow::Result<()> { +fn index_node( + mut parent: DocumentSymbol, + node: &Node, + contents: &Rope, +) -> anyhow::Result { // Check if the node is a comment and matches the markdown-style comment patterns if node.node_type() == NodeType::Comment { let comment_text = contents.node_slice(&node)?.to_string(); @@ -162,10 +172,10 @@ fn index_node(node: &Node, contents: &Rope, parent: &mut DocumentSymbol) -> anyh let end = convert_point_to_position(contents, node.end_position()); let symbol = new_symbol(title, SymbolKind::STRING, None, Range { start, end }); - push_child(parent, symbol); + push_child(&mut parent, symbol); // Return early to avoid further processing - return Ok(()); + return Ok(parent); } } @@ -174,33 +184,25 @@ fn index_node(node: &Node, contents: &Rope, parent: &mut DocumentSymbol) -> anyh NodeType::BinaryOperator(BinaryOperatorType::LeftAssignment) | NodeType::BinaryOperator(BinaryOperatorType::EqualsAssignment) ) { - match index_assignment(node, contents, parent) { - Ok(()) => { - return Ok(()); - }, - Err(err) => error!("{err:?}"), - } + parent = index_assignment(parent, node, contents)?; } // Recurse into children let mut cursor = node.walk(); for child in node.children(&mut cursor) { if is_indexable(&child) { - let result = index_node(&child, contents, parent); - if let Err(error) = result { - error!("{:?}", error); - } + parent = index_node(parent, &child, contents)?; } } - Ok(()) + Ok(parent) } fn index_assignment( + mut parent: DocumentSymbol, node: &Node, contents: &Rope, - parent: &mut DocumentSymbol, -) -> anyhow::Result<()> { +) -> anyhow::Result { // check for assignment matches!( node.node_type(), @@ -217,7 +219,7 @@ fn index_assignment( let function = lhs.is_identifier_or_string() && rhs.is_function_definition(); if function { - return index_assignment_with_function(node, contents, parent); + return index_assignment_with_function(parent, node, contents); } // otherwise, just index as generic object @@ -227,16 +229,16 @@ fn index_assignment( let end = convert_point_to_position(contents, lhs.end_position()); let symbol = new_symbol(name, SymbolKind::VARIABLE, None, Range { start, end }); - push_child(parent, symbol); + push_child(&mut parent, symbol); - Ok(()) + Ok(parent) } fn index_assignment_with_function( + mut parent: DocumentSymbol, node: &Node, contents: &Rope, - parent: &mut DocumentSymbol, -) -> anyhow::Result<()> { +) -> anyhow::Result { // check for lhs, rhs let lhs = node.child_by_field_name("lhs").into_result()?; let rhs = node.child_by_field_name("rhs").into_result()?; @@ -260,12 +262,13 @@ fn index_assignment_with_function( end: convert_point_to_position(contents, rhs.end_position()), }; let symbol = new_symbol(name, SymbolKind::FUNCTION, Some(detail), range); - push_child(parent, symbol); - let parent = parent.children.as_mut().unwrap().last_mut().unwrap(); - index_node(&rhs, contents, parent)?; + // Recurse into the function node + let symbol = index_node(symbol, &rhs, contents)?; - Ok(()) + // Set as child after recursing, now that we own the symbol again + push_child(&mut parent, symbol); + Ok(parent) } #[cfg(test)] @@ -282,7 +285,7 @@ mod tests { let start = convert_point_to_position(&doc.contents, node.start_position()); let end = convert_point_to_position(&doc.contents, node.end_position()); - let mut root = DocumentSymbol { + let root = DocumentSymbol { name: String::from(""), kind: SymbolKind::NULL, children: Some(Vec::new()), @@ -293,7 +296,7 @@ mod tests { selection_range: Range { start, end }, }; - index_node(&node, &doc.contents, &mut root).unwrap(); + let root = index_node(root, &node, &doc.contents).unwrap(); root.children.unwrap_or_default() } From 0b4f9891c72f7ca7232f154ce2dcbe8c7f1dd044 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 21 Oct 2024 11:11:09 +0200 Subject: [PATCH 05/10] Move children while recursing --- crates/ark/src/lsp/symbols.rs | 186 ++++++++++++++++++++++------------ 1 file changed, 119 insertions(+), 67 deletions(-) diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index be3136bd4..e4e30dcb6 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -31,6 +31,18 @@ use crate::treesitter::BinaryOperatorType; use crate::treesitter::NodeType; use crate::treesitter::NodeTypeExt; +fn new_symbol_node( + name: String, + kind: SymbolKind, + detail: Option, + range: Range, + children: Vec, +) -> DocumentSymbol { + let mut symbol = new_symbol(name, kind, detail, range); + symbol.children = Some(children); + symbol +} + fn new_symbol( name: String, kind: SymbolKind, @@ -104,25 +116,14 @@ pub(crate) fn document_symbols( let node = ast.root_node(); - let start = convert_point_to_position(contents, node.start_position()); - let end = convert_point_to_position(contents, node.end_position()); - - // Construct a root symbol, so we always have something to append to - let range = Range { start, end }; - let root = new_symbol("".to_string(), SymbolKind::NULL, None, range); - // Index from the root - let root = match index_node(root, &node, &contents) { - Ok(root) => root, + match index_node(&node, vec![], &contents) { + Ok(children) => Ok(children), Err(err) => { log::error!("Error indexing node: {err:?}"); return Ok(Vec::new()); }, - }; - - // Return the children we found. Safety: We always set the children to an - // empty vector. - Ok(root.children.unwrap()) + } } fn is_indexable(node: &Node) -> bool { @@ -134,12 +135,6 @@ fn is_indexable(node: &Node) -> bool { true } -fn push_child(node: &mut DocumentSymbol, child: DocumentSymbol) { - // Safety: The LSP protocol wraps the list of children in an option but we - // always set it to an empty vector. - node.children.as_mut().unwrap().push(child); -} - // Function to parse a comment and return the section level and title fn parse_comment_as_section(comment: &str) -> Option<(usize, String)> { // Match lines starting with one or more '#' followed by some non-empty content and must end with 4 or more '-', '#', or `=` @@ -157,10 +152,10 @@ fn parse_comment_as_section(comment: &str) -> Option<(usize, String)> { } fn index_node( - mut parent: DocumentSymbol, node: &Node, + mut store: Vec, contents: &Rope, -) -> anyhow::Result { +) -> anyhow::Result> { // Check if the node is a comment and matches the markdown-style comment patterns if node.node_type() == NodeType::Comment { let comment_text = contents.node_slice(&node)?.to_string(); @@ -172,10 +167,10 @@ fn index_node( let end = convert_point_to_position(contents, node.end_position()); let symbol = new_symbol(title, SymbolKind::STRING, None, Range { start, end }); - push_child(&mut parent, symbol); + store.push(symbol); // Return early to avoid further processing - return Ok(parent); + return Ok(store); } } @@ -184,26 +179,26 @@ fn index_node( NodeType::BinaryOperator(BinaryOperatorType::LeftAssignment) | NodeType::BinaryOperator(BinaryOperatorType::EqualsAssignment) ) { - parent = index_assignment(parent, node, contents)?; + return index_assignment(node, store, contents); } - // Recurse into children + // Recurse into children. We're in the same outline section so use the same store. let mut cursor = node.walk(); for child in node.children(&mut cursor) { if is_indexable(&child) { - parent = index_node(parent, &child, contents)?; + store = index_node(&child, store, contents)?; } } - Ok(parent) + Ok(store) } fn index_assignment( - mut parent: DocumentSymbol, node: &Node, + mut store: Vec, contents: &Rope, -) -> anyhow::Result { - // check for assignment +) -> anyhow::Result> { + // Check for assignment matches!( node.node_type(), NodeType::BinaryOperator(BinaryOperatorType::LeftAssignment) | @@ -219,7 +214,7 @@ fn index_assignment( let function = lhs.is_identifier_or_string() && rhs.is_function_definition(); if function { - return index_assignment_with_function(parent, node, contents); + return index_assignment_with_function(node, store, contents); } // otherwise, just index as generic object @@ -229,16 +224,16 @@ fn index_assignment( let end = convert_point_to_position(contents, lhs.end_position()); let symbol = new_symbol(name, SymbolKind::VARIABLE, None, Range { start, end }); - push_child(&mut parent, symbol); + store.push(symbol); - Ok(parent) + Ok(store) } fn index_assignment_with_function( - mut parent: DocumentSymbol, node: &Node, + mut store: Vec, contents: &Rope, -) -> anyhow::Result { +) -> anyhow::Result> { // check for lhs, rhs let lhs = node.child_by_field_name("lhs").into_result()?; let rhs = node.child_by_field_name("rhs").into_result()?; @@ -261,14 +256,15 @@ fn index_assignment_with_function( start: convert_point_to_position(contents, lhs.start_position()), end: convert_point_to_position(contents, rhs.end_position()), }; - let symbol = new_symbol(name, SymbolKind::FUNCTION, Some(detail), range); - // Recurse into the function node - let symbol = index_node(symbol, &rhs, contents)?; + // At this point we increase the nesting level. Recurse into the function + // node with a new store of children nodes. + let children = index_node(&rhs, vec![], contents)?; + + let symbol = new_symbol_node(name, SymbolKind::FUNCTION, Some(detail), range, children); + store.push(symbol); - // Set as child after recursing, now that we own the symbol again - push_child(&mut parent, symbol); - Ok(parent) + Ok(store) } #[cfg(test)] @@ -282,22 +278,7 @@ mod tests { let doc = Document::new(code, None); let node = doc.ast.root_node(); - let start = convert_point_to_position(&doc.contents, node.start_position()); - let end = convert_point_to_position(&doc.contents, node.end_position()); - - let root = DocumentSymbol { - name: String::from(""), - kind: SymbolKind::NULL, - children: Some(Vec::new()), - deprecated: None, - tags: None, - detail: None, - range: Range { start, end }, - selection_range: Range { start, end }, - }; - - let root = index_node(root, &node, &doc.contents).unwrap(); - root.children.unwrap_or_default() + index_node(&node, vec![], &doc.contents).unwrap() } #[test] @@ -327,15 +308,86 @@ mod tests { character: 10, }, }; - assert_eq!(test_symbol("# foo ----"), vec![DocumentSymbol { - name: String::from("foo"), - kind: SymbolKind::STRING, - children: Some(Vec::new()), - deprecated: None, - tags: None, - detail: None, + assert_eq!(test_symbol("# foo ----"), vec![new_symbol( + String::from("foo"), + SymbolKind::STRING, + None, + range + )]); + } + + #[test] + fn test_symbol_assignment() { + let range = Range { + start: Position { + line: 0, + character: 0, + }, + end: Position { + line: 0, + character: 3, + }, + }; + assert_eq!(test_symbol("foo <- 1"), vec![new_symbol( + String::from("foo"), + SymbolKind::OBJECT, + None, range, - selection_range: range, - }]); + )]); + } + + #[test] + fn test_symbol_assignment_function() { + let range = Range { + start: Position { + line: 0, + character: 0, + }, + end: Position { + line: 0, + character: 20, + }, + }; + assert_eq!(test_symbol("foo <- function() {}"), vec![new_symbol( + String::from("foo"), + SymbolKind::FUNCTION, + Some(String::from("function()")), + range, + )]); + } + + #[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::OBJECT, None, 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, + Some(String::from("function()")), + range, + ); + foo.children = Some(vec![bar]); + + assert_eq!(test_symbol("foo <- function() { bar <- 1 }"), vec![foo]); } } From 9552ee99384c0c82d01ed107b35058aa3e6db295 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 21 Oct 2024 11:34:11 +0200 Subject: [PATCH 06/10] Move helper functions down below --- crates/ark/src/lsp/symbols.rs | 50 +++++++++++++++++------------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index e4e30dcb6..9bb2d51e3 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -126,31 +126,6 @@ pub(crate) fn document_symbols( } } -fn is_indexable(node: &Node) -> bool { - // don't index 'arguments' or 'parameters' - if matches!(node.node_type(), NodeType::Arguments | NodeType::Parameters) { - return false; - } - - true -} - -// Function to parse a comment and return the section level and title -fn parse_comment_as_section(comment: &str) -> Option<(usize, String)> { - // Match lines starting with one or more '#' followed by some non-empty content and must end with 4 or more '-', '#', or `=` - // Ensure that there's actual content between the start and the trailing symbols. - if let Some(caps) = indexer::RE_COMMENT_SECTION.captures(comment) { - let hashes = caps.get(1)?.as_str().len(); // Count the number of '#' - let title = caps.get(2)?.as_str().trim().to_string(); // Extract the title text without trailing punctuations - if title.is_empty() { - return None; // Return None for lines with only hashtags - } - return Some((hashes, title)); // Return the level based on the number of '#' and the title - } - - None -} - fn index_node( node: &Node, mut store: Vec, @@ -267,6 +242,31 @@ fn index_assignment_with_function( Ok(store) } +fn is_indexable(node: &Node) -> bool { + // Don't index 'arguments' or 'parameters' + if matches!(node.node_type(), NodeType::Arguments | NodeType::Parameters) { + return false; + } + + true +} + +// Function to parse a comment and return the section level and title +fn parse_comment_as_section(comment: &str) -> Option<(usize, String)> { + // Match lines starting with one or more '#' followed by some non-empty content and must end with 4 or more '-', '#', or `=` + // Ensure that there's actual content between the start and the trailing symbols. + if let Some(caps) = indexer::RE_COMMENT_SECTION.captures(comment) { + let hashes = caps.get(1)?.as_str().len(); // Count the number of '#' + let title = caps.get(2)?.as_str().trim().to_string(); // Extract the title text without trailing punctuations + if title.is_empty() { + return None; // Return None for lines with only hashtags + } + return Some((hashes, title)); // Return the level based on the number of '#' and the title + } + + None +} + #[cfg(test)] mod tests { use tower_lsp::lsp_types::Position; From e5af4c37ddc722005270b3128e93853cc336e7a3 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 21 Oct 2024 11:44:24 +0200 Subject: [PATCH 07/10] Move assignment and comment handling in loop --- crates/ark/src/lsp/symbols.rs | 71 +++++++++++++++++------------------ 1 file changed, 34 insertions(+), 37 deletions(-) diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index 9bb2d51e3..9e819150e 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -131,39 +131,45 @@ fn index_node( mut store: Vec, contents: &Rope, ) -> anyhow::Result> { - // Check if the node is a comment and matches the markdown-style comment patterns - if node.node_type() == NodeType::Comment { - let comment_text = contents.node_slice(&node)?.to_string(); + let mut cursor = node.walk(); - // Check if the comment starts with one or more '#' followed by any text and ends with 4+ punctuations - if let Some((_level, title)) = parse_comment_as_section(&comment_text) { - // Create a symbol based on the parsed comment - let start = convert_point_to_position(contents, node.start_position()); - let end = convert_point_to_position(contents, node.end_position()); + for child in node.children(&mut cursor) { + store = match child.node_type() { + // Index comment sections + NodeType::Comment => index_comments(&child, store, contents)?, + // Don't index argument and parameter lists + NodeType::Arguments | NodeType::Parameters => store, + // Index assignments as object or function symbols + NodeType::BinaryOperator(BinaryOperatorType::LeftAssignment) | + NodeType::BinaryOperator(BinaryOperatorType::EqualsAssignment) => { + index_assignment(&child, store, contents)? + }, + // Recurse + _ => index_node(&child, store, contents)?, + }; + } - let symbol = new_symbol(title, SymbolKind::STRING, None, Range { start, end }); - store.push(symbol); + Ok(store) +} - // Return early to avoid further processing - return Ok(store); - } - } +fn index_comments( + node: &Node, + mut store: Vec, + contents: &Rope, +) -> anyhow::Result> { + let comment_text = contents.node_slice(&node)?.to_string(); - if matches!( - node.node_type(), - NodeType::BinaryOperator(BinaryOperatorType::LeftAssignment) | - NodeType::BinaryOperator(BinaryOperatorType::EqualsAssignment) - ) { - return index_assignment(node, store, contents); - } + // Check if the comment starts with one or more '#' followed by any text and ends with 4+ punctuations + let Some((_level, title)) = parse_comment_as_section(&comment_text) else { + return Ok(store); + }; - // Recurse into children. We're in the same outline section so use the same store. - let mut cursor = node.walk(); - for child in node.children(&mut cursor) { - if is_indexable(&child) { - store = index_node(&child, store, contents)?; - } - } + // Create a symbol based on the parsed comment + let start = convert_point_to_position(contents, node.start_position()); + let end = convert_point_to_position(contents, node.end_position()); + + let symbol = new_symbol(title, SymbolKind::STRING, None, Range { start, end }); + store.push(symbol); Ok(store) } @@ -242,15 +248,6 @@ fn index_assignment_with_function( Ok(store) } -fn is_indexable(node: &Node) -> bool { - // Don't index 'arguments' or 'parameters' - if matches!(node.node_type(), NodeType::Arguments | NodeType::Parameters) { - return false; - } - - true -} - // Function to parse a comment and return the section level and title fn parse_comment_as_section(comment: &str) -> Option<(usize, String)> { // Match lines starting with one or more '#' followed by some non-empty content and must end with 4 or more '-', '#', or `=` From 37fde641e111efadaaf93c1d4ff1144adc5f3b00 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 21 Oct 2024 12:30:15 +0200 Subject: [PATCH 08/10] Index braced lists --- crates/ark/src/lsp/symbols.rs | 55 ++++++++++++++++++++++++++++------- 1 file changed, 44 insertions(+), 11 deletions(-) diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index 9e819150e..120a45bab 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -127,6 +127,29 @@ pub(crate) fn document_symbols( } fn index_node( + node: &Node, + store: Vec, + contents: &Rope, +) -> anyhow::Result> { + Ok(match node.node_type() { + // Handle comment sections in expression lists + NodeType::Program | NodeType::BracedExpression => { + index_expression_list(&node, store, contents)? + }, + // Index assignments as object or function symbols + NodeType::BinaryOperator(BinaryOperatorType::LeftAssignment) | + NodeType::BinaryOperator(BinaryOperatorType::EqualsAssignment) => { + index_assignment(&node, store, contents)? + }, + // Nothing to index. FIXME: We should handle argument lists, e.g. to + // index inside functions passed as arguments, or inside `test_that()` + // blocks. + _ => store, + }) +} + +// Handles root node and braced lists +fn index_expression_list( node: &Node, mut store: Vec, contents: &Rope, @@ -135,18 +158,9 @@ fn index_node( for child in node.children(&mut cursor) { store = match child.node_type() { - // Index comment sections NodeType::Comment => index_comments(&child, store, contents)?, - // Don't index argument and parameter lists - NodeType::Arguments | NodeType::Parameters => store, - // Index assignments as object or function symbols - NodeType::BinaryOperator(BinaryOperatorType::LeftAssignment) | - NodeType::BinaryOperator(BinaryOperatorType::EqualsAssignment) => { - index_assignment(&child, store, contents)? - }, - // Recurse _ => index_node(&child, store, contents)?, - }; + } } Ok(store) @@ -238,9 +252,11 @@ fn index_assignment_with_function( end: convert_point_to_position(contents, rhs.end_position()), }; + let body = rhs.child_by_field_name("body").into_result()?; + // At this point we increase the nesting level. Recurse into the function // node with a new store of children nodes. - let children = index_node(&rhs, vec![], contents)?; + let children = index_node(&body, vec![], contents)?; let symbol = new_symbol_node(name, SymbolKind::FUNCTION, Some(detail), range, children); store.push(symbol); @@ -387,4 +403,21 @@ mod tests { assert_eq!(test_symbol("foo <- function() { bar <- 1 }"), vec![foo]); } + + #[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::OBJECT, None, range); + + assert_eq!(test_symbol("{ foo <- 1 }"), vec![foo]); + } } From 201e9aacf039756c7ea40b05616b520cf7cae411 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 21 Oct 2024 12:32:53 +0200 Subject: [PATCH 09/10] Remove outdated comment --- crates/ark/src/lsp/symbols.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index 120a45bab..61f52c095 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -53,7 +53,6 @@ fn new_symbol( name, kind, detail, - // Safety: We assume `children` can't be `None` children: Some(Vec::new()), deprecated: None, tags: None, From 2684b1e3867ac427cb5155f20ffa00ea322ea02b Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 21 Oct 2024 12:35:19 +0200 Subject: [PATCH 10/10] Remove `detail` from constructor --- crates/ark/src/lsp/symbols.rs | 64 ++++++++++++++--------------------- 1 file changed, 26 insertions(+), 38 deletions(-) diff --git a/crates/ark/src/lsp/symbols.rs b/crates/ark/src/lsp/symbols.rs index 61f52c095..1a8dab7ac 100644 --- a/crates/ark/src/lsp/symbols.rs +++ b/crates/ark/src/lsp/symbols.rs @@ -31,28 +31,11 @@ use crate::treesitter::BinaryOperatorType; use crate::treesitter::NodeType; use crate::treesitter::NodeTypeExt; -fn new_symbol_node( - name: String, - kind: SymbolKind, - detail: Option, - range: Range, - children: Vec, -) -> DocumentSymbol { - let mut symbol = new_symbol(name, kind, detail, range); - symbol.children = Some(children); - symbol -} - -fn new_symbol( - name: String, - kind: SymbolKind, - detail: Option, - range: Range, -) -> DocumentSymbol { +fn new_symbol(name: String, kind: SymbolKind, range: Range) -> DocumentSymbol { DocumentSymbol { name, kind, - detail, + detail: None, children: Some(Vec::new()), deprecated: None, tags: None, @@ -61,6 +44,17 @@ fn new_symbol( } } +fn new_symbol_node( + name: String, + kind: SymbolKind, + range: Range, + children: Vec, +) -> DocumentSymbol { + let mut symbol = new_symbol(name, kind, range); + symbol.children = Some(children); + symbol +} + pub fn symbols(params: &WorkspaceSymbolParams) -> anyhow::Result> { let query = ¶ms.query; let mut info: Vec = Vec::new(); @@ -181,7 +175,7 @@ fn index_comments( let start = convert_point_to_position(contents, node.start_position()); let end = convert_point_to_position(contents, node.end_position()); - let symbol = new_symbol(title, SymbolKind::STRING, None, Range { start, end }); + let symbol = new_symbol(title, SymbolKind::STRING, Range { start, end }); store.push(symbol); Ok(store) @@ -217,7 +211,7 @@ fn index_assignment( 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, None, Range { start, end }); + let symbol = new_symbol(name, SymbolKind::VARIABLE, Range { start, end }); store.push(symbol); Ok(store) @@ -257,7 +251,8 @@ fn index_assignment_with_function( // node with a new store of children nodes. let children = index_node(&body, vec![], contents)?; - let symbol = new_symbol_node(name, SymbolKind::FUNCTION, Some(detail), range, children); + let mut symbol = new_symbol_node(name, SymbolKind::FUNCTION, range, children); + symbol.detail = Some(detail); store.push(symbol); Ok(store) @@ -323,7 +318,6 @@ mod tests { assert_eq!(test_symbol("# foo ----"), vec![new_symbol( String::from("foo"), SymbolKind::STRING, - None, range )]); } @@ -343,7 +337,6 @@ mod tests { assert_eq!(test_symbol("foo <- 1"), vec![new_symbol( String::from("foo"), SymbolKind::OBJECT, - None, range, )]); } @@ -360,12 +353,11 @@ mod tests { character: 20, }, }; - assert_eq!(test_symbol("foo <- function() {}"), vec![new_symbol( - String::from("foo"), - SymbolKind::FUNCTION, - Some(String::from("function()")), - range, - )]); + + let mut foo = new_symbol(String::from("foo"), SymbolKind::FUNCTION, range); + foo.detail = Some(String::from("function()")); + + assert_eq!(test_symbol("foo <- function() {}"), vec![foo]); } #[test] @@ -380,7 +372,7 @@ mod tests { character: 23, }, }; - let bar = new_symbol(String::from("bar"), SymbolKind::OBJECT, None, range); + let bar = new_symbol(String::from("bar"), SymbolKind::OBJECT, range); let range = Range { start: Position { @@ -392,13 +384,9 @@ mod tests { character: 30, }, }; - let mut foo = new_symbol( - String::from("foo"), - SymbolKind::FUNCTION, - Some(String::from("function()")), - range, - ); + 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]); } @@ -415,7 +403,7 @@ mod tests { character: 5, }, }; - let foo = new_symbol(String::from("foo"), SymbolKind::OBJECT, None, range); + let foo = new_symbol(String::from("foo"), SymbolKind::OBJECT, range); assert_eq!(test_symbol("{ foo <- 1 }"), vec![foo]); }