Name the Value conversion functions more clearly (#11851)

# Description
This PR renames the conversion functions on `Value` to be more consistent.
It follows the Rust [API guidelines](https://rust-lang.github.io/api-guidelines/naming.html#ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv) for ad-hoc conversions.
The conversion functions on `Value` now come in a few forms:
- `coerce_{type}` takes a `&Value` and attempts to convert the value to
`type` (e.g., `i64` are converted to `f64`). This is the old behavior of
some of the `as_{type}` functions -- these functions have simply been
renamed to better reflect what they do.
- The new `as_{type}` functions take a `&Value` and returns an `Ok`
result only if the value is of `type` (no conversion is attempted). The
returned value will be borrowed if `type` is non-`Copy`, otherwise an
owned value is returned.
- `into_{type}` exists for non-`Copy` types, but otherwise does not
attempt conversion just like `as_type`. It takes an owned `Value` and
always returns an owned result.
- `coerce_into_{type}` has the same relationship with `coerce_{type}` as
`into_{type}` does with `as_{type}`.
- `to_{kind}_string`: conversion to different string formats (debug,
abbreviated, etc.). Only two of the old string conversion functions were
removed, the rest have been renamed only.
- `to_{type}`: other conversion functions. Currently, only `to_path`
exists. (And `to_string` through `Display`.)

This table summaries the above:
| Form | Cost | Input Ownership | Output Ownership | Converts `Value`
case/`type` |
| ---------------------------- | ----- | --------------- |
---------------- | -------- |
| `as_{type}` | Cheap | Borrowed | Borrowed/Owned | No |
| `into_{type}` | Cheap | Owned | Owned | No |
| `coerce_{type}` | Cheap | Borrowed | Borrowed/Owned | Yes |
| `coerce_into_{type}` | Cheap | Owned | Owned | Yes |
| `to_{kind}_string` | Expensive | Borrowed | Owned | Yes |
| `to_{type}` | Expensive | Borrowed | Owned | Yes |

# User-Facing Changes
Breaking API change for `Value` in `nu-protocol` which is exposed as
part of the plugin API.
This commit is contained in:
Ian Manske
2024-02-17 18:14:16 +00:00
committed by GitHub
parent 360ebeb0bc
commit 1c49ca503a
117 changed files with 903 additions and 745 deletions

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.as_string() {
if let Ok(v) = value.coerce_string() {
match v.parse() {
Ok(format) => {
*config_point = format;

View File

@ -320,7 +320,7 @@ impl Value {
process_int_config(value, &mut errors, &mut config.max_external_completion_results);
}
"completer" => {
if let Ok(v) = value.as_block() {
if let Ok(v) = value.coerce_block() {
config.external_completer = Some(v)
} else {
match value {
@ -539,7 +539,7 @@ impl Value {
process_bool_config(value, &mut errors, &mut config.filesize_metric);
}
"format" => {
if let Ok(v) = value.as_string() {
if let Ok(v) = value.coerce_string() {
config.filesize_format = v.to_lowercase();
} else {
report_invalid_value("should be a string", span, &mut errors);
@ -707,14 +707,14 @@ impl Value {
let span = value.span();
match key2 {
"normal" => {
if let Ok(v) = value.as_string() {
if let Ok(v) = value.coerce_string() {
config.datetime_normal_format = Some(v);
} else {
report_invalid_value("should be a string", span, &mut errors);
}
}
"table" => {
if let Ok(v) = value.as_string() {
if let Ok(v) = value.coerce_string() {
config.datetime_table_format = Some(v);
} else {
report_invalid_value("should be a string", span, &mut errors);

View File

@ -251,7 +251,7 @@ pub(super) fn try_parse_trim_strategy(
}
TrimStrategy::Truncate { suffix } => {
if let Some(value) = map.get("truncating_suffix") {
if let Ok(v) = value.as_string() {
if let Ok(v) = value.coerce_string() {
*suffix = Some(v);
} else {
errors.push(ShellError::GenericError {
@ -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.as_string() {
if let Ok(value) = value.coerce_string() {
match value.to_lowercase().as_str() {
"wrapping" => {
return Some(TrimStrategy::Wrap {

View File

@ -871,7 +871,7 @@ impl EngineState {
pub fn get_cwd(&self) -> Option<String> {
if let Some(pwd_value) = self.get_env_var(PWD_ENV) {
pwd_value.as_string().ok()
pwd_value.coerce_string().ok()
} else {
None
}
@ -900,7 +900,7 @@ impl EngineState {
pub fn current_work_dir(&self) -> String {
self.get_env_var("PWD")
.map(|d| d.as_string().unwrap_or_default())
.map(|d| d.coerce_string().unwrap_or_default())
.unwrap_or_default()
}

View File

@ -581,7 +581,8 @@ impl<'a> StateWorkingSet<'a> {
.permanent_state
.get_env_var(PWD_ENV)
.expect("internal error: can't find PWD");
pwd.as_string().expect("internal error: PWD not a string")
pwd.coerce_string()
.expect("internal error: PWD not a string")
}
pub fn get_env_var(&self, name: &str) -> Option<&Value> {

View File

@ -59,7 +59,7 @@ pub trait Eval {
match item {
RecordItem::Pair(col, val) => {
// avoid duplicate cols
let col_name = Self::eval(state, mut_state, col)?.as_string()?;
let col_name = Self::eval(state, mut_state, col)?.coerce_into_string()?;
if let Some(orig_span) = col_names.get(&col_name) {
return Err(ShellError::ColumnDefinedTwice {
col_name,
@ -102,7 +102,7 @@ pub trait Eval {
Expr::Table(headers, vals) => {
let mut output_headers = vec![];
for expr in headers {
let header = Self::eval(state, mut_state, expr)?.as_string()?;
let header = Self::eval(state, mut_state, expr)?.coerce_into_string()?;
if let Some(idx) = output_headers
.iter()
.position(|existing| existing == &header)

View File

@ -176,7 +176,7 @@ impl PipelineData {
if s.is_binary {
let mut output = vec![];
for item in items {
match item.as_binary() {
match item.coerce_into_binary() {
Ok(item) => {
output.extend(item);
}
@ -192,7 +192,7 @@ impl PipelineData {
} else {
let mut output = String::new();
for item in items {
match item.as_string() {
match item.coerce_into_string() {
Ok(s) => output.push_str(&s),
Err(err) => {
return Value::error(err, span);
@ -319,7 +319,7 @@ impl PipelineData {
pub fn collect_string(self, separator: &str, config: &Config) -> Result<String, ShellError> {
match self {
PipelineData::Empty => Ok(String::new()),
PipelineData::Value(v, ..) => Ok(v.into_string(separator, config)),
PipelineData::Value(v, ..) => Ok(v.to_expanded_string(separator, config)),
PipelineData::ListStream(s, ..) => Ok(s.into_string(separator, config)),
PipelineData::ExternalStream { stdout: None, .. } => Ok(String::new()),
PipelineData::ExternalStream {
@ -330,7 +330,7 @@ impl PipelineData {
let mut output = String::new();
for val in s {
output.push_str(&val?.as_string()?);
output.push_str(&val?.coerce_into_string()?);
}
if trim_end_newline {
output.truncate(output.trim_end_matches(LINE_ENDING_PATTERN).len());
@ -783,9 +783,9 @@ impl PipelineData {
is_err = true;
format_error(&working_set, &*error)
} else if no_newline {
item.into_string("", config)
item.to_expanded_string("", config)
} else {
item.into_string("\n", config)
item.to_expanded_string("\n", config)
};
if !no_newline {
@ -859,12 +859,12 @@ pub fn print_if_stream(
if let Some(stream) = stream {
for s in stream {
let s_live = s?;
let bin_output = s_live.as_binary()?;
let bin_output = s_live.coerce_into_binary()?;
if !to_stderr {
stdout_write_all_and_flush(bin_output)?
stdout_write_all_and_flush(&bin_output)?
} else {
stderr_write_all_and_flush(bin_output)?
stderr_write_all_and_flush(&bin_output)?
}
}
}

File diff suppressed because it is too large Load Diff

View File

@ -55,7 +55,7 @@ impl RawStream {
if nu_utils::ctrl_c::was_pressed(ctrlc) {
break;
}
output.push_str(&item?.as_string()?);
output.push_str(&item?.coerce_into_string()?);
}
Ok(Spanned { item: output, span })
@ -189,7 +189,7 @@ pub struct ListStream {
impl ListStream {
pub fn into_string(self, separator: &str, config: &Config) -> String {
self.map(|x: Value| x.into_string(", ", config))
self.map(|x: Value| x.to_expanded_string(", ", config))
.collect::<Vec<String>>()
.join(separator)
}