Skip to content

Commit e1ce67e

Browse files
authored
fix(lsp): malfunctioning on unsaved files, empty workspaceFolder parameter, overlay use with prefixes (nushell#16568)
Fixes some minor issues of nu-lsp, including nushell#16559 and nushell#16539, as well as most of the key takeaways of nushell#16540 ## Release notes summary - What our users need to know ## Tasks after submitting - [ ] add test cases, I'll leave them to future PRs where I'm going to refactor the verbose testing code of nu-lsp.
1 parent 4381458 commit e1ce67e

File tree

8 files changed

+197
-137
lines changed

8 files changed

+197
-137
lines changed

crates/nu-lsp/src/ast.rs

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -134,21 +134,33 @@ fn try_find_id_in_def(
134134
}
135135
}
136136
}
137-
let (name, span) = strip_quotes(span?, working_set);
138-
let decl_id = Id::Declaration(working_set.find_decl(&name).or_else(|| {
139-
// for defs inside def
137+
138+
let block_span_of_this_def = call.positional_iter().last()?.span;
139+
let decl_on_spot = |decl_id: &DeclId| -> bool {
140+
working_set
141+
.get_decl(*decl_id)
142+
.block_id()
143+
.and_then(|block_id| working_set.get_block(block_id).span)
144+
.is_some_and(|block_span| block_span == block_span_of_this_def)
145+
};
146+
147+
let (_, span) = strip_quotes(span?, working_set);
148+
let id_found = if let Some(id_r) = id_ref {
149+
let Id::Declaration(decl_id_ref) = id_r else {
150+
return None;
151+
};
152+
decl_on_spot(decl_id_ref).then_some(id_r.clone())?
153+
} else {
154+
// Find declaration by name, e.g. `workspace.find_decl`, is not reliable
155+
// considering shadowing and overlay prefixes
140156
// TODO: get scope by position
141157
// https://github.com/nushell/nushell/issues/15291
142-
(0..working_set.num_decls()).rev().find_map(|id| {
158+
Id::Declaration((0..working_set.num_decls()).rev().find_map(|id| {
143159
let decl_id = DeclId::new(id);
144-
let decl = working_set.get_decl(decl_id);
145-
let span = working_set.get_block(decl.block_id()?).span?;
146-
call.span().contains_span(span).then_some(decl_id)
147-
})
148-
})?);
149-
id_ref
150-
.is_none_or(|id_r| decl_id == *id_r)
151-
.then_some((decl_id, span))
160+
decl_on_spot(&decl_id).then_some(decl_id)
161+
})?)
162+
};
163+
Some((id_found, span))
152164
}
153165

154166
/// For situations like

