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

clippy: "warning: passing a unit value to a function" #1495

Closed
NyxCode opened this issue Dec 19, 2020 · 6 comments
Closed

clippy: "warning: passing a unit value to a function" #1495

NyxCode opened this issue Dec 19, 2020 · 6 comments
Labels
upstream An unresolvable issue: an upstream dependency bug

Comments

@NyxCode
Copy link

NyxCode commented Dec 19, 2020

version: latest master

os: arch/x86_64

description:

When a route returns a unit (()), clippy emits this warning:

warning: passing a unit value to a function
  --> src/rest/api/auth/mod.rs:26:8
   |
26 | pub fn logout(ctx: &App, jwt: &JWT) {
   |        ^^^^^^
   |
   = 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...
   |
26 | pub fn logout;
   |        ^^^^^^^
help: ...and use a unit literal instead
   |
26 | pub fn ()(ctx: &App, jwt: &JWT) {
   |    

adding a return type resolves this warning.
The snipped which provoked this warning is this:

#[post("/logout")]
pub fn logout(ctx: &App, jwt: &JWT) {
    ctx.jwt.invalidated_users.invalidate(jwt.user_id);
}
@jebrosen
Copy link
Collaborator

The generated code for such a route includes a pattern similar to this:

{
    let ___responder = hello2();
    rocket::handler::Outcome::from(__req, ___responder)
})

I think clippy is too eager to lint this; there's a similar case at rust-lang/rust-clippy#6447. If there is a reasonable way to generate code differently to avoid the lint I'd be interested, but it looks like the code is generated this way in order to provide better error messages.

@jebrosen jebrosen added the upstream An unresolvable issue: an upstream dependency bug label Dec 23, 2020
@NyxCode
Copy link
Author

NyxCode commented Dec 23, 2020

couldn't we just put a #[allow(clippy::unit_arg)] in there?

@jebrosen
Copy link
Collaborator

In principle we can (and I have opened a PR doing that). I'm not super happy about adding arbitrary clippy exemptions in generated code, since the exemptions may hide errors introduced in the future and this one really does look like a bug in clippy.

@SergioBenitez
Copy link
Member

This appears to have either been resolved upstream or no longer occur with the latest codegen:

❯ cat src/main.rs                                                                                                                                                                                                                
use rocket::*;

#[get("/")]
fn index() { }

#[launch]
fn rocket() -> _ {
    rocket::build().mount("/", routes![index])
}

❯ cargo clippy                                                                                                                                                                                                                   master ●
    Finished dev [unoptimized + debuginfo] target(s) in 0.05s

Closing as "resolved".

@tobymurray
Copy link

Using 0.5.0-dec (rocket v0.5.0-dev (https://github.com/SergioBenitez/Rocket.git?branch=master#801e04bd))and clippy 0.1.51 (2fd73fa 2021-03-23) I see this warning on routes that accept data. Maybe it came back?

Is there anything an end user had to do to silence these?

@jebrosen
Copy link
Collaborator

rust-lang/rust-clippy#6447 was merged which probably fixed this, but that fix might not have made it to the stable releases yet. It ought to hit stable the week of May 6 (or June 17, if I miscounted).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
upstream An unresolvable issue: an upstream dependency bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants