dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.92k stars 4.02k forks source link

Can't bang away warning when nullability of constraint type doesn't match #36510

Closed safern closed 3 years ago

safern commented 5 years ago
using System;
using System.Collections.Generic;
using System.Runtime.CompilerServices;

#nullable enable

public class C {
    public int FooMethodIComparer<T, TComparer>(T value, TComparer comparer) where TComparer : IComparer<T>?
    {
        if (comparer == null)
        {
            return FooMethodIComparable(value, new Foo<T, Comparer<T>>(value, Comparer<T>.Default));
        }

        // warning CS8631: The type 'TComparer' cannot be used as type parameter 'TComparer' in 
        // the generic type or method 'Foo<T, TComparer>'. Nullability of type argument 
        // 'TComparer' doesn't match constraint type 'System.Collections.Generic.IComparer<T>'.
        return FooMethodIComparable(value, new Foo<T, TComparer>(value, comparer));
    }

    public int FooMethodIComparable<T, TComparable>(T value, TComparable comparable) where TComparable : IComparable<T>
    {

        return comparable.CompareTo(value);
    }
}

public class Foo<T, TComparer> : IComparable<T> where TComparer : IComparer<T>
{
    private readonly T _value;
    private readonly TComparer _comparer;

    public Foo(T value, TComparer comparer)
    {
        _value = value;
        _comparer = comparer;
    }

    public int CompareTo(T other) => _comparer.Compare(_value, other);
}

I think people should be allowed to bang this warning away on the generic type use

return FooMethodIComparable(value, new Foo<T, TComparer!>(value, comparer));

Gives a syntax error. So this forces me to either use a pragma warning disable, or make my class Foo<T, TComparer> to accept an IComparer<T>? and then bang away the call to _comparer!.Compare(_value, other) because I know _comparer will not be null.

Also, when I'm creating the instance of this class, the compiler can identify that TComparer is will not be null at the callsite because I'm doing a null check on it:

if (comparer == null)
{
    return FooMethodIComparable(value, new Foo<T, Comparer<T>>(value, Comparer<T>.Default));
}

return FooMethodIComparable(value, new Foo<T, TComparer>(value, comparer));

Repro can be found here

So maybe the warning on FooMethodIComparable(value, new Foo<T, TComparer> should never be produced?

cc: @dotnet/nullablefc @cston

jaredpar commented 3 years ago

Closing as this behavior is currently "By Design". The null forgiving operator today is only allowed in an expression context, not in a type context. Not opposed to this but we'd need to make this a language change proposal on dotnet/csharplang.