From 6ea31b721fb34acde03d0b5d5ab2bb83384d2bb9 Mon Sep 17 00:00:00 2001 From: Dirkjan Ochtman Date: Wed, 25 Oct 2023 17:05:49 +0200 Subject: [PATCH] Remove impl FromXml for &str This is a bit opinionated, but it helps avoid run-time panics from doing the simple thing (&str) which is wrong in the face of escaping strings. The overhead from `Cow` (both in terms of developer experience and run-time performance) seems limited enough. This also makes the next part a little easier. --- instant-xml/src/impls.rs | 39 ----------------------------------- instant-xml/tests/entities.rs | 21 +++++-------------- instant-xml/tests/option.rs | 8 +++++-- instant-xml/tests/scalar.rs | 22 ++++++++++---------- 4 files changed, 22 insertions(+), 68 deletions(-) diff --git a/instant-xml/src/impls.rs b/instant-xml/src/impls.rs index eb3b7cd..6f4d159 100644 --- a/instant-xml/src/impls.rs +++ b/instant-xml/src/impls.rs @@ -281,45 +281,6 @@ impl<'xml> FromXml<'xml> for String { const KIND: Kind = Kind::Scalar; } -impl<'xml> FromXml<'xml> for &'xml str { - #[inline] - fn matches(id: Id<'_>, field: Option>) -> bool { - match field { - Some(field) => id == field, - None => false, - } - } - - fn deserialize<'cx>( - into: &mut Self::Accumulator, - field: &'static str, - deserializer: &mut Deserializer<'cx, 'xml>, - ) -> Result<(), Error> { - if into.is_some() { - return Err(Error::DuplicateValue); - } - - let value = match deserializer.take_str()? { - Some(value) => value, - None => return Ok(()), - }; - - match decode(value)? { - Cow::Borrowed(str) => *into = Some(str), - Cow::Owned(_) => { - return Err(Error::UnexpectedValue(format!( - "string with escape characters cannot be deserialized as &str for {field}: '{value}'", - ))) - } - } - - Ok(()) - } - - type Accumulator = Option<&'xml str>; - const KIND: Kind = Kind::Scalar; -} - impl<'xml, 'a> FromXml<'xml> for Cow<'a, str> { #[inline] fn matches(id: Id<'_>, field: Option>) -> bool { diff --git a/instant-xml/tests/entities.rs b/instant-xml/tests/entities.rs index 4aa7a03..60b3901 100644 --- a/instant-xml/tests/entities.rs +++ b/instant-xml/tests/entities.rs @@ -2,13 +2,12 @@ use std::borrow::Cow; use similar_asserts::assert_eq; -use instant_xml::{from_str, to_string, Error, FromXml, ToXml}; +use instant_xml::{from_str, to_string, FromXml, ToXml}; #[derive(Debug, PartialEq, Eq, FromXml, ToXml)] #[xml(ns("URI"))] struct StructSpecialEntities<'a> { string: String, - str: &'a str, #[xml(borrow)] cow: Cow<'a, str>, } @@ -17,26 +16,17 @@ struct StructSpecialEntities<'a> { fn escape_back() { assert_eq!( from_str( - "<>&"'adsad"strstr&" + "<>&"'adsad"str&" ), Ok(StructSpecialEntities { string: String::from("<>&\"'adsad\""), - str: "str", cow: Cow::Owned("str&".to_string()), }) ); - // Wrong str char - assert_eq!( - from_str( - "<>&"'adsad"str&" - ), - Err::(Error::UnexpectedValue("string with escape characters cannot be deserialized as &str for StructSpecialEntities::str: 'str&'".to_owned())) - ); - // Borrowed let escape_back = from_str::( - "<>&"'adsad"strstr" + "<>&"'adsad"str" ) .unwrap(); @@ -46,7 +36,7 @@ fn escape_back() { // Owned let escape_back = from_str::( - "<>&"'adsad"strstr&" + "<>&"'adsad"str&" ) .unwrap(); @@ -60,9 +50,8 @@ fn special_entities() { assert_eq!( to_string(&StructSpecialEntities{ string: "&\"<>\'aa".to_string(), - str: "&\"<>\'bb", cow: Cow::from("&\"<>\'cc"), }).unwrap(), - "&"<>'aa&"<>'bb&"<>'cc", + "&"<>'aa&"<>'cc", ); } diff --git a/instant-xml/tests/option.rs b/instant-xml/tests/option.rs index 17dcc47..5a597a2 100644 --- a/instant-xml/tests/option.rs +++ b/instant-xml/tests/option.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + use similar_asserts::assert_eq; use instant_xml::{from_str, to_string, FromXml, ToXml}; @@ -21,12 +23,14 @@ fn option_vec() { #[derive(Debug, Eq, FromXml, PartialEq, ToXml)] struct Bar<'a> { #[xml(attribute, borrow)] - maybe: Option<&'a str>, + maybe: Option>, } #[test] fn option_borrow() { - let v = Bar { maybe: Some("a") }; + let v = Bar { + maybe: Some("a".into()), + }; let xml = r#""#; assert_eq!(xml, to_string(&v).unwrap()); diff --git a/instant-xml/tests/scalar.rs b/instant-xml/tests/scalar.rs index 6511adb..4f6acf0 100644 --- a/instant-xml/tests/scalar.rs +++ b/instant-xml/tests/scalar.rs @@ -8,7 +8,7 @@ use instant_xml::{from_str, FromXml, ToXml}; #[xml(ns("URI"))] struct NestedLifetimes<'a> { flag: bool, - str_type_a: &'a str, + str_type_a: Cow<'a, str>, } #[derive(Debug, PartialEq, FromXml, ToXml)] @@ -18,13 +18,13 @@ struct StructDeserializerScalars<'a, 'b> { i8_type: i8, u32_type: u32, string_type: String, - str_type_a: &'a str, - str_type_b: &'b str, + str_type_a: Cow<'a, str>, + str_type_b: Cow<'b, str>, char_type: char, f32_type: f32, nested: NestedLifetimes<'a>, cow: Cow<'a, str>, - option: Option<&'a str>, + option: Option>, slice: Cow<'a, [u8]>, } @@ -39,13 +39,13 @@ fn scalars() { i8_type: 1, u32_type: 42, string_type: "string".to_string(), - str_type_a: "lifetime a", - str_type_b: "lifetime b", + str_type_a: "lifetime a".into(), + str_type_b: "lifetime b".into(), char_type: 'c', f32_type: 1.20, nested: NestedLifetimes { flag: true, - str_type_a: "asd" + str_type_a: "asd".into() }, cow: Cow::from("123"), option: None, @@ -63,16 +63,16 @@ fn scalars() { i8_type: 1, u32_type: 42, string_type: "string".to_string(), - str_type_a: "lifetime a", - str_type_b: "lifetime b", + str_type_a: "lifetime a".into(), + str_type_b: "lifetime b".into(), char_type: 'c', f32_type: 1.20, nested: NestedLifetimes { flag: true, - str_type_a: "asd" + str_type_a: "asd".into(), }, cow: Cow::from("123"), - option: Some("asd"), + option: Some("asd".into()), slice: Cow::Borrowed(&[1, 2, 3]), }) );