Confusing nullability of `IDatabase.ListRightPop()` (and possibly more?)

[bdach]

The interface signature of the method in question is:

https://github.com/StackExchange/StackExchange.Redis/blob/cb8b20df0e2975717bde97ce95ac20e8e8353572/src/StackExchange.Redis/Interfaces/IDatabase.cs#L1082-L1091

The <returns> tag says that the method can return nil, but the actual method signature says that it can't. (This came up because there was actually a null-check in our code, and it started displaying a warning with a package bump.)

Judging by the implementation of the method I found and looked at for 5 minutes:

https://github.com/StackExchange/StackExchange.Redis/blob/cb8b20df0e2975717bde97ce95ac20e8e8353572/src/StackExchange.Redis/RedisDatabase.cs#L1270-L1274

it appears that the method signature is probably correct and the documented return value should be changed to "empty array" instead? That said because I took 5 minutes to look at this, I'm not gonna attempt to claim correctness of that without confirmation.

It would probably be a good idea to grep through all usages of nil in the interface xmldocs and double-check them too because I see some more suspect ones like

https://github.com/StackExchange/StackExchange.Redis/blob/cb8b20df0e2975717bde97ce95ac20e8e8353572/src/StackExchange.Redis/Interfaces/IDatabase.cs#L1492-L1499

RedisValue is a struct, so going off the type annotations alone this can at worst return a default(RedisValue), which I wouldn't generally interpret to be the same as nil.

[mgravell]

Short version: yes, I think the comments could be clearer

Longer version:

• for the single value version, there is an instance property that allows "null" (not C# null, but null nonetheless) to be conveyed
• for arrays, I suspect you're right and we always return a non-empty array

[NickCraver]

I remember when we designed here - we already returned a non-empty array to prevent breaks for a lot of scenarios and definitely didn't want to add any breaks. Agreed, it's likely we want to change the docs here.

[bdach]

Thanks for the confirmation. I can probably get a PR going for this if it helps?

[NickCraver]

@bdach That'd be very welcome :)