Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Equals [NotNullWhen(true)]
  • Loading branch information
maxkoshevoi committed Aug 14, 2021
commit 6b1fed815e034ee390f70a70ab0f2c5409488a53
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public partial interface IChangeToken
public bool Equals(Microsoft.Extensions.Primitives.StringSegment other) { throw null; }
public static bool Equals(Microsoft.Extensions.Primitives.StringSegment a, Microsoft.Extensions.Primitives.StringSegment b, System.StringComparison comparisonType) { throw null; }
public bool Equals(Microsoft.Extensions.Primitives.StringSegment other, System.StringComparison comparisonType) { throw null; }
public override bool Equals(object? obj) { throw null; }
public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhen(true)] object? obj) { throw null; }
public bool Equals(string? text) { throw null; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this throw for null?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't allow me to make it non-nullable...

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interestingly, in implementation text is non-nullable and everything compiles fine

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't allow me to make it non-nullable...

You need to change the type of the interface being implemented to IEquatable<string?>.

in implementation text is non-nullable and everything compiles fine

Yes, because the code never tries to dereference a maybe-null value. The first thing the code does is check whether it's null and throw if it is.

Copy link
Contributor Author

@maxkoshevoi maxkoshevoi Aug 14, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to change the type of the interface being implemented to IEquatable<string?>

Still doesn't allow non-nullable text

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I misread what you'd written. IEquatable<T> is defined to expect Equals always allows null (it accepts a T?), which is why this is complaining about trying to specify a non-nullable value. I think the implementation should be changed to return false rather than throwing, and then typed as string?, but I'll defer here to @dotnet/area-microsoft-extensions area owners.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made it return false.

public bool Equals(string text, System.StringComparison comparisonType) { throw null; }
public override int GetHashCode() { throw null; }
Expand Down Expand Up @@ -138,7 +138,7 @@ public void Reset() { }
public override bool Equals(object? obj) { throw null; }
public bool Equals(string? other) { throw null; }
public static bool Equals(string? left, Microsoft.Extensions.Primitives.StringValues right) { throw null; }
public bool Equals(string?[]? other) { throw null; }
public bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhen(true)] string?[]? other) { throw null; }
public static bool Equals(string?[] left, Microsoft.Extensions.Primitives.StringValues right) { throw null; }
public Microsoft.Extensions.Primitives.StringValues.Enumerator GetEnumerator() { throw null; }
public override int GetHashCode() { throw null; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public static int Compare(StringSegment a, StringSegment b, StringComparison com
/// </summary>
/// <param name="obj">An object to compare with this object.</param>
/// <returns><see langword="true" /> if the current object is equal to the other parameter; otherwise, <see langword="false" />.</returns>
public override bool Equals(object? obj)
public override bool Equals([NotNullWhen(true)] object? obj)
{
return obj is StringSegment segment && Equals(segment);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@ public static bool Equals(StringValues left, StringValues right)
/// </summary>
/// <param name="other">The string array to compare to this instance.</param>
/// <returns><c>true</c> if the value of <paramref name="other"/> is the same as this instance; otherwise, <c>false</c>.</returns>
public bool Equals(string?[]? other) => other != null && Equals(this, new StringValues(other));
public bool Equals([NotNullWhen(true)] string?[]? other) => other != null && Equals(this, new StringValues(other));

/// <inheritdoc cref="Equals(StringValues, string)" />
public static bool operator ==(StringValues left, string? right) => Equals(left, new StringValues(right));
Expand Down