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

Improved nullability annotations in MVVM Toolkit #4135

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
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.IO;
using System.Linq;
using System.Reflection;
using System.Text;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Text;

namespace Microsoft.Toolkit.Mvvm.SourceGenerators
{
/// <summary>
/// A source generator for necessary nullability attributes.
/// </summary>
[Generator]
public sealed class NullabilityAttributesGenerator : ISourceGenerator
{
/// <inheritdoc/>
public void Initialize(GeneratorInitializationContext context)
{
}

/// <inheritdoc/>
public void Execute(GeneratorExecutionContext context)
{
AddSourceCodeIfTypeIsNotPresent(context, "System.Diagnostics.CodeAnalysis.NotNullAttribute");
AddSourceCodeIfTypeIsNotPresent(context, "System.Diagnostics.CodeAnalysis.NotNullIfNotNullAttribute");
}

/// <summary>
/// Adds the source for a given attribute type if it's not present already in the compilation.
/// </summary>
private void AddSourceCodeIfTypeIsNotPresent(GeneratorExecutionContext context, string typeFullName)
{
if (context.Compilation.GetTypeByMetadataName(typeFullName) is not null)
{
return;
}

string
typeName = typeFullName.Split('.').Last(),
filename = $"Microsoft.Toolkit.Mvvm.SourceGenerators.EmbeddedResources.{typeName}.cs";

Stream stream = Assembly.GetExecutingAssembly().GetManifestResourceStream(filename);
StreamReader reader = new(stream);

string
originalSource = reader.ReadToEnd(),
outputSource = originalSource.Replace("NETSTANDARD2_0", "true");

context.AddSource($"{typeFullName}.cs", SourceText.From(outputSource, Encoding.UTF8));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
using System.Threading.Tasks;

Expand Down Expand Up @@ -50,7 +51,7 @@ protected void OnPropertyChanged([CallerMemberName] string? propertyName = null)
/// <remarks>
/// The <see cref="PropertyChanged"/> event is not raised if the current and new value for the target property are the same.
/// </remarks>
protected bool SetProperty<T>(ref T field, T newValue, [CallerMemberName] string? propertyName = null)
protected bool SetProperty<T>([NotNullIfNotNull("newValue")] ref T field, T newValue, [CallerMemberName] string? propertyName = null)
{
if (EqualityComparer<T>.Default.Equals(field, newValue))
{
Expand All @@ -75,7 +76,7 @@ protected bool SetProperty<T>(ref T field, T newValue, [CallerMemberName] string
/// <param name="comparer">The <see cref="IEqualityComparer{T}"/> instance to use to compare the input values.</param>
/// <param name="propertyName">(optional) The name of the property that changed.</param>
/// <returns><see langword="true"/> if the property was changed, <see langword="false"/> otherwise.</returns>
protected bool SetProperty<T>(ref T field, T newValue, IEqualityComparer<T> comparer, [CallerMemberName] string? propertyName = null)
protected bool SetProperty<T>([NotNullIfNotNull("newValue")] ref T field, T newValue, IEqualityComparer<T> comparer, [CallerMemberName] string? propertyName = null)
{
if (comparer.Equals(field, newValue))
{
Expand Down Expand Up @@ -280,7 +281,7 @@ protected bool SetProperty<TModel, T>(T oldValue, T newValue, IEqualityComparer<
/// <paramref name="taskNotifier"/> is different than the previous one, and it does not mean the new
/// <see cref="Task"/> instance passed as argument is in any particular state.
/// </remarks>
protected bool SetPropertyAndNotifyOnCompletion(ref TaskNotifier? taskNotifier, Task? newValue, [CallerMemberName] string? propertyName = null)
protected bool SetPropertyAndNotifyOnCompletion([NotNull] ref TaskNotifier? taskNotifier, Task? newValue, [CallerMemberName] string? propertyName = null)
{
return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new(), newValue, static _ => { }, propertyName);
}
Expand All @@ -300,7 +301,7 @@ protected bool SetPropertyAndNotifyOnCompletion(ref TaskNotifier? taskNotifier,
/// <remarks>
/// The <see cref="PropertyChanged"/> event is not raised if the current and new value for the target property are the same.
/// </remarks>
protected bool SetPropertyAndNotifyOnCompletion(ref TaskNotifier? taskNotifier, Task? newValue, Action<Task?> callback, [CallerMemberName] string? propertyName = null)
protected bool SetPropertyAndNotifyOnCompletion([NotNull] ref TaskNotifier? taskNotifier, Task? newValue, Action<Task?> callback, [CallerMemberName] string? propertyName = null)
{
return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new(), newValue, callback, propertyName);
}
Expand Down Expand Up @@ -338,7 +339,7 @@ protected bool SetPropertyAndNotifyOnCompletion(ref TaskNotifier? taskNotifier,
/// <paramref name="taskNotifier"/> is different than the previous one, and it does not mean the new
/// <see cref="Task{TResult}"/> instance passed as argument is in any particular state.
/// </remarks>
protected bool SetPropertyAndNotifyOnCompletion<T>(ref TaskNotifier<T>? taskNotifier, Task<T>? newValue, [CallerMemberName] string? propertyName = null)
protected bool SetPropertyAndNotifyOnCompletion<T>([NotNull] ref TaskNotifier<T>? taskNotifier, Task<T>? newValue, [CallerMemberName] string? propertyName = null)
{
return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new(), newValue, static _ => { }, propertyName);
}
Expand All @@ -359,7 +360,7 @@ protected bool SetPropertyAndNotifyOnCompletion<T>(ref TaskNotifier<T>? taskNoti
/// <remarks>
/// The <see cref="PropertyChanged"/> event is not raised if the current and new value for the target property are the same.
/// </remarks>
protected bool SetPropertyAndNotifyOnCompletion<T>(ref TaskNotifier<T>? taskNotifier, Task<T>? newValue, Action<Task<T>?> callback, [CallerMemberName] string? propertyName = null)
protected bool SetPropertyAndNotifyOnCompletion<T>([NotNull] ref TaskNotifier<T>? taskNotifier, Task<T>? newValue, Action<Task<T>?> callback, [CallerMemberName] string? propertyName = null)
{
return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new(), newValue, callback, propertyName);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

<ItemGroup>
<Compile Remove="EmbeddedResources\INotifyPropertyChanged.cs" />
<Compile Remove="EmbeddedResources\NotNullAttribute.cs" />
<Compile Remove="EmbeddedResources\NotNullIfNotNullAttribute.cs" />
<Compile Remove="EmbeddedResources\ObservableObject.cs" />
<Compile Remove="EmbeddedResources\ObservableRecipient.cs" />
</ItemGroup>
Expand All @@ -17,6 +19,12 @@
<EmbeddedResource Include="EmbeddedResources\INotifyPropertyChanged.cs">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</EmbeddedResource>
<EmbeddedResource Include="..\Microsoft.Toolkit.Mvvm\Attributes\NotNullAttribute.cs" Link="EmbeddedResources\NotNullAttribute.cs">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</EmbeddedResource>
<EmbeddedResource Include="..\Microsoft.Toolkit.Mvvm\Attributes\NotNullIfNotNullAttribute.cs" Link="EmbeddedResources\NotNullIfNotNullAttribute.cs">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</EmbeddedResource>
<EmbeddedResource Include="..\Microsoft.Toolkit.Mvvm\ComponentModel\ObservableObject.cs" Link="EmbeddedResources\ObservableObject.cs">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</EmbeddedResource>
Expand Down
20 changes: 20 additions & 0 deletions Microsoft.Toolkit.Mvvm/Attributes/NotNullAttribute.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#if NETSTANDARD2_0

namespace System.Diagnostics.CodeAnalysis
{
/// <summary>
/// Specifies that an output will not be null even if the corresponding type allows it.
/// Specifies that an input argument was not null when the call returns.
/// </summary>
/// <remarks>Internal copy from the BCL attribute.</remarks>
[AttributeUsage(AttributeTargets.Field | AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue, Inherited = false)]
internal sealed class NotNullAttribute : Attribute
{
}
}

#endif
27 changes: 27 additions & 0 deletions Microsoft.Toolkit.Mvvm/Attributes/NotNullIfNotNullAttribute.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#if NETSTANDARD2_0

namespace System.Diagnostics.CodeAnalysis
{
/// <summary>
/// Specifies that the output will be non-null if the named parameter is non-null.
/// </summary>
/// <remarks>Internal copy from the BCL attribute.</remarks>
[AttributeUsage(AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.ReturnValue, AllowMultiple = true, Inherited = false)]
internal sealed class NotNullIfNotNullAttribute : Attribute
{
/// <summary>
/// Initializes a new instance of the <see cref="NotNullIfNotNullAttribute"/> class.
/// </summary>
/// <param name="parameterName"> The associated parameter name. The output will be non-null if the argument to the parameter specified is non-null.</param>
public NotNullIfNotNullAttribute(string parameterName) => ParameterName = parameterName;

/// <summary>Gets the associated parameter name.</summary>
public string ParameterName { get; }
}
}

#endif
13 changes: 7 additions & 6 deletions Microsoft.Toolkit.Mvvm/ComponentModel/ObservableObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
using System.Threading.Tasks;

Expand Down Expand Up @@ -84,7 +85,7 @@ protected void OnPropertyChanging([CallerMemberName] string? propertyName = null
/// The <see cref="PropertyChanging"/> and <see cref="PropertyChanged"/> events are not raised
/// if the current and new value for the target property are the same.
/// </remarks>
protected bool SetProperty<T>(ref T field, T newValue, [CallerMemberName] string? propertyName = null)
protected bool SetProperty<T>([NotNullIfNotNull("newValue")] ref T field, T newValue, [CallerMemberName] string? propertyName = null)
{
// We duplicate the code here instead of calling the overload because we can't
// guarantee that the invoked SetProperty<T> will be inlined, and we need the JIT
Expand Down Expand Up @@ -120,7 +121,7 @@ protected bool SetProperty<T>(ref T field, T newValue, [CallerMemberName] string
/// <param name="comparer">The <see cref="IEqualityComparer{T}"/> instance to use to compare the input values.</param>
/// <param name="propertyName">(optional) The name of the property that changed.</param>
/// <returns><see langword="true"/> if the property was changed, <see langword="false"/> otherwise.</returns>
protected bool SetProperty<T>(ref T field, T newValue, IEqualityComparer<T> comparer, [CallerMemberName] string? propertyName = null)
protected bool SetProperty<T>([NotNullIfNotNull("newValue")] ref T field, T newValue, IEqualityComparer<T> comparer, [CallerMemberName] string? propertyName = null)
{
if (comparer.Equals(field, newValue))
{
Expand Down Expand Up @@ -340,7 +341,7 @@ protected bool SetProperty<TModel, T>(T oldValue, T newValue, IEqualityComparer<
/// indicates that the new value being assigned to <paramref name="taskNotifier"/> is different than the previous one,
/// and it does not mean the new <see cref="Task"/> instance passed as argument is in any particular state.
/// </remarks>
protected bool SetPropertyAndNotifyOnCompletion(ref TaskNotifier? taskNotifier, Task? newValue, [CallerMemberName] string? propertyName = null)
protected bool SetPropertyAndNotifyOnCompletion([NotNull] ref TaskNotifier? taskNotifier, Task? newValue, [CallerMemberName] string? propertyName = null)
{
// We invoke the overload with a callback here to avoid code duplication, and simply pass an empty callback.
// The lambda expression here is transformed by the C# compiler into an empty closure class with a
Expand Down Expand Up @@ -368,7 +369,7 @@ protected bool SetPropertyAndNotifyOnCompletion(ref TaskNotifier? taskNotifier,
/// The <see cref="PropertyChanging"/> and <see cref="PropertyChanged"/> events are not raised
/// if the current and new value for the target property are the same.
/// </remarks>
protected bool SetPropertyAndNotifyOnCompletion(ref TaskNotifier? taskNotifier, Task? newValue, Action<Task?> callback, [CallerMemberName] string? propertyName = null)
protected bool SetPropertyAndNotifyOnCompletion([NotNull] ref TaskNotifier? taskNotifier, Task? newValue, Action<Task?> callback, [CallerMemberName] string? propertyName = null)
{
return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new(), newValue, callback, propertyName);
}
Expand Down Expand Up @@ -407,7 +408,7 @@ protected bool SetPropertyAndNotifyOnCompletion(ref TaskNotifier? taskNotifier,
/// indicates that the new value being assigned to <paramref name="taskNotifier"/> is different than the previous one,
/// and it does not mean the new <see cref="Task{TResult}"/> instance passed as argument is in any particular state.
/// </remarks>
protected bool SetPropertyAndNotifyOnCompletion<T>(ref TaskNotifier<T>? taskNotifier, Task<T>? newValue, [CallerMemberName] string? propertyName = null)
protected bool SetPropertyAndNotifyOnCompletion<T>([NotNull] ref TaskNotifier<T>? taskNotifier, Task<T>? newValue, [CallerMemberName] string? propertyName = null)
{
return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new(), newValue, static _ => { }, propertyName);
}
Expand All @@ -430,7 +431,7 @@ protected bool SetPropertyAndNotifyOnCompletion<T>(ref TaskNotifier<T>? taskNoti
/// The <see cref="PropertyChanging"/> and <see cref="PropertyChanged"/> events are not raised
/// if the current and new value for the target property are the same.
/// </remarks>
protected bool SetPropertyAndNotifyOnCompletion<T>(ref TaskNotifier<T>? taskNotifier, Task<T>? newValue, Action<Task<T>?> callback, [CallerMemberName] string? propertyName = null)
protected bool SetPropertyAndNotifyOnCompletion<T>([NotNull] ref TaskNotifier<T>? taskNotifier, Task<T>? newValue, Action<Task<T>?> callback, [CallerMemberName] string? propertyName = null)
{
return SetPropertyAndNotifyOnCompletion(taskNotifier ??= new(), newValue, callback, propertyName);
}
Expand Down
12 changes: 3 additions & 9 deletions Microsoft.Toolkit.Mvvm/ComponentModel/ObservableRecipient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,9 @@
// This file is inspired from the MvvmLight library (lbugnion/MvvmLight),
// more info in ThirdPartyNotices.txt in the root of the project.

// ================================= NOTE =================================
michael-hawker marked this conversation as resolved.
Show resolved Hide resolved
// This file is mirrored in the ObservableRecipient annotated copy
// (for debugging info) in the Mvvm.SourceGenerators project.
// If any changes are made to this file, they should also be appropriately
// ported to that file as well to keep the behavior consistent.
// ========================================================================

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
using Microsoft.Toolkit.Mvvm.Messaging;
using Microsoft.Toolkit.Mvvm.Messaging.Messages;
Expand Down Expand Up @@ -145,7 +139,7 @@ protected virtual void Broadcast<T>(T oldValue, T newValue, string? propertyName
/// the <see cref="ObservableObject.PropertyChanging"/> and <see cref="ObservableObject.PropertyChanged"/> events
/// are not raised if the current and new value for the target property are the same.
/// </remarks>
protected bool SetProperty<T>(ref T field, T newValue, bool broadcast, [CallerMemberName] string? propertyName = null)
protected bool SetProperty<T>([NotNullIfNotNull("newValue")] ref T field, T newValue, bool broadcast, [CallerMemberName] string? propertyName = null)
{
T oldValue = field;

Expand Down Expand Up @@ -174,7 +168,7 @@ protected bool SetProperty<T>(ref T field, T newValue, bool broadcast, [CallerMe
/// <param name="broadcast">If <see langword="true"/>, <see cref="Broadcast{T}"/> will also be invoked.</param>
/// <param name="propertyName">(optional) The name of the property that changed.</param>
/// <returns><see langword="true"/> if the property was changed, <see langword="false"/> otherwise.</returns>
protected bool SetProperty<T>(ref T field, T newValue, IEqualityComparer<T> comparer, bool broadcast, [CallerMemberName] string? propertyName = null)
protected bool SetProperty<T>([NotNullIfNotNull("newValue")] ref T field, T newValue, IEqualityComparer<T> comparer, bool broadcast, [CallerMemberName] string? propertyName = null)
{
T oldValue = field;

Expand Down
5 changes: 3 additions & 2 deletions Microsoft.Toolkit.Mvvm/ComponentModel/ObservableValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Collections.Generic;
using System.ComponentModel;
using System.ComponentModel.DataAnnotations;
using System.Diagnostics.CodeAnalysis;
using System.Diagnostics.Contracts;
using System.Linq;
using System.Linq.Expressions;
Expand Down Expand Up @@ -137,7 +138,7 @@ protected ObservableValidator(ValidationContext validationContext)
/// the <see cref="ObservableObject.PropertyChanging"/> and <see cref="ObservableObject.PropertyChanged"/> events
/// are not raised if the current and new value for the target property are the same.
/// </remarks>
protected bool SetProperty<T>(ref T field, T newValue, bool validate, [CallerMemberName] string? propertyName = null)
protected bool SetProperty<T>([NotNullIfNotNull("newValue")] ref T field, T newValue, bool validate, [CallerMemberName] string? propertyName = null)
{
bool propertyChanged = SetProperty(ref field, newValue, propertyName);

Expand All @@ -162,7 +163,7 @@ protected bool SetProperty<T>(ref T field, T newValue, bool validate, [CallerMem
/// <param name="validate">If <see langword="true"/>, <paramref name="newValue"/> will also be validated.</param>
/// <param name="propertyName">(optional) The name of the property that changed.</param>
/// <returns><see langword="true"/> if the property was changed, <see langword="false"/> otherwise.</returns>
protected bool SetProperty<T>(ref T field, T newValue, IEqualityComparer<T> comparer, bool validate, [CallerMemberName] string? propertyName = null)
protected bool SetProperty<T>([NotNullIfNotNull("newValue")] ref T field, T newValue, IEqualityComparer<T> comparer, bool validate, [CallerMemberName] string? propertyName = null)
{
bool propertyChanged = SetProperty(ref field, newValue, comparer, propertyName);

Expand Down