crates/nu-lsp/src/completion.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ use nu_protocol::{
1414

1515
impl LanguageServer {
1616
pub(crate) fn complete(&mut self, params: &CompletionParams) -> Option<CompletionResponse> {
17-
let path_uri = params.text_document_position.text_document.uri.to_owned();
17+
let path_uri = &params.text_document_position.text_document.uri;
1818
let docs = self.docs.lock().ok()?;
19-
let file = docs.get_document(&path_uri)?;
19+
let file = docs.get_document(path_uri)?;
2020
let location = file.offset_at(params.text_document_position.position) as usize;
2121
let file_text = file.get_content(None).to_owned();
2222
drop(docs);
@@ -30,18 +30,18 @@ impl LanguageServer {
3030
.is_some_and(|c| c.is_whitespace() || "|(){}[]<>,:;".contains(c));
3131

3232
self.need_parse |= need_fallback;
33-
let engine_state = Arc::new(self.new_engine_state(Some(&path_uri)));
33+
let engine_state = Arc::new(self.new_engine_state(Some(path_uri)));
3434
let completer = NuCompleter::new(engine_state.clone(), Arc::new(Stack::new()));
3535
let results = if need_fallback {
3636
completer.fetch_completions_at(&file_text[..location], location)
3737
} else {
38-
let file_path = uri_to_path(&path_uri);
38+
let file_path = uri_to_path(path_uri);
3939
let filename = file_path.to_str()?;
4040
completer.fetch_completions_within_file(filename, location, &file_text)
4141
};
4242

4343
let docs = self.docs.lock().ok()?;
44-
let file = docs.get_document(&path_uri)?;
44+
let file = docs.get_document(path_uri)?;
4545
(!results.is_empty()).then_some(CompletionResponse::Array(
4646
results
4747
.into_iter()

crates/nu-lsp/src/goto.rs

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,16 @@ impl LanguageServer {
1515
for cached_file in files.into_iter() {
1616
if cached_file.covered_span.contains(span.start) {
1717
let path = Path::new(&*cached_file.name);
18-
if !path.is_file() {
19-
return None;
20-
}
2118
let target_uri = path_to_uri(path);
2219
if let Some(file) = self.docs.lock().ok()?.get_document(&target_uri) {
2320
return Some(Location {
2421
uri: target_uri,
2522
range: span_to_range(span, file, cached_file.covered_span.start),
2623
});
2724
} else {
25+
if !path.is_file() {
26+
return None;
27+
}
2828
// in case where the document is not opened yet,
2929
// typically included by the `use/source` command
3030
let temp_doc = FullTextDocument::new(
@@ -77,16 +77,12 @@ impl LanguageServer {
7777
&mut self,
7878
params: &GotoDefinitionParams,
7979
) -> Option<GotoDefinitionResponse> {
80-
let path_uri = params
81-
.text_document_position_params
82-
.text_document
83-
.uri
84-
.to_owned();
85-
let mut engine_state = self.new_engine_state(Some(&path_uri));
80+
let path_uri = &params.text_document_position_params.text_document.uri;
81+
let mut engine_state = self.new_engine_state(Some(path_uri));
8682
let (working_set, id, _, _) = self
8783
.parse_and_find(
8884
&mut engine_state,
89-
&path_uri,
85+
path_uri,
9086
params.text_document_position_params.position,
9187
)
9288
.ok()?;

crates/nu-lsp/src/hover.rs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -117,16 +117,12 @@ impl LanguageServer {
117117
}
118118

119119
pub(crate) fn hover(&mut self, params: &HoverParams) -> Option<Hover> {
120-
let path_uri = params
121-
.text_document_position_params
122-
.text_document
123-
.uri
124-
.to_owned();
125-
let mut engine_state = self.new_engine_state(Some(&path_uri));
120+
let path_uri = &params.text_document_position_params.text_document.uri;
121+
let mut engine_state = self.new_engine_state(Some(path_uri));
126122
let (working_set, id, _, _) = self
127123
.parse_and_find(
128124
&mut engine_state,
129-
&path_uri,
125+
path_uri,
130126
params.text_document_position_params.position,
131127
)
132128
.ok()?;

crates/nu-lsp/src/lib.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,14 @@ impl LanguageServer {
234234
})
235235
}
236236
request::Rename::METHOD => {
237+
if self.channels.is_some() {
238+
self.send_error_message(
239+
request.id.clone(),
240+
3,
241+
"Please wait for renaming preparation to complete.".into(),
242+
)?;
243+
continue;
244+
}
237245
Self::handle_lsp_request(request, |params| self.rename(params))
238246
}
239247
request::SemanticTokensFullRequest::METHOD => {
@@ -393,6 +401,7 @@ impl LanguageServer {
393401
let file_path = uri_to_path(uri);
394402
let file_path_str = file_path.to_str()?;
395403
let contents = file.get_content(None).as_bytes();
404+
// For `const foo = path self .`
396405
let _ = working_set.files.push(file_path.clone(), Span::unknown());
397406
let block = nu_parser::parse(&mut working_set, Some(file_path_str), contents, false);
398407
let span = working_set.get_span_for_filename(file_path_str)?;

crates/nu-lsp/src/signature.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -118,26 +118,22 @@ impl LanguageServer {
118118
&mut self,
119119
params: &SignatureHelpParams,
120120
) -> Option<SignatureHelp> {
121-
let path_uri = params
122-
.text_document_position_params
123-
.text_document
124-
.uri
125-
.to_owned();
121+
let path_uri = &params.text_document_position_params.text_document.uri;
126122
let docs = self.docs.lock().ok()?;
127-
let file = docs.get_document(&path_uri)?;
123+
let file = docs.get_document(path_uri)?;
128124
let location = file.offset_at(params.text_document_position_params.position) as usize;
129125
let file_text = file.get_content(None).to_owned();
130126
drop(docs);
131127

132-
let engine_state = self.new_engine_state(Some(&path_uri));
128+
let engine_state = self.new_engine_state(Some(path_uri));
133129
let mut working_set = StateWorkingSet::new(&engine_state);
134130

135131
// NOTE: in case the cursor is at the end of the call expression
136132
let need_placeholder = location == 0
137133
|| file_text
138134
.get(location - 1..location)
139135
.is_some_and(|s| s.chars().all(|c| c.is_whitespace()));
140-
let file_path = uri_to_path(&path_uri);
136+
let file_path = uri_to_path(path_uri);
141137
let filename = if need_placeholder {
142138
"lsp_signature_helper_temp_file"
143139
} else {

crates/nu-lsp/src/symbols.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -274,12 +274,12 @@ impl LanguageServer {
274274
&mut self,
275275
params: &DocumentSymbolParams,
276276
) -> Option<DocumentSymbolResponse> {
277-
let uri = params.text_document.uri.to_owned();
278-
let engine_state = self.new_engine_state(Some(&uri));
277+
let uri = &params.text_document.uri;
278+
let engine_state = self.new_engine_state(Some(uri));
279279
let docs = self.docs.lock().ok()?;
280-
self.symbol_cache.update(&uri, &engine_state, &docs);
280+
self.symbol_cache.update(uri, &engine_state, &docs);
281281
self.symbol_cache
282-
.get_symbols_by_uri(&uri)
282+
.get_symbols_by_uri(uri)
283283
.map(DocumentSymbolResponse::Flat)
284284
}
285285

0 commit comments

Comments
 (0)