BurntSushi / quickcheck

Automated property based testing for Rust (with shrinking).
The Unlicense
2.4k stars 149 forks source link

Documentation issue: how to implement `Arbitrary` #259

Closed nbraud closed 3 years ago

nbraud commented 4 years ago

I'd like to report what is probably a documentation issue: when trying to implement Arbitrary for a custom datatype (one that models integer factorisations), I quickly discovered that the quickcheck documentation does not explain how to (usefully) implement it, and in particular how to do anything with a quickcheck::Gen.

My implementation is:

#[cfg(test)]
impl quickcheck::Arbitrary for Factors {
    fn arbitrary<G: quickcheck::Gen>(g: &mut G) -> Self {
        use quickcheck::RngCore;
        use rand::Rng;
        let mut f = Factors::one();
        let mut n = g.size() as u64;

        // Adam Kalai's algorithm for generating uniformly-distributed
        // integers and their factorisation.
        //
        // See Generating Random Factored Numbers, Easily, J. Cryptology (2003)
        while n > 1 {
            n = g.gen_range(1, n);
            if miller_rabin::is_prime(n) {
                f.push(n);
            }
        }

        f
    }
}

The build output is:

warning: unused import: `quickcheck::RngCore`
   --> src/uu/factor/src/factor.rs:160:13
    |
160 |         use quickcheck::RngCore;
    |             ^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(unused_imports)]` on by default

error[E0599]: no method named `gen_range` found for mutable reference `&mut G` in the current scope
   --> src/uu/factor/src/factor.rs:170:19
    |
170 |             n = g.gen_range(1, n);
    |                   ^^^^^^^^^ method not found in `&mut G`
    |
    = note: the method `gen_range` exists but the following trait bounds were not satisfied:
            `G: factor::rand::RngCore`
            which is required by `G: factor::rand::Rng`
            `&mut G: factor::rand::RngCore`
            which is required by `&mut G: factor::rand::Rng`
    = help: items from traits can only be used if the trait is in scope
    = note: the following trait is implemented but not in scope; perhaps add a `use` for it:
            `use rand::Rng;`

warning: unused import: `rand::Rng`
   --> src/uu/factor/src/factor.rs:161:13
    |
161 |         use rand::Rng;
    |             ^^^^^^^^^```

Confusingly, the documentation for quickcheck::Gen states it is a quickcheck::RngCore, whose own documentation states that “End users should normally use the Rng trait from the rand crate”... when end users cannot possibly implement traits like Arbitrary without interacting with Gen and RngCore. Even simply discovering the gen_range method required me to look at quickcheck's own implementations of Arbitrary.

nbraud commented 4 years ago

PS: My actual issue was caused by a mismatch in rand versions, so that's fixed, but it seems worth improving the documentation anyhow.

mhuesch commented 4 years ago

Thanks, I think you just saved me a bunch of time with this post :pray: :slightly_smiling_face: :crab: :rocket: