From f3bbdfc31c1cc2eec20d5c5a15657f65ea7d3f12 Mon Sep 17 00:00:00 2001 From: Bradford Hovinen Date: Sat, 23 Mar 2024 11:47:45 +0100 Subject: [PATCH] Remove the need for allocation from `is_utf8_string`. Previously, `IsEncodedStringMatcher` converted its actual value input to a `Vec` and constructed a `String` out of that in order to match it. This is because using a string slice caused problems with the borrow checker as described in #323. The fundamental problem is that the type against which the inner matcher is matching is a reference whose lifetime must be named in the trait bound: ``` impl + Debug, InnerMatcherT> Matcher for IsEncodedStringMatcher where InnerMatcherT: Matcher, ^^^^ What goes here??? ``` One option is just to remove the reference: ``` impl + Debug, InnerMatcherT> Matcher for IsEncodedStringMatcher where InnerMatcherT: Matcher, ``` This doesn't work when the inner matcher is `eq`, since one would then be comparing an `str` and a string slice: ``` verify_that!("A string".as_bytes(), is_utf8_string(eq("A string"))) // Compile error: cannot compare str and &str ``` However, this problem does not occur with `StrMatcher`: ``` verify_that!("A string".as_bytes(), is_utf8_string(constains_substring("A string"))) // Passes ``` So this also introduces an easy way to obtain a `StrMatcher` to check string equality: ``` verify_that!("A string".as_bytes(), is_utf8_string(eq_str("A string"))) // Passes ``` This is slightly inconvenient, since one must use a different matcher to compare string equality in this case. But it is well-documented and not too inconvenient to use. So it should be okay. The introduction of `eq_str` also allows fixing a longstanding limitation of the matcher `property!` -- that it could not match on properties returning string slices. This PR therefore also updates the documentation accordingly and adds an appropriate test. --- googletest/crate_docs.md | 4 ++ .../src/matchers/is_encoded_string_matcher.rs | 50 ++++++++------ googletest/src/matchers/mod.rs | 2 +- googletest/src/matchers/property_matcher.rs | 11 +-- googletest/src/matchers/str_matcher.rs | 69 ++++++++++++++++--- googletest/tests/property_matcher_test.rs | 15 ++++ 6 files changed, 116 insertions(+), 35 deletions(-) diff --git a/googletest/crate_docs.md b/googletest/crate_docs.md index 8c8b47cf..360c6058 100644 --- a/googletest/crate_docs.md +++ b/googletest/crate_docs.md @@ -127,6 +127,7 @@ The following matchers are provided in GoogleTest Rust: | [`ends_with`] | A string ending with the given suffix. | | [`eq`] | A value equal to the argument, in the sense of the [`PartialEq`] trait. | | [`eq_deref_of`] | A value equal to the dereferenced value of the argument. | +| [`eq_str`] | A string equal to the argument. | | [`err`] | A [`Result`][std::result::Result] containing an `Err` variant the argument matches. | | [`field!`] | A struct or enum with a given field whose value the argument matches. | | [`ge`] | A [`PartialOrd`] value greater than or equal to the given value. | @@ -134,6 +135,7 @@ The following matchers are provided in GoogleTest Rust: | [`has_entry`] | A [`HashMap`] containing a given key whose value the argument matches. | | [`is_contained_in!`] | A container each of whose elements is matched by some given matcher. | | [`is_nan`] | A floating point number which is NaN. | +| [`is_utf8_string`] | A byte sequence representing a UTF-8 encoded string which the argument matches. | | [`le`] | A [`PartialOrd`] value less than or equal to the given value. | | [`len`] | A container whose number of elements the argument matches. | | [`lt`] | A [`PartialOrd`] value strictly less than the given value. | @@ -169,6 +171,7 @@ The following matchers are provided in GoogleTest Rust: [`empty`]: matchers::empty [`ends_with`]: matchers::ends_with [`eq`]: matchers::eq +[`eq_str`]: matchers::eq_str [`eq_deref_of`]: matchers::eq_deref_of [`err`]: matchers::err [`field!`]: matchers::field @@ -177,6 +180,7 @@ The following matchers are provided in GoogleTest Rust: [`has_entry`]: matchers::has_entry [`is_contained_in!`]: matchers::is_contained_in [`is_nan`]: matchers::is_nan +[`is_utf8_string`]: matchers::is_utf8_string [`le`]: matchers::le [`len`]: matchers::len [`lt`]: matchers::lt diff --git a/googletest/src/matchers/is_encoded_string_matcher.rs b/googletest/src/matchers/is_encoded_string_matcher.rs index d2fb2596..f6140cb4 100644 --- a/googletest/src/matchers/is_encoded_string_matcher.rs +++ b/googletest/src/matchers/is_encoded_string_matcher.rs @@ -25,18 +25,21 @@ use std::{fmt::Debug, marker::PhantomData}; /// /// The input may be a slice `&[u8]` or a `Vec` of bytes. /// +/// When asserting the equality of the actual value with a given string, use +/// [`eq_str`] rather than [`eq`]. +/// /// ``` /// # use googletest::prelude::*; /// # fn should_pass() -> Result<()> { /// let bytes: &[u8] = "A string".as_bytes(); -/// verify_that!(bytes, is_utf8_string(eq("A string")))?; // Passes +/// verify_that!(bytes, is_utf8_string(eq_str("A string")))?; // Passes /// let bytes: Vec = "A string".as_bytes().to_vec(); -/// verify_that!(bytes, is_utf8_string(eq("A string")))?; // Passes +/// verify_that!(bytes, is_utf8_string(eq_str("A string")))?; // Passes /// # Ok(()) /// # } /// # fn should_fail_1() -> Result<()> { /// # let bytes: &[u8] = "A string".as_bytes(); -/// verify_that!(bytes, is_utf8_string(eq("Another string")))?; // Fails (inner matcher does not match) +/// verify_that!(bytes, is_utf8_string(eq_str("Another string")))?; // Fails (inner matcher does not match) /// # Ok(()) /// # } /// # fn should_fail_2() -> Result<()> { @@ -48,11 +51,14 @@ use std::{fmt::Debug, marker::PhantomData}; /// # should_fail_1().unwrap_err(); /// # should_fail_2().unwrap_err(); /// ``` -pub fn is_utf8_string<'a, ActualT: AsRef<[u8]> + Debug + 'a, InnerMatcherT>( +/// +/// [`eq`]: crate::matchers::eq +/// [`eq_str`]: crate::matchers::eq_str +pub fn is_utf8_string + Debug, InnerMatcherT>( inner: InnerMatcherT, ) -> impl Matcher where - InnerMatcherT: Matcher, + InnerMatcherT: Matcher, { IsEncodedStringMatcher { inner, phantom: Default::default() } } @@ -62,15 +68,15 @@ struct IsEncodedStringMatcher { phantom: PhantomData, } -impl<'a, ActualT: AsRef<[u8]> + Debug + 'a, InnerMatcherT> Matcher +impl + Debug, InnerMatcherT> Matcher for IsEncodedStringMatcher where - InnerMatcherT: Matcher, + InnerMatcherT: Matcher, { type ActualT = ActualT; fn matches(&self, actual: &Self::ActualT) -> MatcherResult { - String::from_utf8(actual.as_ref().to_vec()) + std::str::from_utf8(actual.as_ref()) .map(|s| self.inner.matches(&s)) .unwrap_or(MatcherResult::NoMatch) } @@ -91,7 +97,7 @@ where } fn explain_match(&self, actual: &Self::ActualT) -> Description { - match String::from_utf8(actual.as_ref().to_vec()) { + match std::str::from_utf8(actual.as_ref()) { Ok(s) => { format!("which is a UTF-8 encoded string {}", self.inner.explain_match(&s)).into() } @@ -107,62 +113,62 @@ mod tests { #[test] fn matches_string_as_byte_slice() -> Result<()> { - verify_that!("A string".as_bytes(), is_utf8_string(eq("A string"))) + verify_that!("A string".as_bytes(), is_utf8_string(eq_str("A string"))) } #[test] fn matches_string_as_byte_vec() -> Result<()> { - verify_that!("A string".as_bytes().to_vec(), is_utf8_string(eq("A string"))) + verify_that!("A string".as_bytes().to_vec(), is_utf8_string(eq_str("A string"))) } #[test] fn matches_string_with_utf_8_encoded_sequences() -> Result<()> { - verify_that!("äöüÄÖÜ".as_bytes().to_vec(), is_utf8_string(eq("äöüÄÖÜ"))) + verify_that!("äöüÄÖÜ".as_bytes().to_vec(), is_utf8_string(eq_str("äöüÄÖÜ"))) } #[test] fn does_not_match_non_equal_string() -> Result<()> { - verify_that!("äöüÄÖÜ".as_bytes().to_vec(), not(is_utf8_string(eq("A string")))) + verify_that!("äöüÄÖÜ".as_bytes().to_vec(), not(is_utf8_string(eq_str("A string")))) } #[test] fn does_not_match_non_utf_8_encoded_byte_sequence() -> Result<()> { - verify_that!(&[192, 64, 255, 32], not(is_utf8_string(eq("A string")))) + verify_that!(&[192, 64, 255, 32], not(is_utf8_string(eq_str("A string")))) } #[test] fn has_correct_description_in_matched_case() -> Result<()> { - let matcher = is_utf8_string::<&[u8], _>(eq("A string")); + let matcher = is_utf8_string::<&[u8], _>(eq_str("A string")); verify_that!( matcher.describe(MatcherResult::Match), - displays_as(eq("is a UTF-8 encoded string which is equal to \"A string\"")) + displays_as(eq_str("is a UTF-8 encoded string which is equal to \"A string\"")) ) } #[test] fn has_correct_description_in_not_matched_case() -> Result<()> { - let matcher = is_utf8_string::<&[u8], _>(eq("A string")); + let matcher = is_utf8_string::<&[u8], _>(eq_str("A string")); verify_that!( matcher.describe(MatcherResult::NoMatch), - displays_as(eq("is not a UTF-8 encoded string which is equal to \"A string\"")) + displays_as(eq_str("is not a UTF-8 encoded string which is equal to \"A string\"")) ) } #[test] fn has_correct_explanation_in_matched_case() -> Result<()> { - let explanation = is_utf8_string(eq("A string")).explain_match(&"A string".as_bytes()); + let explanation = is_utf8_string(eq_str("A string")).explain_match(&"A string".as_bytes()); verify_that!( explanation, - displays_as(eq("which is a UTF-8 encoded string which is equal to \"A string\"")) + displays_as(eq_str("which is a UTF-8 encoded string which is equal to \"A string\"")) ) } #[test] fn has_correct_explanation_when_byte_array_is_not_utf8_encoded() -> Result<()> { - let explanation = is_utf8_string(eq("A string")).explain_match(&&[192, 128, 0, 64]); + let explanation = is_utf8_string(eq_str("A string")).explain_match(&&[192, 128, 0, 64]); verify_that!(explanation, displays_as(starts_with("which is not a UTF-8 encoded string: "))) } @@ -170,7 +176,7 @@ mod tests { #[test] fn has_correct_explanation_when_inner_matcher_does_not_match() -> Result<()> { let explanation = - is_utf8_string(eq("A string")).explain_match(&"Another string".as_bytes()); + is_utf8_string(eq_str("A string")).explain_match(&"Another string".as_bytes()); verify_that!( explanation, diff --git a/googletest/src/matchers/mod.rs b/googletest/src/matchers/mod.rs index 147c8ab1..e4faf555 100644 --- a/googletest/src/matchers/mod.rs +++ b/googletest/src/matchers/mod.rs @@ -85,7 +85,7 @@ pub use points_to_matcher::points_to; pub use predicate_matcher::{predicate, PredicateMatcher}; pub use some_matcher::some; pub use str_matcher::{ - contains_substring, ends_with, starts_with, StrMatcher, StrMatcherConfigurator, + contains_substring, ends_with, eq_str, starts_with, StrMatcher, StrMatcherConfigurator, }; pub use subset_of_matcher::subset_of; pub use superset_of_matcher::superset_of; diff --git a/googletest/src/matchers/property_matcher.rs b/googletest/src/matchers/property_matcher.rs index 19b48627..63dbba6f 100644 --- a/googletest/src/matchers/property_matcher.rs +++ b/googletest/src/matchers/property_matcher.rs @@ -78,10 +78,10 @@ /// # .unwrap(); /// ``` /// -/// Unfortunately, this matcher does *not* work with methods returning string -/// slices: +/// When the property returns a string slice and you wish to assert its +/// equality, use [`eq_str`] instead of [`eq`]: /// -/// ```compile_fail +/// ``` /// # use googletest::prelude::*; /// #[derive(Debug)] /// pub struct MyStruct { @@ -92,7 +92,7 @@ /// } /// /// let value = MyStruct { a_string: "A string".into() }; -/// verify_that!(value, property!(*MyStruct.get_a_string(), eq("A string"))) // Does not compile +/// verify_that!(value, property!(*MyStruct.get_a_string(), eq_str("A string"))) /// # .unwrap(); /// ``` /// @@ -101,6 +101,9 @@ /// rather than accessing a field. /// /// The list of arguments may optionally have a trailing comma. +/// +/// [`eq`]: crate::matchers::eq +/// [`eq_str`]: crate::matchers::eq_str #[macro_export] #[doc(hidden)] macro_rules! __property { diff --git a/googletest/src/matchers/str_matcher.rs b/googletest/src/matchers/str_matcher.rs index f4f158e5..c9ce4615 100644 --- a/googletest/src/matchers/str_matcher.rs +++ b/googletest/src/matchers/str_matcher.rs @@ -26,6 +26,59 @@ use std::fmt::Debug; use std::marker::PhantomData; use std::ops::Deref; +/// Matches a string equal to the given string. +/// +/// This has the same effect as [`eq`]. Unlike [`eq`], [`eq_str`] can be used +/// in cases where the actual type being matched is `str`. In paticular, one +/// can use [`eq_str`] together with: +/// +/// * [`property!`] where the property returns a string slice, and +/// * [`is_utf8_string`]. +/// +/// ``` +/// # use googletest::prelude::*; +/// # fn should_pass_1() -> Result<()> { +/// verify_that!("Some value", eq_str("Some value"))?; // Passes +/// verify_that!("Some value".to_string(), eq_str("Some value"))?; // Passes +/// # Ok(()) +/// # } +/// # fn should_fail() -> Result<()> { +/// verify_that!("Another value", eq_str("Some value"))?; // Fails +/// # Ok(()) +/// # } +/// # fn should_pass_2() -> Result<()> { +/// let value_as_bytes = "Some value".as_bytes(); +/// verify_that!(value_as_bytes, is_utf8_string(eq_str("Some value")))?; // Passes +/// # Ok(()) +/// # } +/// # should_pass_1().unwrap(); +/// # should_fail().unwrap_err(); +/// # should_pass_2().unwrap(); +/// +/// #[derive(Debug)] +/// pub struct MyStruct { +/// a_string: String, +/// } +/// impl MyStruct { +/// pub fn get_a_string(&self) -> &str { &self.a_string } +/// } +/// +/// let value = MyStruct { a_string: "A string".into() }; +/// verify_that!(value, property!(*MyStruct.get_a_string(), eq_str("A string"))) // Passes +/// # .unwrap(); +/// ``` +/// +/// [`eq`]: crate::matchers::eq +/// [`is_utf8_string`]: crate::matchers::is_utf8_string +/// [`property!`]: crate::matchers::property +pub fn eq_str(expected: T) -> StrMatcher { + StrMatcher { + configuration: Configuration { mode: MatchMode::Equals, ..Default::default() }, + expected, + phantom: Default::default(), + } +} + /// Matches a string containing a given substring. /// /// Both the actual value and the expected substring may be either a `String` or @@ -729,8 +782,8 @@ mod tests { } #[test] - fn matches_string_containing_expected_value_in_contains_mode_while_ignoring_ascii_case() - -> Result<()> { + fn matches_string_containing_expected_value_in_contains_mode_while_ignoring_ascii_case( + ) -> Result<()> { verify_that!("Some string", contains_substring("STR").ignoring_ascii_case()) } @@ -879,8 +932,8 @@ mod tests { } #[test] - fn describes_itself_for_matching_result_ignoring_ascii_case_and_leading_whitespace() - -> Result<()> { + fn describes_itself_for_matching_result_ignoring_ascii_case_and_leading_whitespace( + ) -> Result<()> { let matcher: StrMatcher<&str, _> = StrMatcher::with_default_config("A string") .ignoring_leading_whitespace() .ignoring_ascii_case(); @@ -1021,8 +1074,8 @@ mod tests { } #[test] - fn match_explanation_for_starts_with_includes_both_versions_of_differing_last_line() - -> Result<()> { + fn match_explanation_for_starts_with_includes_both_versions_of_differing_last_line( + ) -> Result<()> { let result = verify_that!( indoc!( " @@ -1123,8 +1176,8 @@ mod tests { } #[test] - fn match_explanation_for_contains_substring_shows_diff_when_first_and_last_line_are_incomplete() - -> Result<()> { + fn match_explanation_for_contains_substring_shows_diff_when_first_and_last_line_are_incomplete( + ) -> Result<()> { let result = verify_that!( indoc!( " diff --git a/googletest/tests/property_matcher_test.rs b/googletest/tests/property_matcher_test.rs index 7092446b..5c560cfa 100644 --- a/googletest/tests/property_matcher_test.rs +++ b/googletest/tests/property_matcher_test.rs @@ -85,6 +85,21 @@ fn matches_struct_with_matching_string_reference_property() -> Result<()> { verify_that!(value, property!(*StructWithString.get_property_ref(), eq("Something"))) } +#[test] +fn matches_struct_with_matching_string_slice_property() -> Result<()> { + #[derive(Debug)] + struct StructWithString { + property: String, + } + impl StructWithString { + fn get_property_ref(&self) -> &str { + &self.property + } + } + let value = StructWithString { property: "Something".into() }; + verify_that!(value, property!(*StructWithString.get_property_ref(), eq_str("Something"))) +} + #[test] fn matches_struct_with_matching_slice_property() -> Result<()> { #[derive(Debug)]