Switch to &mut self builder

[dtolnay]

According to the builders API guideline, we like &mut self builders.

The RequestBuilder method signatures should change to &mut self:

- fn(self, ...) -> Self
+ fn(&mut self, ...) -> &mut Self

Also they should fail fast where applicable:

- fn(self, ...) -> Self /* eventually error when send is called */
+ fn(&mut self, ...) -> Result<&mut Self, Error>

The state inside the builder should be stored in a way that enables send (or any other terminal method) to accept &mut self as well but take ownership of the state inside, for example by storing Option<T> from which the terminal method can take() the value.

Any further use of the builder after send has been called should panic, and this behavior should be documented.

[seanmonstar]

Adding some more details, an example of how the code could look is:

pub struct RequestBuilder {
    client: ClientRef,
    request: Option<Request>,
}

Where Request would be a new type, discussed in #85. Then, methods would be like this:

impl RequestBuilder {
    fn request_mut(&mut self) -> &mut Request {
        self.request.as_mut().expect("Builder cannot be reused after building a Request")
    }
    pub fn json<T: Serialize>(&mut self, val: T) -> reqwest::Result<&mut RequestBuilder> {
        let json = serde_json::to_vec(val)?;
        self.request_mut().set_body(Body::from(json));
        self
    }
}

There would be 2 public methods on the builder to "consume" it:

impl RequestBuilder {
    fn take_request(&mut self) -> Request {
        self.request.take().expect("Builder cannot be reused after building a Request")
    }

    pub fn send(&mut self) -> reqwest::Result<Response> {
        self.client.send(self.take_request())
    }

    pub fn build(&mut self) -> Request {
        self.take_request()
    }
}

[dtantsur]

Sorry for necroposting, but this is a bit an unfortunate decision. I understand why it was made, but it also makes it harder to return RequestBuilder from functions. Imagine (pseudo-rust):

 fn custom_request(method: Method, url: Url) -> RequestBuilder {
   let extra_hdrs = calculate_some_headers();
   make_client().request(method, url).headers(extra_hdrs)
 }

This is what I need to do (custom authentication, if it matters), and as you see it does not work, because headers() returns a mutable reference to a temporary object. What I have to do is roughly this (keeping in mind I have all lints as fatal, including unused_results):

 fn custom_request(method: Method, url: Url) -> RequestBuilder {
   let extra_hdrs = calculate_some_headers();
   let builder = make_client().request(method, url);
   {
     let _unused = builder.headers(extra_hdrs);
   }
   builder
 }

If this &mut approach was added to some guidelines, was this problem even considered?

[KodrAus]

Hi @dtantsur! The API guidelines are always open to more insight, we've got an open issue there about &mut self builders where you could raise this.