Repository: ParkMyCar/compact_str
Pull request #147 (2022-06-30 05:14:02)

feat: Implement the Arbitrary trait from various crate
This PR implements the `Arbitrary` trait from the [`arbitrary`](https://github.com/rust-fuzz/arbitrary), [`proptest`](https://github.com/AltSysrq/proptest), and [`quickcheck`](https://github.com/BurntSushi/quickcheck) crates. It then makes those crates optional dependencies.

This makes it easier for folks to include a `CompactString` in their types, and then use randomized testing to cover those types

ParkMyCar: Thanks for the review @NobodyXu!

Patch:
diff --git a/.github/workflows/msrv.yml b/.github/workflows/msrv.yml
index 7f354993..38d1f258 100644
--- a/.github/workflows/msrv.yml
+++ b/.github/workflows/msrv.yml
@@ -15,7 +15,7 @@ env:
   RUSTFLAGS: "-D warnings"
 
 jobs:
-  test:
+  msrv:
     name: cargo test msrv..
     runs-on: ubuntu-latest
     steps:
@@ -49,4 +49,32 @@ jobs:
       - name: cargo test msrv..
         run: |
           cd compact_str
-          cargo hack test --feature-powerset --optional-deps --version-range 1.57..
+          cargo hack test --all-features --version-range 1.57..
+
+  feature_powerset:
+    name: cargo check feature-powerset
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          profile: minimal
+          toolchain: stable
+      - name: Cargo Build Cache
+        uses: actions/cache@v3
+        with:
+          path: |
+            ~/.cargo/bin/
+            ~/.cargo/registry/index/
+            ~/.cargo/registry/cache/
+            ~/.cargo/git/db/
+            target/
+          key: ${{ runner.os }}-${{ hashFiles('**/Cargo.toml') }}-stable-cargo-hack
+          restore-keys: |
+            ${{ runner.os }}-${{ hashFiles('**/Cargo.toml') }}-stable
+      - name: install cargo hack
+        run: cargo install cargo-hack --force
+      - name: cargo test msrv..
+        run: |
+          cd compact_str
+          cargo hack check --feature-powerset --optional-deps
diff --git a/compact_str/Cargo.toml b/compact_str/Cargo.toml
index f35411bf..99ebb980 100644
--- a/compact_str/Cargo.toml
+++ b/compact_str/Cargo.toml
@@ -12,7 +12,10 @@ keywords = ["string", "compact", "small", "memory", "mutable"]
 categories = ["encoding", "parsing", "memory-management", "text-processing"]
 
 [dependencies]
+arbitrary = { version = "1", optional = true, default-features = false }
 bytes = { version = "1", optional = true }
+proptest = { version = "1", optional = true, default-features = false }
+quickcheck = { version = "1", optional = true, default-features = false }
 serde = { version = "1", optional = true }
 
 castaway = "0.2.1"
@@ -21,5 +24,6 @@ ryu = "1"
 
 [dev-dependencies]
 proptest = { version = "1", default-features = false, features = ["std"] }
+quickcheck_macros = "1"
 rayon = "1"
 test-strategy = "0.2"
diff --git a/compact_str/src/features/arbitrary.rs b/compact_str/src/features/arbitrary.rs
new file mode 100644
index 00000000..e0b4f91b
--- /dev/null
+++ b/compact_str/src/features/arbitrary.rs
@@ -0,0 +1,52 @@
+//! Implements the [`arbitrary::Arbitrary`] trait for [`CompactString`]
+
+use arbitrary::{
+    Arbitrary,
+    Result,
+    Unstructured,
+};
+
+use crate::CompactString;
+
+impl<'a> Arbitrary<'a> for CompactString {
+    fn arbitrary(u: &mut Unstructured<'a>) -> Result<Self> {
+        <&str as Arbitrary>::arbitrary(u).map(CompactString::new)
+    }
+
+    fn arbitrary_take_rest(u: Unstructured<'a>) -> Result<Self> {
+        <&str as Arbitrary>::arbitrary_take_rest(u).map(CompactString::new)
+    }
+
+    #[inline]
+    fn size_hint(depth: usize) -> (usize, Option<usize>) {
+        <&str as Arbitrary>::size_hint(depth)
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use arbitrary::{
+        Arbitrary,
+        Unstructured,
+    };
+
+    use crate::CompactString;
+
+    #[test]
+    fn arbitrary_sanity() {
+        let mut data = Unstructured::new(&[42; 50]);
+        let compact = CompactString::arbitrary(&mut data).expect("generate a CompactString");
+
+        // we don't really care what the content of the CompactString is, just that one's generated
+        assert!(!compact.is_empty());
+    }
+
+    #[test]
+    fn arbitrary_inlines_strings() {
+        let mut data = Unstructured::new(&[42; 20]);
+        let compact = CompactString::arbitrary(&mut data).expect("generate a CompactString");
+
+        // running this manually, we generate the string "**"
+        assert!(!compact.is_heap_allocated());
+    }
+}
diff --git a/compact_str/src/features/mod.rs b/compact_str/src/features/mod.rs
index 6474acba..732db548 100644
--- a/compact_str/src/features/mod.rs
+++ b/compact_str/src/features/mod.rs
@@ -1,6 +1,12 @@
 //! A module that contains the implementations for optional features. For example `serde` support
 
+#[cfg(feature = "arbitrary")]
+mod arbitrary;
 #[cfg(feature = "bytes")]
 mod bytes;
+#[cfg(feature = "proptest")]
+mod proptest;
+#[cfg(feature = "quickcheck")]
+mod quickcheck;
 #[cfg(feature = "serde")]
 mod serde;
diff --git a/compact_str/src/features/quickcheck.rs b/compact_str/src/features/quickcheck.rs
new file mode 100644
index 00000000..d05682fa
--- /dev/null
+++ b/compact_str/src/features/quickcheck.rs
@@ -0,0 +1,55 @@
+//! Implements the [`quickcheck::Arbitrary`] trait for [`CompactString`]
+
+use quickcheck::{
+    Arbitrary,
+    Gen,
+};
+
+use crate::CompactString;
+
+impl Arbitrary for CompactString {
+    fn arbitrary(g: &mut Gen) -> CompactString {
+        let max = g.size();
+
+        // pick some value in [0, max]
+        let x = usize::arbitrary(g);
+        let ratio = (x as f64) / (usize::MAX as f64);
+        let size = (ratio * max as f64) as usize;
+
+        (0..size).map(|_| char::arbitrary(g)).collect()
+    }
+
+    fn shrink(&self) -> Box<dyn Iterator<Item = CompactString>> {
+        // Shrink a string by shrinking a vector of its characters.
+        let chars: Vec<char> = self.chars().collect();
+        Box::new(
+            chars
+                .shrink()
+                .map(|x| x.into_iter().collect::<CompactString>()),
+        )
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use quickcheck_macros::quickcheck;
+
+    use crate::CompactString;
+
+    #[quickcheck]
+    #[cfg_attr(miri, ignore)]
+    fn quickcheck_sanity(compact: CompactString) {
+        let control: String = compact.clone().into();
+        assert_eq!(control, compact);
+    }
+
+    #[quickcheck]
+    #[cfg_attr(miri, ignore)]
+    fn quickcheck_inlines_strings(compact: CompactString) {
+        if compact.len() <= std::mem::size_of::<String>() {
+            assert!(!compact.is_heap_allocated())
+        } else {
+            assert!(compact.is_heap_allocated())
+        }
+    }
+}


Test patch:
diff --git a/compact_str/src/features/proptest.rs b/compact_str/src/features/proptest.rs
new file mode 100644
index 00000000..346694aa
--- /dev/null
+++ b/compact_str/src/features/proptest.rs
@@ -0,0 +1,52 @@
+//! Implements the [`proptest::arbitrary::Arbitrary`] trait for [`CompactString`]
+
+use proptest::arbitrary::{
+    Arbitrary,
+    StrategyFor,
+};
+use proptest::prelude::*;
+use proptest::strategy::{
+    MapInto,
+    Strategy,
+};
+use proptest::string::StringParam;
+
+use crate::CompactString;
+
+impl Arbitrary for CompactString {
+    type Parameters = StringParam;
+    type Strategy = MapInto<StrategyFor<String>, Self>;
+
+    fn arbitrary_with(a: Self::Parameters) -> Self::Strategy {
+        any_with::<String>(a).prop_map_into()
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use proptest::prelude::*;
+
+    use crate::CompactString;
+
+    proptest! {
+        #[test]
+        #[cfg_attr(miri, ignore)]
+        fn proptest_sanity(compact: CompactString) {
+            let control: String = compact.clone().into();
+            assert_eq!(control, compact);
+        }
+
+        /// We rely on [`proptest`]'s `String` strategy for generating a `CompactString`. When
+        /// converting from a `String` into a `CompactString`, our O(1) converstion kicks in and we
+        /// reuse the buffer, unless empty, and thus all non-empty strings will be heap allocated
+        #[test]
+        #[cfg_attr(miri, ignore)]
+        fn proptest_does_not_inline_strings(compact: CompactString) {
+            if compact.is_empty() {
+                assert!(!compact.is_heap_allocated());
+            } else {
+                assert!(compact.is_heap_allocated());
+            }
+        }
+    }
+}
diff --git a/compact_str/src/tests.rs b/compact_str/src/tests.rs
index e31f4574..7c354238 100644
--- a/compact_str/src/tests.rs
+++ b/compact_str/src/tests.rs
@@ -187,38 +187,6 @@ fn proptest_extend_chars_allocated_properly(
     assert_allocated_properly(&compact);
 }
 
-#[proptest]
-#[cfg_attr(miri, ignore)]
-fn proptest_truncate(#[strategy(rand_unicode())] mut control: String, val: u8) {
-    let initial_len = control.len();
-    let mut compact = CompactString::new(&control);
-
-    // turn the arbitrary number `val` into character indices
-    let new_len = control
-        .char_indices()
-        .into_iter()
-        .cycle()
-        .nth(val as usize)
-        .unwrap_or_default()
-        .0;
-
-    // then truncate both strings string
-    control.truncate(new_len);
-    compact.truncate(new_len);
-
-    // assert they're equal
-    prop_assert_eq!(&control, &compact);
-    prop_assert_eq!(control.len(), compact.len());
-
-    // If we started as heap allocated, we should stay heap allocated. This prevents us from
-    // needing to deallocate the buffer on the heap
-    if initial_len > MAX_SIZE {
-        prop_assert!(compact.is_heap_allocated());
-    } else {
-        prop_assert!(!compact.is_heap_allocated());
-    }
-}
-
 #[test]
 fn test_const_creation() {
     const EMPTY: CompactString = CompactString::new_inline("");
@@ -913,28 +881,3 @@ fn test_into_string_empty_str() {
     assert_eq!(str_len, new_str_len);
     assert_eq!(str_len, new_str_cap);
 }
-
-#[test]
-fn test_truncate_noops_if_new_len_greater_than_current() {
-    let mut short = CompactString::from("short");
-    let short_cap = short.capacity();
-    short.truncate(100);
-
-    assert_eq!(short.len(), 5);
-    assert_eq!(short.capacity(), short_cap);
-
-    let mut long = CompactString::from("i am a long string that will be allocated on the heap");
-    let long_cap = long.capacity();
-    long.truncate(500);
-
-    assert_eq!(long.len(), 53);
-    assert_eq!(long.capacity(), long_cap);
-}
-
-#[test]
-#[should_panic(expected = "new_len must lie on char boundary")]
-fn test_truncate_panics_on_non_char_boundary() {
-    let mut emojis = CompactString::from("😀😀😀😀");
-    assert!('😀'.len_utf8() > 1);
-    emojis.truncate(1);
-}