Add a NotNullIfNull code analysis attribute

[twoltjer]

Considering this code, there's no good attribute that can be used to indicate the nullability of the returned value:

// Precondition: points has at least one valid value between FirstValidPointIndex and LastValidPointIndex, and all points in that range are valid
// Return is not null if predicate is null
public static double? FirstXValue(this PointCollection points, Func<double, bool>? predicate = null)
{
	for (var i = points.FirstValidPointIndex; i <= points.LastValidPointIndex; i++)
	{
		var x = points.GetXFromPointIndex(i);
		if (predicate?.Invoke(x) ?? true)
			return x;
	}
	return null;
}

I'd be happy to help out with development, but haven't worked much with Roslyn code before (just making analyzers) so will likely require some help with the prototype.

[RikkiGibson]

Moved to csharplang since this suggests a the nullable reference types language feature.

I think this attribute would be tricky for a number of reasons. First is that nullable analysis doesn't actually track that a value is null--it tracks that it may be null. So it would actually be difficult for us to do the right thing in a scenario like the following:

void M(PointCollection points)
{
    Func<double, bool>? predicate = null;
    var xValue = points.FirstXValue(predicate);
    xValue.Value.ToString();  // shouldn't warn
}

When predicate is read here we just know that it might be null. In that case, we would want to assume the return value is maybe-null, even if a NotNullIfNull attribute is used. In order to realize the return is not-null, we would need to add a definitely-null flow state to nullable analysis. It's a significant amount of work to add a flow state and make sure it is handled correctly everywhere.

I think it's worthwhile to gauge whether there's a wider interest/benefit from this, however, my instinctive thought about the scenario you've shared here is that an overload double FirstXValue(this PointCollection points) should be added.

[yaakov-h]

The returned value is a double? a.k.a Nullable<double>, which is a nullable value type. This doesn't seem to have anything to do with the nullable reference types feature as it stands.

Are there NRT warnings for value types now?

[jnm2]

NRT warnings have applied to nullable value types all along.

[yaakov-h]

TIL, guess I just haven't come across it.