Fix needs_quotes() in to nuon (closes #6989) (#7056)

* to nuon: fix needs_quotes()

Also, null now serialises as null instead of $nothing.

* Clippy

* Add missing quote

* Remove two unnecessary characters

* Add short datetime tests

* Make regex simplificatified

* Alphabetise 'use' statements

* Improve perf by putting case-insensitive cases in regex

* Fix 1 test
This commit is contained in:
Leon 2022-11-19 21:09:39 +10:00 committed by GitHub
parent 6454bf69aa
commit c98a6705e6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 61 additions and 29 deletions

5
Cargo.lock generated
View File

@ -2617,6 +2617,7 @@ dependencies = [
"num 0.4.0",
"num-format",
"num-traits",
"once_cell",
"pathdiff",
"polars",
"powierza-coefficient",
@ -3045,9 +3046,9 @@ checksum = "e7461858c5ac9bde3fcdeedc17f58ed0469ec74f2d737b6369fc31c0b0ef576c"
[[package]]
name = "once_cell"
version = "1.15.0"
version = "1.16.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e82dad04139b71a90c080c8463fe0dc7902db5192d939bd0950f074d014339e1"
checksum = "86f0b0d4bf799edbc74508c1e8bf170ff5f41238e5f8225603ca7caaae2b7860"
[[package]]
name = "oorandom"

View File

@ -61,6 +61,7 @@ mime = "0.3.16"
notify = "4.0.17"
num = { version = "0.4.0", optional = true }
num-traits = "0.2.14"
once_cell = "1.0"
pathdiff = "0.2.1"
powierza-coefficient = "1.0.1"
quick-xml = "0.23.0"

View File

@ -85,6 +85,7 @@ impl Command for FromNuon {
let mut error = None;
// Most of the 'work' in converting from Nuon is simply pushing it through the Nu parser.
let (lexed, err) = nu_parser::lex(string_input.as_bytes(), 0, &[b'\n', b'\r'], &[], true);
error = error.or(err);

View File

@ -1,4 +1,5 @@
use core::fmt::Write;
use fancy_regex::Regex;
use nu_engine::get_columns;
use nu_parser::escape_quote_string;
use nu_protocol::ast::{Call, RangeInclusion};
@ -6,6 +7,7 @@ use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{
Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, Type, Value,
};
use once_cell::sync::Lazy;
#[derive(Clone)]
pub struct ToNuon;
@ -55,7 +57,7 @@ fn value_to_string(v: &Value, span: Span) -> Result<String, ShellError> {
for byte in val {
if write!(s, "{:02X}", byte).is_err() {
return Err(ShellError::UnsupportedInput(
"binary could not translate to string".into(),
"could not convert binary to string".into(),
span,
));
}
@ -63,7 +65,7 @@ fn value_to_string(v: &Value, span: Span) -> Result<String, ShellError> {
Ok(format!("0x[{}]", s))
}
Value::Block { .. } => Err(ShellError::UnsupportedInput(
"block not supported".into(),
"blocks are currently not nuon-compatible".into(),
span,
)),
Value::Closure { .. } => Err(ShellError::UnsupportedInput(
@ -78,21 +80,24 @@ fn value_to_string(v: &Value, span: Span) -> Result<String, ShellError> {
}
}
Value::CellPath { .. } => Err(ShellError::UnsupportedInput(
"cellpath not supported".to_string(),
"cellpaths are currently not nuon-compatible".to_string(),
span,
)),
Value::CustomValue { .. } => Err(ShellError::UnsupportedInput(
"custom not supported".to_string(),
"customs are currently not nuon-compatible".to_string(),
span,
)),
Value::Date { val, .. } => Ok(val.to_rfc3339()),
// FIXME: make duratiobs use the shortest lossless representation.
Value::Duration { val, .. } => Ok(format!("{}ns", *val)),
Value::Error { .. } => Err(ShellError::UnsupportedInput(
"error not supported".to_string(),
"errors are currently not nuon-compatible".to_string(),
span,
)),
// FIXME: make filesizes use the shortest lossless representation.
Value::Filesize { val, .. } => Ok(format!("{}b", *val)),
Value::Float { val, .. } => {
// This serialises these as 'nan', 'inf' and '-inf', respectively.
if &val.round() == val
&& val != &f64::NAN
&& val != &f64::INFINITY
@ -146,7 +151,7 @@ fn value_to_string(v: &Value, span: Span) -> Result<String, ShellError> {
Ok(format!("[{}]", collection.join(", ")))
}
}
Value::Nothing { .. } => Ok("$nothing".to_string()),
Value::Nothing { .. } => Ok("null".to_string()),
Value::Range { val, .. } => Ok(format!(
"{}..{}{}",
value_to_string(&val.from, span)?,
@ -172,6 +177,8 @@ fn value_to_string(v: &Value, span: Span) -> Result<String, ShellError> {
}
Ok(format!("{{{}}}", collection.join(", ")))
}
// All strings outside data structures are quoted because they are in 'command position'
// (could be mistaken for commands by the Nu parser)
Value::String { val, .. } => Ok(escape_quote_string(val)),
}
}
@ -195,26 +202,24 @@ fn to_nuon(call: &Call, input: PipelineData) -> Result<String, ShellError> {
value_to_string(&v, call.head)
}
// This hits, in order:
// • Any character of []:`{}#'";()|$,
// • Any digit (\d)
// • Any whitespace (\s)
// • Case-insensitive sign-insensitive float "keywords" inf, infinity and nan.
static NEEDS_QUOTES_REGEX: Lazy<Regex> = Lazy::new(|| {
Regex::new(r#"[\[\]:`\{\}#'";\(\)\|\$,\d\s]|(?i)^[+\-]?(inf(inity)?|nan)$"#)
.expect("internal error: NEEDS_QUOTES_REGEX didn't compile")
});
fn needs_quotes(string: &str) -> bool {
string.contains(' ')
|| string.contains('[')
|| string.contains(']')
|| string.contains(':')
|| string.contains('`')
|| string.contains('{')
|| string.contains('}')
|| string.contains('#')
|| string.contains('\'')
|| string.contains(';')
|| string.contains('(')
|| string.contains(')')
|| string.contains('|')
|| string.contains('$')
|| string.contains(',')
|| string.contains('\t')
|| string.contains('\n')
|| string.contains('\r')
|| string.contains('\"')
// These are case-sensitive keywords
match string {
"true" | "false" | "null" => return true,
_ => (),
};
// All other cases are handled here
NEEDS_QUOTES_REGEX.is_match(string).unwrap_or(false)
}
#[cfg(test)]

View File

@ -84,6 +84,6 @@ fn test_list() {
);
assert_eq!(
actual.out,
r##"{name: C64, black: "#090300", red: "#883932", green: "#55a049", yellow: "#bfce72", blue: "#40318d", purple: "#8b3f96", cyan: "#67b6bd", white: "#ffffff", brightBlack: "#000000", brightRed: "#883932", brightGreen: "#55a049", brightYellow: "#bfce72", brightBlue: "#40318d", brightPurple: "#8b3f96", brightCyan: "#67b6bd", brightWhite: "#f7f7f7", background: "#40318d", foreground: "#7869c4"}"##
r##"{name: "C64", black: "#090300", red: "#883932", green: "#55a049", yellow: "#bfce72", blue: "#40318d", purple: "#8b3f96", cyan: "#67b6bd", white: "#ffffff", brightBlack: "#000000", brightRed: "#883932", brightGreen: "#55a049", brightYellow: "#bfce72", brightBlue: "#40318d", brightPurple: "#8b3f96", brightCyan: "#67b6bd", brightWhite: "#f7f7f7", background: "#40318d", foreground: "#7869c4"}"##
);
}

View File

@ -297,7 +297,7 @@ fn to_nuon_converts_columns_with_spaces() {
}
#[test]
fn to_nuon_does_not_quote_unnecessarily() {
fn does_not_quote_strings_unnecessarily() {
let actual = nu!(
cwd: "tests/fixtures/formats", pipeline(
r#"
@ -314,6 +314,30 @@ fn to_nuon_does_not_quote_unnecessarily() {
assert_eq!(actual.out, "{\"ro name\": sam, rank: 10}");
}
#[test]
fn quotes_some_strings_necessarily() {
let actual = nu!(
cwd: "tests/fixtures/formats", pipeline(
r#"
['true','false','null',
'NaN','NAN','nan','+nan','-nan',
'inf','+inf','-inf','INF',
'Infinity','+Infinity','-Infinity','INFINITY',
'+19.99','-19.99', '19.99b',
'19.99kb','19.99mb','19.99gb','19.99tb','19.99pb','19.99eb','19.99zb',
'19.99kib','19.99mib','19.99gib','19.99tib','19.99pib','19.99eib','19.99zib',
'19ns', '19us', '19ms', '19sec', '19min', '19hr', '19day', '19wk',
'-11.0..-15.0', '11.0..-15.0', '-11.0..15.0',
'-11.0..<-15.0', '11.0..<-15.0', '-11.0..<15.0',
'-11.0..', '11.0..', '..15.0', '..-15.0', '..<15.0', '..<-15.0',
'2000-01-01', '2022-02-02T14:30:00', '2022-02-02T14:30:00+05:00'
] | to nuon | from nuon | describe
"#
));
assert_eq!(actual.out, "list<string>");
}
proptest! {
#[test]
fn to_nuon_from_nuon(c: char) {