nullable tracking doesn't work well with linq

[YairHalberstadt]

Version Used: master

Steps to Reproduce:

using System;
using System.Collections.Generic;
using System.Linq;

#nullable enable
public class C {
    public IEnumerable<string> WhereNotNull(IEnumerable<string?> strings)
    {
        return strings.Where(x => x != null);
    }
}

Expected Behavior:

No warning

Actual Behavior:

warning CS8619: Nullability of reference types in value of type 'IEnumerable<string?>' doesn't match target type 'IEnumerable'.

[sharwell]

I hit this recently. I noticed that strings.OfType<string>() is functionally equivalent and works with nullable reference types, but I'm not sure it would perform as well.

[RikkiGibson]

I think a helper like this would be nice to have in the standard library for this very simple case:

static IEnumerable<T> WhereNotNull<T>(this IEnumerable<T?> enumerable) where T : class
{
    foreach (var t in enumerable)
    {
        if (t != null)
        {
            yield return t;
        }
    }
}

It's conceivable that nullable analysis could track flow state between lambdas or track the element null state of collections to support slightly more complex cases like the ones below:

class Widget
{
    string? Prop { get; set; }

    static void Query1(List<Widget> widgets)
    {
        // hypothetically, we carry over the WhenTrue state
        // from the Where clause over to the Select
        _ = widgets
            .Where(w => w.Prop != null)
            .Select(w => w.Prop.GetHashCode());
    }

    static void Query2(List<Widget?> widgets)
    {
        if (widgets.All(w => w != null)
        {
            // `widgets` is considered a List<Widget> here
            _ = widgets[0].ToString();
        }
    }
}

While I think this is worth thinking about, it seems very fraught and limited to a small subset of real-world queries. Ideally the user should be able to figure out why the compiler thinks something is null, so arguably it's better to avoid doing the analysis even for the simple LINQ cases if it's just going to cause confusion about why they're getting warnings in this query but not that one.

[YairHalberstadt]

Related is dotnet/csharplang#2388

[YairHalberstadt]

I raised an issue in CoreFx to see what they thought of adding a WhereNotNull method:

https://github.com/dotnet/corefx/issues/39798

[rosenbjerg]

I don't see how the expected behaviour is automatic casting/conversion from string? to string.
What has led you to expect this?

[YairHalberstadt]

@rosenbjerg
One of the aim of NRTs is to work well with existing C# patterns. Given that Where(x => x != null) is commonly used to return a collection containing non-null elements, NRTs should recognize this pattern, and treat the returned enumerable as having a non-nullable type parameter.

[rosenbjerg]

@YairHalberstadt I agree that it makes sense for this simple use case, but won't it be hard to get consistent analysis results for more complex scenarios? That might not actually be a problem, but I would value consistent behaviour higher than extra "smartness" in certain cases

[Spongman]

I'm having the same issue, but with IObservable<T?>.Where(t => t != null). the solution is not quite as obvious.

[redodin2]

Looks like an extension method lost the context. However, you can use query syntax

public static IEnumerable<T> WhereNotNull<T>(this IEnumerable<T?> that) 
    where T : class => from item in that where item != null select item;

[SpaceOgre]

Is there a fix for problems like this:

var myClasses = list
	.Where(x => x.Prop != null)
	.Select(x => new MyClass(x.Prop));

Atm I get a CS8604 on new MyClass(x.Prop) where Prop obviously can't be null. To a pragma or something for every case like this is not good since it makes the code harder to read.

And I would rather not have to write code like this just to not get the warning:

var res = new List<MyClass>();
foreach (var item in list)
{
	if (item.Prop== null) continue;
	res.Add(new MyClass(item.Prop));
}

At this stage I'm just thinking of hiding the CS8604 warning so I don't have to deal with all the extra issues it causes and that is bad.

Oh and why is this not giving me an warning 🤔:

var myClasses = list
	.Where(x => x.Prop != null)
	.Select(x => new { x.Prop });
	.Select(x => new MyClass(x.Prop));

[canton7]

@SpaceOgre The easiest way to suppress individual nullability warnings is to use the null-forgiving operator !:

var myClasses = list
	.Where(x => x.Prop != null)
	.Select(x => new MyClass(x.Prop!));

[SpaceOgre]

@SpaceOgre The easiest way to suppress individual nullability warnings is to use the null-forgiving operator !:

var myClasses = list
	.Where(x => x.Prop != null)
	.Select(x => new MyClass(x.Prop!));

Awsome! Thank you 👍

[sjb-sjb]

Here's another example:

 Delegate[] invocationList = userCallback.GetInvocationList(); // userCallback is a multicast delegate
 [...] from @delegate in invocationList where @delegate?.Target != null select new WeakReference<object>(@delegate.Target) [...]

The WeakReference constructor requires a non-nullable reference argument, and the where clause means that the argument is definitely not null, yet C# 9 gives a warning that the argument is possibly null.

The reason I don't want to just throw ! onto the @delegate.Target is that our coding standard says ! can only be added in places where an exception will occur if the expression is null -- e.g. just before a dereference. The purpose of that standard is to prevent null from being accidentally propagated around and throughout the code, which after all is one of the main reasons for having nullability checking in the first place.

[Pzixel]

Sometimes LINQ code doesn't provoke a NRT warning while the equivalent method chain does:

using System;
using System.Collections.Generic;
using System.Linq;

#nullable enable

public class C {
    public void M(IEnumerable<string?> s) {
        var x = from c in s
        select c.Length;
    }
    
    public void N(IEnumerable<string?> s) {
        var x = s.Select(c => c.Length);
    }
}

Expected Behavior:

Both lines show "warning CS8602: Dereference of a possibly null reference."

Actual Behavior:

LINQ version doesn't produce any warnings and allows to silently defererence null

[kapsiR]

It seems there is a similar issue with Task<T> and Task<T?>.

warning CS8620: Argument of type 'Task' cannot be used for parameter 'task' of type 'Task<string?>' in 'Task Extensions.Bar(Task<string?> task)' due to differences in the nullability of reference types.

SharpLab example (Task<string> as parameter for a method that accepts Task<string?>)

Should I create a separate issue for that case?

[RikkiGibson]

See dotnet/csharplang#3950 and related issues.

[VAllens]

This scene, can be tracked?

class Program {
    static void Main(string[] args) {
        List<Model> models = new();
        models.Add(new Model {
            Id = Guid.NewGuid()
        });
        models.Add(new Model {
            Id = Guid.NewGuid(),
            Address = "Address1"
        });

        List<string> addresses = models.Where(x => x.Address != null).Select(x => x.Address).ToList();
    }
}

[RikkiGibson]

The nullability improvements working group discussed this issue yesterday.

[mungojam]

To cover the main case, maybe the BCL could add a function .WhereNotNull().

[RikkiGibson]

I think if we didn't want to introduce any "real" tracking of the query variable, I would personally prefer adding WhereNotNull over only recognizing .Where(x => x != null).

[TKharaishvili]

In TypeScript I do this:

strings.flatMap(x => x != null ? [x] : [])

looks much uglier in C#:

strings.SelectMany(x => x != null ? new[] { x } : Array.Empty<string>())

but maybe that could be helped by this - dotnet/csharplang#5354

my two cents...