Skip to content
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

Custom statement_range() LSP message #85

Merged
merged 3 commits into from
Sep 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions crates/ark/src/lsp/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,11 @@ use crate::lsp::globals;
use crate::lsp::hover::hover;
use crate::lsp::indexer;
use crate::lsp::signature_help::signature_help;
use crate::lsp::statement_range;
use crate::lsp::symbols;
use crate::request::KernelRequest;

#[macro_export]
macro_rules! backend_trace {
Comment on lines +52 to 53
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it seem reasonable to do this? Is there anything else I need to do besides this tag?


($self: expr, $($rest: expr),*) => {{
Expand Down Expand Up @@ -581,11 +583,6 @@ impl LanguageServer for Backend {
// https://github.com/Microsoft/vscode-languageserver-node/blob/18fad46b0e8085bb72e1b76f9ea23a379569231a/client/src/common/client.ts#L802-L838
// https://github.com/Microsoft/vscode-languageserver-node/blob/18fad46b0e8085bb72e1b76f9ea23a379569231a/client/src/common/client.ts#L701-L752
impl Backend {
async fn request(&self, params: Option<Value>) -> Result<i32> {
info!("Received Positron request: {:?}", params);
Ok(42)
}

async fn notification(&self, params: Option<Value>) {
info!("Received Positron notification: {:?}", params);
}
Expand Down Expand Up @@ -633,7 +630,10 @@ pub async fn start_lsp(
};

let (service, socket) = LspService::build(init)
.custom_method("positron/request", Backend::request)
.custom_method(
statement_range::POSITRON_STATEMENT_RANGE_REQUEST,
Backend::statement_range,
)
.custom_method("positron/notification", Backend::notification)
.finish();

Expand Down
1 change: 1 addition & 0 deletions crates/ark/src/lsp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ pub mod markdown;
pub mod references;
pub mod show_message;
pub mod signature_help;
pub mod statement_range;
pub mod symbols;
pub mod traits;
pub mod treesitter;
Expand Down
84 changes: 84 additions & 0 deletions crates/ark/src/lsp/statement_range.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
//
// statement_range.rs
//
// Copyright (C) 2023 Posit Software, PBC. All rights reserved.
//
//

use serde::Deserialize;
use serde::Serialize;
use stdext::unwrap;
use tower_lsp::jsonrpc::Result;
use tower_lsp::lsp_types::Position;
use tower_lsp::lsp_types::Range;
use tower_lsp::lsp_types::VersionedTextDocumentIdentifier;

use crate::backend_trace;
use crate::lsp::backend::Backend;
use crate::lsp::traits::cursor::TreeCursorExt;
use crate::lsp::traits::point::PointExt;
use crate::lsp::traits::position::PositionExt;

pub static POSITRON_STATEMENT_RANGE_REQUEST: &'static str = "positron/textDocument/statementRange";

#[derive(Debug, Eq, PartialEq, Clone, Deserialize, Serialize)]
#[serde(rename_all = "camelCase")]
pub struct StatementRangeParams {
/// The document to provide a statement range for.
pub text_document: VersionedTextDocumentIdentifier,
/// The location of the cursor.
pub position: Position,
}

#[derive(Debug, Eq, PartialEq, Clone, Deserialize, Serialize)]
#[serde(rename_all = "camelCase")]
pub struct StatementRangeResponse {
/// The document range the statement covers.
pub range: Range,
}

impl Backend {
pub async fn statement_range(
&self,
params: StatementRangeParams,
) -> Result<Option<StatementRangeResponse>> {
backend_trace!(self, "statement_range({:?})", params);

let uri = &params.text_document.uri;
let document = unwrap!(self.documents.get_mut(uri), None => {
backend_trace!(self, "statement_range(): No document associated with URI {uri}");
return Ok(None);
});

let position = params.position;
let point = position.as_point();

let root = document.ast.root_node();
let mut cursor = root.walk();

if !cursor.goto_first_child_for_point_patched(point) {
// TODO: Uncommenting this causes a compile error???
// backend_trace!(self, "statement_range(): No child associated with point.");
Comment on lines +60 to +61
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevinushey I think we should talk about this and see if the backend_trace!() macro has a bug in it somehow

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to work above; maybe the macro doesn't handle invocations lacking any arguments?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or perhaps it's because logging here requires a call to .await and that doesn't play nicely when you have treesitter nodes on the stack?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has something to do with the latter. If i put the message before the creation of root then everything is fine

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2023-09-05 at 3 44 01 PM

The exact error is this horrible thing

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That thing gives C++ template compilation errors a run for its money 💀

We could probably adjust the macro to either avoid the await, or spawn a separate execution context wherein it could be awaited safely and independently from the current call... not sure if you have any strong feelings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely no strong feelings

return Ok(None);
}

let node = cursor.node();

// Tree-sitter `Point`s
let start_point = node.start_position();
let end_point = node.end_position();

// To LSP `Position`s
let start_position = start_point.as_position();
let end_position = end_point.as_position();

let range = Range {
start: start_position,
end: end_position,
};

let response = StatementRangeResponse { range };

Ok(Some(response))
}
}
36 changes: 36 additions & 0 deletions crates/ark/src/lsp/traits/cursor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,19 @@ pub trait TreeCursorExt {
// Find a leaf node in the AST. The leaf node either at the requested point,
// or the leaf node closest (but not after) the requested point, will be returned.
fn find_leaf(&mut self, point: Point) -> Node;

/// Move this cursor to the first child of its current node that extends
/// beyond or touches the given point. Returns `true` if a child node was found,
/// otherwise returns `false`.
///
/// TODO: In theory we should be using `cursor.goto_first_child_for_point()`,
/// but it is reported to be broken, and indeed does not work right if I
/// substitute it in.
/// https://github.com/tree-sitter/tree-sitter/issues/2012
///
/// This simple reimplementation is based on this Emacs hot patch
/// https://git.savannah.gnu.org/cgit/emacs.git/commit/?h=emacs-29&id=7c61a304104fe3a35c47d412150d29b93a697c5e
fn goto_first_child_for_point_patched(&mut self, point: Point) -> bool;
}

impl TreeCursorExt for TreeCursor<'_> {
Expand All @@ -111,4 +124,27 @@ impl TreeCursorExt for TreeCursor<'_> {
let node = self.node();
_find_leaf_impl(node, point)
}

fn goto_first_child_for_point_patched(&mut self, point: Point) -> bool {
if !self.goto_first_child() {
return false;
}

let mut node = self.node();

// The emacs patch used `<=` in the while condition, but we want the
// following to execute all of `fn()` if the cursor is placed at the `|`
// fn <- function() {
// }|
while node.end_position() < point {
if self.goto_next_sibling() {
node = self.node();
} else {
// Reached the end and still can't find a valid sibling
return false;
}
}

return true;
}
}