refactor(completion): AST traverse to find the inner-most expression to complete (#14973)

# Description

As discussed
[here](https://github.com/nushell/nushell/pull/14856#issuecomment-2623393017)
and [here](https://github.com/nushell/nushell/discussions/14868).

I feel this method is generally better. As for the new-parser, we can
simply modify the implementation in `traverse.rs` to accommodate.

Next, I'm gonna overhaul the `Completer` trait, so before it gets really
messy, I' think this is the step to put this open for review so we can
check if I'm on track.

This PR closes #13897 (the `|` part)

# User-Facing Changes

# After Submitting
This commit is contained in:
zc he
2025-02-06 20:49:13 +08:00
committed by GitHub
parent 0b2d1327d2
commit 164a089656
6 changed files with 619 additions and 466 deletions

View File

@ -1,172 +1,11 @@
use crate::Id;
use nu_protocol::{
ast::{
Argument, Block, Call, Expr, Expression, ExternalArgument, ListItem, MatchPattern,
PathMember, Pattern, PipelineRedirection, RecordItem,
},
ast::{Argument, Block, Call, Expr, Expression, FindMapResult, ListItem, PathMember, Traverse},
engine::StateWorkingSet,
Span,
};
use std::sync::Arc;
/// similar to flatten_block, but allows extra map function
pub fn ast_flat_map<'a, T, F>(
ast: &'a Arc<Block>,
working_set: &'a StateWorkingSet,
f_special: &F,
) -> Vec<T>
where
F: Fn(&'a Expression) -> Option<Vec<T>>,
{
ast.pipelines
.iter()
.flat_map(|pipeline| {
pipeline.elements.iter().flat_map(|element| {
expr_flat_map(&element.expr, working_set, f_special)
.into_iter()
.chain(
element
.redirection
.as_ref()
.map(|redir| redirect_flat_map(redir, working_set, f_special))
.unwrap_or_default(),
)
})
})
.collect()
}
/// generic function that do flat_map on an expression
/// concats all recursive results on sub-expressions
///
/// # Arguments
/// * `f_special` - function that overrides the default behavior
pub fn expr_flat_map<'a, T, F>(
expr: &'a Expression,
working_set: &'a StateWorkingSet,
f_special: &F,
) -> Vec<T>
where
F: Fn(&'a Expression) -> Option<Vec<T>>,
{
// behavior overridden by f_special
if let Some(vec) = f_special(expr) {
return vec;
}
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, f_special)
}
Expr::Range(range) => [&range.from, &range.next, &range.to]
.iter()
.filter_map(|e| e.as_ref())
.flat_map(recur)
.collect(),
Expr::Call(call) => call
.arguments
.iter()
.filter_map(|arg| arg.expr())
.flat_map(recur)
.collect(),
Expr::ExternalCall(head, args) => recur(head)
.into_iter()
.chain(args.iter().flat_map(|arg| match arg {
ExternalArgument::Regular(e) | ExternalArgument::Spread(e) => recur(e),
}))
.collect(),
Expr::UnaryNot(expr) | Expr::Collect(_, expr) => recur(expr),
Expr::BinaryOp(lhs, op, rhs) => recur(lhs)
.into_iter()
.chain(recur(op))
.chain(recur(rhs))
.collect(),
Expr::MatchBlock(matches) => matches
.iter()
.flat_map(|(pattern, expr)| {
match_pattern_flat_map(pattern, working_set, f_special)
.into_iter()
.chain(recur(expr))
})
.collect(),
Expr::List(items) => items
.iter()
.flat_map(|item| match item {
ListItem::Item(expr) | ListItem::Spread(_, expr) => recur(expr),
})
.collect(),
Expr::Record(items) => items
.iter()
.flat_map(|item| match item {
RecordItem::Spread(_, expr) => recur(expr),
RecordItem::Pair(key, val) => [key, val].into_iter().flat_map(recur).collect(),
})
.collect(),
Expr::Table(table) => table
.columns
.iter()
.flat_map(recur)
.chain(table.rows.iter().flat_map(|row| row.iter().flat_map(recur)))
.collect(),
Expr::ValueWithUnit(vu) => recur(&vu.expr),
Expr::FullCellPath(fcp) => recur(&fcp.head),
Expr::Keyword(kw) => recur(&kw.expr),
Expr::StringInterpolation(vec) | Expr::GlobInterpolation(vec, _) => {
vec.iter().flat_map(recur).collect()
}
_ => Vec::new(),
}
}
/// flat_map on match patterns
fn match_pattern_flat_map<'a, T, F>(
pattern: &'a MatchPattern,
working_set: &'a StateWorkingSet,
f_special: &F,
) -> Vec<T>
where
F: Fn(&'a Expression) -> Option<Vec<T>>,
{
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) => {
patterns.iter().flat_map(recur_match).collect()
}
Pattern::Record(entries) => entries.iter().flat_map(|(_, p)| recur_match(p)).collect(),
_ => Vec::new(),
}
.into_iter()
.chain(pattern.guard.as_ref().map(|g| recur(g)).unwrap_or_default())
.collect()
}
/// flat_map on redirections
fn redirect_flat_map<'a, T, F>(
redir: &'a PipelineRedirection,
working_set: &'a StateWorkingSet,
f_special: &F,
) -> Vec<T>
where
F: Fn(&'a Expression) -> Option<Vec<T>>,
{
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]
.iter()
.filter_map(|t| t.expr())
.flat_map(recur)
.collect(),
}
}
/// 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));
@ -436,22 +275,22 @@ fn find_id_in_expr(
expr: &Expression,
working_set: &StateWorkingSet,
location: &usize,
) -> Option<Vec<(Id, Span)>> {
) -> FindMapResult<(Id, Span)> {
// skip the entire expression if the location is not in it
if !expr.span.contains(*location) {
return Some(Vec::new());
return FindMapResult::Stop;
}
let span = expr.span;
match &expr.expr {
Expr::VarDecl(var_id) => Some(vec![(Id::Variable(*var_id), span)]),
Expr::VarDecl(var_id) => FindMapResult::Found((Id::Variable(*var_id), span)),
// trim leading `$` sign
Expr::Var(var_id) => Some(vec![(
Expr::Var(var_id) => FindMapResult::Found((
Id::Variable(*var_id),
Span::new(span.start.saturating_add(1), span.end),
)]),
)),
Expr::Call(call) => {
if call.head.contains(*location) {
Some(vec![(Id::Declaration(call.decl_id), call.head)])
FindMapResult::Found((Id::Declaration(call.decl_id), call.head))
} else {
try_find_id_in_def(call, working_set, Some(location), None)
.or(try_find_id_in_mod(call, working_set, Some(location), None))
@ -462,19 +301,20 @@ fn find_id_in_expr(
Some(location),
None,
))
.map(|p| vec![p])
.map(FindMapResult::Found)
.unwrap_or_default()
}
}
Expr::FullCellPath(fcp) => {
if fcp.head.span.contains(*location) {
None
FindMapResult::Continue
} else {
let Expression {
expr: Expr::Var(var_id),
..
} = fcp.head
else {
return None;
return FindMapResult::Continue;
};
let tail: Vec<PathMember> = fcp
.tail
@ -482,11 +322,13 @@ fn find_id_in_expr(
.into_iter()
.take_while(|pm| pm.span().start <= *location)
.collect();
let span = tail.last()?.span();
Some(vec![(Id::CellPath(var_id, tail), span)])
let Some(span) = tail.last().map(|pm| pm.span()) else {
return FindMapResult::Stop;
};
FindMapResult::Found((Id::CellPath(var_id, tail), span))
}
}
Expr::Overlay(Some(module_id)) => Some(vec![(Id::Module(*module_id), span)]),
Expr::Overlay(Some(module_id)) => FindMapResult::Found((Id::Module(*module_id), span)),
// terminal value expressions
Expr::Bool(_)
| Expr::Binary(_)
@ -500,8 +342,8 @@ fn find_id_in_expr(
| Expr::Nothing
| Expr::RawString(_)
| Expr::Signature(_)
| Expr::String(_) => Some(vec![(Id::Value(expr.ty.clone()), span)]),
_ => None,
| Expr::String(_) => FindMapResult::Found((Id::Value(expr.ty.clone()), span)),
_ => FindMapResult::Continue,
}
}
@ -512,7 +354,7 @@ pub(crate) fn find_id(
location: &usize,
) -> Option<(Id, Span)> {
let closure = |e| find_id_in_expr(e, working_set, location);
ast_flat_map(ast, working_set, &closure).first().cloned()
ast.find_map(working_set, &closure)
}
fn find_reference_by_id_in_expr(
@ -521,7 +363,6 @@ fn find_reference_by_id_in_expr(
id: &Id,
) -> Option<Vec<Span>> {
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
@ -536,7 +377,7 @@ fn find_reference_by_id_in_expr(
.arguments
.iter()
.filter_map(|arg| arg.expr())
.flat_map(recur)
.flat_map(|e| e.flat_map(working_set, &closure))
.collect();
if matches!(id, Id::Declaration(decl_id) if call.decl_id == *decl_id) {
occurs.push(call.head);
@ -560,7 +401,7 @@ pub(crate) fn find_reference_by_id(
working_set: &StateWorkingSet,
id: &Id,
) -> Vec<Span> {
ast_flat_map(ast, working_set, &|e| {
ast.flat_map(working_set, &|e| {
find_reference_by_id_in_expr(e, working_set, id)
})
}

View File

@ -1,4 +1,3 @@
use crate::ast::{ast_flat_map, expr_flat_map};
use crate::{span_to_range, LanguageServer};
use lsp_textdocument::FullTextDocument;
use lsp_types::{
@ -6,7 +5,7 @@ use lsp_types::{
MarkupKind, Position, Range,
};
use nu_protocol::{
ast::{Argument, Block, Expr, Expression, Operator},
ast::{Argument, Block, Expr, Expression, Operator, Traverse},
engine::StateWorkingSet,
Type,
};
@ -29,11 +28,12 @@ fn extract_inlay_hints_from_expression(
file: &FullTextDocument,
) -> Option<Vec<InlayHint>> {
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<InlayHint> =
[lhs, op, rhs].into_iter().flat_map(|e| recur(e)).collect();
let mut hints: Vec<InlayHint> = [lhs, op, rhs]
.into_iter()
.flat_map(|e| e.flat_map(working_set, &closure))
.collect();
if let Expr::Operator(Operator::Assignment(_)) = op.expr {
let position = span_to_range(&lhs.span, file, *offset).end;
let type_rhs = type_short_name(&rhs.ty);
@ -103,13 +103,13 @@ fn extract_inlay_hints_from_expression(
match arg {
// skip the rest when spread/unknown arguments encountered
Argument::Spread(expr) | Argument::Unknown(expr) => {
hints.extend(recur(expr));
hints.extend(expr.flat_map(working_set, &closure));
sig_idx = signatures.len();
continue;
}
// skip current for flags
Argument::Named((_, _, Some(expr))) => {
hints.extend(recur(expr));
hints.extend(expr.flat_map(working_set, &closure));
continue;
}
Argument::Positional(expr) => {
@ -130,7 +130,7 @@ fn extract_inlay_hints_from_expression(
padding_right: None,
});
}
hints.extend(recur(expr));
hints.extend(expr.flat_map(working_set, &closure));
}
_ => {
continue;
@ -154,7 +154,7 @@ impl LanguageServer {
offset: usize,
file: &FullTextDocument,
) -> Vec<InlayHint> {
ast_flat_map(block, working_set, &|e| {
block.flat_map(working_set, &|e| {
extract_inlay_hints_from_expression(e, working_set, &offset, file)
})
}