Rocket: Invalid lint warning: #[warn(unmounted_route)] on mounted routes

[aloucks]

This lint warning is showing up even when the routes are mounted. I noticed this when defining routes in sub-modules and creating a helper function to create the routes vec. This is observed on the current master branch.

Example:

#![feature(plugin)]
#![plugin(rocket_codegen)]

extern crate rocket;

use rocket::Route;

#[get("/")]
fn index() -> &'static str {
    "Hello!"
}

#[get("/goodbye")]
fn goodbye() -> &'static str {
    "See ya!"
}

fn routes() -> Vec<Route> {
    routes![index, goodbye]
}

fn main() {
    rocket::ignite().mount("/", routes()).launch();
}

Warnings:

warning: the 'index' route is not mounted, #[warn(unmounted_route)] on by default
  --> src\main.rs:9:1
   |
9  |   fn index() -> &'static str {
   |  _^ starting here...
10 | |     "Hello!"
11 | | }
   | |_^ ...ending here
   |
   = note: Rocket will not dispatch requests to unmounted routes.
help: maybe add a call to `mount` here?
  --> src\main.rs:23:5
   |
23 |     rocket::ignite().mount("/", routes()).launch();
   |     ^^^^^^^^^^^^^^^^

warning: the 'goodbye' route is not mounted, #[warn(unmounted_route)] on by default
  --> src\main.rs:14:1
   |
14 |   fn goodbye() -> &'static str {
   |  _^ starting here...
15 | |     "See ya!"
16 | | }
   | |_^ ...ending here
   |
   = note: Rocket will not dispatch requests to unmounted routes.
help: maybe add a call to `mount` here?
  --> src\main.rs:23:5
   |
23 |     rocket::ignite().mount("/", routes()).launch();
   |     ^^^^^^^^^^^^^^^^

They're definitely mounted:

$ curl http://localhost:8000
Hello!
$ curl http://localhost:8000/goodbye
See ya!

[SergioBenitez]

The lint cannot track across function boundaries. One thing you can do is move the ignite and mount calls into the routes function (and likely rename it to rocket) and then call rocket().launch() in main.

[aloucks]

@SergioBenitez That doesn't help if you want to have multiple mounts.

let rocket = rocket::ignite();
rocket.mount("/foo", foo::routes())
      .mount("/bar", bar::routes())
      .launch();

I like the spirit of the lint, but reporting false positives due to limitations of the lint seems like a bug.

[SergioBenitez]

Not sure how it doesn't help with multiple mounts; you can simply inline those routes as well. If you want to keep returning the routes from a function, then yes, the lint won't help you there. But I'm curious; why do this in the first place? The only use case I can see is so that you don't have to retype all of the routes during testing. But you can achieve the same thing, and be even terser, by bundling all of the initialization into a rocket() function as I described earlier.

It's not a 'bug'; the lint was designed with this shortcoming in mind. Handling this case is quite involved, and it's likely not worth the complexity. The lint can't handle every case - it's simply not possible - so I've chosen to make it handle cases that are 1) possible and 2) likely. If the lint does you more harm than good, which I presume is unlikely in the vast majority of cases, simply add #[allow(unmounted_route)] to your crate attributes to disable the lint.

[radix]

fwiw, there is another, simpler case that this lint isn't handling and I ran into it:

  let routes = routes![get_app,
                       poll_app,
                       options_handler,
                       post_app,
                       combat_movement_options,
                       movement_options,
                       target_options];
  rocket::ignite().mount("/", routes).manage(pt).launch();

[lholden]

The pattern of calling out to various routes() functions seem fairly reasonable. We have been using one per controller. I totally understand why this would be hard for the linter to work out though.

[kestred]

In one of my projects, it was convenient to follow this pattern as well, eg.

pub fn routes() -> Vec<Route> {
    return routes![
        manage,
        new_network,
        new_place,
        post_new_network,
        post_new_place,
    ];
}

    rocket::ignite()
        .manage(bss::database::Pool::new())
        .mount("/", home::routes())
        .mount("/devices", devices::routes())
        .mount("/locations", locations::routes())

        ...



md5-392dd3be630adec91292e1650697f7d3



warning: function is never used: `routes`
  --> src/handlers/locations.rs:10:1
   |
10 |   pub fn routes() -> Vec<Route> {
   |  _^ starting here...
11 | |     return routes![
12 | |         manage,
13 | |         new_network,
14 | |         new_place,
15 | |         post_new_network,
16 | |         post_new_place,
17 | |     ];
18 | | }
   | |_^ ...ending here
   |
   = note: #[warn(dead_code)] on by default

warning: function is never used: `rocket_route_fn_manage`
  --> src/handlers/locations.rs:20:1
   |
20 | #[get("/")]
   | ^^^^^^^^^^^
   |
   = note: #[warn(dead_code)] on by default

warning: static item is never used: `static_rocket_route_info_for_manage`
  --> src/handlers/locations.rs:20:1
   |
20 | #[get("/")]
   | ^^^^^^^^^^^
   |
   = note: #[warn(dead_code)] on by default

warning: function is never used: `manage`
  --> src/handlers/locations.rs:21:1
   |
21 | fn manage(db: State<database::Pool>, sess: Option<Session>) -> Result<Template, Redirect> {
   | ^
   |
   = note: #[warn(dead_code)] on by default

---

On that note, the lint's only benefit should be providing a simpler and easier to understand error message, since rust is already helping with this?

[SergioBenitez]

Since returning routes from a function in a module seems to be pretty common, it would be nice if the lint supported (at least) this case. I would be happy to review and eventually accept a PR that improves the lint to do this.

[xentec]

Modifying the routes vector also triggers this warning.

let mut routes = routes![index_root, index_path];
// status parses requests manually and is not handled by codegen
routes.push(Route::new(Method::Get, "/?status", status)); 
rocket::ignite().mount("/", routes).launch();

[SergioBenitez]

This is no longer applicable as Rocket no longer has this lint.

[clee]

That's certainly one way to close it!