-
Notifications
You must be signed in to change notification settings - Fork 23
Create Rowan parse tree, store document contents as text, and use Biome position encoding converters #974
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Create Rowan parse tree, store document contents as text, and use Biome position encoding converters #974
Changes from all commits
6e0e50d
d9b2c9e
432af99
bc4c0b7
82b0032
dd41ff3
434518d
0261074
ffb1588
43390c4
f49769f
7fbf498
b3cb224
c5fc08e
260dd0f
7b8a451
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,12 +16,16 @@ amalthea = { path = "../amalthea" } | |
| anyhow = "1.0.80" | ||
| async-trait = "0.1.66" | ||
| base64 = "0.21.0" | ||
| biome_line_index.workspace = true | ||
| bus = "2.3.0" | ||
| cfg-if = "1.0.0" | ||
| crossbeam = { version = "0.8.2", features = ["crossbeam-channel"] } | ||
| ctor = "0.1.26" | ||
| dap = { git = "https://github.com/sztomi/dap-rs", branch = "main" } | ||
| dashmap = "5.4.0" | ||
| aether_parser.workspace = true | ||
| aether_syntax.workspace = true | ||
| aether_lsp_utils.workspace = true | ||
| ego-tree = "0.6.2" | ||
| harp = { path = "../harp" } | ||
| http = "0.2.9" | ||
|
|
@@ -38,16 +42,16 @@ regex = "1.10.0" | |
| reqwest = { version = "0.12.5", default-features = false, features = ["json"] } | ||
| reqwest-retry = "0.6.1" | ||
| reqwest-middleware = "0.3.3" | ||
| ropey = "1.6.0" | ||
| rust-embed = "8.0.0" | ||
| scraper = "0.15.0" | ||
| serde = { version = "1.0.183", features = ["derive"] } | ||
| serde_json = { version = "1.0.94", features = ["preserve_order"] } | ||
| stdext = { path = "../stdext" } | ||
| streaming-iterator = "0.1.9" | ||
| tokio = { version = "1.26.0", features = ["full"] } | ||
| tower-lsp = "0.19.0" | ||
| tree-sitter = "0.23.0" | ||
| tree-sitter-r = { git = "https://github.com/r-lib/tree-sitter-r", rev = "95aff097aa927a66bb357f715b58cde821be8867" } | ||
| tower-lsp.workspace = true | ||
| tree-sitter = "0.24.7" | ||
| tree-sitter-r = { git = "https://github.com/r-lib/tree-sitter-r", rev = "daa26a2ff0d9546e9125c7d8bcec046027f02070" } | ||
|
Comment on lines
+53
to
+54
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we please keep the exact tree-sitter and tree-sitter-r versions as before? This actually downgrades tree-sitter-r from where we were previously. The exact tree-sitter and tree-sitter-r versions are extremely sensitive and typically require a lot of mental thought, and I'd prefer to keep any changes to them isolated to their own PRs if possible. |
||
| uuid = "1.3.0" | ||
| url = "2.4.1" | ||
| walkdir = "2" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,14 +5,14 @@ | |
| // | ||
| // | ||
|
|
||
| use stdext::result::ResultExt; | ||
| use tower_lsp::lsp_types; | ||
| use tower_lsp::lsp_types::Range; | ||
| use tree_sitter::Node; | ||
|
|
||
| use crate::lsp::document_context::DocumentContext; | ||
| use crate::lsp::encoding::convert_point_to_position; | ||
| use crate::lsp::encoding::convert_tree_sitter_range_to_lsp_range; | ||
| use crate::lsp::traits::node::NodeExt; | ||
| use crate::treesitter::node_find_parent_call; | ||
| use crate::treesitter::node_text; | ||
| use crate::treesitter::BinaryOperatorType; | ||
| use crate::treesitter::NodeType; | ||
| use crate::treesitter::NodeTypeExt; | ||
|
|
@@ -54,25 +54,24 @@ pub(crate) enum ArgumentsStatus { | |
| } | ||
|
|
||
| impl FunctionContext { | ||
| pub(crate) fn new(document_context: &DocumentContext) -> Self { | ||
| pub(crate) fn new(document_context: &DocumentContext) -> anyhow::Result<Self> { | ||
| let completion_node = document_context.node; | ||
|
|
||
| let Some(effective_function_node) = get_effective_function_node(completion_node) else { | ||
| // We shouldn't ever attempt to instantiate a FunctionContext or | ||
| // function-flavored CompletionItem in this degenerate case, but we | ||
| // return a dummy FunctionContext just to be safe. | ||
| let node_end = convert_point_to_position( | ||
| &document_context.document.contents, | ||
| completion_node.range().end_point, | ||
| ); | ||
| let node_end = document_context | ||
| .document | ||
| .lsp_position_from_tree_sitter_point(completion_node.range().end_point)?; | ||
|
|
||
| return Self { | ||
| return Ok(Self { | ||
| name: String::new(), | ||
| range: tower_lsp::lsp_types::Range::new(node_end, node_end), | ||
| range: lsp_types::Range::new(node_end, node_end), | ||
| usage: FunctionRefUsage::Call, | ||
| arguments_status: ArgumentsStatus::Absent, | ||
| cursor_is_at_end: true, | ||
| }; | ||
| }); | ||
| }; | ||
|
|
||
| let usage = determine_function_usage( | ||
|
|
@@ -93,7 +92,7 @@ impl FunctionContext { | |
| cursor.row == node_range.end_point.row && cursor.column == node_range.end_point.column; | ||
|
|
||
| let name = match function_name_node { | ||
| Some(node) => node_text(&node, &document_context.document.contents).unwrap_or_default(), | ||
| Some(node) => node.node_to_string(&document_context.document.contents)?, | ||
| None => String::new(), | ||
|
Comment on lines
94
to
96
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks like a change in behavior? Subtle, but maybe keep |
||
| }; | ||
|
|
||
|
|
@@ -107,26 +106,26 @@ impl FunctionContext { | |
| "FunctionContext created with name: '{name}', usage: {usage:?}, arguments: {arguments_status:?}, cursor at end: {is_cursor_at_end}" | ||
| ); | ||
|
|
||
| Self { | ||
| Ok(Self { | ||
| name, | ||
| range: match function_name_node { | ||
| Some(node) => convert_tree_sitter_range_to_lsp_range( | ||
| &document_context.document.contents, | ||
| node.range(), | ||
| ), | ||
| Some(node) => document_context | ||
| .document | ||
| .lsp_range_from_tree_sitter_range(node.range())?, | ||
| None => { | ||
| // Create a zero-width range at the end of the effective_function_node | ||
| let node_end = convert_point_to_position( | ||
| &document_context.document.contents, | ||
| effective_function_node.range().end_point, | ||
| ); | ||
| tower_lsp::lsp_types::Range::new(node_end, node_end) | ||
| let node_end = document_context | ||
| .document | ||
| .lsp_position_from_tree_sitter_point( | ||
| effective_function_node.range().end_point, | ||
| )?; | ||
| lsp_types::Range::new(node_end, node_end) | ||
| }, | ||
| }, | ||
| usage, | ||
| arguments_status, | ||
| cursor_is_at_end: is_cursor_at_end, | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -165,7 +164,7 @@ static FUNCTIONS_EXPECTING_A_FUNCTION_REFERENCE: &[&str] = &[ | |
| "str", | ||
| ]; | ||
|
|
||
| fn is_inside_special_function(node: &Node, contents: &ropey::Rope) -> bool { | ||
| fn is_inside_special_function(node: &Node, contents: &str) -> bool { | ||
| let Some(call_node) = node_find_parent_call(node) else { | ||
| return false; | ||
| }; | ||
|
|
@@ -174,9 +173,12 @@ fn is_inside_special_function(node: &Node, contents: &ropey::Rope) -> bool { | |
| return false; | ||
| }; | ||
|
|
||
| let call_name = node_text(&call_name_node, contents).unwrap_or_default(); | ||
| let call_name = call_name_node | ||
| .node_as_str(contents) | ||
| .log_err() | ||
| .unwrap_or_default(); | ||
|
Comment on lines
175
to
+179
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea that |
||
|
|
||
| FUNCTIONS_EXPECTING_A_FUNCTION_REFERENCE.contains(&call_name.as_str()) | ||
| FUNCTIONS_EXPECTING_A_FUNCTION_REFERENCE.contains(&call_name) | ||
| } | ||
|
|
||
| /// Checks if the node is inside a help operator context like `?foo` or `method?foo` | ||
|
|
@@ -226,7 +228,7 @@ fn determine_arguments_status(function_container_node: &Node) -> ArgumentsStatus | |
| } | ||
| } | ||
|
|
||
| fn determine_function_usage(node: &Node, contents: &ropey::Rope) -> FunctionRefUsage { | ||
| fn determine_function_usage(node: &Node, contents: &str) -> FunctionRefUsage { | ||
| if is_inside_special_function(node, contents) || is_inside_help_operator(node) { | ||
| FunctionRefUsage::Value | ||
| } else { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alpahbetical order please?