Ogeon / rustful

[OUTDATED] A light HTTP framework for Rust
https://docs.rs/rustful
Apache License 2.0
862 stars 51 forks source link

'variable does not need to be mutable' warning from `insert_routes!` #133

Open fintelia opened 7 years ago

fintelia commented 7 years ago

Example code to reproduce:

let mut router = TreeRouter::new();
insert_routes! {
    &mut router => {
        "graph" => Get: Box::new(move |_: Context, _: Response| {}) as Box<Handler>,
    }
};

which expands to:

let mut router = TreeRouter::new();
{
    use ::router::Router;
    let mut router = &mut router;
    {
        let method =
        {
            #[allow(unused_imports)]
            use ::Method::*;
            Get
        };
        let path = &["graph"];
        router.insert(method, path,
                      Box::new(move |_: Context, _: Response| { }) as
                      Box<Handler>);
        { };
    };
    router
};

And the actual warning message:

warning: variable does not need to be mutable
  --> src/lib.rs:8:5
   |
8  | /     insert_routes! {
9  | |         &mut router => {
10 | |             "graph" => Get: Box::new(move |_: Context, _: Response| {}) as Box<Handler>,
11 | |         }
12 | |     };
   | |______^
   |
   = note: #[warn(unused_mut)] on by default
   = note: this error originates in a macro outside of the current crate
Ogeon commented 7 years ago

Hi! Thank you for reporting this! This macro has been removed in master, due to how inflexible it is, but I can make a maintenance branch if there's no way to work around this and a fix can be figured out. The tricky part is to detect if mut is necessary or not.

Do you own the router, as in the example above? In that case you can try something like this:

let mut router = TreeRouter::new();
let router = insert_routes! {
    router => {
        "graph" => Get: Box::new(move |_: Context, _: Response| {}) as Box<Handler>,
    }
};
fintelia commented 7 years ago

I think fixing the warning should simply be a matter of having #[allow(unused_mut)] on the assignment statement.

However, in my use case there is no particular reason that I have to use insert_routes, and it would probably be better to move away from it if the macro has been deprecated. What is the suggested replacement for it? For reference, the code that is currently relying on it is here.

Ogeon commented 7 years ago

I think fixing the warning should simply be a matter of having #[allow(unused_mut)] on the assignment statement.

I'm forgetting that this is possible these days... That could be a good idea.

In the meantime, it looks like you can use the method I showed before, but if you want to move away from the macro, it seems like you will be fine with the insert method. That's what's used behind the scenes, and you are not really using the tree structure of the macro anyway.

The released version doesn't have the new ways of building routers, so your tools are listed here: https://docs.rs/rustful/0.9.0/rustful/struct.TreeRouter.html, unless you feel like using the master branch. It's a bit different, but you will not get any surprise breaks if you lock it to a commit.