From ee84435a0eedff47f13d4a05caf05941c618ac40 Mon Sep 17 00:00:00 2001 From: zc he Date: Tue, 21 Jan 2025 03:03:03 +0800 Subject: [PATCH] fix(lsp): missing references in `use` command (#14861) # Description This PR fixes the issue of the missing references in `use` command image However, as described in [this discussion](https://github.com/nushell/nushell/discussions/14854), the returned reference list is still not complete due to the inconsistent IDs. As a side effect, `hover/goto def` now also works on the `use` command arguments image Actions including `goto def/hover/references/rename` now work with module (maybe some edge cases of `overlay` are not covered) image # User-Facing Changes # Tests + Formatting Added 1. the test for heavy requests cancellation. 2. expected Edit for the missing ref of `use` to the existing rename test. 3. `goto/hover` on module name # After Submitting --- crates/nu-lsp/src/ast.rs | 339 +++++++++++++++++++------- crates/nu-lsp/src/goto.rs | 62 +++++ crates/nu-lsp/src/hints.rs | 23 +- crates/nu-lsp/src/lib.rs | 82 +++++-- crates/nu-lsp/src/workspace.rs | 118 +++++++-- tests/fixtures/lsp/goto/module.nu | 4 + tests/fixtures/lsp/goto/use_module.nu | 1 + 7 files changed, 490 insertions(+), 139 deletions(-) create mode 100644 tests/fixtures/lsp/goto/module.nu create mode 100644 tests/fixtures/lsp/goto/use_module.nu diff --git a/crates/nu-lsp/src/ast.rs b/crates/nu-lsp/src/ast.rs index ad53f57f9d..bee0c007f3 100644 --- a/crates/nu-lsp/src/ast.rs +++ b/crates/nu-lsp/src/ast.rs @@ -1,36 +1,35 @@ -use std::sync::Arc; - use nu_protocol::{ ast::{ Argument, Block, Call, Expr, Expression, ExternalArgument, ListItem, MatchPattern, Pattern, PipelineRedirection, RecordItem, }, engine::StateWorkingSet, - DeclId, Span, + Span, }; +use std::{path::PathBuf, sync::Arc}; use crate::Id; /// similar to flatten_block, but allows extra map function -pub fn ast_flat_map( - ast: &Arc, - working_set: &StateWorkingSet, - extra_args: &E, - f_special: fn(&Expression, &StateWorkingSet, &E) -> Option>, -) -> Vec { +pub fn ast_flat_map<'a, T, F>( + ast: &'a Arc, + working_set: &'a StateWorkingSet, + f_special: &F, +) -> Vec +where + F: Fn(&'a Expression) -> Option>, +{ ast.pipelines .iter() .flat_map(|pipeline| { pipeline.elements.iter().flat_map(|element| { - expr_flat_map(&element.expr, working_set, extra_args, f_special) + expr_flat_map(&element.expr, working_set, f_special) .into_iter() .chain( element .redirection .as_ref() - .map(|redir| { - redirect_flat_map(redir, working_set, extra_args, f_special) - }) + .map(|redir| redirect_flat_map(redir, working_set, f_special)) .unwrap_or_default(), ) }) @@ -43,24 +42,26 @@ pub fn ast_flat_map( /// /// # Arguments /// * `f_special` - function that overrides the default behavior -pub fn expr_flat_map( - expr: &Expression, - working_set: &StateWorkingSet, - extra_args: &E, - f_special: fn(&Expression, &StateWorkingSet, &E) -> Option>, -) -> Vec { +pub fn expr_flat_map<'a, T, F>( + expr: &'a Expression, + working_set: &'a StateWorkingSet, + f_special: &F, +) -> Vec +where + F: Fn(&'a Expression) -> Option>, +{ // behavior overridden by f_special - if let Some(vec) = f_special(expr, working_set, extra_args) { + if let Some(vec) = f_special(expr) { return vec; } - let recur = |expr| expr_flat_map(expr, working_set, extra_args, f_special); + let recur = |expr| expr_flat_map(expr, working_set, f_special); match &expr.expr { Expr::RowCondition(block_id) | Expr::Subexpression(block_id) | Expr::Block(block_id) | Expr::Closure(block_id) => { let block = working_set.get_block(block_id.to_owned()); - ast_flat_map(block, working_set, extra_args, f_special) + ast_flat_map(block, working_set, f_special) } Expr::Range(range) => [&range.from, &range.next, &range.to] .iter() @@ -88,7 +89,7 @@ pub fn expr_flat_map( Expr::MatchBlock(matches) => matches .iter() .flat_map(|(pattern, expr)| { - match_pattern_flat_map(pattern, working_set, extra_args, f_special) + match_pattern_flat_map(pattern, working_set, f_special) .into_iter() .chain(recur(expr)) }) @@ -124,14 +125,16 @@ pub fn expr_flat_map( } /// flat_map on match patterns -fn match_pattern_flat_map( - pattern: &MatchPattern, - working_set: &StateWorkingSet, - extra_args: &E, - f_special: fn(&Expression, &StateWorkingSet, &E) -> Option>, -) -> Vec { - let recur = |expr| expr_flat_map(expr, working_set, extra_args, f_special); - let recur_match = |p| match_pattern_flat_map(p, working_set, extra_args, f_special); +fn match_pattern_flat_map<'a, T, F>( + pattern: &'a MatchPattern, + working_set: &'a StateWorkingSet, + f_special: &F, +) -> Vec +where + F: Fn(&'a Expression) -> Option>, +{ + let recur = |expr| expr_flat_map(expr, working_set, f_special); + let recur_match = |p| match_pattern_flat_map(p, working_set, f_special); match &pattern.pattern { Pattern::Expression(expr) => recur(expr), Pattern::List(patterns) | Pattern::Or(patterns) => { @@ -146,13 +149,15 @@ fn match_pattern_flat_map( } /// flat_map on redirections -fn redirect_flat_map( - redir: &PipelineRedirection, - working_set: &StateWorkingSet, - extra_args: &E, - f_special: fn(&Expression, &StateWorkingSet, &E) -> Option>, -) -> Vec { - let recur = |expr| expr_flat_map(expr, working_set, extra_args, f_special); +fn redirect_flat_map<'a, T, F>( + redir: &'a PipelineRedirection, + working_set: &'a StateWorkingSet, + f_special: &F, +) -> Vec +where + F: Fn(&'a Expression) -> Option>, +{ + let recur = |expr| expr_flat_map(expr, working_set, f_special); match redir { PipelineRedirection::Single { target, .. } => target.expr().map(recur).unwrap_or_default(), PipelineRedirection::Separate { out, err } => [out, err] @@ -163,6 +168,19 @@ fn redirect_flat_map( } } +/// Adjust span if quoted +fn strip_quotes(span: Span, working_set: &StateWorkingSet) -> Span { + let text = String::from_utf8_lossy(working_set.get_span_contents(span)); + if text.len() > 1 + && ((text.starts_with('"') && text.ends_with('"')) + || (text.starts_with('\'') && text.ends_with('\''))) + { + Span::new(span.start.saturating_add(1), span.end.saturating_sub(1)) + } else { + span + } +} + /// For situations like /// ```nushell /// def foo [] {} @@ -171,22 +189,21 @@ fn redirect_flat_map( /// `def` is an internal call with name/signature/closure as its arguments /// /// # Arguments -/// - `location`: None if no `contains` check required, typically when we want to compare the `decl_id` directly +/// - `location`: None if no `contains` check required +/// - `id`: None if no id equal check required fn try_find_id_in_def( call: &Call, working_set: &StateWorkingSet, location: Option<&usize>, + id_ref: Option<&Id>, ) -> Option<(Id, Span)> { - let call_name = String::from_utf8(working_set.get_span_contents(call.head).to_vec()).ok()?; - if !(call_name == "def" || call_name == "export def") { + let call_name = working_set.get_span_contents(call.head); + if call_name != "def".as_bytes() && call_name != "export def".as_bytes() { return None; }; let mut span = None; for arg in call.arguments.iter() { - if location - .map(|pos| arg.span().contains(*pos)) - .unwrap_or(true) - { + if location.map_or(true, |pos| arg.span().contains(*pos)) { // String means this argument is the name if let Argument::Positional(expr) = arg { if let Expr::String(_) = &expr.expr { @@ -201,30 +218,180 @@ fn try_find_id_in_def( } } } - let mut span = span?; - // adjust span if quoted - let text = String::from_utf8(working_set.get_span_contents(span).to_vec()).ok()?; - if text.len() > 1 - && ((text.starts_with('"') && text.ends_with('"')) - || (text.starts_with('\'') && text.ends_with('\''))) - { - span = Span::new(span.start.saturating_add(1), span.end.saturating_sub(1)); + let span = strip_quotes(span?, working_set); + let name = working_set.get_span_contents(span); + let decl_id = Id::Declaration(working_set.find_decl(name)?); + id_ref + .map_or(true, |id_r| decl_id == *id_r) + .then_some((decl_id, span)) +} + +/// For situations like +/// ```nushell +/// module foo {} +/// # |__________ location +/// ``` +/// `module` is an internal call with name/signature/closure as its arguments +/// +/// # Arguments +/// - `location`: None if no `contains` check required +/// - `id`: None if no id equal check required +fn try_find_id_in_mod( + call: &Call, + working_set: &StateWorkingSet, + location: Option<&usize>, + id_ref: Option<&Id>, +) -> Option<(Id, Span)> { + let call_name = working_set.get_span_contents(call.head); + if call_name != "module".as_bytes() && call_name != "export module".as_bytes() { + return None; + }; + let check_location = |span: &Span| location.map_or(true, |pos| span.contains(*pos)); + + call.arguments.first().and_then(|arg| { + if !check_location(&arg.span()) { + return None; + } + match arg { + Argument::Positional(expr) => { + let name = expr.as_string()?; + let module_id = working_set.find_module(name.as_bytes())?; + let found_id = Id::Module(module_id); + let found_span = strip_quotes(arg.span(), working_set); + id_ref + .map_or(true, |id_r| found_id == *id_r) + .then_some((found_id, found_span)) + } + _ => None, + } + }) +} + +/// Find id in use command +/// `use foo.nu bar` or `use foo.nu [bar baz]` +/// NOTE: `call.parser_info` contains a 'import_pattern' field for `use` commands, +/// but sometimes it is missing, so fall back to `call_name == "use"` here. +/// One drawback is that the `module_id` is harder to get +/// +/// # Arguments +/// - `location`: None if no `contains` check required +/// - `id`: None if no id equal check required +fn try_find_id_in_use( + call: &Call, + working_set: &StateWorkingSet, + location: Option<&usize>, + id: Option<&Id>, +) -> Option<(Id, Span)> { + let call_name = working_set.get_span_contents(call.head); + if call_name != "use".as_bytes() { + return None; } - let call_span = call.span(); - // find decl_ids whose span is covered by the `def` call - let mut matched_ids: Vec<(Id, Span)> = (0..working_set.num_decls()) - .filter_map(|id| { - let decl_id = DeclId::new(id); - let block_id = working_set.get_decl(decl_id).block_id()?; - let decl_span = working_set.get_block(block_id).span?; - // find those within the `def` call - call_span - .contains_span(decl_span) - .then_some((Id::Declaration(decl_id), decl_span)) + let find_by_name = |name: &str| { + match id { + Some(Id::Variable(var_id_ref)) => { + if let Some(var_id) = working_set.find_variable(name.as_bytes()) { + if var_id == *var_id_ref { + return Some(Id::Variable(var_id)); + } + } + } + Some(Id::Declaration(decl_id_ref)) => { + if let Some(decl_id) = working_set.find_decl(name.as_bytes()) { + if decl_id == *decl_id_ref { + return Some(Id::Declaration(decl_id)); + } + } + } + Some(Id::Module(module_id_ref)) => { + if let Some(module_id) = working_set.find_module(name.as_bytes()) { + if module_id == *module_id_ref { + return Some(Id::Module(module_id)); + } + } + } + None => { + if let Some(var_id) = working_set.find_variable(name.as_bytes()) { + return Some(Id::Variable(var_id)); + } + if let Some(decl_id) = working_set.find_decl(name.as_bytes()) { + return Some(Id::Declaration(decl_id)); + } + if let Some(module_id) = working_set.find_module(name.as_bytes()) { + return Some(Id::Module(module_id)); + } + } + _ => (), + } + None + }; + let check_location = |span: &Span| location.map_or(true, |pos| span.contains(*pos)); + let get_module_id = |span: Span| { + let span = strip_quotes(span, working_set); + let name = String::from_utf8_lossy(working_set.get_span_contents(span)); + let path = PathBuf::from(name.as_ref()); + let stem = path.file_stem().and_then(|fs| fs.to_str()).unwrap_or(&name); + let module_id = working_set.find_module(stem.as_bytes())?; + let found_id = Id::Module(module_id); + id.map_or(true, |id_r| found_id == *id_r) + .then_some((found_id, span)) + }; + + // Get module id if required + let module_name = call.arguments.first()?; + let span = module_name.span(); + if let Some(Id::Module(_)) = id { + // still need to check the second argument + if let Some(res) = get_module_id(span) { + return Some(res); + } + } + if let Some(pos) = location { + if span.contains(*pos) { + return get_module_id(span); + } + } + + let search_in_list_items = |items: &Vec| { + items.iter().find_map(|item| { + let item_expr = item.expr(); + check_location(&item_expr.span) + .then_some(item_expr) + .and_then(|e| { + let name = e.as_string()?; + Some(( + find_by_name(&name)?, + strip_quotes(item_expr.span, working_set), + )) + }) }) - .collect(); - matched_ids.sort_by_key(|(_, s)| s.start); - matched_ids.first().cloned().map(|(id, _)| (id, span)) + }; + + // the imported name is always at the second argument + if let Argument::Positional(expr) = call.arguments.get(1)? { + if check_location(&expr.span) { + match &expr.expr { + Expr::String(name) => { + if let Some(id) = find_by_name(name) { + return Some((id, strip_quotes(expr.span, working_set))); + } + } + Expr::List(items) => { + if let Some(res) = search_in_list_items(items) { + return Some(res); + } + } + Expr::FullCellPath(fcp) => { + if let Expr::List(items) = &fcp.head.expr { + if let Some(res) = search_in_list_items(items) { + return Some(res); + } + } + } + _ => (), + } + } + } + None } fn find_id_in_expr( @@ -255,10 +422,12 @@ fn find_id_in_expr( if call.head.contains(*location) { Some(vec![(Id::Declaration(call.decl_id), call.head)]) } else { - try_find_id_in_def(call, working_set, Some(location)).map(|p| vec![p]) + try_find_id_in_def(call, working_set, Some(location), None) + .or(try_find_id_in_mod(call, working_set, Some(location), None)) + .or(try_find_id_in_use(call, working_set, Some(location), None)) + .map(|p| vec![p]) } } - // TODO: module id of `use` Expr::Overlay(Some(module_id)) => Some(vec![(Id::Module(*module_id), span)]), // terminal value expressions Expr::Bool(_) @@ -284,18 +453,17 @@ pub fn find_id( working_set: &StateWorkingSet, location: &usize, ) -> Option<(Id, Span)> { - ast_flat_map(ast, working_set, location, find_id_in_expr) - .first() - .cloned() + let closure = |e| find_id_in_expr(e, working_set, location); + ast_flat_map(ast, working_set, &closure).first().cloned() } -// TODO: module id support fn find_reference_by_id_in_expr( expr: &Expression, working_set: &StateWorkingSet, id: &Id, ) -> Option> { - let recur = |expr| expr_flat_map(expr, working_set, id, find_reference_by_id_in_expr); + let closure = |e| find_reference_by_id_in_expr(e, working_set, id); + let recur = |expr| expr_flat_map(expr, working_set, &closure); match (&expr.expr, id) { (Expr::Var(vid1), Id::Variable(vid2)) if *vid1 == *vid2 => Some(vec![Span::new( // we want to exclude the `$` sign for renaming @@ -303,21 +471,26 @@ fn find_reference_by_id_in_expr( expr.span.end, )]), (Expr::VarDecl(vid1), Id::Variable(vid2)) if *vid1 == *vid2 => Some(vec![expr.span]), - (Expr::Call(call), Id::Declaration(decl_id)) => { + // also interested in `var_id` in call.arguments of `use` command + // and `module_id` in `module` command + (Expr::Call(call), _) => { let mut occurs: Vec = call .arguments .iter() .filter_map(|arg| arg.expr()) .flat_map(recur) .collect(); - if *decl_id == call.decl_id { - occurs.push(call.head); - } - if let Some((id_found, span_found)) = try_find_id_in_def(call, working_set, None) { - if id_found == *id { - occurs.push(span_found); + if let Id::Declaration(decl_id) = id { + if *decl_id == call.decl_id { + occurs.push(call.head); } } + if let Some((_, span_found)) = try_find_id_in_def(call, working_set, None, Some(id)) + .or(try_find_id_in_mod(call, working_set, None, Some(id))) + .or(try_find_id_in_use(call, working_set, None, Some(id))) + { + occurs.push(span_found); + } Some(occurs) } _ => None, @@ -325,5 +498,7 @@ fn find_reference_by_id_in_expr( } pub fn find_reference_by_id(ast: &Arc, working_set: &StateWorkingSet, id: &Id) -> Vec { - ast_flat_map(ast, working_set, id, find_reference_by_id_in_expr) + ast_flat_map(ast, working_set, &|e| { + find_reference_by_id_in_expr(e, working_set, id) + }) } diff --git a/crates/nu-lsp/src/goto.rs b/crates/nu-lsp/src/goto.rs index 72e576d674..a1a5527675 100644 --- a/crates/nu-lsp/src/goto.rs +++ b/crates/nu-lsp/src/goto.rs @@ -348,4 +348,66 @@ mod tests { }) ); } + + #[test] + fn goto_definition_of_module() { + let (client_connection, _recv) = initialize_language_server(None); + + let mut script = fixtures(); + script.push("lsp"); + script.push("goto"); + script.push("module.nu"); + let script = path_to_uri(&script); + + open_unchecked(&client_connection, script.clone()); + + let resp = send_goto_definition_request(&client_connection, script.clone(), 3, 15); + let result = if let Message::Response(response) = resp { + response.result + } else { + panic!() + }; + + assert_json_eq!( + result, + serde_json::json!({ + "uri": script, + "range": { + "start": { "line": 1, "character": 29 }, + "end": { "line": 1, "character": 30 } + } + }) + ); + } + + #[test] + fn goto_definition_of_module_in_another_file() { + let (client_connection, _recv) = initialize_language_server(None); + + let mut script = fixtures(); + script.push("lsp"); + script.push("goto"); + script.push("use_module.nu"); + let script = path_to_uri(&script); + + open_unchecked(&client_connection, script.clone()); + + let resp = send_goto_definition_request(&client_connection, script.clone(), 0, 23); + let result = if let Message::Response(response) = resp { + response.result + } else { + panic!() + }; + + assert_json_eq!( + result, + serde_json::json!({ + "uri": script.to_string().replace("use_module", "module"), + "range": { + "start": { "line": 1, "character": 29 }, + "end": { "line": 1, "character": 30 } + } + }) + ); + } } diff --git a/crates/nu-lsp/src/hints.rs b/crates/nu-lsp/src/hints.rs index 39719ad0b3..44f4ab66b7 100644 --- a/crates/nu-lsp/src/hints.rs +++ b/crates/nu-lsp/src/hints.rs @@ -26,17 +26,11 @@ fn type_short_name(t: &Type) -> String { fn extract_inlay_hints_from_expression( expr: &Expression, working_set: &StateWorkingSet, - extra_args: &(usize, &FullTextDocument), + offset: &usize, + file: &FullTextDocument, ) -> Option> { - let (offset, file) = extra_args; - let recur = |expr| { - expr_flat_map( - expr, - working_set, - extra_args, - extract_inlay_hints_from_expression, - ) - }; + let closure = |e| extract_inlay_hints_from_expression(e, working_set, offset, file); + let recur = |expr| expr_flat_map(expr, working_set, &closure); match &expr.expr { Expr::BinaryOp(lhs, op, rhs) => { let mut hints: Vec = @@ -162,12 +156,9 @@ impl LanguageServer { offset: usize, file: &FullTextDocument, ) -> Vec { - ast_flat_map( - block, - working_set, - &(offset, file), - extract_inlay_hints_from_expression, - ) + ast_flat_map(block, working_set, &|e| { + extract_inlay_hints_from_expression(e, working_set, &offset, file) + }) } } diff --git a/crates/nu-lsp/src/lib.rs b/crates/nu-lsp/src/lib.rs index b6759fe99e..94c3d1ff0d 100644 --- a/crates/nu-lsp/src/lib.rs +++ b/crates/nu-lsp/src/lib.rs @@ -409,16 +409,23 @@ impl LanguageServer { ) .ok()?; + let markdown_hover = |content: String| { + Some(Hover { + contents: HoverContents::Markup(MarkupContent { + kind: MarkupKind::Markdown, + value: content, + }), + // TODO + range: None, + }) + }; + match id { Id::Variable(var_id) => { let var = working_set.get_variable(var_id); - let contents = format!("{}{}", if var.mutable { "mutable " } else { "" }, var.ty); - - Some(Hover { - contents: HoverContents::Scalar(lsp_types::MarkedString::String(contents)), - // TODO - range: None, - }) + let contents = + format!("{} `{}`", if var.mutable { "mutable " } else { "" }, var.ty); + markdown_hover(contents) } Id::Declaration(decl_id) => { let decl = working_set.get_decl(decl_id); @@ -559,24 +566,19 @@ impl LanguageServer { )); } } - - Some(Hover { - contents: HoverContents::Markup(MarkupContent { - kind: MarkupKind::Markdown, - value: description, - }), - // TODO - range: None, - }) + markdown_hover(description) } - Id::Value(t) => { - Some(Hover { - contents: HoverContents::Scalar(lsp_types::MarkedString::String(t.to_string())), - // TODO - range: None, - }) + Id::Module(module_id) => { + let mut description = String::new(); + for cmt_span in working_set.get_module_comments(module_id)? { + description.push_str( + String::from_utf8_lossy(working_set.get_span_contents(*cmt_span)).as_ref(), + ); + description.push('\n'); + } + markdown_hover(description) } - _ => None, + Id::Value(t) => markdown_hover(format!("`{}`", t)), } } @@ -842,9 +844,7 @@ mod tests { assert_json_eq!( result, - serde_json::json!({ - "contents": "table" - }) + serde_json::json!({ "contents": { "kind": "markdown", "value": " `table`" } }) ); } @@ -908,6 +908,36 @@ mod tests { ); } + #[test] + fn hover_on_module() { + let (client_connection, _recv) = initialize_language_server(None); + + let mut script = fixtures(); + script.push("lsp"); + script.push("goto"); + script.push("module.nu"); + let script = path_to_uri(&script); + + open_unchecked(&client_connection, script.clone()); + + let resp = send_hover_request(&client_connection, script.clone(), 3, 12); + let result = if let Message::Response(response) = resp { + response.result + } else { + panic!() + }; + + assert_eq!( + result + .unwrap() + .pointer("/contents/value") + .unwrap() + .to_string() + .replace("\\r", ""), + "\"# module doc\\n\"" + ); + } + fn send_complete_request( client_connection: &Connection, uri: Uri, diff --git a/crates/nu-lsp/src/workspace.rs b/crates/nu-lsp/src/workspace.rs index 8161719e61..37eb81182c 100644 --- a/crates/nu-lsp/src/workspace.rs +++ b/crates/nu-lsp/src/workspace.rs @@ -286,7 +286,8 @@ impl LanguageServer { let len = scripts.len(); for (i, fp) in scripts.iter().enumerate() { - // std::thread::sleep(std::time::Duration::from_millis(500)); + #[cfg(test)] + std::thread::sleep(std::time::Duration::from_millis(200)); // cancel the loop on cancellation message from main thread if cancel_receiver.try_recv().is_ok() { data_sender @@ -366,7 +367,7 @@ mod tests { use nu_test_support::fs::fixtures; use crate::path_to_uri; - use crate::tests::{initialize_language_server, open_unchecked}; + use crate::tests::{initialize_language_server, open_unchecked, send_hover_request}; fn send_reference_request( client_connection: &Connection, @@ -411,6 +412,7 @@ mod tests { line: u32, character: u32, num: usize, + immediate_cancellation: bool, ) -> Vec { client_connection .sender @@ -425,6 +427,11 @@ mod tests { })) .unwrap(); + // use a hover request to interrupt + if immediate_cancellation { + send_hover_request(client_connection, uri.clone(), line, character); + } + (0..num) .map(|_| { client_connection @@ -576,8 +583,14 @@ mod tests { open_unchecked(&client_connection, script.clone()); let message_num = 5; - let messages = - send_rename_prepare_request(&client_connection, script.clone(), 6, 11, message_num); + let messages = send_rename_prepare_request( + &client_connection, + script.clone(), + 6, + 11, + message_num, + false, + ); assert_eq!(messages.len(), message_num); for message in messages { match message { @@ -605,15 +618,23 @@ mod tests { } ]) ); - assert_json_eq!( - changes[script.to_string().replace("foo", "bar")], - serde_json::json!([ - { - "range": { "start": { "line": 5, "character": 4 }, "end": { "line": 5, "character": 11 } }, - "newText": "new" - } - ]) - ); + let changs_bar = changes[script.to_string().replace("foo", "bar")] + .as_array() + .unwrap(); + assert!( + changs_bar.contains( + &serde_json::json!({ + "range": { "start": { "line": 5, "character": 4 }, "end": { "line": 5, "character": 11 } }, + "newText": "new" + }) + )); + assert!( + changs_bar.contains( + &serde_json::json!({ + "range": { "start": { "line": 0, "character": 20 }, "end": { "line": 0, "character": 27 } }, + "newText": "new" + }) + )); } else { panic!() } @@ -637,8 +658,14 @@ mod tests { open_unchecked(&client_connection, script.clone()); let message_num = 5; - let messages = - send_rename_prepare_request(&client_connection, script.clone(), 3, 5, message_num); + let messages = send_rename_prepare_request( + &client_connection, + script.clone(), + 3, + 5, + message_num, + false, + ); assert_eq!(messages.len(), message_num); for message in messages { match message { @@ -677,4 +704,65 @@ mod tests { panic!() } } + + #[test] + fn rename_cancelled() { + let mut script = fixtures(); + script.push("lsp"); + script.push("workspace"); + let (client_connection, _recv) = initialize_language_server(Some(InitializeParams { + workspace_folders: Some(vec![WorkspaceFolder { + uri: path_to_uri(&script), + name: "random name".to_string(), + }]), + ..Default::default() + })); + script.push("foo.nu"); + let script = path_to_uri(&script); + + open_unchecked(&client_connection, script.clone()); + + let message_num = 3; + let messages = send_rename_prepare_request( + &client_connection, + script.clone(), + 6, + 11, + message_num, + true, + ); + assert_eq!(messages.len(), message_num); + if let Some(Message::Notification(cancel_notification)) = &messages.last() { + assert_json_eq!( + cancel_notification.params["value"], + serde_json::json!({ "kind": "end", "message": "interrupted." }) + ); + } else { + panic!("Progress not cancelled"); + }; + for message in messages { + match message { + Message::Notification(n) => assert_eq!(n.method, "$/progress"), + // the response of the preempting hover request + Message::Response(r) => assert_json_eq!( + r.result, + serde_json::json!({ + "contents": { + "kind": "markdown", + "value": "\n-----\n### Usage \n```nu\n foo str {flags}\n```\n\n### Flags\n\n `-h`, `--help` - Display the help message for this command\n\n" + } + }), + ), + _ => panic!("unexpected message type"), + } + } + + if let Message::Response(r) = send_rename_request(&client_connection, script.clone(), 6, 11) + { + // should not return any changes + assert_json_eq!(r.result.unwrap()["changes"], serde_json::json!({})); + } else { + panic!() + } + } } diff --git a/tests/fixtures/lsp/goto/module.nu b/tests/fixtures/lsp/goto/module.nu new file mode 100644 index 0000000000..028045db7a --- /dev/null +++ b/tests/fixtures/lsp/goto/module.nu @@ -0,0 +1,4 @@ +# module doc +export module "module name" { } + +use "module name" diff --git a/tests/fixtures/lsp/goto/use_module.nu b/tests/fixtures/lsp/goto/use_module.nu new file mode 100644 index 0000000000..0bbc2e19a5 --- /dev/null +++ b/tests/fixtures/lsp/goto/use_module.nu @@ -0,0 +1 @@ +use module.nu "module name"