fix(completion): edge cases of operator completions (#15169)

# Description

Improves the completeness of operator completions.
Check the new test cases for details.

# User-Facing Changes

# Tests + Formatting

+4

# After Submitting
This commit is contained in:
zc he 2025-03-01 02:39:59 +08:00 committed by GitHub
parent c5a14bb8ff
commit 52a35827c7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 989 additions and 637 deletions

1
Cargo.lock generated
View File

@ -3538,6 +3538,7 @@ dependencies = [
"percent-encoding",
"reedline",
"rstest",
"strum",
"sysinfo",
"tempfile",
"unicode-segmentation",

View File

@ -155,6 +155,8 @@ serde_urlencoded = "0.7.1"
serde_yaml = "0.9.33"
sha2 = "0.10"
strip-ansi-escapes = "0.2.0"
strum = "0.26"
strum_macros = "0.26"
syn = "2.0"
sysinfo = "0.33"
tabled = { version = "0.17.0", default-features = false }

View File

@ -40,6 +40,7 @@ miette = { workspace = true, features = ["fancy-no-backtrace"] }
nucleo-matcher = { workspace = true }
percent-encoding = { workspace = true }
sysinfo = { workspace = true }
strum = { workspace = true }
unicode-segmentation = { workspace = true }
uuid = { workspace = true, features = ["v4"] }
which = { workspace = true }
@ -49,4 +50,4 @@ plugin = ["nu-plugin-engine"]
system-clipboard = ["reedline/system_clipboard"]
[lints]
workspace = true
workspace = true

View File

@ -31,6 +31,7 @@ pub enum SuggestionKind {
Command(nu_protocol::engine::CommandType),
Type(nu_protocol::Type),
Module,
Operator,
}
impl From<Suggestion> for SemanticSuggestion {

View File

@ -1,10 +1,10 @@
use crate::completions::{Completer, CompletionOptions, SemanticSuggestion, SuggestionKind};
use nu_engine::{column::get_columns, eval_variable};
use nu_protocol::{
ast::{Expr, FullCellPath, PathMember},
ast::{Expr, Expression, FullCellPath, PathMember},
engine::{Stack, StateWorkingSet},
eval_const::eval_constant,
Span, Value,
ShellError, Span, Value,
};
use reedline::Suggestion;
@ -42,37 +42,53 @@ impl Completer for CellPathCompletion<'_> {
};
let mut matcher = NuMatcher::new(prefix_str, options);
// evaluate the head expression to get its value
let value = if let Expr::Var(var_id) = self.full_cell_path.head.expr {
working_set
.get_variable(var_id)
.const_val
.to_owned()
.or_else(|| eval_variable(working_set.permanent_state, stack, var_id, span).ok())
} else {
eval_constant(working_set, &self.full_cell_path.head).ok()
}
let value = eval_cell_path(
working_set,
stack,
&self.full_cell_path.head,
path_members,
span,
)
.unwrap_or_default();
for suggestion in nested_suggestions(&value, path_members, current_span) {
for suggestion in get_suggestions_by_value(&value, current_span) {
matcher.add_semantic_suggestion(suggestion);
}
matcher.results()
}
}
// Find recursively the values for cell_path
fn nested_suggestions(
val: &Value,
/// Follow cell path to get the value
/// NOTE: This is a relatively lightweight implementation,
/// so it may fail to get the exact value when the expression is complicated.
/// One failing example would be `[$foo].0`
pub(crate) fn eval_cell_path(
working_set: &StateWorkingSet,
stack: &Stack,
head: &Expression,
path_members: &[PathMember],
span: Span,
) -> Result<Value, ShellError> {
// evaluate the head expression to get its value
let head_value = if let Expr::Var(var_id) = head.expr {
working_set
.get_variable(var_id)
.const_val
.to_owned()
.map_or_else(
|| eval_variable(working_set.permanent_state, stack, var_id, span),
Ok,
)
} else {
eval_constant(working_set, head)
}?;
head_value.follow_cell_path(path_members, false)
}
fn get_suggestions_by_value(
value: &Value,
current_span: reedline::Span,
) -> Vec<SemanticSuggestion> {
let value = val
.clone()
.follow_cell_path(path_members, false)
.unwrap_or_default();
let kind = SuggestionKind::Type(value.get_type());
let str_to_suggestion = |s: String| SemanticSuggestion {
suggestion: Suggestion {

View File

@ -2,155 +2,258 @@ use crate::completions::{
completion_options::NuMatcher, Completer, CompletionOptions, SemanticSuggestion, SuggestionKind,
};
use nu_protocol::{
ast::{Expr, Expression},
ast::{self, Comparison, Expr, Expression},
engine::{Stack, StateWorkingSet},
Span, Type,
Span, Type, Value, ENV_VARIABLE_ID,
};
use reedline::Suggestion;
use strum::{EnumMessage, IntoEnumIterator};
use super::cell_path_completions::eval_cell_path;
#[derive(Clone)]
pub struct OperatorCompletion<'a> {
pub left_hand_side: &'a Expression,
}
struct OperatorItem {
pub symbols: String,
pub description: String,
}
fn operator_to_item<T: EnumMessage + AsRef<str>>(op: T) -> OperatorItem {
OperatorItem {
symbols: op.as_ref().into(),
description: op.get_message().unwrap_or_default().into(),
}
}
fn common_comparison_ops() -> Vec<OperatorItem> {
vec![
operator_to_item(Comparison::In),
operator_to_item(Comparison::NotIn),
operator_to_item(Comparison::Equal),
operator_to_item(Comparison::NotEqual),
]
}
fn collection_comparison_ops() -> Vec<OperatorItem> {
let mut ops = common_comparison_ops();
ops.push(operator_to_item(Comparison::Has));
ops.push(operator_to_item(Comparison::NotHas));
ops
}
fn number_comparison_ops() -> Vec<OperatorItem> {
Comparison::iter()
.filter(|op| {
!matches!(
op,
Comparison::RegexMatch
| Comparison::NotRegexMatch
| Comparison::StartsWith
| Comparison::EndsWith
| Comparison::Has
| Comparison::NotHas
)
})
.map(operator_to_item)
.collect()
}
fn math_ops() -> Vec<OperatorItem> {
ast::Math::iter()
.filter(|op| !matches!(op, ast::Math::Concatenate | ast::Math::Pow))
.map(operator_to_item)
.collect()
}
fn bit_ops() -> Vec<OperatorItem> {
ast::Bits::iter().map(operator_to_item).collect()
}
fn numeric_assignment_ops() -> Vec<OperatorItem> {
ast::Assignment::iter()
.filter(|op| !matches!(op, ast::Assignment::ConcatenateAssign))
.map(operator_to_item)
.collect()
}
fn concat_assignment_ops() -> Vec<OperatorItem> {
vec![
operator_to_item(ast::Assignment::Assign),
operator_to_item(ast::Assignment::ConcatenateAssign),
]
}
fn valid_int_ops() -> Vec<OperatorItem> {
let mut ops = valid_float_ops();
ops.extend(bit_ops());
ops
}
fn valid_float_ops() -> Vec<OperatorItem> {
let mut ops = valid_value_with_unit_ops();
ops.push(operator_to_item(ast::Math::Pow));
ops
}
fn valid_string_ops() -> Vec<OperatorItem> {
let mut ops: Vec<OperatorItem> = Comparison::iter().map(operator_to_item).collect();
ops.push(operator_to_item(ast::Math::Concatenate));
ops.push(OperatorItem {
symbols: "like".into(),
description: Comparison::RegexMatch
.get_message()
.unwrap_or_default()
.into(),
});
ops.push(OperatorItem {
symbols: "not-like".into(),
description: Comparison::NotRegexMatch
.get_message()
.unwrap_or_default()
.into(),
});
ops
}
fn valid_list_ops() -> Vec<OperatorItem> {
let mut ops = collection_comparison_ops();
ops.push(operator_to_item(ast::Math::Concatenate));
ops
}
fn valid_binary_ops() -> Vec<OperatorItem> {
let mut ops = number_comparison_ops();
ops.extend(bit_ops());
ops.push(operator_to_item(ast::Math::Concatenate));
ops
}
fn valid_bool_ops() -> Vec<OperatorItem> {
let mut ops: Vec<OperatorItem> = ast::Boolean::iter().map(operator_to_item).collect();
ops.extend(common_comparison_ops());
ops
}
fn valid_value_with_unit_ops() -> Vec<OperatorItem> {
let mut ops = number_comparison_ops();
ops.extend(math_ops());
ops
}
fn ops_by_value(value: &Value, mutable: bool) -> Vec<OperatorItem> {
let mut ops = match value {
Value::Int { .. } => valid_int_ops(),
Value::Float { .. } => valid_float_ops(),
Value::String { .. } => valid_string_ops(),
Value::Binary { .. } => valid_binary_ops(),
Value::Bool { .. } => valid_bool_ops(),
Value::Date { .. } => number_comparison_ops(),
Value::Filesize { .. } | Value::Duration { .. } => valid_value_with_unit_ops(),
Value::Range { .. } | Value::Record { .. } => collection_comparison_ops(),
Value::List { .. } => valid_list_ops(),
_ => common_comparison_ops(),
};
if mutable {
ops.extend(match value {
Value::Int { .. }
| Value::Float { .. }
| Value::Filesize { .. }
| Value::Duration { .. } => numeric_assignment_ops(),
Value::String { .. } | Value::Binary { .. } | Value::List { .. } => {
concat_assignment_ops()
}
_ => vec![operator_to_item(ast::Assignment::Assign)],
})
}
ops
}
fn is_expression_mutable(expr: &Expr, working_set: &StateWorkingSet) -> bool {
let Expr::FullCellPath(path) = expr else {
return false;
};
let Expr::Var(id) = path.head.expr else {
return false;
};
if id == ENV_VARIABLE_ID {
return true;
}
let var = working_set.get_variable(id);
var.mutable
}
impl Completer for OperatorCompletion<'_> {
fn fetch(
&mut self,
working_set: &StateWorkingSet,
_stack: &Stack,
stack: &Stack,
prefix: impl AsRef<str>,
span: Span,
offset: usize,
options: &CompletionOptions,
) -> Vec<SemanticSuggestion> {
//Check if int, float, or string
let possible_operations = match &self.left_hand_side.expr {
Expr::Int(_) => vec![
("+", "Add (Plus)"),
("-", "Subtract (Minus)"),
("*", "Multiply"),
("/", "Divide"),
("==", "Equal to"),
("!=", "Not equal to"),
("//", "Floor division"),
("<", "Less than"),
(">", "Greater than"),
("<=", "Less than or equal to"),
(">=", "Greater than or equal to"),
("mod", "Floor division remainder (Modulo)"),
("**", "Power of"),
("bit-or", "Bitwise OR"),
("bit-xor", "Bitwise exclusive OR"),
("bit-and", "Bitwise AND"),
("bit-shl", "Bitwise shift left"),
("bit-shr", "Bitwise shift right"),
("in", "Is a member of (doesn't use regex)"),
("not-in", "Is not a member of (doesn't use regex)"),
],
Expr::String(_) => vec![
("=~", "Contains regex match"),
("like", "Contains regex match"),
("!~", "Does not contain regex match"),
("not-like", "Does not contain regex match"),
(
"++",
"Concatenates two lists, two strings, or two binary values",
),
("in", "Is a member of (doesn't use regex)"),
("not-in", "Is not a member of (doesn't use regex)"),
("starts-with", "Starts with"),
("ends-with", "Ends with"),
],
Expr::Float(_) => vec![
("+", "Add (Plus)"),
("-", "Subtract (Minus)"),
("*", "Multiply"),
("/", "Divide"),
("==", "Equal to"),
("!=", "Not equal to"),
("//", "Floor division"),
("<", "Less than"),
(">", "Greater than"),
("<=", "Less than or equal to"),
(">=", "Greater than or equal to"),
("mod", "Floor division remainder (Modulo)"),
("**", "Power of"),
("in", "Is a member of (doesn't use regex)"),
("not-in", "Is not a member of (doesn't use regex)"),
],
Expr::Bool(_) => vec![
(
"and",
"Both values are true (short-circuits when first value is false)",
),
(
"or",
"Either value is true (short-circuits when first value is true)",
),
("xor", "One value is true and the other is false"),
("not", "Negates a value or expression"),
("in", "Is a member of (doesn't use regex)"),
("not-in", "Is not a member of (doesn't use regex)"),
],
Expr::FullCellPath(path) => match path.head.expr {
Expr::List(_) => vec![
(
"++",
"Concatenates two lists, two strings, or two binary values",
),
("has", "Contains a value of (doesn't use regex)"),
("not-has", "Does not contain a value of (doesn't use regex)"),
],
Expr::Var(id) => get_variable_completions(id, working_set),
_ => vec![],
let mut needs_assignment_ops = true;
// Complete according expression type
// TODO: type inference on self.left_hand_side to get more accurate completions
let mut possible_operations: Vec<OperatorItem> = match &self.left_hand_side.ty {
Type::Int | Type::Number => valid_int_ops(),
Type::Float => valid_float_ops(),
Type::String => valid_string_ops(),
Type::Binary => valid_binary_ops(),
Type::Bool => valid_bool_ops(),
Type::Date => number_comparison_ops(),
Type::Filesize | Type::Duration => valid_value_with_unit_ops(),
Type::Record(_) | Type::Range => collection_comparison_ops(),
Type::List(_) | Type::Table(_) => valid_list_ops(),
// Unknown type, resort to evaluated values
Type::Any => match &self.left_hand_side.expr {
Expr::FullCellPath(path) => {
// for `$ <tab>`
if matches!(path.head.expr, Expr::Garbage) {
return vec![];
}
let value =
eval_cell_path(working_set, stack, &path.head, &path.tail, path.head.span)
.unwrap_or_default();
let mutable = is_expression_mutable(&self.left_hand_side.expr, working_set);
// to avoid duplication
needs_assignment_ops = false;
ops_by_value(&value, mutable)
}
_ => common_comparison_ops(),
},
_ => vec![],
_ => common_comparison_ops(),
};
// If the left hand side is a variable, add assignment operators if mutable
if needs_assignment_ops && is_expression_mutable(&self.left_hand_side.expr, working_set) {
possible_operations.extend(match &self.left_hand_side.ty {
Type::Int | Type::Float | Type::Number => numeric_assignment_ops(),
Type::Filesize | Type::Duration => numeric_assignment_ops(),
Type::String | Type::Binary | Type::List(_) => concat_assignment_ops(),
_ => vec![operator_to_item(ast::Assignment::Assign)],
});
}
let mut matcher = NuMatcher::new(prefix, options);
for (symbol, desc) in possible_operations.into_iter() {
for OperatorItem {
symbols,
description,
} in possible_operations
{
matcher.add_semantic_suggestion(SemanticSuggestion {
suggestion: Suggestion {
value: symbol.to_string(),
description: Some(desc.to_string()),
value: symbols.to_owned(),
description: Some(description.to_owned()),
span: reedline::Span::new(span.start - offset, span.end - offset),
append_whitespace: true,
..Suggestion::default()
},
kind: Some(SuggestionKind::Command(
nu_protocol::engine::CommandType::Builtin,
)),
kind: Some(SuggestionKind::Operator),
});
}
matcher.results()
}
}
pub fn get_variable_completions<'a>(
id: nu_protocol::Id<nu_protocol::marker::Var>,
working_set: &StateWorkingSet,
) -> Vec<(&'a str, &'a str)> {
let var = working_set.get_variable(id);
if !var.mutable {
return vec![];
}
match var.ty {
Type::List(_) | Type::String | Type::Binary => vec![
(
"++=",
"Concatenates two lists, two strings, or two binary values",
),
("=", "Assigns a value to a variable."),
],
Type::Int | Type::Float => vec![
("=", "Assigns a value to a variable."),
("+=", "Adds a value to a variable."),
("-=", "Subtracts a value from a variable."),
("*=", "Multiplies a variable by a value"),
("/=", "Divides a variable by a value."),
],
_ => vec![],
}
}

File diff suppressed because it is too large Load Diff

View File

@ -217,7 +217,7 @@ pub fn new_partial_engine() -> (AbsolutePathBuf, String, EngineState, Stack) {
}
/// match a list of suggestions with the expected values
pub fn match_suggestions(expected: &Vec<String>, suggestions: &Vec<Suggestion>) {
pub fn match_suggestions(expected: &Vec<&str>, suggestions: &Vec<Suggestion>) {
let expected_len = expected.len();
let suggestions_len = suggestions.len();
if expected_len != suggestions_len {
@ -230,12 +230,18 @@ pub fn match_suggestions(expected: &Vec<String>, suggestions: &Vec<Suggestion>)
let suggestions_str = suggestions
.iter()
.map(|it| it.value.clone())
.map(|it| it.value.as_str())
.collect::<Vec<_>>();
assert_eq!(expected, &suggestions_str);
}
/// match a list of suggestions with the expected values
pub fn match_suggestions_by_string(expected: &[String], suggestions: &Vec<Suggestion>) {
let expected = expected.iter().map(|it| it.as_str()).collect::<Vec<_>>();
match_suggestions(&expected, suggestions);
}
/// append the separator to the converted path
pub fn folder(path: impl Into<PathBuf>) -> String {
let mut converted_path = file(path);

View File

@ -1,3 +1,5 @@
pub mod completions_helpers;
pub use completions_helpers::{file, folder, match_suggestions, merge_input, new_engine};
pub use completions_helpers::{
file, folder, match_suggestions, match_suggestions_by_string, merge_input, new_engine,
};

View File

@ -705,7 +705,8 @@ impl LanguageServer {
.map(|kind| match kind {
SuggestionKind::Type(t) => t.to_string(),
SuggestionKind::Command(cmd) => cmd.to_string(),
SuggestionKind::Module => "".to_string(),
SuggestionKind::Module => "module".to_string(),
SuggestionKind::Operator => "operator".to_string(),
})
.map(|s| CompletionItemLabelDetails {
detail: None,
@ -754,6 +755,7 @@ impl LanguageServer {
nu_protocol::engine::CommandType::External => Some(CompletionItemKind::INTERFACE),
_ => None,
},
SuggestionKind::Operator => Some(CompletionItemKind::OPERATOR),
SuggestionKind::Module => Some(CompletionItemKind::MODULE),
})
}

View File

@ -35,6 +35,8 @@ num-format = { workspace = true }
rmp-serde = { workspace = true, optional = true }
serde = { workspace = true }
serde_json = { workspace = true }
strum = { workspace = true }
strum_macros = { workspace = true }
thiserror = "2.0"
typetag = "0.2"
os_pipe = { workspace = true, optional = true, features = ["io_safety"] }
@ -64,8 +66,6 @@ plugin = [
[dev-dependencies]
serde_json = { workspace = true }
strum = "0.26"
strum_macros = "0.26"
nu-test-support = { path = "../nu-test-support", version = "0.102.1" }
nu-utils = { path = "../nu-utils", version = "0.102.1" }
pretty_assertions = { workspace = true }

View File

@ -2,22 +2,37 @@ use super::{Expr, Expression};
use crate::{ShellError, Span};
use serde::{Deserialize, Serialize};
use std::fmt;
use strum_macros::{EnumIter, EnumMessage};
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, EnumIter, EnumMessage)]
pub enum Comparison {
#[strum(message = "Equal to")]
Equal,
#[strum(message = "Not equal to")]
NotEqual,
#[strum(message = "Less than")]
LessThan,
#[strum(message = "Greater than")]
GreaterThan,
#[strum(message = "Less than or equal to")]
LessThanOrEqual,
#[strum(message = "Greater than or equal to")]
GreaterThanOrEqual,
#[strum(message = "Contains regex match")]
RegexMatch,
#[strum(message = "Does not contain regex match")]
NotRegexMatch,
#[strum(message = "Is a member of (doesn't use regex)")]
In,
#[strum(message = "Is not a member of (doesn't use regex)")]
NotIn,
#[strum(message = "Contains a value of (doesn't use regex)")]
Has,
#[strum(message = "Does not contain a value of (doesn't use regex)")]
NotHas,
#[strum(message = "Starts with")]
StartsWith,
#[strum(message = "Ends with")]
EndsWith,
}
@ -42,21 +57,35 @@ impl Comparison {
}
}
impl AsRef<str> for Comparison {
fn as_ref(&self) -> &str {
self.as_str()
}
}
impl fmt::Display for Comparison {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str(self.as_str())
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, EnumIter, EnumMessage)]
pub enum Math {
#[strum(message = "Add (Plus)")]
Add,
#[strum(message = "Subtract (Minus)")]
Subtract,
#[strum(message = "Multiply")]
Multiply,
#[strum(message = "Divide")]
Divide,
#[strum(message = "Floor division")]
FloorDivide,
#[strum(message = "Floor division remainder (Modulo)")]
Modulo,
#[strum(message = "Power of")]
Pow,
#[strum(message = "Concatenates two lists, two strings, or two binary values")]
Concatenate,
}
@ -75,16 +104,25 @@ impl Math {
}
}
impl AsRef<str> for Math {
fn as_ref(&self) -> &str {
self.as_str()
}
}
impl fmt::Display for Math {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str(self.as_str())
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, EnumIter, EnumMessage)]
pub enum Boolean {
#[strum(message = "Logical OR (short-circuiting)")]
Or,
#[strum(message = "Logical XOR")]
Xor,
#[strum(message = "Logical AND (short-circuiting)")]
And,
}
@ -98,21 +136,38 @@ impl Boolean {
}
}
impl AsRef<str> for Boolean {
fn as_ref(&self) -> &str {
self.as_str()
}
}
impl fmt::Display for Boolean {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str(self.as_str())
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, EnumIter, EnumMessage)]
pub enum Bits {
#[strum(message = "Bitwise OR")]
BitOr,
#[strum(message = "Bitwise exclusive OR")]
BitXor,
#[strum(message = "Bitwise AND")]
BitAnd,
#[strum(message = "Bitwise shift left")]
ShiftLeft,
#[strum(message = "Bitwise shift right")]
ShiftRight,
}
impl AsRef<str> for Bits {
fn as_ref(&self) -> &str {
self.as_str()
}
}
impl Bits {
pub const fn as_str(&self) -> &'static str {
match self {
@ -131,16 +186,28 @@ impl fmt::Display for Bits {
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, EnumIter, EnumMessage)]
pub enum Assignment {
#[strum(message = "Assigns a value to a variable.")]
Assign,
#[strum(message = "Adds a value to a variable.")]
AddAssign,
#[strum(message = "Subtracts a value from a variable.")]
SubtractAssign,
#[strum(message = "Multiplies a variable by a value")]
MultiplyAssign,
#[strum(message = "Divides a variable by a value.")]
DivideAssign,
#[strum(message = "Concatenates a variable with a list, string or binary.")]
ConcatenateAssign,
}
impl AsRef<str> for Assignment {
fn as_ref(&self) -> &str {
self.as_str()
}
}
impl Assignment {
pub const fn as_str(&self) -> &'static str {
match self {
@ -221,7 +288,7 @@ impl fmt::Display for Operator {
}
}
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize, EnumIter)]
pub enum RangeInclusion {
Inclusive,
RightExclusive,