lipanski / mockito

HTTP mocking for Rust!
MIT License
695 stars 59 forks source link

Warning on missing .create()? #112

Closed dbr closed 4 years ago

dbr commented 4 years ago

Minor thing, but it can be easy to forget to put the .create() call when creating a builder (something I found particularly when using with_body() with multi-lines strings)

Perhaps the impl Drop could warn about this in some way - e.g

struct Mock {
    x: u64,
    created: bool,
}

impl Mock {
    pub fn new() -> Self {
        Self {
            x: 10,
            created: false,
        }
    }
    pub fn with_x(mut self, x: u64) -> Self {
        self.x = x;
        self
    }
    #[must_use]
    pub fn create(mut self) -> Mock {
        self.created = true;
        // ...
        self
    }
}

impl Drop for Mock {
    fn drop(&mut self) {
        if !self.created {
            eprintln!("Missing call to Mock.create()");
        }
    }
}

fn main() {
    Mock::new().with_x(123).create(); // `warning: unused return value of `Mock::create` that must be used`
    let _m = Mock::new().with_x(123); // `Missing call to Mock.create()`
    let _m = Mock::new().with_x(123).create(); // Happy
}

Additionally, adding the must_use attribute to the return of create() may help people avoid forgetting to assign the mocks to objects as per these docs

lipanski commented 4 years ago

@dbr both valid points. care to open a PR? otherwise I'll add it to my list but won't get to it too soon.

dbr commented 4 years ago

Yep shall do!

lipanski commented 4 years ago

@dbr released in 0.24.0 - thanks!

dimo414 commented 3 years ago

Is there any chance the Mock API could be split into a separate Builder, so that it's harder to accidentally drop .create() or mix up the mocking and asserting/expecting methods?