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

[Group 1] Enable nullable annotations for Microsoft.Extensions.Primitives #57395

Merged
merged 21 commits into from
Aug 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);netcoreapp3.1;netstandard2.0;net461</TargetFrameworks>
<Nullable>enable</Nullable>
</PropertyGroup>
<ItemGroup>
<Compile Include="Microsoft.Extensions.Primitives.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public CancellationChangeToken(CancellationToken cancellationToken)
private CancellationToken Token { get; }

/// <inheritdoc />
public IDisposable RegisterChangeCallback(Action<object> callback, object state)
public IDisposable RegisterChangeCallback(Action<object?> callback, object? state)
{
#if NETCOREAPP || NETSTANDARD2_1
try
Expand Down
26 changes: 13 additions & 13 deletions src/libraries/Microsoft.Extensions.Primitives/src/ChangeToken.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public static class ChangeToken
/// <param name="changeTokenProducer">Produces the change token.</param>
/// <param name="changeTokenConsumer">Action called when the token changes.</param>
/// <returns></returns>
public static IDisposable OnChange(Func<IChangeToken> changeTokenProducer, Action changeTokenConsumer)
public static IDisposable OnChange(Func<IChangeToken?> changeTokenProducer, Action changeTokenConsumer)
{
if (changeTokenProducer == null)
{
Expand All @@ -39,7 +39,7 @@ public static IDisposable OnChange(Func<IChangeToken> changeTokenProducer, Actio
/// <param name="changeTokenConsumer">Action called when the token changes.</param>
/// <param name="state">state for the consumer.</param>
/// <returns></returns>
public static IDisposable OnChange<TState>(Func<IChangeToken> changeTokenProducer, Action<TState> changeTokenConsumer, TState state)
public static IDisposable OnChange<TState>(Func<IChangeToken?> changeTokenProducer, Action<TState> changeTokenConsumer, TState state)
{
if (changeTokenProducer == null)
{
Expand All @@ -55,20 +55,20 @@ public static IDisposable OnChange<TState>(Func<IChangeToken> changeTokenProduce

private sealed class ChangeTokenRegistration<TState> : IDisposable
{
private readonly Func<IChangeToken> _changeTokenProducer;
private readonly Func<IChangeToken?> _changeTokenProducer;
private readonly Action<TState> _changeTokenConsumer;
private readonly TState _state;
private IDisposable _disposable;
private IDisposable? _disposable;

private static readonly NoopDisposable _disposedSentinel = new NoopDisposable();

public ChangeTokenRegistration(Func<IChangeToken> changeTokenProducer, Action<TState> changeTokenConsumer, TState state)
public ChangeTokenRegistration(Func<IChangeToken?> changeTokenProducer, Action<TState> changeTokenConsumer, TState state)
{
_changeTokenProducer = changeTokenProducer;
_changeTokenConsumer = changeTokenConsumer;
_state = state;

IChangeToken token = changeTokenProducer();
IChangeToken? token = changeTokenProducer();

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

try
{
Expand All @@ -93,14 +93,14 @@ private void OnChangeTokenFired()
}
}

private void RegisterChangeTokenCallback(IChangeToken token)
private void RegisterChangeTokenCallback(IChangeToken? token)
{
if (token is null)
{
return;
}

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

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

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

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

if (previous == _disposedSentinel)
{
Expand All @@ -129,7 +129,7 @@ private void SetDisposable(IDisposable disposable)
}
else if (previous == current)
{
// We successfuly assigned the _disposable field to disposable
// We successfully assigned the _disposable field to disposable
}
else
{
Expand All @@ -142,7 +142,7 @@ public void Dispose()
{
// If the previous value is disposable then dispose it, otherwise,
// now we've set the disposed sentinel
Interlocked.Exchange(ref _disposable, _disposedSentinel).Dispose();
Interlocked.Exchange(ref _disposable, _disposedSentinel)?.Dispose();
maxkoshevoi marked this conversation as resolved.
Show resolved Hide resolved
}

private sealed class NoopDisposable : IDisposable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Threading;

namespace Microsoft.Extensions.Primitives
Expand All @@ -13,11 +14,14 @@ namespace Microsoft.Extensions.Primitives
/// </summary>
public class CompositeChangeToken : IChangeToken
{
private static readonly Action<object> _onChangeDelegate = OnChange;
private readonly object _callbackLock = new object();
private CancellationTokenSource _cancellationTokenSource;
private bool _registeredCallbackProxy;
private List<IDisposable> _disposables;
private static readonly Action<object?> _onChangeDelegate = OnChange;
private readonly object _callbackLock = new();
private CancellationTokenSource? _cancellationTokenSource;
private List<IDisposable>? _disposables;

[MemberNotNullWhen(true, nameof(_cancellationTokenSource))]
[MemberNotNullWhen(true, nameof(_disposables))]
krwq marked this conversation as resolved.
Show resolved Hide resolved
private bool RegisteredCallbackProxy { get; set; }

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

/// <inheritdoc />
public IDisposable RegisterChangeCallback(Action<object> callback, object state)
public IDisposable RegisterChangeCallback(Action<object?> callback, object? state)
{
EnsureCallbacksInitialized();
return _cancellationTokenSource.Token.Register(callback, state);
Expand Down Expand Up @@ -74,16 +78,18 @@ public bool HasChanged
/// <inheritdoc />
public bool ActiveChangeCallbacks { get; }

[MemberNotNull(nameof(_cancellationTokenSource))]
[MemberNotNull(nameof(_disposables))]
private void EnsureCallbacksInitialized()
{
if (_registeredCallbackProxy)
if (RegisteredCallbackProxy)
{
return;
}

lock (_callbackLock)
{
if (_registeredCallbackProxy)
if (RegisteredCallbackProxy)
{
return;
}
Expand All @@ -98,12 +104,14 @@ private void EnsureCallbacksInitialized()
_disposables.Add(disposable);
}
}
_registeredCallbackProxy = true;
RegisteredCallbackProxy = true;
}
}

private static void OnChange(object state)
private static void OnChange(object? state)
{
Debug.Assert(state != null);

var compositeChangeTokenState = (CompositeChangeToken)state;
if (compositeChangeTokenState._cancellationTokenSource == null)
{
Expand All @@ -121,13 +129,12 @@ private static void OnChange(object state)
}
}

List<IDisposable> disposables = compositeChangeTokenState._disposables;
List<IDisposable>? disposables = compositeChangeTokenState._disposables;
Debug.Assert(disposables != null);
for (int i = 0; i < disposables.Count; i++)
{
disposables[i].Dispose();
}

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,6 @@ public interface IChangeToken
/// <param name="callback">The <see cref="Action{Object}"/> to invoke.</param>
/// <param name="state">State to be passed into the callback.</param>
/// <returns>An <see cref="IDisposable"/> that is used to unregister the callback.</returns>
IDisposable RegisterChangeCallback(Action<object> callback, object state);
IDisposable RegisterChangeCallback(Action<object?> callback, object? state);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFrameworks>$(NetCoreAppCurrent);netcoreapp3.1;netstandard2.0;net461</TargetFrameworks>
<Nullable>enable</Nullable>
<EnableDefaultItems>true</EnableDefaultItems>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<!-- Use targeting pack references instead of granular ones in the project file. -->
Expand Down
21 changes: 11 additions & 10 deletions src/libraries/Microsoft.Extensions.Primitives/src/StringSegment.cs
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;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;

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

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

/// <summary>
Expand Down Expand Up @@ -188,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 Expand Up @@ -239,7 +241,7 @@ public static bool Equals(StringSegment a, StringSegment b, StringComparison com
/// </summary>
/// <param name="text">The <see cref="string"/> to compare with the current <see cref="StringSegment"/>.</param>
/// <returns><see langword="true" /> if the specified <see cref="string"/> is equal to the current <see cref="StringSegment"/>; otherwise, <see langword="false" />.</returns>
public bool Equals(string text)
public bool Equals([NotNullWhen(true)] string? text)
{
return Equals(text, StringComparison.Ordinal);
}
Expand All @@ -250,15 +252,12 @@ public bool Equals(string text)
/// <param name="text">The <see cref="string"/> to compare with the current <see cref="StringSegment"/>.</param>
/// <param name="comparisonType">One of the enumeration values that specifies the rules to use in the comparison.</param>
/// <returns><see langword="true" /> if the specified <see cref="string"/> is equal to the current <see cref="StringSegment"/>; otherwise, <see langword="false" />.</returns>
/// <exception cref="ArgumentNullException">
/// <paramref name="text"/> is <see langword="null" />.
/// </exception>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool Equals(string text, StringComparison comparisonType)
public bool Equals([NotNullWhen(true)] string? text, StringComparison comparisonType)
{
if (text == null)
{
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.text);
return false;
}

if (!HasValue)
Expand Down Expand Up @@ -681,7 +680,8 @@ private static void CheckStringComparison(StringComparison comparisonType)

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

[DoesNotReturn]
private void ThrowInvalidArguments(int offset, int length, ExceptionArgument offsetOrStart)
{
throw GetInvalidArgumentsException(HasValue);
Expand All @@ -733,7 +734,7 @@ Exception GetInvalidArgumentsException(bool hasValue)
}

/// <inheritdoc />
bool IEquatable<string>.Equals(string other)
bool IEquatable<string>.Equals(string? other)
{
// Explicit interface implementation for IEquatable<string> because
// the interface's Equals method allows null strings, which we return
Expand Down
Loading