Search code examples
c#genericsnullablec#-8.0nullable-reference-types

How to solve "The issue with T?"/nullable constraint on type parameter?


I'm designing an interface in C# 8.0 with nullable enabled, targeting .Net Standard 2.0 (using the Nullable package) and 2.1. I am now facing The issue with T?.

In my example, I am building an interface for a cache which stores Streams/byte data, identified by a string key, i.e. the file system could by a trivial implementation. Every entry is additionally identified by a version, which should be generic. This version could for example be another string key (like an etag), an int or a date.

public interface ICache<TVersionIdentifier> where TVersionIdentifier : notnull
{
    // this method should return a nullable version of TVersionIdentifier, but this is not expressable due to 
    // "The issue with T?" https://devblogs.microsoft.com/dotnet/try-out-nullable-reference-types/
    Task<TVersionIdentifier> GetVersionAsync(string file, CancellationToken cancellationToken = default);

    // TVersionIdentifier should be not nullable here, which is what we get with the given code
    Task<Stream> GetAsync(string file, TVersionIdentifier version, CancellationToken cancellationToken = default);

    // ...
}

While I understand what the issue with T? is and why it is a non trivial problem for the compiler, I don't know how to handle this situation.

I thought of some options but neither of them seems to be optimal:

  1. Disable nullable for the interface, manually tag non-nullable occurrences of TVersionIdentifier:

    #nullable disable
    public interface ICache<TVersionIdentifier>
    {
        Task<TVersionIdentifier> GetVersionAsync(string file, CancellationToken cancellationToken = default);
        // notice the DisallowNullAttribute
        Task<Stream> GetAsync(string file, [DisallowNull] TVersionIdentifier version, CancellationToken cancellationToken = default);
        // ..
    }
    #nullable restore
    

    This does not seem to help. When implementing ICache<string> in a nullable-enabled context, Task<string?> GetVersionAsync generates a warning as the signatures don't match. Most likely the compiler knows that the type given for TVersionIdentifier is non-nullable and enforces it's rules, even though ICache doesn't know about it. For popular interfaces like IList<T> this makes sense.

    This results in warnings so this does not seem to be a real option.

  2. Disable nullable for the implementation of the member. While warnings are produced in either case it seems consequent to disable nullable for the interface then though (does this really make sense?).

    #nullable disable
        public Task<string> GetVersionAsync(string file, CancellationToken cancellationToken = default)
        {
            return Task.FromResult((string)null);
        }
    #nullable restore
    
  3. Like (2) but disable nullable for the whole implementing class (and the interface too). Maybe this is most consequent, as it clearly expresses the concept of nullable reference types/generics/... does not work for this class and the callers have to handle this situation as they had to earlier (pre C# 8.0).

    #nullable disable
    class FileSystemCache : ICache<string>
    {
        // ...
    }
    #nullable restore
    
  4. Option (2) or (3) but suppressing the compiler warning instead of disabling nullable. Maybe the compiler draws wrong conclusions afterwards, so this is a bad idea?

  5. Like (1) but with convention for implementers: Disable nullable for the interface, but annotate with [DisallowNull] and [NotNull] manually (see code in (1)). Use nullable types as TVersionIdentifier in all implementations manually (we can not enforce this). This might get us as close as we can get regarding correctly annotated assemblies. The consumers of our implemementation get warned when using nulls where they shouldn't and they get correctly annotated return values. This way is not very self-documenting though. Any possible implementer needs to read our docs to fully understand our intent. Thus, our interface isn't a good model for possible implementations, as it misses some aspects. People might not expect this.

Which way to go? Is there another way I missed? Are there any relevant aspects I missed?

I think it would have been great if Microsoft advised a possible workaround in the blog post.


Solution

  • Starting with C# 9, the interface could be declared like this:

    public interface ICache<TVersionIdentifier> where TVersionIdentifier : notnull
    {
        Task<Stream> GetAsync(string file, TVersionIdentifier version, CancellationToken cancellationToken = default);
        Task<TVersionIdentifier?> GetVersionAsync(string file, CancellationToken cancellationToken = default);
        // ...
    }
    

    It doesn't achieve what I try to achieve though.

    An implementation of ICache<string> will correctly have the members:

    public class StringVersionedCache : ICache<string>
    {
        public Task<Stream> GetAsync(string file, string version, CancellationToken cancellationToken = default)
        {
            // ...
        }
    
        public Task<string?> GetVersionAsync(string file, CancellationToken cancellationToken = default)
        {
            // ...
        }
    
        // ...
    }
    

    An implementation of ICache<int> will incorrectly have these members though:

    public class IntVersionedCache : ICache<int>
    {
        public Task<Stream> GetAsync(string file, int version, CancellationToken cancellationToken = default)
        {
            // ...
        }
    
        // WRONG: needs to be Task<int?>
        public Task<int> GetVersionAsync(string file, CancellationToken cancellationToken = default)
        {
            // ...
        }
    
        // ...
    }
    

    @rikki-gibson mentioned Nullable reference types: How to specify "T?" type without constraining to class or struct in the comments, thanks. It is realted and further helps understanding the issue, but doesn't solve the problem though (see above).

    The imho cleanest solution I was able to come up with is described in Returning nullable and null in single C# generic method?.

    Declare the interface this way:

    public interface ICache<TVersionIdentifier> where TVersionIdentifier : notnull
    {
        Task<Stream> GetAsync(string file, [DisallowNull] TVersionIdentifier version, CancellationToken cancellationToken = default);
        Task<bool> TryGetVersionAsync(string file, [NotNullWhen(true)] out TVersionIdentifier? result, CancellationToken cancellationToken = default);
        // ...
    }
    

    Also note the [NotNullWhen(true)] attribute and see the corresponding docs. Thanks to C# 9 it is possible to have a nullable out parameter out TVersionIdentifier? result. If TVersionIdentifier is int, this will be int not int? (as described above). Given how Try* methods are typically used in .Net, the intention and semantics of the function are easily understandable though. Using the NotNullWhen attribute we get proper compiler null checks too. Thus, this solution meets my design goals.