x11rb-async async functions should have `#[must_use]` attributes

[Antikyth]

It is easy to forget to .await an async fn, and without a #[must_use] attribute, you won't get a warning for forgetting to do so.

[Antikyth]

This should probably go for cookies too actually.

[psychon]

My first thought is "this is not x11rb-async specific, but should rather be a rust ecosystem thing", but I cannot find much compiler or clippy lints that would warn about this. The closest I found are https://rust-lang.github.io/rust-clippy/master/index.html#/let_underscore_future and https://rust-lang.github.io/rust-clippy/master/index.html#/unused_async but neither would exactly hit this case.

Actually... the Future trait is marked must_use, so shouldn't that already be enough for a warning? I'm not sure.

But I just tried this and I couldn't get a warning. conn.query_tree(window); doesn't do anything. At least conn.query_tree(window).await; generates a warning due to the Result. conn.query_tree(window).await?; then again doesn't warn. With conn.query_tree(window).await?.reply(); I finally get warning: unused implementer of Future that must be used. conn.query_tree(window).await?.reply().await; again warns due to the unused Result.

Ahh, query_tree does not return a Future, but a Pin<Box<dyn Future<....>>>. That's why there is no warning here. x11rb_async::protocol::xproto::query_tree(conn, window); also causes a warning due to the naked future.

This should probably go for cookies too actually.

I'm not quite sure. The cookie by itself doesn't do much. The request was already sent and there are valid cases for ignoring a cookie. I guess must VoidCookies are immediately dropped and I bet I could even come up with a case where dropping a Cookie would be the right thing to do.

Edit: Now that I know that this is about boxed futures, I found https://github.com/rust-lang/futures-rs/issues/2017 which leads to https://github.com/rust-lang/rust/issues/67387 and https://github.com/rust-lang/rust/issues/73417

[notgull]

Actually... the Future trait is marked must_use, so shouldn't that already be enough for a warning? I'm not sure.

At least for async it should be. Like you mentioned it's a footgun that should be fixed on the Rust level.