unit_arg lint for Rocket handlers returning unit causes a warning

[LunaBorowska]

Lint name:

unit_arg

I tried this code:

#[rocket::get("/")]
fn handler() {}

fn main() {
    println!("Hello, world!");
}

with the following Cargo.toml:

[package]
name = "rocket-example"
version = "0.1.0"
edition = "2018"

[dependencies.rocket]
git = "https://github.com/SergioBenitez/Rocket.git"
rev = "9671115796e42865eaebd020ac27542802027b02"

I expected to see this happen: no warning

Instead, this happened:

warning: passing a unit value to a function
 --> src/main.rs:2:4
  |
2 | fn handler() {}
  |    ^^^^^^^
  |
  = note: `#[warn(clippy::unit_arg)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg
help: move the expression in front of the call and replace it with the unit literal `()`
  |
2 | fn handler;
3 | ()() {}
  |

warning: 1 warning emitted

See also rwf2/Rocket#1495.

Meta

• cargo clippy -V: clippy 0.0.212 (2987785 2020-12-28)
• rustc -Vv:

  rustc 1.51.0-nightly (2987785df 2020-12-28)
  binary: rustc
  commit-hash: 2987785df3d46d5ff144a5c67fbb8f5cca798d78
  commit-date: 2020-12-28
  host: x86_64-unknown-linux-gnu
  release: 1.51.0-nightly
  

[phansch]

Hm, I'm having trouble reproducing this bug in a Clippy testcase without rocket as a dependency. Currently I have this:

// file: tests/ui/auxiliary/proc_macro_derive_unit_arg.rs
// compile-flags: --emit=link
// no-prefer-dynamic

#![crate_type = "proc-macro"]
#![feature(repr128, proc_macro_quote)]
#![allow(incomplete_features)]

// Allow clippy::unit_arg while compiling the auxiliary crate.
#![allow(clippy::unit_arg)]

extern crate proc_macro;

use proc_macro::TokenStream;

#[allow(clippy::no_effect, unused)]
#[proc_macro_attribute]
pub fn rocket_get(_args: TokenStream, input: TokenStream) -> TokenStream {
    pub fn foo5(t: ()) {
        println!("{:?}", t);
    }

    pub fn foo6() {
        foo5({ 1; });
    }
    input
}

// file: tests/ui/unit_arg_repro.rs 
// aux-build:proc_macro_derive_unit_arg.rs

#![warn(clippy::unit_arg)]

#[macro_use]
extern crate proc_macro_derive_unit_arg;

#[rocket_get("/")]
fn get_index() {}

fn main() {}

With these files present, running TESTNAME=unit_arg_repro cargo uitest should produce the false positive, but currently doesn't and I don't know why.

This expanded output also triggers the false positive, not sure if it helps

#![feature(prelude_import)]
#[prelude_import]
fn handler() {}
#[doc(hidden)]
#[allow(non_camel_case_types)]
/// Rocket code generated proxy structure.
struct handler {}
/// Rocket code generated proxy static conversion implementation.
impl From<handler> for rocket::StaticRouteInfo {
    fn from(_: handler) -> rocket::StaticRouteInfo {
        fn monomorphized_function<'_b>(
            __req: &'_b rocket::request::Request,
            __data: rocket::data::Data,
        ) -> rocket::handler::HandlerFuture<'_b> {

        ::std::boxed::Box::pin(async move {
                let ___responder = handler();
                rocket::handler::Outcome::from(__req, ___responder)
            })
        }
        rocket::StaticRouteInfo {
            name: "handler",
            method: ::rocket::http::Method::Get,
            path: "/",
            handler: monomorphized_function,
            format: ::std::option::Option::None,
            rank: ::std::option::Option::None,
        }
    }
}
/// Rocket code generated proxy conversion implementation.
impl From<handler> for rocket::Route {
    #[inline]
    fn from(_: handler) -> rocket::Route {
        rocket::StaticRouteInfo::from(handler {}).into()
    }
}
#[doc(hidden)]
fn main() {}

[phansch]

Alright, I managed to extract a proc_macro_attribute function from Rocket's codegen code that reproduces the issue:

#[proc_macro_attribute]
pub fn rocket_get(_args: TokenStream, input: TokenStream) -> TokenStream {
    let input_fn: syn::ItemFn = syn::parse(input).unwrap();

    // This needs to be quote_spanned! with the span of the input signature otherwise the issue couldn't be reproduced
    let spanned = quote_spanned! { input_fn.sig.ident.span() =>
        fn unitfn(_: ()) {}
        #[allow(clippy::no_effect)]
        unitfn({ 1; });
    };

    let output = quote! {
        #[allow(unused, clippy::no_effect)]
        #input_fn

        fn foo() {
            #spanned
        }
    };
    output.into()
}

Unfortunately a simple external macro check didn't fix the issue, so I will continue working on this tomorrow =)

[phansch]

Unfortunately, neither Clippy nor rustc currently have a way to tell apart a quote_spanned span from a non-macro span, so this is currently impossible to fix on our side (Also see #6249 (comment)).

[camsteffen]

I don't have much experience with procedural macros. Is this something that can/should be fixed in the library? Maybe using Span::mixed_site or something like that?