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

needless_return triggered without a return statement #13481

Closed
dominikwilkowski opened this issue Sep 30, 2024 · 2 comments · Fixed by #13464
Closed

needless_return triggered without a return statement #13481

dominikwilkowski opened this issue Sep 30, 2024 · 2 comments · Fixed by #13464

Comments

@dominikwilkowski
Copy link

dominikwilkowski commented Sep 30, 2024

Description

This may very well be hidden in the prog macro or something else is going on but at the very least the clippy suggestion is wrong if not the entire warning.

This is my code:

#[cfg(feature = "ssr")]
#[tokio::main]
async fn main() {
    use sqlx::migrate::Migrator;
    static MIGRATOR: Migrator = sqlx::migrate!("./migrations");

    use crate::{
        app::App,
        db::ssr::{get_db, init_db},
        fileserv::file_and_error_handler,
    };

    use axum::Router;
    use leptos::*;
    use leptos_axum::{generate_route_list, LeptosRoutes};

    // Init the pool into static
    init_db().await.expect("Initialization of database failed");

    if let Err(e) = MIGRATOR.run(get_db()).await {
        eprintln!("{e:?}");
    }

    // Setting get_configuration(None) means we'll be using cargo-leptos's env values
    // For deployment these variables are:
    // https://github.com/leptos-rs/start-axum#executing-a-server-on-a-remote-machine-without-the-toolchain
    // Alternately a file can be specified such as Some("Cargo.toml")
    // The file would need to be included with the executable when moved to deployment
    let conf = get_configuration(None).await.unwrap();
    let leptos_options = conf.leptos_options;
    let addr = leptos_options.site_addr;
    let routes = generate_route_list(App);

    // build our application with a route
    let app = Router::new()
        .leptos_routes(&leptos_options, routes, App)
        .fallback(file_and_error_handler)
        .with_state(leptos_options);

    let listener = tokio::net::TcpListener::bind(&addr).await.unwrap();
    axum::serve(listener, app.into_make_service()).await.unwrap();
}

(Almost unchanged from the leptos starter template)

And this is the warning I'm getting when running cargo clippy --all-features

warning: unneeded `return` statement
  --> src/main.rs:77:63
   |
77 |     axum::serve(listener, app.into_make_service()).await.unwrap();
   |                                                                  ^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
   = note: `#[warn(clippy::needless_return)]` on by default
help: remove `return`
   |
77 |     axum::serve(listener, app.into_make_service()).await.unwrap()axum::serve(listener, app.into_make_service()).await.unwrap();
   |                                                                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Now axum::serve(listener, app.into_make_service()).await.unwrap()axum::serve(listener, app.into_make_service()).await.unwrap(); isn't valid rust code for one but I also don't see a return statement?

On the side I tried to suppress the warning and got this:

warning: unneeded `return` statement
  --> src/main.rs:77:63
   |
77 |     axum::serve(listener, app.into_make_service()).await.unwrap();
   |                                                                  ^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
   = note: `#[warn(clippy::needless_return)]` on by default
help: remove `return`
   |
77 ~     axum::serve(listener, app.into_make_service()).await.unwrap()#[allow(clippy::needless_return)]
78 ~     axum::serve(listener, app.into_make_service()).await.unwrap();
   |

Side note: Adding the #[allow(clippy::needless_return)] to the top of the function works to suppress the warning.

Version

rustc 1.83.0-nightly (7608018cb 2024-09-29)
binary: rustc
commit-hash: 7608018cbdac9e55d0d13529cf43adc33d53efcf
commit-date: 2024-09-29
host: aarch64-apple-darwin
release: 1.83.0-nightly
LLVM version: 19.1.0

Additional Labels

No response

@y21
Copy link
Member

y21 commented Sep 30, 2024

Duplicate of #13458, will be fixed by #13464

@dominikwilkowski
Copy link
Author

Ah thank you! I did search the issues for needless_return and didn't find this issue. Closing

bors added a commit that referenced this issue Oct 10, 2024
Don't warn on proc macro generated code in `needless_return`

Fixes #13458
Fixes #13457
Fixes #13467
Fixes #13479
Fixes #13481
Fixes #13526
Fixes #13486

The fix is unfortunately a little more convoluted than just simply adding a `is_from_proc_macro`. That check *does*  fix the issue, however it also introduces a bunch of false negatives in the tests, specifically when the returned expression is in a different syntax context, e.g. `return format!(..)`.

The proc macro check builds up a start and end pattern based on the HIR nodes and compares it to a snippet of the span, however that would currently fail for `return format!(..)` because we would have the patterns `("return", <something inside of the format macro>)`, which doesn't compare equal. So we now return an empty string pattern for when it's in a different syntax context.

"Hide whitespace" helps a bit for reviewing the proc macro detection change

changelog: none
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Oct 18, 2024
Don't warn on proc macro generated code in `needless_return`

Fixes rust-lang#13458
Fixes rust-lang#13457
Fixes rust-lang#13467
Fixes rust-lang#13479
Fixes rust-lang#13481
Fixes rust-lang#13526
Fixes rust-lang#13486

The fix is unfortunately a little more convoluted than just simply adding a `is_from_proc_macro`. That check *does*  fix the issue, however it also introduces a bunch of false negatives in the tests, specifically when the returned expression is in a different syntax context, e.g. `return format!(..)`.

The proc macro check builds up a start and end pattern based on the HIR nodes and compares it to a snippet of the span, however that would currently fail for `return format!(..)` because we would have the patterns `("return", <something inside of the format macro>)`, which doesn't compare equal. So we now return an empty string pattern for when it's in a different syntax context.

"Hide whitespace" helps a bit for reviewing the proc macro detection change

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants