Fix from toml to handle toml datetime correctly (#13315)

# Description

fixed #12699

When bare dates or naive times are specified in toml files, `from toml`
returns invalid dates or times.
This PR fixes the problem to correctly handle toml datetime.

The current version command returns the default datetime
(`chrono::DateTime::default()`) if the datetime parse fails. However, I
felt that this behavior was a bit unfriendly, so I changed it to return
`Value::string`.

# User-Facing Changes

The command returns a date with default time and timezone if a bare date
is specified.

```
~/Development/nushell> "dob = 2023-05-27" | from toml
╭─────┬────────────╮
│ dob │ a year ago │
╰─────┴────────────╯
~/Development/nushell> "dob = 2023-05-27" | from toml |
Sat, 27 May 2023 00:00:00 +0000 (a year ago)
~/Development/nushell>                            
```

If a bare time is given, a time string is returned.

```
~/Development/nushell> "tm = 11:00:00" | from toml
╭────┬──────────╮
│ tm │ 11:00:00 │
╰────┴──────────╯
~/Development/nushell> "tm = 11:00:00" | from toml | get tm
11:00:00
~/Development/nushell>  
```

# Tests + Formatting

When I ran tests, `commands::touch::change_file_mtime_to_reference`
failed with the following error.
The error also occurs in the master branch, so it's probably unrelated
to these changes.
(maybe a problem with my dev environment)

```
$ ~/Development/nushell> toolkit check pr

~~~~~~~~

test usage_start_uppercase ... ok
test format_conversions::yaml::convert_dict_to_yaml_with_integer_floats_key ... ok
test format_conversions::yaml::convert_dict_to_yaml_with_boolean_key ... ok
test format_conversions::yaml::table_to_yaml_text_and_from_yaml_text_back_into_table ... ok
test quickcheck_parse ... ok
test format_conversions::yaml::convert_dict_to_yaml_with_integer_key ... ok

failures:

---- commands::touch::change_file_mtime_to_reference stdout ----
=== stderr

thread 'commands::touch::change_file_mtime_to_reference' panicked at crates/nu-command/tests/commands/touch.rs:298:9:
assertion `left == right` failed
  left: SystemTime { tv_sec: 1720344745, tv_nsec: 862392750 }
 right: SystemTime { tv_sec: 1720344745, tv_nsec: 887670417 }


failures:
    commands::touch::change_file_mtime_to_reference

test result: FAILED. 1542 passed; 1 failed; 32 ignored; 0 measured; 0 filtered out; finished in 12.04s

error: test failed, to rerun pass `-p nu-command --test main`
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🔴 `toolkit test`
-  `toolkit test stdlib`

~/Development/nushell> toolkit test stdlib
   Compiling nu v0.95.1 (/Users/hiroki/Development/nushell)
   Compiling nu-cmd-lang v0.95.1 (/Users/hiroki/Development/nushell/crates/nu-cmd-lang)
    Finished dev [unoptimized + debuginfo] target(s) in 6.64s
     Running `target/debug/nu --no-config-file -c '
        use crates/nu-std/testing.nu
        testing run-tests --path crates/nu-std
    '`
2024-07-07T19:00:20.423|INF|Running from_jsonl_invalid_object in module test_formats
2024-07-07T19:00:20.436|INF|Running env_log-prefix in module test_logger_env

~~~~~~~~~~~

2024-07-07T19:00:22.196|INF|Running debug_short in module test_basic_commands
~/Development/nushell> 
```

# After Submitting

nothing
This commit is contained in:
goldfish 2024-07-07 21:55:06 +09:00 committed by GitHub
parent 32db5d3aa3
commit 5af8d62666
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -1,5 +1,5 @@
use nu_engine::command_prelude::*;
use std::str::FromStr;
use toml::value::{Datetime, Offset};
#[derive(Clone)]
pub struct FromToml;
@ -56,6 +56,54 @@ b = [1, 2]' | from toml",
}
}
fn convert_toml_datetime_to_value(dt: &Datetime, span: Span) -> Value {
match &dt.clone() {
toml::value::Datetime {
date: Some(_),
time: _,
offset: _,
} => (),
_ => return Value::string(dt.to_string(), span),
}
let date = match dt.date {
Some(date) => {
chrono::NaiveDate::from_ymd_opt(date.year.into(), date.month.into(), date.day.into())
}
None => Some(chrono::NaiveDate::default()),
};
let time = match dt.time {
Some(time) => chrono::NaiveTime::from_hms_nano_opt(
time.hour.into(),
time.minute.into(),
time.second.into(),
time.nanosecond,
),
None => Some(chrono::NaiveTime::default()),
};
let tz = match dt.offset {
Some(offset) => match offset {
Offset::Z => chrono::FixedOffset::east_opt(0),
Offset::Custom { minutes: min } => chrono::FixedOffset::east_opt(min as i32 * 60),
},
None => chrono::FixedOffset::east_opt(0),
};
let datetime = match (date, time, tz) {
(Some(date), Some(time), Some(tz)) => chrono::NaiveDateTime::new(date, time)
.and_local_timezone(tz)
.earliest(),
_ => None,
};
match datetime {
Some(datetime) => Value::date(datetime, span),
None => Value::string(dt.to_string(), span),
}
}
fn convert_toml_to_value(value: &toml::Value, span: Span) -> Value {
match value {
toml::Value::Array(array) => {
@ -76,13 +124,7 @@ fn convert_toml_to_value(value: &toml::Value, span: Span) -> Value {
span,
),
toml::Value::String(s) => Value::string(s.clone(), span),
toml::Value::Datetime(d) => match chrono::DateTime::from_str(&d.to_string()) {
Ok(nushell_date) => Value::date(nushell_date, span),
// in the unlikely event that parsing goes wrong, this function still returns a valid
// nushell date (however the default one). This decision was made to make the output of
// this function uniform amongst all eventualities
Err(_) => Value::date(chrono::DateTime::default(), span),
},
toml::Value::Datetime(dt) => convert_toml_datetime_to_value(dt, span),
}
}
@ -113,32 +155,6 @@ mod tests {
test_examples(FromToml {})
}
#[test]
fn from_toml_creates_nushell_date() {
let toml_date = toml::Value::Datetime(Datetime {
date: Option::from(toml::value::Date {
year: 1980,
month: 10,
day: 12,
}),
time: Option::from(toml::value::Time {
hour: 10,
minute: 12,
second: 44,
nanosecond: 0,
}),
offset: Option::from(toml::value::Offset::Custom { minutes: 120 }),
});
let span = Span::test_data();
let reference_date = Value::date(Default::default(), Span::test_data());
let result = convert_toml_to_value(&toml_date, span);
//positive test (from toml returns a nushell date)
assert_eq!(result.get_type(), reference_date.get_type());
}
#[test]
fn from_toml_creates_correct_date() {
let toml_date = toml::Value::Datetime(Datetime {
@ -206,4 +222,113 @@ mod tests {
assert!(result.is_err());
}
#[test]
fn convert_toml_datetime_to_value_date_time_offset() {
let toml_date = Datetime {
date: Option::from(toml::value::Date {
year: 2000,
month: 1,
day: 1,
}),
time: Option::from(toml::value::Time {
hour: 12,
minute: 12,
second: 12,
nanosecond: 0,
}),
offset: Option::from(toml::value::Offset::Custom { minutes: 120 }),
};
let span = Span::test_data();
let reference_date = Value::date(
chrono::FixedOffset::east_opt(60 * 120)
.unwrap()
.with_ymd_and_hms(2000, 1, 1, 12, 12, 12)
.unwrap(),
span,
);
let result = convert_toml_datetime_to_value(&toml_date, span);
assert_eq!(result, reference_date);
}
#[test]
fn convert_toml_datetime_to_value_date_time() {
let toml_date = Datetime {
date: Option::from(toml::value::Date {
year: 2000,
month: 1,
day: 1,
}),
time: Option::from(toml::value::Time {
hour: 12,
minute: 12,
second: 12,
nanosecond: 0,
}),
offset: None,
};
let span = Span::test_data();
let reference_date = Value::date(
chrono::FixedOffset::east_opt(0)
.unwrap()
.with_ymd_and_hms(2000, 1, 1, 12, 12, 12)
.unwrap(),
span,
);
let result = convert_toml_datetime_to_value(&toml_date, span);
assert_eq!(result, reference_date);
}
#[test]
fn convert_toml_datetime_to_value_date() {
let toml_date = Datetime {
date: Option::from(toml::value::Date {
year: 2000,
month: 1,
day: 1,
}),
time: None,
offset: None,
};
let span = Span::test_data();
let reference_date = Value::date(
chrono::FixedOffset::east_opt(0)
.unwrap()
.with_ymd_and_hms(2000, 1, 1, 0, 0, 0)
.unwrap(),
span,
);
let result = convert_toml_datetime_to_value(&toml_date, span);
assert_eq!(result, reference_date);
}
#[test]
fn convert_toml_datetime_to_value_only_time() {
let toml_date = Datetime {
date: None,
time: Option::from(toml::value::Time {
hour: 12,
minute: 12,
second: 12,
nanosecond: 0,
}),
offset: None,
};
let span = Span::test_data();
let reference_date = Value::string(toml_date.to_string(), span);
let result = convert_toml_datetime_to_value(&toml_date, span);
assert_eq!(result, reference_date);
}
}