Skip to content

Commit edfed86

Browse files
maxkoshevoimaxkoshevoi
authored
[Group 1] Enable nullable annotations for Microsoft.Extensions.Primitives (#57395)
* Annotate src * StringValues array is non-nullable * Anotate ref * Fix analizer error * Update Equals implementation * Result of IChangeToken.RegisterChangeCallback is nullable * Revert "Result of IChangeToken.RegisterChangeCallback is nullable" This reverts commit e005156. * Dont use using in ref * Commit to rerun CI * Revert "Commit to rerun CI" This reverts commit d7230db. * StringValues can contain string?[] * Equals [NotNullWhen(true)] * text = null - retrun false * text = null - retrun false (tests) * Remove unneeded ?. * IChangeToken can be null * Update doc * NullTokenDisposeShouldNotThrow * _registeredCallbackProxy -> RegisteredCallbackProxy * StringValues can be created from string?[]? * Debug.Fail -> Assert.True(false)
1 parent c674260 commit edfed86

12 files changed

+208
-198
lines changed

‎src/libraries/Microsoft.Extensions.Primitives/ref/Microsoft.Extensions.Primitives.cs‎

Lines changed: 56 additions & 55 deletions
Large diffs are not rendered by default.

‎src/libraries/Microsoft.Extensions.Primitives/ref/Microsoft.Extensions.Primitives.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
33
<TargetFrameworks>$(NetCoreAppCurrent);netcoreapp3.1;netstandard2.0;net461</TargetFrameworks>
4+
<Nullable>enable</Nullable>
45
</PropertyGroup>
56
<ItemGroup>
67
<Compile Include="Microsoft.Extensions.Primitives.cs" />

‎src/libraries/Microsoft.Extensions.Primitives/src/CancellationChangeToken.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public CancellationChangeToken(CancellationToken cancellationToken)
2929
private CancellationToken Token { get; }
3030

3131
/// <inheritdoc />
32-
public IDisposable RegisterChangeCallback(Action<object> callback, object state)
32+
public IDisposable RegisterChangeCallback(Action<object?> callback, object? state)
3333
{
3434
#if NETCOREAPP || NETSTANDARD2_1
3535
try

‎src/libraries/Microsoft.Extensions.Primitives/src/ChangeToken.cs‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public static class ChangeToken
1818
/// <param name="changeTokenProducer">Produces the change token.</param>
1919
/// <param name="changeTokenConsumer">Action called when the token changes.</param>
2020
/// <returns></returns>
21-
public static IDisposable OnChange(Func<IChangeToken> changeTokenProducer, Action changeTokenConsumer)
21+
public static IDisposable OnChange(Func<IChangeToken?> changeTokenProducer, Action changeTokenConsumer)
2222
{
2323
if (changeTokenProducer == null)
2424
{
@@ -39,7 +39,7 @@ public static IDisposable OnChange(Func<IChangeToken> changeTokenProducer, Actio
3939
/// <param name="changeTokenConsumer">Action called when the token changes.</param>
4040
/// <param name="state">state for the consumer.</param>
4141
/// <returns></returns>
42-
public static IDisposable OnChange<TState>(Func<IChangeToken> changeTokenProducer, Action<TState> changeTokenConsumer, TState state)
42+
public static IDisposable OnChange<TState>(Func<IChangeToken?> changeTokenProducer, Action<TState> changeTokenConsumer, TState state)
4343
{
4444
if (changeTokenProducer == null)
4545
{
@@ -55,20 +55,20 @@ public static IDisposable OnChange<TState>(Func<IChangeToken> changeTokenProduce
5555

5656
private sealed class ChangeTokenRegistration<TState> : IDisposable
5757
{
58-
private readonly Func<IChangeToken> _changeTokenProducer;
58+
private readonly Func<IChangeToken?> _changeTokenProducer;
5959
private readonly Action<TState> _changeTokenConsumer;
6060
private readonly TState _state;
61-
private IDisposable _disposable;
61+
private IDisposable? _disposable;
6262

6363
private static readonly NoopDisposable _disposedSentinel = new NoopDisposable();
6464

65-
public ChangeTokenRegistration(Func<IChangeToken> changeTokenProducer, Action<TState> changeTokenConsumer, TState state)
65+
public ChangeTokenRegistration(Func<IChangeToken?> changeTokenProducer, Action<TState> changeTokenConsumer, TState state)
6666
{
6767
_changeTokenProducer = changeTokenProducer;
6868
_changeTokenConsumer = changeTokenConsumer;
6969
_state = state;
7070

71-
IChangeToken token = changeTokenProducer();
71+
IChangeToken? token = changeTokenProducer();
7272

7373
RegisterChangeTokenCallback(token);
7474
}
@@ -80,7 +80,7 @@ private void OnChangeTokenFired()
8080
//
8181
// If the token changes after we take the token, then we'll process the update immediately upon
8282
// registering the callback.
83-
IChangeToken token = _changeTokenProducer();
83+
IChangeToken? token = _changeTokenProducer();
8484

8585
try
8686
{
@@ -93,14 +93,14 @@ private void OnChangeTokenFired()
9393
}
9494
}
9595

96-
private void RegisterChangeTokenCallback(IChangeToken token)
96+
private void RegisterChangeTokenCallback(IChangeToken? token)
9797
{
9898
if (token is null)
9999
{
100100
return;
101101
}
102102

103-
IDisposable registraton = token.RegisterChangeCallback(s => ((ChangeTokenRegistration<TState>)s).OnChangeTokenFired(), this);
103+
IDisposable registraton = token.RegisterChangeCallback(s => ((ChangeTokenRegistration<TState>?)s)!.OnChangeTokenFired(), this);
104104

105105
SetDisposable(registraton);
106106
}
@@ -110,7 +110,7 @@ private void SetDisposable(IDisposable disposable)
110110
// We don't want to transition from _disposedSentinel => anything since it's terminal
111111
// but we want to allow going from previously assigned disposable, to another
112112
// disposable.
113-
IDisposable current = Volatile.Read(ref _disposable);
113+
IDisposable? current = Volatile.Read(ref _disposable);
114114

115115
// If Dispose was called, then immediately dispose the disposable
116116
if (current == _disposedSentinel)
@@ -120,7 +120,7 @@ private void SetDisposable(IDisposable disposable)
120120
}
121121

122122
// Otherwise, try to update the disposable
123-
IDisposable previous = Interlocked.CompareExchange(ref _disposable, disposable, current);
123+
IDisposable? previous = Interlocked.CompareExchange(ref _disposable, disposable, current);
124124

125125
if (previous == _disposedSentinel)
126126
{
@@ -129,7 +129,7 @@ private void SetDisposable(IDisposable disposable)
129129
}
130130
else if (previous == current)
131131
{
132-
// We successfuly assigned the _disposable field to disposable
132+
// We successfully assigned the _disposable field to disposable
133133
}
134134
else
135135
{
@@ -142,7 +142,7 @@ public void Dispose()
142142
{
143143
// If the previous value is disposable then dispose it, otherwise,
144144
// now we've set the disposed sentinel
145-
Interlocked.Exchange(ref _disposable, _disposedSentinel).Dispose();
145+
Interlocked.Exchange(ref _disposable, _disposedSentinel)?.Dispose();
146146
}
147147

148148
private sealed class NoopDisposable : IDisposable

‎src/libraries/Microsoft.Extensions.Primitives/src/CompositeChangeToken.cs‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Collections.Generic;
66
using System.Diagnostics;
7+
using System.Diagnostics.CodeAnalysis;
78
using System.Threading;
89

910
namespace Microsoft.Extensions.Primitives
@@ -13,11 +14,14 @@ namespace Microsoft.Extensions.Primitives
1314
/// </summary>
1415
public class CompositeChangeToken : IChangeToken
1516
{
16-
private static readonly Action<object> _onChangeDelegate = OnChange;
17-
private readonly object _callbackLock = new object();
18-
private CancellationTokenSource _cancellationTokenSource;
19-
private bool _registeredCallbackProxy;
20-
private List<IDisposable> _disposables;
17+
private static readonly Action<object?> _onChangeDelegate = OnChange;
18+
private readonly object _callbackLock = new();
19+
private CancellationTokenSource? _cancellationTokenSource;
20+
private List<IDisposable>? _disposables;
21+
22+
[MemberNotNullWhen(true, nameof(_cancellationTokenSource))]
23+
[MemberNotNullWhen(true, nameof(_disposables))]
24+
private bool RegisteredCallbackProxy { get; set; }
2125

2226
/// <summary>
2327
/// Creates a new instance of <see cref="CompositeChangeToken"/>.
@@ -42,7 +46,7 @@ public CompositeChangeToken(IReadOnlyList<IChangeToken> changeTokens)
4246
public IReadOnlyList<IChangeToken> ChangeTokens { get; }
4347

4448
/// <inheritdoc />
45-
public IDisposable RegisterChangeCallback(Action<object> callback, object state)
49+
public IDisposable RegisterChangeCallback(Action<object?> callback, object? state)
4650
{
4751
EnsureCallbacksInitialized();
4852
return _cancellationTokenSource.Token.Register(callback, state);
@@ -74,16 +78,18 @@ public bool HasChanged
7478
/// <inheritdoc />
7579
public bool ActiveChangeCallbacks { get; }
7680

81+
[MemberNotNull(nameof(_cancellationTokenSource))]
82+
[MemberNotNull(nameof(_disposables))]
7783
private void EnsureCallbacksInitialized()
7884
{
79-
if (_registeredCallbackProxy)
85+
if (RegisteredCallbackProxy)
8086
{
8187
return;
8288
}
8389

8490
lock (_callbackLock)
8591
{
86-
if (_registeredCallbackProxy)
92+
if (RegisteredCallbackProxy)
8793
{
8894
return;
8995
}
@@ -98,12 +104,14 @@ private void EnsureCallbacksInitialized()
98104
_disposables.Add(disposable);
99105
}
100106
}
101-
_registeredCallbackProxy = true;
107+
RegisteredCallbackProxy = true;
102108
}
103109
}
104110

105-
private static void OnChange(object state)
111+
private static void OnChange(object? state)
106112
{
113+
Debug.Assert(state != null);
114+
107115
var compositeChangeTokenState = (CompositeChangeToken)state;
108116
if (compositeChangeTokenState._cancellationTokenSource == null)
109117
{
@@ -121,13 +129,12 @@ private static void OnChange(object state)
121129
}
122130
}
123131

124-
List<IDisposable> disposables = compositeChangeTokenState._disposables;
132+
List<IDisposable>? disposables = compositeChangeTokenState._disposables;
125133
Debug.Assert(disposables != null);
126134
for (int i = 0; i < disposables.Count; i++)
127135
{
128136
disposables[i].Dispose();
129137
}
130-
131138
}
132139
}
133140
}

‎src/libraries/Microsoft.Extensions.Primitives/src/IChangeToken.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,6 @@ public interface IChangeToken
2828
/// <param name="callback">The <see cref="Action{Object}"/> to invoke.</param>
2929
/// <param name="state">State to be passed into the callback.</param>
3030
/// <returns>An <see cref="IDisposable"/> that is used to unregister the callback.</returns>
31-
IDisposable RegisterChangeCallback(Action<object> callback, object state);
31+
IDisposable RegisterChangeCallback(Action<object?> callback, object? state);
3232
}
3333
}

‎src/libraries/Microsoft.Extensions.Primitives/src/Microsoft.Extensions.Primitives.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
33
<TargetFrameworks>$(NetCoreAppCurrent);netcoreapp3.1;netstandard2.0;net461</TargetFrameworks>
4+
<Nullable>enable</Nullable>
45
<EnableDefaultItems>true</EnableDefaultItems>
56
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
67
<!-- Use targeting pack references instead of granular ones in the project file. -->

‎src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System;
5+
using System.Diagnostics.CodeAnalysis;
56
using System.Runtime.CompilerServices;
67

78
namespace Microsoft.Extensions.Primitives
@@ -76,11 +77,12 @@ public StringSegment(string buffer, int offset, int length)
7677
/// <summary>
7778
/// Gets the value of this segment as a <see cref="string"/>.
7879
/// </summary>
79-
public string Value => HasValue ? Buffer.Substring(Offset, Length) : null;
80+
public string? Value => HasValue ? Buffer.Substring(Offset, Length) : null;
8081

8182
/// <summary>
8283
/// Gets whether this <see cref="StringSegment"/> contains a valid value.
8384
/// </summary>
85+
[MemberNotNullWhen(true, nameof(Buffer))]
8486
public bool HasValue => Buffer != null;
8587

8688
/// <summary>
@@ -188,7 +190,7 @@ public static int Compare(StringSegment a, StringSegment b, StringComparison com
188190
/// </summary>
189191
/// <param name="obj">An object to compare with this object.</param>
190192
/// <returns><see langword="true" /> if the current object is equal to the other parameter; otherwise, <see langword="false" />.</returns>
191-
public override bool Equals(object obj)
193+
public override bool Equals([NotNullWhen(true)] object? obj)
192194
{
193195
return obj is StringSegment segment && Equals(segment);
194196
}
@@ -239,7 +241,7 @@ public static bool Equals(StringSegment a, StringSegment b, StringComparison com
239241
/// </summary>
240242
/// <param name="text">The <see cref="string"/> to compare with the current <see cref="StringSegment"/>.</param>
241243
/// <returns><see langword="true" /> if the specified <see cref="string"/> is equal to the current <see cref="StringSegment"/>; otherwise, <see langword="false" />.</returns>
242-
public bool Equals(string text)
244+
public bool Equals([NotNullWhen(true)] string? text)
243245
{
244246
return Equals(text, StringComparison.Ordinal);
245247
}
@@ -250,15 +252,12 @@ public bool Equals(string text)
250252
/// <param name="text">The <see cref="string"/> to compare with the current <see cref="StringSegment"/>.</param>
251253
/// <param name="comparisonType">One of the enumeration values that specifies the rules to use in the comparison.</param>
252254
/// <returns><see langword="true" /> if the specified <see cref="string"/> is equal to the current <see cref="StringSegment"/>; otherwise, <see langword="false" />.</returns>
253-
/// <exception cref="ArgumentNullException">
254-
/// <paramref name="text"/> is <see langword="null" />.
255-
/// </exception>
256255
[MethodImpl(MethodImplOptions.AggressiveInlining)]
257-
public bool Equals(string text, StringComparison comparisonType)
256+
public bool Equals([NotNullWhen(true)] string? text, StringComparison comparisonType)
258257
{
259258
if (text == null)
260259
{
261-
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.text);
260+
return false;
262261
}
263262

264263
if (!HasValue)
@@ -681,7 +680,8 @@ private static void CheckStringComparison(StringComparison comparisonType)
681680

682681
// Methods that do no return (i.e. throw) are not inlined
683682
// https://github.com/dotnet/coreclr/pull/6103
684-
private static void ThrowInvalidArguments(string buffer, int offset, int length)
683+
[DoesNotReturn]
684+
private static void ThrowInvalidArguments(string? buffer, int offset, int length)
685685
{
686686
// Only have single throw in method so is marked as "does not return" and isn't inlined to caller
687687
throw GetInvalidArgumentsException();
@@ -707,6 +707,7 @@ Exception GetInvalidArgumentsException()
707707
}
708708
}
709709

710+
[DoesNotReturn]
710711
private void ThrowInvalidArguments(int offset, int length, ExceptionArgument offsetOrStart)
711712
{
712713
throw GetInvalidArgumentsException(HasValue);
@@ -733,7 +734,7 @@ Exception GetInvalidArgumentsException(bool hasValue)
733734
}
734735

735736
/// <inheritdoc />
736-
bool IEquatable<string>.Equals(string other)
737+
bool IEquatable<string>.Equals(string? other)
737738
{
738739
// Explicit interface implementation for IEquatable<string> because
739740
// the interface's Equals method allows null strings, which we return

0 commit comments

Comments
 (0)