Fix nightly compilation warnings.

[nihohit]

Adds missing generic declarations or return types for (), when until the 2024 edition ! was inferred. This fixes this warning:

warning: this function depends on never type fallback being `()`
 --> redis/examples/async-await.rs:4:7
  |
4 | async fn main() -> redis::RedisResult<()> {
  |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
  = note: for more information, see issue #123748 <https://github.com/rust-lang/rust/issues/123748>
  = help: specify the types explicitly
note: in edition 2024, the requirement `!: FromRedisValue` will fail
 --> redis/examples/async-await.rs:8:9
  |
8 |     con.set("key1", b"foo").await?;
  |         ^^^
  = note: `-D dependency-on-unit-never-type-fallback` implied by `-D warnings`
  = help: to override `-D warnings` add `#[allow(dependency_on_unit_never_type_fallback)]`

In order to make this easier, this also changes the generics used in Cmd/Pipeline::query_async/Script::invoke_async, so that the function declaration would only need to define a single generic value. This changed the usage from cmd.query_async::<_, ()>(&mut conn).await; to cmd.query_async::<()>(&mut conn).await;. This was done since the compiler doesn't always suggest to use _ for the connection type, so the user might think they need to give a concrete connection type, which might be onerous to the user.
This can be reverted and replaced with let _: () = cmd.query_async(&mut conn).await;, as is done with other calls.

[jaymell]

Yowza

[nihohit]

Yowza

a-yup.

[nihohit]

#1228

[jaymell]

I just wonder if there's a better way we can solve this. Maybe different methods if you're not anticipating a result, that exxplicitly return Result<()>?

Obviously we will still need to make some breaking changes here, but having exec / exec_async methods might offer a more intuitive way. Arguably, query is not the best name for a method where you're not expecting a result anyway. 🤷

EDIT: Ok, I see we already have an execute method (though not an async variant?), but I'm not sure the unwrap/lack of return type is the cleanest way to handle things 😅. Maybe a bit of cleaning up there is a good way to go and offers a better way to clean up some of these warnings. All these useless generic params make my soul hurt.

[nihohit]

We can definitely add a call/_async functions that return RedisResult<()>, or exec/_async (although in such a case I would consider deprecating execute in order to remove confusion between similarily named functions),
but I'm not sure about how easy would it be to find these functions.

I'd also be happy to somehow solve the issue for all of the sets around, but I'm not sure what's the solution then.

[jaymell]

We can definitely add a call/_async functions that return RedisResult<()>, or exec/_async (although in such a case I would consider deprecating execute in order to remove confusion between similarily named functions),

Yep I think deprecating execute is definitely a good idea.

but I'm not sure about how easy would it be to find these functions.

What do you mean exactly here?

I'd also be happy to somehow solve the issue for all of the sets around, but I'm not sure what's the solution then.

What if we modify the macros, or add a separate one for write-only commands that calls exec_async instead of query_async / with a hard-coded return value of RedisResult<()>? I guess that could get tricky for variants of commands that potentially return things, ie SET ... GET, but we could probably handle most of the use cases.

[nihohit]

but I'm not sure about how easy would it be to find these functions.

What do you mean exactly here?

mostly discoverability. we'll need to add those functions to the docs, and explain when to use one function or the other.

What if we modify the macros, or add a separate one for write-only commands that calls exec_async instead of query_async / with a hard-coded return value of RedisResult<()>?

If we modify the macros to change the return type, it's a breaking change. If anyone anywhere expects the result of set to be String, their code will be broken. If we add a new set of macros, then we'll need to guide users when to use one set or the other, not to mention adding verbosity in the case when a user imports both macros, and needs to explain to the compiler which set did they intend.
And above that, we'd need to actually go through the code and understand which commands should be changed. There are quite a few set variants.

[jaymell]

If we modify the macros to change the return type, it's a breaking change. If anyone anywhere expects the result of set to be String, their code will be broken. If we add a new set of macros, then we'll need to guide users when to use one set or the other, not to mention adding verbosity in the case when a user imports both macros, and needs to explain to the compiler which set did they intend.
And above that, we'd need to actually go through the code and understand which commands should be changed. There are quite a few set variants.

Yep, there are a lot of potential issues here, but I think there could be other solutions. The high-level command API in this library is already confusing to many people, and I feel like having to specify even more generics is going to make the problem worse.

[nihohit]

@jaymell I added exec/_async (didn't deprecate execute yet - I'm undecided about that, since we have enough breaking changes ATM).
As for the macros system - I'm not sure what/how/where to do it in a way that will minimize the damage.

[nihohit]

BTW, I think that we need nightly to generate docs, so we should fix this before the next release 🤦 .

[jaymell]

@jaymell I added exec/_async (didn't deprecate execute yet - I'm undecided about that, since we have enough breaking changes ATM).

I'll defer to you, but I'm totally in favor of deprecating here. What's one more breaking change? 🤷 More seriously, though, the unwrap in there is just bad from my perspective. I'd have advocated for this much earlier if I realized the command existed 🤣 😭

[jaymell]

I wonder if we can word this a bit more simply/descriptively, something like:

This is an alternative to query that can be used if you want to be able to handle a command's success or failure but don't care about the command's response. For example, this is useful for "write" commands for which the response other is not important. It avoids the need to define generic bounds for ().

[jaymell]

... and same here of course.

[jaymell]

As for the macros system - I'm not sure what/how/where to do it in a way that will minimize the damage.

I'm not sure either. I'm even imagining something like a set of commands with _no_return appended to the name, but I don't think that's much of an improvement. I think what you've got here is about as good as we're going to get for now.