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

Still clippy warning after macros: fix wrong error messages #4067 #4245

Closed
ChristianBeilschmidt opened this issue Nov 18, 2021 · 1 comment · Fixed by #4252
Closed

Still clippy warning after macros: fix wrong error messages #4067 #4245

ChristianBeilschmidt opened this issue Nov 18, 2021 · 1 comment · Fixed by #4252
Assignees
Labels
A-tokio-macros Area: The tokio-macros crate C-bug Category: This is a bug.

Comments

@ChristianBeilschmidt
Copy link

Version
1.13.0

Platform

  • Rust Playground
  • Linux 5.11.0-40-generic #44~20.04.2-Ubuntu SMP Tue Oct 26 18:07:44 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Description
In #4067, there was a discussion about a falsely Clippy warning clippy::semicolon_if_nothing_returned in tokio's main and test.
Most problems were solved in #4176.
However, I encountered some leftovers that still pose the same problem.

I could reproduce it in Rust Playground:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=39638811ab983f5bea782a42d5d184b9

#![warn(clippy::semicolon_if_nothing_returned)]

use tokio; // 1.13.0

async fn foo() -> &'static str {
    "foo"
}

#[tokio::main]
async fn main() {
    let _foo = foo().await;
}

The output of Clippy is

Checking playground v0.0.1 (/playground)
warning: this import is redundant
--> src/main.rs:3:1
|
3 | use tokio; // 1.13.0
| ^^^^^^^^^^ help: remove it entirely
|
= note: #[warn(clippy::single_component_path_imports)] on by default
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_component_path_imports

warning: consider adding a ; to the last statement for consistent formatting
--> src/main.rs:11:5
|
11 | let _foo = foo().await;
| ^^^^^^^^^^^^^^^^^^^^^^^ help: add a ; here: let _foo = foo().await;;
|
note: the lint level is defined here
--> src/main.rs:1:9
|
1 | #![warn(clippy::semicolon_if_nothing_returned)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_if_nothing_returned

warning: playground (bin "playground") generated 2 warnings
Finished dev [unoptimized + debuginfo] target(s) in 2.09s

My current state of investigation is, that this problem arises when ….async; is the last command in the function that is rewritten by tokio's main or test.

An expansion brings the following output:

#![feature(prelude_import)]
#![warn(clippy::semicolon_if_nothing_returned)]
#[prelude_import]
use std::prelude::rust_2021::*;
#[macro_use]
extern crate std;
use tokio;
async fn foo() -> &'static str {
    "foo"
}
fn main() {
    let body = async {
        let _foo = foo().await;
    };
    #[allow(clippy::expect_used)]
    tokio::runtime::Builder::new_multi_thread()
        .enable_all()
        .build()
        .expect("Failed building the Runtime")
        .block_on(body)
}

So, the semicolon is missing here. But if ….async; isn't the last expression, then there is a ;.

@ChristianBeilschmidt ChristianBeilschmidt added A-tokio Area: The main tokio crate C-bug Category: This is a bug. labels Nov 18, 2021
@ChristianBeilschmidt ChristianBeilschmidt changed the title Still wrong clippy error after macros: fix wrong error messages #4067 Still clippy warning after macros: fix wrong error messages #4067 Nov 18, 2021
@taiki-e
Copy link
Member

taiki-e commented Nov 18, 2021

we need to handle syn::Stmt::Local the same as non-return syn::Stmt::Semi.

let (tail_return, tail_semicolon) = match body.stmts.last() {
Some(syn::Stmt::Semi(expr, _)) => match expr {

@taiki-e taiki-e added A-tokio-macros Area: The tokio-macros crate and removed A-tokio Area: The main tokio crate labels Nov 18, 2021
@taiki-e taiki-e self-assigned this Nov 21, 2021
bors bot added a commit to geo-engine/geoengine that referenced this issue Dec 22, 2021
425: Build-rs r=jdroenner a=ChristianBeilschmidt

I did the following things:

1. Removed `anyhow` from our build-dependencies and just unwrapped the result in the `build.rs` file.
2. Clippy wanted to add #[must_use] labels to functions that take `&self` and return `Self`. This makes sense since not using would mean you did something wrong.
3. The arrow example was not working anymore 🤷. I changed the pointer cast. Fortunately, it seems that our in-code conversions still work.
4. I updated `tokio` and could remove some Clippy lint exclusions (`// TODO: remove when tokio-rs/tokio#4245 is fixed`).
5. I updated Actix-web and they removed `AnyBody`. This means I had to wrap our error handler differently. You now have to put it into an EitherBody-enum and this takes a BoxBody. So I had to call `boxed()` here as well. Please check that the HTTP responses still work, for me, it looks fine.

Co-authored-by: Christian Beilschmidt <christian.beilschmidt@geoengine.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio-macros Area: The tokio-macros crate C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants