Skip to content

Commit

Permalink
Use [NotNullWhen(true)] in more places (#47598)
Browse files Browse the repository at this point in the history
* Use [NotNullWhen(true)] in more places

Did a quick search/audit for methods that returned bool and took nullable object as the first input, and added [NotNullWhen(true)] where it was obviously correct.

* Address PR feedback
  • Loading branch information
stephentoub committed Jan 30, 2021
1 parent 14453d6 commit 2b0e278
Show file tree
Hide file tree
Showing 269 changed files with 716 additions and 554 deletions.
1 change: 1 addition & 0 deletions docs/coding-guidelines/api-guidelines/nullability.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ The C# compiler respects a set of attributes that impact its flow analysis. We
- **DO** annotate properties where a getter will never return `null` but a setter allows `null` as being non-nullable but also `[AllowNull]`.
- **DO** annotate properties where a getter may return `null` but a setter throws for `null` as being nullable but also `[DisallowNull]`.
- **DO** add `[NotNullWhen(true)]` to nullable arguments of `Try` methods that will definitively be non-`null` if the method returns `true`. For example, if `Int32.TryParse(string? s)` returns `true`, `s` is known to not be `null`, and so the method should be `public static bool TryParse([NotNullWhen(true)] string? s, out int result)`.
- **DO** add `[NotNullWhen(true)]` to overrides of `public virtual bool Equals(object? obj)`, except in the extremely rare circumstance where a non-null instance may compare equally to `null` (one example of where it's not valid is on `Nullable<T>`).
- **DO** add `[NotNullIfNotNull(string)]` if nullable ref argument will be non-`null` upon exit, when an other argument passed evaluated to non-`null`, pass that argument name as string. Example: `public void Exchange([NotNullIfNotNull("value")] ref object? location, object? value);`.
- **DO** add `[return: NotNullIfNotNull(string)]` if a method would not return `null` in case an argument passed evaluated to non-`null`, pass that argument name as string. Example: `[return: NotNullIfNotNull("name")] public string? FormatName(string? name);`
- **DO** add `[MemberNotNull(string fieldName)]` to a helper method which initializes member field(s), passing in the field name. Example: `[MemberNotNull("_buffer")] private void InitializeBuffer()`. This will help to avoid spurious warnings at call sites that call the initialization method and then proceed to use the specified field. Note that there are two constructors to `MemberNotNull`; one that takes a single `string`, and one that takes a `params string[]`. When the number of fields initialized is small (e.g. <= 3), it's preferable to use multiple `[MemberNotNull(string)]` attributes on the method rather than one `[MemberNotNull(string, string, string, ...)]` attribute, as the latter is not CLS compliant and will likely require `#pragma warning disable` and `#pragma warning restore` around the line to suppress warnings.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ protected Delegate([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.Al
}


public override bool Equals(object? obj)
public override bool Equals([NotNullWhen(true)] object? obj)
{
if (obj == null || !InternalEqualTypes(this, obj))
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
**
===========================================================*/

using System.Diagnostics.CodeAnalysis;

namespace System.Diagnostics.SymbolStore
{
internal struct SymbolToken
Expand All @@ -21,7 +23,7 @@ internal struct SymbolToken

public override int GetHashCode() { return m_token; }

public override bool Equals(object? obj)
public override bool Equals([NotNullWhen(true)] object? obj)
{
if (obj is SymbolToken)
return Equals((SymbolToken)obj);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
Expand All @@ -13,7 +14,7 @@ public abstract partial class Enum
private static extern void GetEnumValuesAndNames(QCallTypeHandle enumType, ObjectHandleOnStack values, ObjectHandleOnStack names, Interop.BOOL getNames);

[MethodImpl(MethodImplOptions.InternalCall)]
public extern override bool Equals(object? obj);
public extern override bool Equals([NotNullWhen(true)] object? obj);

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern object InternalBoxEnum(RuntimeType enumType, long value);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public override void GetObjectData(SerializationInfo info, StreamingContext cont

// equals returns true IIF the delegate is not null and has the
// same target, method and invocation list as this object
public sealed override bool Equals(object? obj)
public sealed override bool Equals([NotNullWhen(true)] object? obj)
{
if (obj == null)
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1246,7 +1246,7 @@ public override int GetHashCode()
return m_ptr != null ? m_ptr.GetHashCode() : 0;
}

public override bool Equals(object? obj)
public override bool Equals([NotNullWhen(true)] object? obj)
{
if (!(obj is ModuleHandle))
return false;
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/System.Private.CoreLib/src/System/ValueType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public abstract class ValueType
{
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern",
Justification = "Trimmed fields don't make a difference for equality")]
public override bool Equals(object? obj)
public override bool Equals([NotNullWhen(true)] object? obj)
{
if (null == obj)
{
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Console/ref/System.Console.cs
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ public enum ConsoleKey
public char KeyChar { get { throw null; } }
public System.ConsoleModifiers Modifiers { get { throw null; } }
public bool Equals(System.ConsoleKeyInfo obj) { throw null; }
public override bool Equals(object? value) { throw null; }
public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? value) { throw null; }
public override int GetHashCode() { throw null; }
public static bool operator ==(System.ConsoleKeyInfo a, System.ConsoleKeyInfo b) { throw null; }
public static bool operator !=(System.ConsoleKeyInfo a, System.ConsoleKeyInfo b) { throw null; }
Expand Down
4 changes: 3 additions & 1 deletion src/libraries/System.Console/src/System/ConsoleKeyInfo.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;

namespace System
{
public readonly struct ConsoleKeyInfo : IEquatable<ConsoleKeyInfo>
Expand Down Expand Up @@ -46,7 +48,7 @@ public ConsoleModifiers Modifiers
get { return _mods; }
}

public override bool Equals(object? value)
public override bool Equals([NotNullWhen(true)] object? value)
{
return value is ConsoleKeyInfo info && Equals(info);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public void CopyTo(System.Span<byte> destination) { }
public static System.Diagnostics.ActivitySpanId CreateFromUtf8String(System.ReadOnlySpan<byte> idData) { throw null; }
public static System.Diagnostics.ActivitySpanId CreateRandom() { throw null; }
public bool Equals(System.Diagnostics.ActivitySpanId spanId) { throw null; }
public override bool Equals(object? obj) { throw null; }
public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? obj) { throw null; }
public override int GetHashCode() { throw null; }
public static bool operator ==(System.Diagnostics.ActivitySpanId spanId1, System.Diagnostics.ActivitySpanId spandId2) { throw null; }
public static bool operator !=(System.Diagnostics.ActivitySpanId spanId1, System.Diagnostics.ActivitySpanId spandId2) { throw null; }
Expand Down Expand Up @@ -171,7 +171,7 @@ public void CopyTo(System.Span<byte> destination) { }
public static System.Diagnostics.ActivityTraceId CreateFromUtf8String(System.ReadOnlySpan<byte> idData) { throw null; }
public static System.Diagnostics.ActivityTraceId CreateRandom() { throw null; }
public bool Equals(System.Diagnostics.ActivityTraceId traceId) { throw null; }
public override bool Equals(object? obj) { throw null; }
public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? obj) { throw null; }
public override int GetHashCode() { throw null; }
public static bool operator ==(System.Diagnostics.ActivityTraceId traceId1, System.Diagnostics.ActivityTraceId traceId2) { throw null; }
public static bool operator !=(System.Diagnostics.ActivityTraceId traceId1, System.Diagnostics.ActivityTraceId traceId2) { throw null; }
Expand Down Expand Up @@ -227,15 +227,15 @@ public readonly struct ActivityEvent
public static bool operator ==(System.Diagnostics.ActivityContext left, System.Diagnostics.ActivityContext right) { throw null; }
public static bool operator !=(System.Diagnostics.ActivityContext left, System.Diagnostics.ActivityContext right) { throw null; }
public bool Equals(System.Diagnostics.ActivityContext value) { throw null; }
public override bool Equals(object? obj) { throw null; }
public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? obj) { throw null; }
public override int GetHashCode() { throw null; }
}
public readonly struct ActivityLink : IEquatable<ActivityLink>
{
public ActivityLink(System.Diagnostics.ActivityContext context, System.Diagnostics.ActivityTagsCollection? tags = null) { throw null; }
public System.Diagnostics.ActivityContext Context { get { throw null; } }
public System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, object?>>? Tags { get { throw null; } }
public override bool Equals(object? obj) { throw null; }
public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? obj) { throw null; }
public bool Equals(System.Diagnostics.ActivityLink value) { throw null; }
public static bool operator ==(System.Diagnostics.ActivityLink left, System.Diagnostics.ActivityLink right) { throw null; }
public static bool operator !=(System.Diagnostics.ActivityLink left, System.Diagnostics.ActivityLink right) { throw null; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1763,7 +1763,7 @@ public bool Equals(ActivityTraceId traceId)
{
return _hexString == traceId._hexString;
}
public override bool Equals(object? obj)
public override bool Equals([NotNullWhen(true)] object? obj)
{
if (obj is ActivityTraceId traceId)
return _hexString == traceId._hexString;
Expand Down Expand Up @@ -1948,7 +1948,7 @@ public bool Equals(ActivitySpanId spanId)
{
return _hexString == spanId._hexString;
}
public override bool Equals(object? obj)
public override bool Equals([NotNullWhen(true)] object? obj)
{
if (obj is ActivitySpanId spanId)
return _hexString == spanId._hexString;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace System.Diagnostics
{
Expand Down Expand Up @@ -95,7 +96,7 @@ public static ActivityContext Parse(string traceParent, string? traceState)

public bool Equals(ActivityContext value) => SpanId.Equals(value.SpanId) && TraceId.Equals(value.TraceId) && TraceFlags == value.TraceFlags && TraceState == value.TraceState && IsRemote == value.IsRemote;

public override bool Equals(object? obj) => (obj is ActivityContext context) ? Equals(context) : false;
public override bool Equals([NotNullWhen(true)] object? obj) => (obj is ActivityContext context) ? Equals(context) : false;
public static bool operator ==(ActivityContext left, ActivityContext right) => left.Equals(right);
public static bool operator !=(ActivityContext left, ActivityContext right) => !(left == right);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;

namespace System.Diagnostics
{
Expand Down Expand Up @@ -34,7 +35,7 @@ public ActivityLink(ActivityContext context, ActivityTagsCollection? tags = null
/// </summary>
public IEnumerable<KeyValuePair<string, object?>>? Tags { get; }

public override bool Equals(object? obj) => (obj is ActivityLink link) && this.Equals(link);
public override bool Equals([NotNullWhen(true)] object? obj) => (obj is ActivityLink link) && this.Equals(link);

public bool Equals(ActivityLink value) => Context == value.Context && value.Tags == Tags;
public static bool operator ==(ActivityLink left, ActivityLink right) => left.Equals(right);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ public readonly partial struct SymbolToken
private readonly int _dummyPrimitive;
public SymbolToken(int val) { throw null; }
public bool Equals(System.Diagnostics.SymbolStore.SymbolToken obj) { throw null; }
public override bool Equals(object? obj) { throw null; }
public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? obj) { throw null; }
public override int GetHashCode() { throw null; }
public int GetToken() { throw null; }
public static bool operator ==(System.Diagnostics.SymbolStore.SymbolToken a, System.Diagnostics.SymbolStore.SymbolToken b) { throw null; }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;

namespace System.Diagnostics.SymbolStore
{
public readonly struct SymbolToken
Expand All @@ -16,7 +18,7 @@ public SymbolToken(int val)

public override int GetHashCode() => _token;

public override bool Equals(object? obj)
public override bool Equals([NotNullWhen(true)] object? obj)
{
if (obj is SymbolToken)
return Equals((SymbolToken)obj);
Expand Down
18 changes: 9 additions & 9 deletions src/libraries/System.Drawing.Common/ref/System.Drawing.Common.cs
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ public partial struct CharacterRange
public CharacterRange(int First, int Length) { throw null; }
public int First { get { throw null; } set { } }
public int Length { get { throw null; } set { } }
public override bool Equals(object? obj) { throw null; }
public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? obj) { throw null; }
public override int GetHashCode() { throw null; }
public static bool operator ==(System.Drawing.CharacterRange cr1, System.Drawing.CharacterRange cr2) { throw null; }
public static bool operator !=(System.Drawing.CharacterRange cr1, System.Drawing.CharacterRange cr2) { throw null; }
Expand Down Expand Up @@ -322,7 +322,7 @@ public Font(string familyName, float emSize, System.Drawing.GraphicsUnit unit) {
public System.Drawing.GraphicsUnit Unit { get { throw null; } }
public object Clone() { throw null; }
public void Dispose() { }
public override bool Equals(object? obj) { throw null; }
public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? obj) { throw null; }
~Font() { }
public static System.Drawing.Font FromHdc(System.IntPtr hdc) { throw null; }
public static System.Drawing.Font FromHfont(System.IntPtr hfont) { throw null; }
Expand Down Expand Up @@ -376,7 +376,7 @@ public FontFamily(string name, System.Drawing.Text.FontCollection? fontCollectio
public static System.Drawing.FontFamily GenericSerif { get { throw null; } }
public string Name { get { throw null; } }
public void Dispose() { }
public override bool Equals(object? obj) { throw null; }
public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? obj) { throw null; }
~FontFamily() { }
public int GetCellAscent(System.Drawing.FontStyle style) { throw null; }
public int GetCellDescent(System.Drawing.FontStyle style) { throw null; }
Expand Down Expand Up @@ -774,7 +774,7 @@ public sealed partial class ImageAnimator
{
internal ImageAnimator() { }
public static void Animate(System.Drawing.Image image, System.EventHandler onFrameChangedHandler) { }
public static bool CanAnimate(System.Drawing.Image? image) { throw null; }
public static bool CanAnimate([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] System.Drawing.Image? image) { throw null; }
public static void StopAnimate(System.Drawing.Image image, System.EventHandler onFrameChangedHandler) { }
public static void UpdateFrames() { }
public static void UpdateFrames(System.Drawing.Image image) { }
Expand Down Expand Up @@ -1266,7 +1266,7 @@ public partial class ToolboxBitmapAttribute : System.Attribute
public ToolboxBitmapAttribute(string imageFile) { }
public ToolboxBitmapAttribute(System.Type t) { }
public ToolboxBitmapAttribute(System.Type t, string name) { }
public override bool Equals(object? value) { throw null; }
public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? value) { throw null; }
public override int GetHashCode() { throw null; }
public System.Drawing.Image? GetImage(object? component) { throw null; }
public System.Drawing.Image? GetImage(object? component, bool large) { throw null; }
Expand Down Expand Up @@ -1660,7 +1660,7 @@ public Matrix(float m11, float m12, float m21, float m22, float dx, float dy) {
public float OffsetY { get { throw null; } }
public System.Drawing.Drawing2D.Matrix Clone() { throw null; }
public void Dispose() { }
public override bool Equals(object? obj) { throw null; }
public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? obj) { throw null; }
~Matrix() { }
public override int GetHashCode() { throw null; }
public void Invert() { }
Expand Down Expand Up @@ -2246,7 +2246,7 @@ public FrameDimension(System.Guid guid) { }
public static System.Drawing.Imaging.FrameDimension Page { get { throw null; } }
public static System.Drawing.Imaging.FrameDimension Resolution { get { throw null; } }
public static System.Drawing.Imaging.FrameDimension Time { get { throw null; } }
public override bool Equals(object? o) { throw null; }
public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? o) { throw null; }
public override int GetHashCode() { throw null; }
public override string ToString() { throw null; }
}
Expand Down Expand Up @@ -2364,7 +2364,7 @@ public ImageFormat(System.Guid guid) { }
public static System.Drawing.Imaging.ImageFormat Png { get { throw null; } }
public static System.Drawing.Imaging.ImageFormat Tiff { get { throw null; } }
public static System.Drawing.Imaging.ImageFormat Wmf { get { throw null; } }
public override bool Equals(object? o) { throw null; }
public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? o) { throw null; }
public override int GetHashCode() { throw null; }
public override string ToString() { throw null; }
}
Expand Down Expand Up @@ -2557,7 +2557,7 @@ public Margins(int left, int right, int top, int bottom) { }
public int Right { get { throw null; } set { } }
public int Top { get { throw null; } set { } }
public object Clone() { throw null; }
public override bool Equals(object? obj) { throw null; }
public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? obj) { throw null; }
public override int GetHashCode() { throw null; }
public static bool operator ==(System.Drawing.Printing.Margins? m1, System.Drawing.Printing.Margins? m2) { throw null; }
public static bool operator !=(System.Drawing.Printing.Margins? m1, System.Drawing.Printing.Margins? m2) { throw null; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public int Length
set => _length = value;
}

public override bool Equals(object? obj)
public override bool Equals([NotNullWhen(true)] object? obj)
{
if (!(obj is CharacterRange cr))
{
Expand Down
Loading

0 comments on commit 2b0e278

Please sign in to comment.