Should `CacheExtensions.GetOrCreate()`/`GetOrCreateAsync()` return `TItem` instead of `TItem?`?

[kevinchalet]

Take this snippet:

using Microsoft.Extensions.Caching.Memory;
using Microsoft.Extensions.DependencyInjection;

var services = new ServiceCollection();
services.AddMemoryCache();

var provider = services.BuildServiceProvider();
var cache = provider.GetRequiredService<IMemoryCache>();

var instance = cache.GetOrCreate("key", entry =>
{
    // ...
    return new
    {
        Message = "Hello World"
    };
});

Console.WriteLine(instance.Message);

On .NET 6.0, you don't get any error or warning as the caching stack was not decorated with nullable annotations.
On .NET 7.0, you get a CS8602 warning on the last line, as GetOrCreate() returns TItem? even if the delegate itself will never return null.

I guess the warning is technically correct because - in theory - nothing prevents me from inserting a null value in the cache before the delegate has a chance to be called, but I'm not sure it's a frequent case (in this case, it would be more logical that the delegate signature use TItem? instead of TItem to account for potential null values).

To make them easier to use, should CacheExtensions.GetOrCreate()/GetOrCreateAsync() return TItem instead of TItem??

Tagging subscribers to this area: @dotnet/area-extensions-caching
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Take this snippet:

using Microsoft.Extensions.Caching.Memory;
using Microsoft.Extensions.DependencyInjection;

var services = new ServiceCollection();
services.AddMemoryCache();

var provider = services.BuildServiceProvider();
var cache = provider.GetRequiredService<IMemoryCache>();

var instance = cache.GetOrCreate("key", entry =>
{
    // ...
    return new
    {
        Message = "Hello World"
    };
});

Console.WriteLine(instance.Message);

On .NET 6.0, you don't get any error or warning as the caching stack was not decorated with nullable annotations.
On .NET 7.0, you get a CS8602 warning on the last line, as GetOrCreate() returns TItem? even if the delegate itself will never return null.

I guess the warning is technically correct because - in theory - nothing prevents me from inserting a null value in the cache before the delegate has a chance to be called, but I'm not sure it's a frequent case (in this case, it would be more logical that the delegate signature use TItem? instead of TItem to account for potential null values).

To make them easier to use, should CacheExtensions.GetOrCreate()/GetOrCreateAsync() return TItem instead of TItem??

Author:	kevinchalet
Assignees:	-
Labels:	area-Extensions-Caching
Milestone:	-

[tarekgh]

@eerhardt what you think about this one?

[eerhardt]

This looks like the opposite of the request in #77264. In #77264, it is showing that you can cache null as a value in the IMemoryCache.

If someone cached null as the value for item, calling GetOrCreate() will return null, and not invoke your factory method.

So I believe the existing annotation is correct.

[kevinchalet]

This looks like the opposite of the request in #77264. In #77264, it is showing that you can cache null as a value in the IMemoryCache.

No, the two are actually unrelated (being able to cache null values has always been supported, otherwise I wouldn't have used it). The other issue is just an oversight affecting an extension.

If someone cached null as the value for item, calling GetOrCreate() will return null, and not invoke your factory method.

I mentioned that in my OP but my point is that GetOrCreate() is typically always used alone - as a one-shot operation - in the wild. You never do something like that:

cache.Set<object?>("key", null);

var instance = cache.GetOrCreate("key", entry =>
{
    // ...
    return new
    {
        Message = "Hello World"
    };
});

While technically correct, it semantically doesn't make much sense that the nullability of the returned type is not respected when it's used alone:

var instance = cache.GetOrCreate("key", entry =>
{
    // ...
    return new
    {
        Message = "Hello World"
    };
});

// You don't expect a null value here as the delegate doesn't return null.

[kevinchalet]

And BTW, Set() + GetOrCreate() is a terrible combination. Take this snippet, similar to the previous one, but using a delegate that returns a non-nullable value type:

using Microsoft.Extensions.Caching.Memory;
using Microsoft.Extensions.DependencyInjection;

var services = new ServiceCollection();
services.AddMemoryCache();

var provider = services.BuildServiceProvider();
var cache = provider.GetRequiredService<IMemoryCache>();

cache.Set<object?>("key", null);

// The returned type is inferred as "int" based on the delegate signature.
var instance = cache.GetOrCreate("key", entry =>
{
    // ...
    return 10;
});

Console.WriteLine(instance);

If you run this snippet, all you'll get is a null-reference exception when calling GetOrCreate().

[eerhardt]

While technically correct

The point of nullable annotations is to be able to answer questions like:

• Is it possible for this method to return null?
• Does this method accept null?

From the method signature. And to statically warn you at compile time when it is possible for you to get NullReferenceExceptions at runtime.

If this API was annotated the way you are suggesting, it would be possible for your code to get NREs at runtime, with no warnings at compile time. This goes against the spirit of the annotations.

So while it is not likely, or not often the case, it is still possible. This is why it is annotated this way.

If IMemoryCache didn't have object as its Value, and instead was generic - IMemoryCache<TValue> (see #48455 and #48567), then this API could be annotated the way you want. But since it can take any object (including null), it is possible for this API to return null.

[eerhardt]

If you run this snippet, all you'll get is a null-reference exception when calling GetOrCreate().

This is another disadvantage of MemoryCache not being generic. You are trying to use strong-typing, but everything is weakly typed underneath. So there are no guarantees your strong typing will work.

This issue has been marked needs-author-action and may be missing some important information.

[tarekgh]

Per @eerhardt replies, it looks preferable to not do the suggested change. @kevinchalet I am closing the issue but feel free to reply if you still have any questions.