Skip to content

Commit 5b0800a

Browse files
blindFSblindFS
authored
fix(completion): missing local variables (nushell#16531)
Closes nushell#15291, nushell#6696
1 parent e1ce67e commit 5b0800a

File tree

6 files changed

+106
-34
lines changed

6 files changed

+106
-34
lines changed

‎crates/nu-cli/src/completions/variable_completions.rs‎

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
use std::collections::HashMap;
2+
13
use crate::completions::{Completer, CompletionOptions, SemanticSuggestion, SuggestionKind};
24
use nu_protocol::{
3-
Span, VarId,
5+
ENV_VARIABLE_ID, IN_VARIABLE_ID, NU_VARIABLE_ID, Span,
46
engine::{Stack, StateWorkingSet},
57
};
68
use reedline::Suggestion;
@@ -26,30 +28,10 @@ impl Completer for VariableCompletion {
2628
};
2729

2830
// Variable completion (e.g: $en<tab> to complete $env)
29-
let builtins = ["$nu", "$in", "$env"];
30-
for builtin in builtins {
31-
matcher.add_semantic_suggestion(SemanticSuggestion {
32-
suggestion: Suggestion {
33-
value: builtin.to_string(),
34-
span: current_span,
35-
description: Some("reserved".into()),
36-
..Suggestion::default()
37-
},
38-
kind: Some(SuggestionKind::Variable),
39-
});
40-
}
41-
42-
let mut add_candidate = |name, var_id: &VarId| {
43-
matcher.add_semantic_suggestion(SemanticSuggestion {
44-
suggestion: Suggestion {
45-
value: String::from_utf8_lossy(name).to_string(),
46-
span: current_span,
47-
description: Some(working_set.get_variable(*var_id).ty.to_string()),
48-
..Suggestion::default()
49-
},
50-
kind: Some(SuggestionKind::Variable),
51-
})
52-
};
31+
let mut variables = HashMap::new();
32+
variables.insert("$nu".into(), &NU_VARIABLE_ID);
33+
variables.insert("$in".into(), &IN_VARIABLE_ID);
34+
variables.insert("$env".into(), &ENV_VARIABLE_ID);
5335

5436
// TODO: The following can be refactored (see find_commands_by_predicate() used in
5537
// command_completions).
@@ -58,7 +40,8 @@ impl Completer for VariableCompletion {
5840
for scope_frame in working_set.delta.scope.iter().rev() {
5941
for overlay_frame in scope_frame.active_overlays(&mut removed_overlays).rev() {
6042
for (name, var_id) in &overlay_frame.vars {
61-
add_candidate(name, var_id);
43+
let name = String::from_utf8_lossy(name).to_string();
44+
variables.insert(name, var_id);
6245
}
6346
}
6447
}
@@ -70,10 +53,23 @@ impl Completer for VariableCompletion {
7053
.rev()
7154
{
7255
for (name, var_id) in &overlay_frame.vars {
73-
add_candidate(name, var_id);
56+
let name = String::from_utf8_lossy(name).to_string();
57+
variables.insert(name, var_id);
7458
}
7559
}
7660

61+
for (name, var_id) in variables {
62+
matcher.add_semantic_suggestion(SemanticSuggestion {
63+
suggestion: Suggestion {
64+
value: name,
65+
span: current_span,
66+
description: Some(working_set.get_variable(*var_id).ty.to_string()),
67+
..Suggestion::default()
68+
},
69+
kind: Some(SuggestionKind::Variable),
70+
});
71+
}
72+
7773
matcher.results()
7874
}
7975
}

‎crates/nu-cli/tests/completions/mod.rs‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1980,6 +1980,48 @@ fn variables_completions() {
19801980
match_suggestions(&expected, &suggestions);
19811981
}
19821982

1983+
#[test]
1984+
fn local_variable_completion() {
1985+
let (_, _, engine, stack) = new_engine();
1986+
let mut completer = NuCompleter::new(Arc::new(engine), Arc::new(stack));
1987+
1988+
let completion_str = "def test [foo?: string, --foo1: bool, ...foo2] { let foo3 = true; $foo";
1989+
let suggestions = completer.complete(completion_str, completion_str.len());
1990+
1991+
// https://github.com/nushell/nushell/issues/15291
1992+
let expected: Vec<_> = vec!["$foo", "$foo1", "$foo2", "$foo3"];
1993+
match_suggestions(&expected, &suggestions);
1994+
1995+
let completion_str = "if true { let foo = true; $foo";
1996+
let suggestions = completer.complete(completion_str, completion_str.len());
1997+
let expected: Vec<_> = vec!["$foo"];
1998+
match_suggestions(&expected, &suggestions);
1999+
2000+
let completion_str = "if true {let foo1 = 1} else {let foo = true; $foo";
2001+
let suggestions = completer.complete(completion_str, completion_str.len());
2002+
let expected: Vec<_> = vec!["$foo"];
2003+
match_suggestions(&expected, &suggestions);
2004+
2005+
let completion_str = "for foo in [1] { let foo1 = true; $foo";
2006+
let suggestions = completer.complete(completion_str, completion_str.len());
2007+
let expected: Vec<_> = vec!["$foo", "$foo1"];
2008+
match_suggestions(&expected, &suggestions);
2009+
2010+
let completion_str = "for foo in [1] { let foo1 = true }; $foo";
2011+
let suggestions = completer.complete(completion_str, completion_str.len());
2012+
assert!(suggestions.is_empty());
2013+
2014+
let completion_str = "(let foo = true; $foo";
2015+
let suggestions = completer.complete(completion_str, completion_str.len());
2016+
let expected: Vec<_> = vec!["$foo"];
2017+
match_suggestions(&expected, &suggestions);
2018+
2019+
let completion_str = "match {a: {b: 3}} {{a: {b: $foo}} => $foo";
2020+
let suggestions = completer.complete(completion_str, completion_str.len());
2021+
let expected: Vec<_> = vec!["$foo"];
2022+
match_suggestions(&expected, &suggestions);
2023+
}
2024+
19832025
#[test]
19842026
fn record_cell_path_completions() {
19852027
let (_, _, mut engine, mut stack) = new_engine();

‎crates/nu-cmd-lang/src/core_commands/for_.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ impl Command for For {
2828
"Range of the loop.",
2929
)
3030
.required("block", SyntaxShape::Block, "The block to run.")
31-
.creates_scope()
3231
.category(Category::Core)
3332
}
3433

‎crates/nu-lsp/src/completion.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,12 @@ impl LanguageServer {
2323
// fallback to default completer where
2424
// the text is truncated to `location` and
2525
// an extra placeholder token is inserted for correct parsing
26+
let is_variable = file_text
27+
.get(..location)
28+
.and_then(|s| s.rsplit(' ').next())
29+
.is_some_and(|last_word| last_word.starts_with('$'));
2630
let need_fallback = location == 0
31+
|| is_variable
2732
|| file_text
2833
.get(location - 1..location)
2934
.and_then(|s| s.chars().next())

‎crates/nu-parser/src/parse_keywords.rs‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,11 +279,22 @@ pub fn parse_for(working_set: &mut StateWorkingSet, lite_command: &LiteCommand)
279279
return garbage(working_set, spans[0]);
280280
}
281281
Some(decl_id) => {
282+
let starting_error_count = working_set.parse_errors.len();
282283
working_set.enter_scope();
283284
let ParsedInternalCall { call, output } =
284285
parse_internal_call(working_set, spans[0], &spans[1..], decl_id);
285286

286-
working_set.exit_scope();
287+
if working_set
288+
.parse_errors
289+
.get(starting_error_count..)
290+
.is_none_or(|new_errors| {
291+
new_errors
292+
.iter()
293+
.all(|e| !matches!(e, ParseError::Unclosed(token, _) if token == "}"))
294+
})
295+
{
296+
working_set.exit_scope();
297+
}
287298

288299
let call_span = Span::concat(spans);
289300
let decl = working_set.get_decl(decl_id);
@@ -595,7 +606,12 @@ fn parse_def_inner(
595606
let mut new_errors = working_set.parse_errors[starting_error_count..].to_vec();
596607
working_set.parse_errors.truncate(starting_error_count);
597608

598-
working_set.exit_scope();
609+
if new_errors
610+
.iter()
611+
.all(|e| !matches!(e, ParseError::Unclosed(token, _) if token == "}"))
612+
{
613+
working_set.exit_scope();
614+
}
599615

600616
let call_span = Span::concat(spans);
601617
let decl = working_set.get_decl(decl_id);

‎crates/nu-parser/src/parser.rs‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2584,6 +2584,7 @@ pub fn parse_full_cell_path(
25842584
let head_span = head.span;
25852585
let mut start = head.span.start;
25862586
let mut end = head.span.end;
2587+
let mut is_closed = true;
25872588

25882589
if bytes.starts_with(b"(") {
25892590
start += 1;
@@ -2592,6 +2593,7 @@ pub fn parse_full_cell_path(
25922593
end -= 1;
25932594
} else {
25942595
working_set.error(ParseError::Unclosed(")".into(), Span::new(end, end)));
2596+
is_closed = false;
25952597
}
25962598

25972599
let span = Span::new(start, end);
@@ -2605,7 +2607,7 @@ pub fn parse_full_cell_path(
26052607

26062608
// Creating a Type scope to parse the new block. This will keep track of
26072609
// the previous input type found in that block
2608-
let output = parse_block(working_set, &output, span, true, true);
2610+
let output = parse_block(working_set, &output, span, is_closed, true);
26092611

26102612
let ty = output.output_type();
26112613

@@ -4848,6 +4850,7 @@ pub fn parse_block_expression(working_set: &mut StateWorkingSet, span: Span) ->
48484850

48494851
let mut start = span.start;
48504852
let mut end = span.end;
4853+
let mut is_closed = true;
48514854

48524855
if bytes.starts_with(b"{") {
48534856
start += 1;
@@ -4859,6 +4862,7 @@ pub fn parse_block_expression(working_set: &mut StateWorkingSet, span: Span) ->
48594862
end -= 1;
48604863
} else {
48614864
working_set.error(ParseError::Unclosed("}".into(), Span::new(end, end)));
4865+
is_closed = false;
48624866
}
48634867

48644868
let inner_span = Span::new(start, end);
@@ -4892,7 +4896,9 @@ pub fn parse_block_expression(working_set: &mut StateWorkingSet, span: Span) ->
48924896

48934897
output.span = Some(span);
48944898

4895-
working_set.exit_scope();
4899+
if is_closed {
4900+
working_set.exit_scope();
4901+
}
48964902

48974903
let block_id = working_set.add_block(Arc::new(output));
48984904

@@ -4904,6 +4910,7 @@ pub fn parse_match_block_expression(working_set: &mut StateWorkingSet, span: Spa
49044910

49054911
let mut start = span.start;
49064912
let mut end = span.end;
4913+
let mut is_closed = true;
49074914

49084915
if bytes.starts_with(b"{") {
49094916
start += 1;
@@ -4915,6 +4922,7 @@ pub fn parse_match_block_expression(working_set: &mut StateWorkingSet, span: Spa
49154922
end -= 1;
49164923
} else {
49174924
working_set.error(ParseError::Unclosed("}".into(), Span::new(end, end)));
4925+
is_closed = false;
49184926
}
49194927

49204928
let inner_span = Span::new(start, end);
@@ -5080,7 +5088,9 @@ pub fn parse_match_block_expression(working_set: &mut StateWorkingSet, span: Spa
50805088
&SyntaxShape::OneOf(vec![SyntaxShape::Block, SyntaxShape::Expression]),
50815089
);
50825090
position += 1;
5083-
working_set.exit_scope();
5091+
if is_closed {
5092+
working_set.exit_scope();
5093+
}
50845094

50855095
output_matches.push((pattern, result));
50865096
}
@@ -5104,6 +5114,7 @@ pub fn parse_closure_expression(
51045114

51055115
let mut start = span.start;
51065116
let mut end = span.end;
5117+
let mut is_closed = true;
51075118

51085119
if bytes.starts_with(b"{") {
51095120
start += 1;
@@ -5115,6 +5126,7 @@ pub fn parse_closure_expression(
51155126
end -= 1;
51165127
} else {
51175128
working_set.error(ParseError::Unclosed("}".into(), Span::new(end, end)));
5129+
is_closed = false;
51185130
}
51195131

51205132
let inner_span = Span::new(start, end);
@@ -5211,7 +5223,9 @@ pub fn parse_closure_expression(
52115223

52125224
output.span = Some(span);
52135225

5214-
working_set.exit_scope();
5226+
if is_closed {
5227+
working_set.exit_scope();
5228+
}
52155229

52165230
let block_id = working_set.add_block(Arc::new(output));
52175231

0 commit comments

Comments
 (0)