Add Value::coerce_str (#11885)

# Description
Following #11851, this PR adds one final conversion function for
`Value`. `Value::coerce_str` takes a `&Value` and converts it to a
`Cow<str>`, creating an owned `String` for types that needed converting.
Otherwise, it returns a borrowed `str` for `String` and `Binary`
`Value`s which avoids a clone/allocation. Where possible, `coerce_str`
and `coerce_into_string` should be used instead of `coerce_string`,
since `coerce_string` always allocates a new `String`.
This commit is contained in:
Ian Manske 2024-02-18 16:47:10 +00:00 committed by GitHub
parent fb4251aba7
commit 68fcd71898
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
34 changed files with 115 additions and 83 deletions

View File

@ -43,9 +43,9 @@ impl CommandCompletion {
if let Some(paths) = paths {
if let Ok(paths) = paths.as_list() {
for path in paths {
let path = path.coerce_string().unwrap_or_default();
let path = path.coerce_str().unwrap_or_default();
if let Ok(mut contents) = std::fs::read_dir(path) {
if let Ok(mut contents) = std::fs::read_dir(path.as_ref()) {
while let Some(Ok(item)) = contents.next() {
if self.engine_state.config.max_external_completion_results
> executables.len() as i64

View File

@ -28,7 +28,7 @@ pub fn evaluate_commands(
let (block, delta) = {
if let Some(ref t_mode) = table_mode {
let mut config = engine_state.get_config().clone();
config.table_mode = t_mode.coerce_string()?.parse().unwrap_or_default();
config.table_mode = t_mode.coerce_str()?.parse().unwrap_or_default();
engine_state.set_config(config);
}
@ -59,7 +59,7 @@ pub fn evaluate_commands(
Ok(pipeline_data) => {
let mut config = engine_state.get_config().clone();
if let Some(t_mode) = table_mode {
config.table_mode = t_mode.coerce_string()?.parse().unwrap_or_default();
config.table_mode = t_mode.coerce_str()?.parse().unwrap_or_default();
}
crate::eval_file::print_table_or_error(engine_state, stack, pipeline_data, &mut config)
}

View File

@ -459,9 +459,7 @@ pub(crate) fn add_ide_menu(
};
ide_menu = match extract_value("description_mode", val, span) {
Ok(description_mode) => {
let description_mode_str = description_mode.coerce_string()?;
match description_mode_str.as_str() {
Ok(description_mode) => match description_mode.coerce_str()?.as_ref() {
"left" => ide_menu.with_description_mode(DescriptionMode::Left),
"right" => ide_menu.with_description_mode(DescriptionMode::Right),
"prefer_right" => ide_menu.with_description_mode(DescriptionMode::PreferRight),
@ -472,8 +470,7 @@ pub(crate) fn add_ide_menu(
span: description_mode.span(),
});
}
}
}
},
Err(_) => ide_menu,
};

View File

@ -6,7 +6,7 @@ pub fn get_matching_brackets_style(default_style: Style, conf: &Config) -> Style
const MATCHING_BRACKETS_CONFIG_KEY: &str = "shape_matching_brackets";
match conf.color_config.get(MATCHING_BRACKETS_CONFIG_KEY) {
Some(int_color) => match int_color.coerce_string() {
Some(int_color) => match int_color.coerce_str() {
Ok(int_color) => merge_styles(default_style, lookup_ansi_color_style(&int_color)),
Err(_) => default_style,
},

View File

@ -262,7 +262,7 @@ fn action(input: &Value, args: &Arguments, head: Span) -> Value {
// Let's try dtparse first
if matches!(input, Value::String { .. }) && dateformat.is_none() {
let span = input.span();
if let Ok(input_val) = input.coerce_string() {
if let Ok(input_val) = input.coerce_str() {
match parse_date_from_string(&input_val, span) {
Ok(date) => return Value::date(date, span),
Err(_) => {

View File

@ -142,7 +142,7 @@ impl Iterator for UpdateCellIterator {
// for a particular datatype. If it does, it will convert the cell to that datatype.
fn process_cell(val: Value, display_as_filesizes: bool, span: Span) -> Result<Value, ShellError> {
// step 1: convert value to string
let val_str = val.coerce_string().unwrap_or_default();
let val_str = val.coerce_str().unwrap_or_default();
// step 2: bounce string up against regexes
if BOOLEAN_RE.is_match(&val_str) {
@ -189,7 +189,7 @@ fn process_cell(val: Value, display_as_filesizes: bool, span: Span) -> Result<Va
Ok(Value::int(ival, span))
}
} else if INTEGER_WITH_DELIMS_RE.is_match(&val_str) {
let mut val_str = val_str;
let mut val_str = val_str.into_owned();
val_str.retain(|x| !['_', ','].contains(&x));
let ival = val_str

View File

@ -222,9 +222,7 @@ fn move_record_columns(
// Find indices of columns to be moved
for column in columns.iter() {
let column_str = column.coerce_string()?;
if let Some(idx) = record.index_of(&column_str) {
if let Some(idx) = record.index_of(&column.coerce_str()?) {
column_idx.push(idx);
} else {
return Err(ShellError::GenericError {

View File

@ -288,7 +288,7 @@ pub fn sort(
.unwrap_or(Ordering::Equal)
}
} else if natural {
match (a.coerce_string(), b.coerce_string()) {
match (a.coerce_str(), b.coerce_str()) {
(Ok(left), Ok(right)) => compare_str(left, right),
_ => Ordering::Equal,
}

View File

@ -194,7 +194,7 @@ pub fn transpose(
match &i.get_data_by_key(desc) {
Some(x) => {
if let Ok(s) = x.coerce_string() {
headers.push(s.to_string());
headers.push(s);
} else {
return Err(ShellError::GenericError {
error: "Header row needs string headers".into(),

View File

@ -72,7 +72,7 @@ fn to_url(input: PipelineData, head: Span) -> Result<PipelineData, ShellError> {
for (k, v) in val {
match v.coerce_string() {
Ok(s) => {
row_vec.push((k.clone(), s.to_string()));
row_vec.push((k.clone(), s));
}
_ => {
return Err(ShellError::UnsupportedInput {

View File

@ -262,29 +262,29 @@ fn merge_record(record: &Record, head: Span, span: Span) -> Result<PathBuf, Shel
#[cfg(windows)]
if let Some(val) = record.get("prefix") {
let p = val.coerce_string()?;
let p = val.coerce_str()?;
if !p.is_empty() {
result.push(p);
result.push(p.as_ref());
}
}
if let Some(val) = record.get("parent") {
let p = val.coerce_string()?;
let p = val.coerce_str()?;
if !p.is_empty() {
result.push(p);
result.push(p.as_ref());
}
}
let mut basename = String::new();
if let Some(val) = record.get("stem") {
let p = val.coerce_string()?;
let p = val.coerce_str()?;
if !p.is_empty() {
basename.push_str(&p);
}
}
if let Some(val) = record.get("extension") {
let p = val.coerce_string()?;
let p = val.coerce_str()?;
if !p.is_empty() {
basename.push('.');
basename.push_str(&p);

View File

@ -738,9 +738,9 @@ fn heavy_lifting(code: Value, escape: bool, osc: bool, call: &Call) -> Result<St
});
}
let code_string = if param_is_string {
code.coerce_string().expect("error getting code as string")
code.coerce_str().expect("error getting code as string")
} else {
"".to_string()
"".into()
};
let param_is_valid_string = param_is_string && !code_string.is_empty();
if (escape || osc) && (param_is_valid_string) {

View File

@ -150,7 +150,7 @@ pub fn sort(
.unwrap_or(Ordering::Equal)
}
} else if natural {
match (a.coerce_string(), b.coerce_string()) {
match (a.coerce_str(), b.coerce_str()) {
(Ok(left), Ok(right)) => compare_str(left, right),
_ => Ordering::Equal,
}

View File

@ -89,7 +89,7 @@ fn process(
new_table_name
);
for (column_name, column_datatype) in record {
match column_datatype.coerce_string()?.as_str() {
match column_datatype.coerce_str()?.as_ref() {
"int" => {
create_stmt.push_str(&format!("{} INTEGER, ", column_name));
}

View File

@ -226,7 +226,7 @@ fn detect_columns(
.iter()
.take(end_index)
.skip(start_index)
.map(|v| v.coerce_string().unwrap_or_default())
.map(|v| v.coerce_str().unwrap_or_default())
.join(" ");
let binding = Value::string(combined, Span::unknown());
let last_seg = vals.split_off(end_index);

View File

@ -124,7 +124,7 @@ fn split_chars_helper(v: &Value, name: Span, graphemes: bool) -> Value {
Value::Error { error, .. } => Value::error(*error.clone(), span),
v => {
let v_span = v.span();
if let Ok(s) = v.coerce_string() {
if let Ok(s) = v.coerce_str() {
Value::list(
if graphemes {
s.graphemes(true)

View File

@ -148,7 +148,7 @@ fn split_column_helper(
collapse_empty: bool,
head: Span,
) -> Vec<Value> {
if let Ok(s) = v.coerce_string() {
if let Ok(s) = v.coerce_str() {
let split_result: Vec<_> = separator
.split(&s)
.filter(|x| !(collapse_empty && x.is_empty()))

View File

@ -160,7 +160,7 @@ enum Matcher {
impl Matcher {
pub fn new(regex: bool, lhs: Value) -> Result<Self, ShellError> {
if regex {
Ok(Matcher::Regex(Regex::new(&lhs.coerce_string()?).map_err(
Ok(Matcher::Regex(Regex::new(&lhs.coerce_str()?).map_err(
|e| ShellError::GenericError {
error: "Error with regular expression".into(),
msg: e.to_string(),
@ -180,7 +180,7 @@ impl Matcher {
pub fn compare(&self, rhs: &Value) -> Result<bool, ShellError> {
Ok(match self {
Matcher::Regex(regex) => {
if let Ok(rhs_str) = rhs.coerce_string() {
if let Ok(rhs_str) = rhs.coerce_str() {
regex.is_match(&rhs_str)
} else {
false

View File

@ -152,7 +152,7 @@ fn split_row_helper(v: &Value, regex: &Regex, max_split: Option<usize>, name: Sp
v => {
let v_span = v.span();
if let Ok(s) = v.coerce_string() {
if let Ok(s) = v.coerce_str() {
match max_split {
Some(max_split) => regex
.splitn(&s, max_split)

View File

@ -158,7 +158,7 @@ fn split_words_helper(v: &Value, word_length: Option<usize>, span: Span, graphem
Value::Error { error, .. } => Value::error(*error.clone(), v_span),
v => {
let v_span = v.span();
if let Ok(s) = v.coerce_string() {
if let Ok(s) = v.coerce_str() {
// let splits = s.unicode_words();
// let words = trim_to_words(s);
// let words: Vec<&str> = s.split_whitespace().collect();

View File

@ -110,10 +110,10 @@ pub fn env_to_string(
Value::List { vals, .. } => {
let paths = vals
.iter()
.map(|v| v.coerce_string())
.map(Value::coerce_str)
.collect::<Result<Vec<_>, _>>()?;
match std::env::join_paths(paths) {
match std::env::join_paths(paths.iter().map(AsRef::as_ref)) {
Ok(p) => Ok(p.to_string_lossy().to_string()),
Err(_) => Err(ShellError::EnvVarNotAString {
envvar_name: env_name.to_string(),

View File

@ -1008,7 +1008,7 @@ fn cmd_input_key_event(buf: &mut CommandBuf, key: &KeyEvent) -> bool {
}
fn value_as_style(style: &mut nu_ansi_term::Style, value: &Value) -> bool {
match value.coerce_string() {
match value.coerce_str() {
Ok(s) => {
*style = lookup_ansi_color_style(&s);
true

View File

@ -331,8 +331,8 @@ impl View for RecordView<'_> {
if let Some(hm) = cfg.config.get("table").and_then(create_map) {
self.theme = theme_from_config(&hm);
if let Some(orientation) = hm.get("orientation").and_then(|v| v.coerce_string().ok()) {
let orientation = match orientation.as_str() {
if let Some(orientation) = hm.get("orientation").and_then(|v| v.coerce_str().ok()) {
let orientation = match orientation.as_ref() {
"left" => Some(Orientation::Left),
"top" => Some(Orientation::Top),
_ => None,
@ -859,7 +859,7 @@ fn config_get_bool(config: &ConfigMap, key: &str, default: bool) -> bool {
fn config_get_usize(config: &ConfigMap, key: &str, default: usize) -> usize {
config
.get(key)
.and_then(|v| v.coerce_string().ok())
.and_then(|v| v.coerce_str().ok())
.and_then(|s| s.parse::<usize>().ok())
.unwrap_or(default)
}

View File

@ -15,7 +15,7 @@ pub(super) fn process_string_enum<T, E>(
E: Display,
{
let span = value.span();
if let Ok(v) = value.coerce_string() {
if let Ok(v) = value.coerce_str() {
match v.parse() {
Ok(format) => {
*config_point = format;

View File

@ -539,7 +539,7 @@ impl Value {
process_bool_config(value, &mut errors, &mut config.filesize_metric);
}
"format" => {
if let Ok(v) = value.coerce_string() {
if let Ok(v) = value.coerce_str() {
config.filesize_format = v.to_lowercase();
} else {
report_invalid_value("should be a string", span, &mut errors);

View File

@ -272,7 +272,7 @@ pub(super) fn try_parse_trim_strategy(
}
fn try_parse_trim_methodology(value: &Value) -> Option<TrimStrategy> {
if let Ok(value) = value.coerce_string() {
if let Ok(value) = value.coerce_str() {
match value.to_lowercase().as_str() {
"wrapping" => {
return Some(TrimStrategy::Wrap {

View File

@ -356,7 +356,7 @@ impl Value {
}
}
/// Returns this `Value` converted to a `String` or an error if it cannot be converted
/// Returns this `Value` converted to a `str` or an error if it cannot be converted
///
/// Only the following `Value` cases will return an `Ok` result:
/// - `Int`
@ -365,9 +365,56 @@ impl Value {
/// - `Binary` (only if valid utf-8)
/// - `Date`
///
/// Prefer [`coerce_into_string`](Self::coerce_into_string)
/// ```
/// # use nu_protocol::Value;
/// for val in Value::test_values() {
/// assert_eq!(
/// matches!(
/// val,
/// Value::Int { .. }
/// | Value::Float { .. }
/// | Value::String { .. }
/// | Value::Binary { .. }
/// | Value::Date { .. }
/// ),
/// val.coerce_str().is_ok(),
/// );
/// }
/// ```
pub fn coerce_str(&self) -> Result<Cow<str>, ShellError> {
match self {
Value::Int { val, .. } => Ok(Cow::Owned(val.to_string())),
Value::Float { val, .. } => Ok(Cow::Owned(val.to_string())),
Value::String { val, .. } => Ok(Cow::Borrowed(val)),
Value::Binary { val, .. } => match std::str::from_utf8(val) {
Ok(s) => Ok(Cow::Borrowed(s)),
Err(_) => self.cant_convert_to("string"),
},
Value::Date { val, .. } => Ok(Cow::Owned(
val.to_rfc3339_opts(chrono::SecondsFormat::Millis, true),
)),
val => val.cant_convert_to("string"),
}
}
/// Returns this `Value` converted to a `String` or an error if it cannot be converted
///
/// # Note
/// This function is equivalent to `value.coerce_str().map(Cow::into_owned)`
/// which might allocate a new `String`.
///
/// To avoid this allocation, prefer [`coerce_str`](Self::coerce_str)
/// if you do not need an owned `String`,
/// or [`coerce_into_string`](Self::coerce_into_string)
/// if you do not need to keep the original `Value` around.
///
/// Only the following `Value` cases will return an `Ok` result:
/// - `Int`
/// - `Float`
/// - `String`
/// - `Binary` (only if valid utf-8)
/// - `Date`
///
/// ```
/// # use nu_protocol::Value;
/// for val in Value::test_values() {
@ -385,17 +432,7 @@ impl Value {
/// }
/// ```
pub fn coerce_string(&self) -> Result<String, ShellError> {
match self {
Value::Int { val, .. } => Ok(val.to_string()),
Value::Float { val, .. } => Ok(val.to_string()),
Value::String { val, .. } => Ok(val.clone()),
Value::Binary { val, .. } => match std::str::from_utf8(val) {
Ok(s) => Ok(s.to_string()),
Err(_) => self.cant_convert_to("string"),
},
Value::Date { val, .. } => Ok(val.to_rfc3339_opts(chrono::SecondsFormat::Millis, true)),
val => val.cant_convert_to("string"),
}
self.coerce_str().map(Cow::into_owned)
}
/// Returns this `Value` converted to a `String` or an error if it cannot be converted
@ -570,13 +607,13 @@ impl Value {
/// Returns this `Value` as a `u8` slice or an error if it cannot be converted
///
/// Prefer [`coerce_into_binary`](Self::coerce_into_binary)
/// if you do not need to keep the original `Value` around.
///
/// Only the following `Value` cases will return an `Ok` result:
/// - `Binary`
/// - `String`
///
/// Prefer [`coerce_into_binary`](Self::coerce_into_binary)
/// if you do not need to keep the original `Value` around.
///
/// ```
/// # use nu_protocol::Value;
/// for val in Value::test_values() {

View File

@ -9,7 +9,7 @@ pub const CMD_NAME: &str = "from ics";
pub fn from_ics_call(call: &EvaluatedCall, input: &Value) -> Result<Value, LabeledError> {
let span = input.span();
let input_string = input.coerce_string()?;
let input_string = input.coerce_str()?;
let head = call.head;
let input_string = input_string

View File

@ -5,7 +5,7 @@ pub const CMD_NAME: &str = "from ini";
pub fn from_ini_call(call: &EvaluatedCall, input: &Value) -> Result<Value, LabeledError> {
let span = input.span();
let input_string = input.coerce_string()?;
let input_string = input.coerce_str()?;
let head = call.head;
let ini_config: Result<ini::Ini, ini::ParseError> = ini::Ini::load_from_str(&input_string);

View File

@ -8,7 +8,7 @@ pub const CMD_NAME: &str = "from vcf";
pub fn from_vcf_call(call: &EvaluatedCall, input: &Value) -> Result<Value, LabeledError> {
let span = input.span();
let input_string = input.coerce_string()?;
let input_string = input.coerce_str()?;
let head = call.head;
let input_string = input_string

View File

@ -8,8 +8,8 @@ pub fn execute_json_query(
input: &Value,
query: Option<Spanned<String>>,
) -> Result<Value, LabeledError> {
let input_string = match &input.coerce_string() {
Ok(s) => s.clone(),
let input_string = match input.coerce_str() {
Ok(s) => s,
Err(e) => {
return Err(LabeledError {
span: Some(call.head),

View File

@ -332,15 +332,15 @@ mod tests {
Span::test_data(),
);
let out = item
.as_list()
.into_list()
.unwrap()
.iter()
.into_iter()
.map(|matches| {
matches
.as_list()
.into_list()
.unwrap()
.iter()
.map(|text_nodes| text_nodes.coerce_string().unwrap())
.into_iter()
.map(|text_nodes| text_nodes.coerce_into_string().unwrap())
.collect::<Vec<String>>()
})
.collect::<Vec<Vec<String>>>();

View File

@ -21,7 +21,7 @@ pub fn execute_xpath_query(
};
let xpath = build_xpath(query_string, span)?;
let input_string = input.coerce_string()?;
let input_string = input.coerce_str()?;
let package = parser::parse(&input_string);
if package.is_err() {

View File

@ -325,9 +325,9 @@ pub fn nu_repl() {
if let Some(cwd) = stack.get_env_var(&engine_state, "PWD") {
let path = cwd
.coerce_string()
.coerce_str()
.unwrap_or_else(|err| outcome_err(&engine_state, &err));
let _ = std::env::set_current_dir(path);
let _ = std::env::set_current_dir(path.as_ref());
engine_state.add_env_var("PWD".into(), cwd);
}
}