diff --git a/Cargo.lock b/Cargo.lock index f6b9a92593..5ad5c1890b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3821,6 +3821,7 @@ dependencies = [ "lsp-server", "lsp-textdocument", "lsp-types", + "memchr", "miette", "nu-cli", "nu-cmd-lang", diff --git a/crates/nu-lsp/Cargo.toml b/crates/nu-lsp/Cargo.toml index fbc444b418..be0d17ca7f 100644 --- a/crates/nu-lsp/Cargo.toml +++ b/crates/nu-lsp/Cargo.toml @@ -18,6 +18,7 @@ crossbeam-channel = { workspace = true } lsp-server = { workspace = true } lsp-textdocument = { workspace = true } lsp-types = { workspace = true } +memchr = { workspace = true } miette = { workspace = true } nucleo-matcher = { workspace = true } serde = { workspace = true } diff --git a/crates/nu-lsp/src/ast.rs b/crates/nu-lsp/src/ast.rs index fb25e9139a..083b8a69ad 100644 --- a/crates/nu-lsp/src/ast.rs +++ b/crates/nu-lsp/src/ast.rs @@ -2,20 +2,25 @@ use crate::Id; use nu_protocol::{ ast::{Argument, Block, Call, Expr, Expression, FindMapResult, ListItem, PathMember, Traverse}, engine::StateWorkingSet, - Span, + ModuleId, Span, }; use std::sync::Arc; /// 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)); +fn strip_quotes(span: Span, working_set: &StateWorkingSet) -> (Box<[u8]>, Span) { + let text = working_set.get_span_contents(span); if text.len() > 1 - && ((text.starts_with('"') && text.ends_with('"')) - || (text.starts_with('\'') && text.ends_with('\''))) + && ((text.starts_with(b"\"") && text.ends_with(b"\"")) + || (text.starts_with(b"'") && text.ends_with(b"'"))) { - Span::new(span.start.saturating_add(1), span.end.saturating_sub(1)) + ( + text.get(1..text.len() - 1) + .expect("Invalid quoted span!") + .into(), + Span::new(span.start.saturating_add(1), span.end.saturating_sub(1)), + ) } else { - span + (text.into(), span) } } @@ -53,6 +58,12 @@ fn try_find_id_in_def( location: Option<&usize>, id_ref: Option<&Id>, ) -> Option<(Id, Span)> { + // skip if the id to search is not a declaration id + if let Some(id_ref) = id_ref { + if !matches!(id_ref, Id::Declaration(_)) { + return None; + } + } let mut span = None; for arg in call.arguments.iter() { if location.is_none_or(|pos| arg.span().contains(*pos)) { @@ -70,9 +81,8 @@ fn try_find_id_in_def( } } } - 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).or_else(|| { + let (name, span) = strip_quotes(span?, working_set); + let decl_id = Id::Declaration(working_set.find_decl(&name).or_else(|| { // for defs inside def // TODO: get scope by position // https://github.com/nushell/nushell/issues/15291 @@ -104,8 +114,14 @@ fn try_find_id_in_mod( location: Option<&usize>, id_ref: Option<&Id>, ) -> Option<(Id, Span)> { - let check_location = |span: &Span| location.is_none_or(|pos| span.contains(*pos)); + // skip if the id to search is not a module id + if let Some(id_ref) = id_ref { + if !matches!(id_ref, Id::Module(_, _)) { + return None; + } + } + let check_location = |span: &Span| location.is_none_or(|pos| span.contains(*pos)); call.arguments.first().and_then(|arg| { if !check_location(&arg.span()) { return None; @@ -113,9 +129,31 @@ fn try_find_id_in_mod( 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); + let module_id = working_set.find_module(name.as_bytes()).or_else(|| { + // in case the module is hidden + let mut any_id = true; + let mut id_num_ref = 0; + if let Some(Id::Module(id_ref, _)) = id_ref { + any_id = false; + id_num_ref = id_ref.get(); + } + let block_span = call.arguments.last()?.span(); + (0..working_set.num_modules()) + .find(|id| { + (any_id || id_num_ref == *id) + && working_set.get_module(ModuleId::new(*id)).span.is_some_and( + |mod_span| { + mod_span.start <= block_span.start + 1 + && block_span.start <= mod_span.start + && block_span.end >= mod_span.end + && block_span.end <= mod_span.end + 1 + }, + ) + }) + .map(ModuleId::new) + })?; + let found_id = Id::Module(module_id, name.as_bytes().into()); + let found_span = strip_quotes(arg.span(), working_set).1; id_ref .is_none_or(|id_r| found_id == *id_r) .then_some((found_id, found_span)) @@ -135,7 +173,7 @@ fn try_find_id_in_use( call: &Call, working_set: &StateWorkingSet, location: Option<&usize>, - id: Option<&Id>, + id_ref: Option<&Id>, ) -> Option<(Id, Span)> { // NOTE: `call.parser_info` contains a 'import_pattern' field for `use`/`hide` commands, // If it's missing, usually it means the PWD env is not correctly set, @@ -151,25 +189,43 @@ fn try_find_id_in_use( let find_by_name = |name: &[u8]| { let module = working_set.get_module(module_id); - match id { - Some(Id::Variable(var_id_ref)) => module + match id_ref { + Some(Id::Variable(var_id_ref, name_ref)) => module .constants .get(name) - .and_then(|var_id| (*var_id == *var_id_ref).then_some(Id::Variable(*var_id))), + .cloned() + .or_else(|| { + // NOTE: This is for the module record variable: + // https://www.nushell.sh/book/modules/using_modules.html#importing-constants + // The definition span is located at the head of the `use` command. + (name_ref.as_ref() == name + && call + .head + .contains_span(working_set.get_variable(*var_id_ref).declaration_span)) + .then_some(*var_id_ref) + }) + .and_then(|var_id| { + (var_id == *var_id_ref).then_some(Id::Variable(var_id, name.into())) + }), Some(Id::Declaration(decl_id_ref)) => module.decls.get(name).and_then(|decl_id| { (*decl_id == *decl_id_ref).then_some(Id::Declaration(*decl_id)) }), // this is only for argument `members` - Some(Id::Module(module_id_ref)) => module.submodules.get(name).and_then(|module_id| { - (*module_id == *module_id_ref).then_some(Id::Module(*module_id)) - }), + Some(Id::Module(module_id_ref, name_ref)) => { + module.submodules.get(name).and_then(|module_id| { + (*module_id == *module_id_ref && name_ref.as_ref() == name) + .then_some(Id::Module(*module_id, name.into())) + }) + } None => module .submodules .get(name) - .cloned() - .map(Id::Module) + .map(|id| Id::Module(*id, name.into())) .or(module.decls.get(name).cloned().map(Id::Declaration)) - .or(module.constants.get(name).cloned().map(Id::Variable)), + .or(module + .constants + .get(name) + .map(|id| Id::Variable(*id, name.into()))), _ => None, } }; @@ -178,16 +234,17 @@ fn try_find_id_in_use( // Get module id if required let module_name = call.arguments.first()?; let span = module_name.span(); - if let Some(Id::Module(id_ref)) = id { + let (span_content, clean_span) = strip_quotes(span, working_set); + if let Some(Id::Module(id_ref, name_ref)) = id_ref { // still need to check the rest, if id not matched - if module_id == *id_ref { - return Some((Id::Module(module_id), strip_quotes(span, working_set))); + if module_id == *id_ref && name_ref == &span_content { + return Some((Id::Module(module_id, span_content), clean_span)); } } if let Some(pos) = location { // first argument of `use`/`hide` should always be module name if span.contains(*pos) { - return Some((Id::Module(module_id), strip_quotes(span, working_set))); + return Some((Id::Module(module_id, span_content), clean_span)); } } @@ -200,14 +257,13 @@ fn try_find_id_in_use( let name = e.as_string()?; Some(( find_by_name(name.as_bytes())?, - strip_quotes(item_expr.span, working_set), + strip_quotes(item_expr.span, working_set).1, )) }) }) }; - let arguments = call.arguments.get(1..)?; - for arg in arguments { + for arg in call.arguments.get(1..)?.iter().rev() { let Argument::Positional(expr) = arg else { continue; }; @@ -216,7 +272,7 @@ fn try_find_id_in_use( } let matched = match &expr.expr { Expr::String(name) => { - find_by_name(name.as_bytes()).map(|id| (id, strip_quotes(expr.span, working_set))) + find_by_name(name.as_bytes()).map(|id| (id, strip_quotes(expr.span, working_set).1)) } Expr::List(items) => search_in_list_items(items), Expr::FullCellPath(fcp) => { @@ -245,10 +301,16 @@ fn try_find_id_in_overlay( call: &Call, working_set: &StateWorkingSet, location: Option<&usize>, - id: Option<&Id>, + id_ref: Option<&Id>, ) -> Option<(Id, Span)> { + // skip if the id to search is not a module id + if let Some(id_ref) = id_ref { + if !matches!(id_ref, Id::Module(_, _)) { + return None; + } + } let check_location = |span: &Span| location.is_none_or(|pos| span.contains(*pos)); - let module_from_parser_info = |span: Span| { + let module_from_parser_info = |span: Span, name: &str| { let Expression { expr: Expr::Overlay(Some(module_id)), .. @@ -256,18 +318,24 @@ fn try_find_id_in_overlay( else { return None; }; - let found_id = Id::Module(*module_id); - id.is_none_or(|id_r| found_id == *id_r) - .then_some((found_id, strip_quotes(span, working_set))) + let found_id = Id::Module(*module_id, name.as_bytes().into()); + id_ref + .is_none_or(|id_r| found_id == *id_r) + .then_some((found_id, strip_quotes(span, working_set).1)) }; - // NOTE: `overlay_expr` doesn't work for `overlay hide` + // NOTE: `overlay_expr` doesn't exist for `overlay hide` let module_from_overlay_name = |name: &str, span: Span| { - let found_id = Id::Module(working_set.find_overlay(name.as_bytes())?.origin); - id.is_none_or(|id_r| found_id == *id_r) - .then_some((found_id, strip_quotes(span, working_set))) + let found_id = Id::Module( + working_set.find_overlay(name.as_bytes())?.origin, + name.as_bytes().into(), + ); + id_ref + .is_none_or(|id_r| found_id == *id_r) + .then_some((found_id, strip_quotes(span, working_set).1)) }; - for arg in call.arguments.iter() { + // check `as alias` first + for arg in call.arguments.iter().rev() { let Argument::Positional(expr) = arg else { continue; }; @@ -275,11 +343,11 @@ fn try_find_id_in_overlay( continue; }; let matched = match &expr.expr { - Expr::String(name) => module_from_parser_info(expr.span) + Expr::String(name) => module_from_parser_info(expr.span, name) .or_else(|| module_from_overlay_name(name, expr.span)), // keyword 'as' Expr::Keyword(kwd) => match &kwd.expr.expr { - Expr::String(name) => module_from_parser_info(kwd.expr.span) + Expr::String(name) => module_from_parser_info(kwd.expr.span, name) .or_else(|| module_from_overlay_name(name, kwd.expr.span)), _ => None, }, @@ -292,6 +360,19 @@ fn try_find_id_in_overlay( None } +/// Trim leading `$` sign For variable references `$foo` +fn strip_dollar_sign(span: Span, working_set: &StateWorkingSet<'_>) -> (Box<[u8]>, Span) { + let content = working_set.get_span_contents(span); + if content.starts_with(b"$") { + ( + content[1..].into(), + Span::new(span.start.saturating_add(1), span.end), + ) + } else { + (content.into(), span) + } +} + fn find_id_in_expr( expr: &Expression, working_set: &StateWorkingSet, @@ -303,12 +384,10 @@ fn find_id_in_expr( } let span = expr.span; match &expr.expr { - Expr::VarDecl(var_id) => FindMapResult::Found((Id::Variable(*var_id), span)), - // trim leading `$` sign - Expr::Var(var_id) => FindMapResult::Found(( - Id::Variable(*var_id), - Span::new(span.start.saturating_add(1), span.end), - )), + Expr::VarDecl(var_id) | Expr::Var(var_id) => { + let (name, clean_span) = strip_dollar_sign(span, working_set); + FindMapResult::Found((Id::Variable(*var_id, name), clean_span)) + } Expr::Call(call) => { if call.head.contains(*location) { FindMapResult::Found((Id::Declaration(call.decl_id), call.head)) @@ -349,7 +428,9 @@ fn find_id_in_expr( FindMapResult::Found((Id::CellPath(var_id, tail), span)) } } - Expr::Overlay(Some(module_id)) => FindMapResult::Found((Id::Module(*module_id), span)), + Expr::Overlay(Some(module_id)) => { + FindMapResult::Found((Id::Module(*module_id, [].into()), span)) + } // terminal value expressions Expr::Bool(_) | Expr::Binary(_) @@ -385,12 +466,12 @@ fn find_reference_by_id_in_expr( ) -> Option> { let closure = |e| find_reference_by_id_in_expr(e, working_set, id); match (&expr.expr, id) { - (Expr::Var(vid1), Id::Variable(vid2)) if *vid1 == *vid2 => Some(vec![Span::new( + (Expr::Var(vid1), Id::Variable(vid2, _)) if *vid1 == *vid2 => Some(vec![Span::new( // we want to exclude the `$` sign for renaming expr.span.start.saturating_add(1), expr.span.end, )]), - (Expr::VarDecl(vid1), Id::Variable(vid2)) if *vid1 == *vid2 => Some(vec![expr.span]), + (Expr::VarDecl(vid1), Id::Variable(vid2, _)) if *vid1 == *vid2 => Some(vec![expr.span]), // also interested in `var_id` in call.arguments of `use` command // and `module_id` in `module` command (Expr::Call(call), _) => { diff --git a/crates/nu-lsp/src/completion.rs b/crates/nu-lsp/src/completion.rs index cd354dd927..2d03acbb27 100644 --- a/crates/nu-lsp/src/completion.rs +++ b/crates/nu-lsp/src/completion.rs @@ -588,7 +588,7 @@ mod tests { ]) ); - let resp = send_complete_request(&client_connection, script.clone(), 7, 15); + let resp = send_complete_request(&client_connection, script, 7, 15); assert_json_include!( actual: result_from_message(resp), expected: serde_json::json!([ diff --git a/crates/nu-lsp/src/goto.rs b/crates/nu-lsp/src/goto.rs index f84d4c0e9e..1a232c5986 100644 --- a/crates/nu-lsp/src/goto.rs +++ b/crates/nu-lsp/src/goto.rs @@ -51,11 +51,11 @@ impl LanguageServer { let block_id = working_set.get_decl(*decl_id).block_id()?; working_set.get_block(block_id).span } - Id::Variable(var_id) => { + Id::Variable(var_id, _) => { let var = working_set.get_variable(*var_id); Some(var.declaration_span) } - Id::Module(module_id) => { + Id::Module(module_id, _) => { let module = working_set.get_module(*module_id); module.span } @@ -147,7 +147,7 @@ mod tests { let mut none_existent_path = root(); none_existent_path.push("none-existent.nu"); let script = path_to_uri(&none_existent_path); - let resp = send_goto_definition_request(&client_connection, script.clone(), 0, 0); + let resp = send_goto_definition_request(&client_connection, script, 0, 0); assert_json_eq!(result_from_message(resp), serde_json::json!(null)); } @@ -195,7 +195,7 @@ mod tests { serde_json::json!({ "line": 1, "character": 10 }) ); - let resp = send_goto_definition_request(&client_connection, script.clone(), 2, 9); + let resp = send_goto_definition_request(&client_connection, script, 2, 9); assert_json_eq!( result_from_message(resp).pointer("/range/start").unwrap(), serde_json::json!({ "line": 1, "character": 17 }) @@ -451,7 +451,7 @@ mod tests { serde_json::json!({ "line": 0, "character": 0 }) ); - let resp = send_goto_definition_request(&client_connection, script.clone(), 2, 30); + let resp = send_goto_definition_request(&client_connection, script, 2, 30); assert_json_eq!( result_from_message(resp).pointer("/range/start").unwrap(), serde_json::json!({ "line": 0, "character": 0 }) diff --git a/crates/nu-lsp/src/hints.rs b/crates/nu-lsp/src/hints.rs index c01966e382..a87edd940d 100644 --- a/crates/nu-lsp/src/hints.rs +++ b/crates/nu-lsp/src/hints.rs @@ -213,7 +213,7 @@ mod tests { let script = path_to_uri(&script); open_unchecked(&client_connection, script.clone()); - let resp = send_inlay_hint_request(&client_connection, script.clone()); + let resp = send_inlay_hint_request(&client_connection, script); assert_json_eq!( result_from_message(resp), @@ -240,7 +240,7 @@ mod tests { let script = path_to_uri(&script); open_unchecked(&client_connection, script.clone()); - let resp = send_inlay_hint_request(&client_connection, script.clone()); + let resp = send_inlay_hint_request(&client_connection, script); assert_json_eq!( result_from_message(resp), @@ -268,7 +268,7 @@ mod tests { let script = path_to_uri(&script); open_unchecked(&client_connection, script.clone()); - let resp = send_inlay_hint_request(&client_connection, script.clone()); + let resp = send_inlay_hint_request(&client_connection, script); assert_json_eq!( result_from_message(resp), @@ -333,7 +333,7 @@ mod tests { let (client_connection, _recv) = initialize_language_server(Some(&config), None); open_unchecked(&client_connection, script.clone()); - let resp = send_inlay_hint_request(&client_connection, script.clone()); + let resp = send_inlay_hint_request(&client_connection, script); assert_json_eq!( result_from_message(resp), diff --git a/crates/nu-lsp/src/hover.rs b/crates/nu-lsp/src/hover.rs index 29e106c751..df6de49fc2 100644 --- a/crates/nu-lsp/src/hover.rs +++ b/crates/nu-lsp/src/hover.rs @@ -142,7 +142,7 @@ impl LanguageServer { }; match id { - Id::Variable(var_id) => { + Id::Variable(var_id, _) => { let var = working_set.get_variable(var_id); let value = var .const_val @@ -178,7 +178,7 @@ impl LanguageServer { working_set.get_decl(decl_id), false, )), - Id::Module(module_id) => { + Id::Module(module_id, _) => { let description = working_set .get_module_comments(module_id)? .iter() @@ -231,7 +231,7 @@ mod hover_tests { let script = path_to_uri(&script); open_unchecked(&client_connection, script.clone()); - let resp = send_hover_request(&client_connection, script.clone(), 2, 0); + let resp = send_hover_request(&client_connection, script, 2, 0); assert_json_eq!( result_from_message(resp), @@ -248,7 +248,6 @@ mod hover_tests { script.push("hover"); script.push("use.nu"); let script = path_to_uri(&script); - open_unchecked(&client_connection, script.clone()); let resp = send_hover_request(&client_connection, script.clone(), 2, 3); @@ -265,12 +264,27 @@ mod hover_tests { serde_json::json!("```\nrecord\n```") ); - let resp = send_hover_request(&client_connection, script.clone(), 2, 11); + let resp = send_hover_request(&client_connection, script, 2, 11); let result = result_from_message(resp); assert_json_eq!( result.pointer("/contents/value").unwrap(), serde_json::json!("```\nint\n```\n---\n2") ); + + let mut script = fixtures(); + script.push("lsp"); + script.push("workspace"); + script.push("baz.nu"); + let script = path_to_uri(&script); + open_unchecked(&client_connection, script.clone()); + + // For module record + let resp = send_hover_request(&client_connection, script, 8, 42); + let result = result_from_message(resp); + assert_json_eq!( + result.pointer("/contents/value").unwrap(), + serde_json::json!("```\nstring\n```\n---\nconst value") + ); } #[test] @@ -284,7 +298,7 @@ mod hover_tests { let script = path_to_uri(&script); open_unchecked(&client_connection, script.clone()); - let resp = send_hover_request(&client_connection, script.clone(), 3, 0); + let resp = send_hover_request(&client_connection, script, 3, 0); assert_json_eq!( result_from_message(resp), @@ -308,7 +322,7 @@ mod hover_tests { let script = path_to_uri(&script); open_unchecked(&client_connection, script.clone()); - let resp = send_hover_request(&client_connection, script.clone(), 9, 7); + let resp = send_hover_request(&client_connection, script, 9, 7); assert_json_eq!( result_from_message(resp), @@ -332,7 +346,7 @@ mod hover_tests { let script = path_to_uri(&script); open_unchecked(&client_connection, script.clone()); - let resp = send_hover_request(&client_connection, script.clone(), 6, 2); + let resp = send_hover_request(&client_connection, script, 6, 2); let hover_text = result_from_message(resp) .pointer("/contents/value") @@ -358,7 +372,7 @@ mod hover_tests { let script = path_to_uri(&script); open_unchecked(&client_connection, script.clone()); - let resp = send_hover_request(&client_connection, script.clone(), 5, 8); + let resp = send_hover_request(&client_connection, script, 5, 8); assert_json_eq!( result_from_message(resp), @@ -377,21 +391,42 @@ mod hover_tests { let mut script = fixtures(); script.push("lsp"); - script.push("goto"); - script.push("module.nu"); + script.push("workspace"); + script.push("foo.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 resp = send_hover_request(&client_connection, script.clone(), 15, 15); let result = result_from_message(resp); - assert_eq!( result .pointer("/contents/value") .unwrap() .to_string() .replace("\\r", ""), - "\"# module doc\"" + "\"# cmt\"" + ); + + let resp = send_hover_request(&client_connection, script.clone(), 17, 27); + let result = result_from_message(resp); + assert_eq!( + result + .pointer("/contents/value") + .unwrap() + .to_string() + .replace("\\r", ""), + "\"# sub cmt\"" + ); + + let resp = send_hover_request(&client_connection, script, 19, 33); + let result = result_from_message(resp); + assert_eq!( + result + .pointer("/contents/value") + .unwrap() + .to_string() + .replace("\\r", ""), + "\"# sub sub cmt\"" ); } @@ -419,7 +454,7 @@ mod hover_tests { "\"```\\nrecord>\\n``` \\n---\\nimmutable\"" ); - let resp = send_hover_request(&client_connection, script_uri.clone(), 0, 22); + let resp = send_hover_request(&client_connection, script_uri, 0, 22); let result = result_from_message(resp); assert!(result diff --git a/crates/nu-lsp/src/lib.rs b/crates/nu-lsp/src/lib.rs index 111a736b8c..090b519eb8 100644 --- a/crates/nu-lsp/src/lib.rs +++ b/crates/nu-lsp/src/lib.rs @@ -40,10 +40,10 @@ mod workspace; #[derive(Debug, Clone, PartialEq)] pub(crate) enum Id { - Variable(VarId), + Variable(VarId, Box<[u8]>), Declaration(DeclId), Value(Type), - Module(ModuleId), + Module(ModuleId, Box<[u8]>), CellPath(VarId, Vec), External(String), } @@ -352,7 +352,7 @@ impl LanguageServer { if self.need_parse { // TODO: incremental parsing // add block to working_set for later references - working_set.add_block(block.clone()); + working_set.add_block(block); self.cached_state_delta = Some(working_set.delta.clone()); self.need_parse = false; } @@ -615,7 +615,7 @@ mod tests { method: DidChangeTextDocument::METHOD.to_string(), params: serde_json::to_value(DidChangeTextDocumentParams { text_document: lsp_types::VersionedTextDocumentIdentifier { - uri: uri.clone(), + uri, version: 2, }, content_changes: vec![TextDocumentContentChangeEvent { diff --git a/crates/nu-lsp/src/notification.rs b/crates/nu-lsp/src/notification.rs index 32073747b8..369ca67f69 100644 --- a/crates/nu-lsp/src/notification.rs +++ b/crates/nu-lsp/src/notification.rs @@ -19,20 +19,19 @@ impl LanguageServer { docs.listen(notification.method.as_str(), ¬ification.params); match notification.method.as_str() { DidOpenTextDocument::METHOD => { - let params: DidOpenTextDocumentParams = - serde_json::from_value(notification.params.clone()) - .expect("Expect receive DidOpenTextDocumentParams"); + let params: DidOpenTextDocumentParams = serde_json::from_value(notification.params) + .expect("Expect receive DidOpenTextDocumentParams"); Some(params.text_document.uri) } DidChangeTextDocument::METHOD => { let params: DidChangeTextDocumentParams = - serde_json::from_value(notification.params.clone()) + serde_json::from_value(notification.params) .expect("Expect receive DidChangeTextDocumentParams"); Some(params.text_document.uri) } DidCloseTextDocument::METHOD => { let params: DidCloseTextDocumentParams = - serde_json::from_value(notification.params.clone()) + serde_json::from_value(notification.params) .expect("Expect receive DidCloseTextDocumentParams"); let uri = params.text_document.uri; self.symbol_cache.drop(&uri); @@ -41,7 +40,7 @@ impl LanguageServer { } DidChangeWorkspaceFolders::METHOD => { let params: DidChangeWorkspaceFoldersParams = - serde_json::from_value(notification.params.clone()) + serde_json::from_value(notification.params) .expect("Expect receive DidChangeWorkspaceFoldersParams"); for added in params.event.added { self.workspace_folders.insert(added.name.clone(), added); @@ -155,7 +154,7 @@ mod tests { let script = path_to_uri(&script); open_unchecked(&client_connection, script.clone()); - let resp = send_hover_request(&client_connection, script.clone(), 0, 0); + let resp = send_hover_request(&client_connection, script, 0, 0); assert_json_eq!( result_from_message(resp), @@ -190,7 +189,7 @@ hello"#, ), None, ); - let resp = send_hover_request(&client_connection, script.clone(), 3, 0); + let resp = send_hover_request(&client_connection, script, 3, 0); assert_json_eq!( result_from_message(resp), @@ -229,7 +228,7 @@ hello"#, }, }), ); - let resp = send_hover_request(&client_connection, script.clone(), 3, 0); + let resp = send_hover_request(&client_connection, script, 3, 0); assert_json_eq!( result_from_message(resp), diff --git a/crates/nu-lsp/src/semantic_tokens.rs b/crates/nu-lsp/src/semantic_tokens.rs index cd5a65f7e0..b57f3ee45c 100644 --- a/crates/nu-lsp/src/semantic_tokens.rs +++ b/crates/nu-lsp/src/semantic_tokens.rs @@ -144,7 +144,7 @@ mod tests { let script = path_to_uri(&script); open_unchecked(&client_connection, script.clone()); - let resp = send_semantic_token_request(&client_connection, script.clone()); + let resp = send_semantic_token_request(&client_connection, script); assert_json_eq!( result_from_message(resp), diff --git a/crates/nu-lsp/src/symbols.rs b/crates/nu-lsp/src/symbols.rs index 0a02d383a8..17791fdefd 100644 --- a/crates/nu-lsp/src/symbols.rs +++ b/crates/nu-lsp/src/symbols.rs @@ -110,7 +110,7 @@ impl SymbolCache { range: span_to_range(&span, doc, doc_span.start), }) } - Id::Variable(var_id) => { + Id::Variable(var_id, _) => { let var = working_set.get_variable(var_id); let span = var.declaration_span; if !doc_span.contains(span.start) || span.end == span.start { @@ -124,7 +124,7 @@ impl SymbolCache { range, }) } - Id::Module(module_id) => { + Id::Module(module_id, _) => { let module = working_set.get_module(module_id); let span = module.span?; if !doc_span.contains(span.start) { @@ -157,7 +157,7 @@ impl SymbolCache { .chain((0..working_set.num_vars()).filter_map(|id| { Self::get_symbol_by_id( working_set, - Id::Variable(VarId::new(id)), + Id::Variable(VarId::new(id), [].into()), doc, &cached_file.covered_span, ) @@ -165,7 +165,7 @@ impl SymbolCache { .chain((0..working_set.num_modules()).filter_map(|id| { Self::get_symbol_by_id( working_set, - Id::Module(ModuleId::new(id)), + Id::Module(ModuleId::new(id), [].into()), doc, &cached_file.covered_span, ) @@ -236,7 +236,7 @@ impl SymbolCache { self.cache .get(uri)? .iter() - .map(|s| s.clone().to_symbol_information(uri)) + .map(|s| s.to_symbol_information(uri)) .collect(), ) } @@ -250,7 +250,7 @@ impl SymbolCache { ); self.cache .iter() - .flat_map(|(uri, symbols)| symbols.iter().map(|s| s.clone().to_symbol_information(uri))) + .flat_map(|(uri, symbols)| symbols.iter().map(|s| s.to_symbol_information(uri))) .filter_map(|s| { let mut buf = Vec::new(); let name = Utf32Str::new(&s.name, &mut buf); @@ -361,7 +361,7 @@ mod tests { let script = path_to_uri(&script); open_unchecked(&client_connection, script.clone()); - let resp = send_document_symbol_request(&client_connection, script.clone()); + let resp = send_document_symbol_request(&client_connection, script); assert_json_eq!(result_from_message(resp), serde_json::json!([])); } @@ -470,7 +470,7 @@ mod tests { script.push("bar.nu"); let script_bar = path_to_uri(&script); - open_unchecked(&client_connection, script_foo.clone()); + open_unchecked(&client_connection, script_foo); open_unchecked(&client_connection, script_bar.clone()); let resp = send_workspace_symbol_request(&client_connection, "br".to_string()); @@ -531,7 +531,7 @@ mod tests { let script_bar = path_to_uri(&script); open_unchecked(&client_connection, script_foo.clone()); - open_unchecked(&client_connection, script_bar.clone()); + open_unchecked(&client_connection, script_bar); let resp = send_workspace_symbol_request(&client_connection, "foo".to_string()); assert_json_eq!( diff --git a/crates/nu-lsp/src/workspace.rs b/crates/nu-lsp/src/workspace.rs index 05701cd7e2..58e9e75193 100644 --- a/crates/nu-lsp/src/workspace.rs +++ b/crates/nu-lsp/src/workspace.rs @@ -59,13 +59,29 @@ struct IDTracker { /// Span of the original instance under the cursor pub span: Span, /// Name of the definition - pub name: String, + pub name: Box<[u8]>, /// Span of the original file where the request comes from pub file_span: Span, /// The redundant parsing should only happen once pub renewed: bool, } +impl IDTracker { + fn new(id: Id, span: Span, file_span: Span, working_set: &StateWorkingSet) -> Self { + let name = match &id { + Id::Variable(_, name) | Id::Module(_, name) => name.clone(), + _ => working_set.get_span_contents(span).into(), + }; + Self { + id, + span, + name, + file_span, + renewed: false, + } + } +} + impl LanguageServer { /// Get initial workspace folders from initialization response pub(crate) fn initialize_workspace_folders( @@ -121,7 +137,6 @@ impl LanguageServer { /// The rename request only happens after the client received a `PrepareRenameResponse`, /// and a new name typed in, could happen before ranges ready for all files in the workspace folder pub(crate) fn rename(&mut self, params: &RenameParams) -> Option { - let new_name = params.new_name.to_owned(); // changes in WorkspaceEdit have mutable key #[allow(clippy::mutable_key_type)] let changes: HashMap> = self @@ -134,7 +149,7 @@ impl LanguageServer { .iter() .map(|range| TextEdit { range: *range, - new_text: new_name.clone(), + new_text: params.new_name.to_owned(), }) .collect(), ) @@ -176,16 +191,9 @@ impl LanguageServer { .to_owned() .unwrap_or(ProgressToken::Number(1)); - let id_tracker = IDTracker { - id, - span, - file_span, - name: String::from_utf8_lossy(working_set.get_span_contents(span)).to_string(), - renewed: false, - }; + let id_tracker = IDTracker::new(id, span, file_span, &working_set); // make sure the parsing result of current file is merged in the state let engine_state = self.new_engine_state(Some(&path_uri)); - self.channels = self .find_reference_in_workspace( engine_state, @@ -270,16 +278,9 @@ impl LanguageServer { .get_workspace_folder_by_uri(&path_uri) .ok_or_else(|| miette!("\nCurrent file is not in any workspace"))?; // now continue parsing on other files in the workspace - let id_tracker = IDTracker { - id, - span, - file_span, - name: String::from_utf8_lossy(working_set.get_span_contents(span)).to_string(), - renewed: false, - }; + let id_tracker = IDTracker::new(id, span, file_span, &working_set); // make sure the parsing result of current file is merged in the state let engine_state = self.new_engine_state(Some(&path_uri)); - self.channels = self .find_reference_in_workspace( engine_state, @@ -338,18 +339,20 @@ impl LanguageServer { file_span: Span, sample_span: Span, ) -> Option { - if let (Id::Variable(_), Some(decl_span)) = (&id, definition_span) { + if let (Id::Variable(_, name_ref), Some(decl_span)) = (&id, definition_span) { if file_span.contains_span(decl_span) && decl_span.end > decl_span.start { - let leading_dashes = working_set - .get_span_contents(decl_span) + let content = working_set.get_span_contents(decl_span); + let leading_dashes = content .iter() // remove leading dashes for flags .take_while(|c| *c == &b'-') .count(); let start = decl_span.start + leading_dashes; - return Some(Span { - start, - end: start + sample_span.end - sample_span.start, + return content.get(leading_dashes..).and_then(|name| { + name.starts_with(name_ref).then_some(Span { + start, + end: start + sample_span.end - sample_span.start, + }) }); } } @@ -391,6 +394,8 @@ impl LanguageServer { .collect(); let len = scripts.len(); let definition_span = Self::find_definition_span_by_id(&working_set, &id_tracker.id); + let bytes_to_search = id_tracker.name.to_owned(); + let finder = memchr::memmem::Finder::new(&bytes_to_search); for (i, fp) in scripts.iter().enumerate() { #[cfg(test)] @@ -411,7 +416,7 @@ impl LanguageServer { let file = if let Some(file) = docs.get_document(&uri) { file } else { - let bytes = match fs::read(fp) { + let file_bytes = match fs::read(fp) { Ok(it) => it, Err(_) => { // continue on fs error @@ -419,15 +424,18 @@ impl LanguageServer { } }; // skip if the file does not contain what we're looking for - let content_string = String::from_utf8_lossy(&bytes); - if !content_string.contains(&id_tracker.name) { + if finder.find(&file_bytes).is_none() { // progress without any data data_sender .send(InternalMessage::OnGoing(token.clone(), percentage)) .into_diagnostic()?; continue; } - &FullTextDocument::new("nu".to_string(), 0, content_string.into()) + &FullTextDocument::new( + "nu".to_string(), + 0, + String::from_utf8_lossy(&file_bytes).into(), + ) }; let _ = Self::find_reference_in_file(&mut working_set, file, fp, &mut id_tracker) .map(|mut refs| { @@ -458,7 +466,7 @@ impl LanguageServer { }); } data_sender - .send(InternalMessage::Finished(token.clone())) + .send(InternalMessage::Finished(token)) .into_diagnostic()?; Ok(()) }); @@ -551,7 +559,7 @@ mod tests { // use a hover request to interrupt if immediate_cancellation { - send_hover_request(client_connection, uri.clone(), line, character); + send_hover_request(client_connection, uri, line, character); } (0..num) @@ -635,7 +643,7 @@ mod tests { script.push("foo.nu"); let script = path_to_uri(&script); - open_unchecked(&client_connection, script.clone()); + open_unchecked(&client_connection, script); } #[test] @@ -659,14 +667,16 @@ mod tests { open_unchecked(&client_connection, script.clone()); - let message_num = 5; + let message_num = 6; let messages = send_reference_request(&client_connection, script.clone(), 0, 12, message_num); assert_eq!(messages.len(), message_num); + let mut has_response = false; for message in messages { match message { Message::Notification(n) => assert_eq!(n.method, "$/progress"), Message::Response(r) => { + has_response = true; let result = r.result.unwrap(); let array = result.as_array().unwrap(); assert!(array.contains(&serde_json::json!( @@ -678,7 +688,7 @@ mod tests { )); assert!(array.contains(&serde_json::json!( { - "uri": script.to_string(), + "uri": script, "range": { "start": { "line": 0, "character": 11 }, "end": { "line": 0, "character": 16 } } } ) @@ -687,6 +697,7 @@ mod tests { _ => panic!("unexpected message type"), } } + assert!(has_response); } #[test] @@ -717,19 +728,21 @@ mod tests { // thus changing the cached `StateDelta` open_unchecked(&client_connection, script_foo); - let message_num = 5; + let message_num = 6; let messages = send_reference_request(&client_connection, script.clone(), 0, 23, message_num); assert_eq!(messages.len(), message_num); + let mut has_response = false; for message in messages { match message { Message::Notification(n) => assert_eq!(n.method, "$/progress"), Message::Response(r) => { + has_response = true; let result = r.result.unwrap(); let array = result.as_array().unwrap(); assert!(array.contains(&serde_json::json!( { - "uri": script.to_string(), + "uri": script, "range": { "start": { "line": 5, "character": 4 }, "end": { "line": 5, "character": 11 } } } ) @@ -745,6 +758,61 @@ mod tests { _ => panic!("unexpected message type"), } } + assert!(has_response); + } + + #[test] + fn module_path_reference_in_workspace() { + let mut script = fixtures(); + script.push("lsp"); + script.push("workspace"); + let (client_connection, _recv) = initialize_language_server( + None, + serde_json::to_value(InitializeParams { + workspace_folders: Some(vec![WorkspaceFolder { + uri: path_to_uri(&script), + name: "random name".to_string(), + }]), + ..Default::default() + }) + .ok(), + ); + script.push("baz.nu"); + let script = path_to_uri(&script); + + open_unchecked(&client_connection, script.clone()); + + let message_num = 6; + let messages = + send_reference_request(&client_connection, script.clone(), 0, 12, message_num); + assert_eq!(messages.len(), message_num); + let mut has_response = false; + for message in messages { + match message { + Message::Notification(n) => assert_eq!(n.method, "$/progress"), + Message::Response(r) => { + has_response = true; + let result = r.result.unwrap(); + let array = result.as_array().unwrap(); + assert!(array.contains(&serde_json::json!( + { + "uri": script.to_string().replace("baz", "bar"), + "range": { "start": { "line": 0, "character": 4 }, "end": { "line": 0, "character": 12 } } + } + ) + )); + assert!(array.contains(&serde_json::json!( + { + "uri": script, + "range": { "start": { "line": 6, "character": 4 }, "end": { "line": 6, "character": 12 } } + } + ) + )); + } + _ => panic!("unexpected message type"), + } + } + assert!(has_response); } #[test] @@ -768,7 +836,7 @@ mod tests { open_unchecked(&client_connection, script.clone()); - let message_num = 5; + let message_num = 6; let messages = send_rename_prepare_request( &client_connection, script.clone(), @@ -778,19 +846,24 @@ mod tests { false, ); assert_eq!(messages.len(), message_num); + let mut has_response = false; for message in messages { match message { Message::Notification(n) => assert_eq!(n.method, "$/progress"), - Message::Response(r) => assert_json_eq!( - r.result, - serde_json::json!({ - "start": { "line": 6, "character": 13 }, - "end": { "line": 6, "character": 20 } - }), - ), + Message::Response(r) => { + has_response = true; + assert_json_eq!( + r.result, + serde_json::json!({ + "start": { "line": 6, "character": 13 }, + "end": { "line": 6, "character": 20 } + }), + ) + } _ => panic!("unexpected message type"), } } + assert!(has_response); if let Message::Response(r) = send_rename_request(&client_connection, script.clone(), 6, 11) { @@ -817,7 +890,7 @@ mod tests { assert!( changs_bar.contains( &serde_json::json!({ - "range": { "start": { "line": 0, "character": 20 }, "end": { "line": 0, "character": 27 } }, + "range": { "start": { "line": 0, "character": 22 }, "end": { "line": 0, "character": 29 } }, "newText": "new" }) )); @@ -847,7 +920,7 @@ mod tests { open_unchecked(&client_connection, script.clone()); - let message_num = 5; + let message_num = 6; let messages = send_rename_prepare_request( &client_connection, script.clone(), @@ -857,19 +930,24 @@ mod tests { false, ); assert_eq!(messages.len(), message_num); + let mut has_response = false; for message in messages { match message { Message::Notification(n) => assert_eq!(n.method, "$/progress"), - Message::Response(r) => assert_json_eq!( - r.result, - serde_json::json!({ - "start": { "line": 3, "character": 3 }, - "end": { "line": 3, "character": 8 } - }), - ), + Message::Response(r) => { + has_response = true; + assert_json_eq!( + r.result, + serde_json::json!({ + "start": { "line": 3, "character": 3 }, + "end": { "line": 3, "character": 8 } + }), + ) + } _ => panic!("unexpected message type"), } } + assert!(has_response); if let Message::Response(r) = send_rename_request(&client_connection, script.clone(), 3, 5) { @@ -934,25 +1012,29 @@ mod tests { } else { panic!("Progress not cancelled"); }; + let mut has_response = false; 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" - } - }), - ), + Message::Response(r) => { + has_response = true; + 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"), } } + assert!(has_response); - if let Message::Response(r) = send_rename_request(&client_connection, script.clone(), 6, 11) - { + if let Message::Response(r) = send_rename_request(&client_connection, script, 6, 11) { // should not return any changes assert_json_eq!(r.result.unwrap()["changes"], serde_json::json!({})); } else { @@ -998,7 +1080,7 @@ mod tests { let (client_connection, _recv) = initialize_language_server(None, None); open_unchecked(&client_connection, script.clone()); - let message = send_document_highlight_request(&client_connection, script.clone(), 3, 5); + let message = send_document_highlight_request(&client_connection, script, 3, 5); let Message::Response(r) = message else { panic!("unexpected message type"); }; @@ -1010,4 +1092,76 @@ mod tests { ]), ); } + + #[test] + fn document_highlight_module_alias() { + let mut script = fixtures(); + script.push("lsp"); + script.push("goto"); + script.push("use_module.nu"); + let script = path_to_uri(&script); + + let (client_connection, _recv) = initialize_language_server(None, None); + open_unchecked(&client_connection, script.clone()); + + let message = send_document_highlight_request(&client_connection, script.clone(), 1, 26); + let Message::Response(r) = message else { + panic!("unexpected message type"); + }; + assert_json_eq!( + r.result, + serde_json::json!([ + { "range": { "start": { "line": 1, "character": 25 }, "end": { "line": 1, "character": 33 } }, "kind": 1 }, + { "range": { "start": { "line": 2, "character": 30 }, "end": { "line": 2, "character": 38 } }, "kind": 1 } + ]), + ); + + let message = send_document_highlight_request(&client_connection, script, 0, 10); + let Message::Response(r) = message else { + panic!("unexpected message type"); + }; + assert_json_eq!( + r.result, + serde_json::json!([ + { "range": { "start": { "line": 0, "character": 4 }, "end": { "line": 0, "character": 13 } }, "kind": 1 }, + { "range": { "start": { "line": 1, "character": 12 }, "end": { "line": 1, "character": 21 } }, "kind": 1 } + ]), + ); + } + + #[test] + fn document_highlight_module_record() { + let mut script = fixtures(); + script.push("lsp"); + script.push("workspace"); + script.push("baz.nu"); + let script = path_to_uri(&script); + + let (client_connection, _recv) = initialize_language_server(None, None); + open_unchecked(&client_connection, script.clone()); + + let message = send_document_highlight_request(&client_connection, script.clone(), 8, 0); + let Message::Response(r) = message else { + panic!("unexpected message type"); + }; + assert_json_eq!( + r.result, + serde_json::json!([ + { "range": { "start": { "line": 6, "character": 26 }, "end": { "line": 6, "character": 33 } }, "kind": 1 }, + { "range": { "start": { "line": 8, "character": 1 }, "end": { "line": 8, "character": 8 } }, "kind": 1 }, + ]), + ); + + let message = send_document_highlight_request(&client_connection, script, 10, 7); + let Message::Response(r) = message else { + panic!("unexpected message type"); + }; + assert_json_eq!( + r.result, + serde_json::json!([ + { "range": { "start": { "line": 10, "character": 4 }, "end": { "line": 10, "character": 12 } }, "kind": 1 }, + { "range": { "start": { "line": 11, "character": 1 }, "end": { "line": 11, "character": 8 } }, "kind": 1 }, + ]), + ); + } } diff --git a/tests/fixtures/lsp/workspace/bar.nu b/tests/fixtures/lsp/workspace/bar.nu index 947043d791..65f4ae09ed 100644 --- a/tests/fixtures/lsp/workspace/bar.nu +++ b/tests/fixtures/lsp/workspace/bar.nu @@ -1,4 +1,4 @@ -use foo.nu [ foooo "foo str" ] +use ./foo.nu [ foooo "foo str" ] export def "bar str" [ ] { diff --git a/tests/fixtures/lsp/workspace/baz.nu b/tests/fixtures/lsp/workspace/baz.nu new file mode 100644 index 0000000000..462655d6c9 --- /dev/null +++ b/tests/fixtures/lsp/workspace/baz.nu @@ -0,0 +1,12 @@ +overlay use ./foo.nu as prefix --prefix +alias aname = prefix mod name sub module cmd name +aname +prefix foo str +overlay hide prefix + +use ./foo.nu [ "mod name" cst_mod ] + +$cst_mod."sub module"."sub sub module".var_name +mod name sub module cmd name +let $cst_mod = 1 +$cst_mod diff --git a/tests/fixtures/lsp/workspace/foo.nu b/tests/fixtures/lsp/workspace/foo.nu index 3af354b032..784ce27581 100644 --- a/tests/fixtures/lsp/workspace/foo.nu +++ b/tests/fixtures/lsp/workspace/foo.nu @@ -5,3 +5,20 @@ export def foooo [ } export def "foo str" [] { "foo" } + +export module "mod name" { + export module "sub module" { + export def "cmd name" [] { } + } +} + +# cmt +export module cst_mod { + # sub cmt + export module "sub module" { + # sub sub cmt + export module "sub sub module" { + export const var_name = "const value" + } + } +}