Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Enumerable.WhereNotNull() #41363

Closed
terrajobst opened this issue Aug 25, 2020 · 5 comments
Closed

Add Enumerable.WhereNotNull() #41363

terrajobst opened this issue Aug 25, 2020 · 5 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Linq untriaged New issue has not been triaged by the area owner
Milestone

Comments

@terrajobst
Copy link
Member

terrajobst commented Aug 25, 2020

The compiler has limitations what it can infer and express with respect to nullability, especially in the context of generics. When using LINQ, it's common to deal with sequences that should not contain null values. Unfortunately, there is no easy way to make the type checker aware when they got removed.

Consider this:

private static IReadOnlyList<ToolCommand> GetCommands()
{
    return typeof(Program).Assembly
                          .GetTypes()
                          .Where(t => typeof(ToolCommand).IsAssignableFrom(t) &&
                                      !t.IsAbstract && t.GetConstructor(Array.Empty<Type>()) != null)
                          .Select(t => (ToolCommand?)Activator.CreateInstance(t))
                          .OrderBy(t => t.Name)
                          .ToArray();
}

Since the Activator.CreateInstance may return a null value, the compiler correctly warns me that .OrderBy(t => t.Name) may null-ref. Sadly, inserting .Where(t => t != null) doesn't fix this because it doesn't change the return type of the Where() method. One also has to insert .Select(t => t!) to tell the type system that the values are now not-null.

private static IReadOnlyList<ToolCommand> GetCommands()
{
    return typeof(Program).Assembly
                          .GetTypes()
                          .Where(t => typeof(ToolCommand).IsAssignableFrom(t) &&
                                      !t.IsAbstract && t.GetConstructor(Array.Empty<Type>()) != null)
                          .Select(t => (ToolCommand?)Activator.CreateInstance(t))
                          .Where(t => t != null)
                          .Select(t => t !)
                          .OrderBy(t => t.Name)
                          .ToArray();
}

Proposed API

We can address this problem by introducing a method that explicitly changes the return type:

namespace System.Linq
{
    public partial class Enumerable
    {
        public static IEnumerable<T> WhereNotNull<T>(this IEnumerable<T?> source)
            where T: class;
    }
}

It also makes the common tasks of excluding null values easier.

Usage Examples

private static IReadOnlyList<ToolCommand> GetCommands()
{
    return typeof(Program).Assembly
                          .GetTypes()
                          .Where(t => typeof(ToolCommand).IsAssignableFrom(t) &&
                                      !t.IsAbstract && t.GetConstructor(Array.Empty<Type>()) != null)
                          .Select(t => (ToolCommand?)Activator.CreateInstance(t))
                          .WhereNotNull()
                          .OrderBy(t => t.Name)
                          .ToArray();
}
@terrajobst terrajobst added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Linq labels Aug 25, 2020
@terrajobst terrajobst added this to the 6.0.0 milestone Aug 25, 2020
@ghost
Copy link

ghost commented Aug 25, 2020

Tagging subscribers to this area: @eiriktsarpalis
See info in area-owners.md if you want to be subscribed.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Aug 25, 2020
@GrabYourPitchforks
Copy link
Member

GrabYourPitchforks commented Aug 26, 2020

Is there any way to remove the where T : class restriction? I can imagine scenarios where I want to use this within a helper function, but my helper function isn't constrained to where T : class. In cases where T is a non-nullable value type the method's implementation would essentially be "return this".

public static class Extensions
{
    public static IEnumerable<T> WhereNotNull<T>(this IEnumerable<T?> source) => throw null;
}

Behavior:

  • If T is a reference type, filter out items which are null.
  • If T is a non-nullable value type, return source.
  • If T is a nullable value type, filter out items which are null, but since you have to preserve the return type you'd still technically be returning IEnumerable<Nullable<T>>. It's not the end of the world. If you want to make this a little better then you can add another overload, as shown below.
public static class Extensions
{
    public static IEnumerable<T> WhereNotNull<T>(this IEnumerable<T?> source) => throw null;
    public static IEnumerable<T> WhereNotNull<T>(this IEnumerable<T?> source) where T : struct => throw null;
}

I confirmed in a sample app that this syntax is legal and that the nullability feature behaves as expected when it sees calls to these APIs.

@mburbea
Copy link

mburbea commented Aug 26, 2020

@FiniteReality
Copy link

FiniteReality commented Aug 26, 2020

@mburbea That's still an extra method call:

var values = mySequence
    .Where(x => x != null)
    .OfType<T>();
// or
var values = mySequence
    .Where(x => x != null)
    .Select(x => x!);

The goal is:

var values = mySequence
    .WhereNotNull();

EDIT: So it turns out I shouldn't be reading issues shortly after I've woken up 🙃
I checked the implementation and only saw if (value is TResult result) and completely forgot that also adds an implicit null check...

@stephentoub
Copy link
Member

Dup of #30381

@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Linq untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

6 participants