Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warning on missing .create()? #112

Closed
dbr opened this issue Feb 29, 2020 · 4 comments
Closed

Warning on missing .create()? #112

dbr opened this issue Feb 29, 2020 · 4 comments

Comments

@dbr
Copy link
Contributor

dbr commented Feb 29, 2020

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
Copy link
Owner

lipanski commented Mar 8, 2020

@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
Copy link
Contributor Author

dbr commented Mar 10, 2020

Yep shall do!

@lipanski
Copy link
Owner

@dbr released in 0.24.0 - thanks!

@dimo414
Copy link

dimo414 commented Jan 25, 2021

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants