Skip to content

Commit

Permalink
Fix nullability checks in type parameter constraints
Browse files Browse the repository at this point in the history
  • Loading branch information
Sergio0694 committed Jan 6, 2025
1 parent f7f3e86 commit fd9faea
Show file tree
Hide file tree
Showing 3 changed files with 202 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// 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 Microsoft.CodeAnalysis;

namespace CommunityToolkit.GeneratedDependencyProperty.Extensions;

/// <summary>
/// Extension methods for <see cref="ITypeParameterSymbol"/> types.
/// </summary>
internal static class ITypeParameterSymbolExtensions
{
/// <summary>
/// Checks whether a given type parameter is a reference type.
/// </summary>
/// <param name="symbol">The input <see cref="ITypeParameterSymbol"/> instance to check.</param>
/// <returns>Whether the input type parameter is a reference type.</returns>
public static bool IsReferenceTypeOrIndirectlyConstrainedToReferenceType(this ITypeParameterSymbol symbol)
{
// The type is definitely a reference type (e.g. it has the 'class' constraint)
if (symbol.IsReferenceType)
{
return true;
}

// The type is definitely a value type (e.g. it has the 'struct' constraint)
if (symbol.IsValueType)
{
return false;
}

foreach (ITypeSymbol constraintType in symbol.ConstraintTypes)
{
// Recurse on the type parameter first (e. g. we might indirectly be getting a 'class' constraint)
if (constraintType is ITypeParameterSymbol typeParameter &&
typeParameter.IsReferenceTypeOrIndirectlyConstrainedToReferenceType())
{
return true;
}

// Special constraint type that type parameters can derive from. Note that for concrete enum
// types, the 'Enum' constraint isn't sufficient, they'd also have e.g. 'struct', which is
// already checked before. If a type parameter only has 'Enum', then it should be considered
// a reference type.
if (constraintType.SpecialType is SpecialType.System_Delegate or SpecialType.System_Enum)
{
return true;
}

// Only check for classes (an interface doesn't guarantee the type argument will be a reference type)
if (constraintType.TypeKind is TypeKind.Class)
{
return true;
}
}

return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public static bool IsDefaultValueNull(this ITypeSymbol symbol)
// If we do have a type parameter, check that it does have some reference type constraint on it.
if (symbol is ITypeParameterSymbol typeParameter)
{
return typeParameter.HasReferenceTypeConstraint;
return typeParameter.IsReferenceTypeOrIndirectlyConstrainedToReferenceType();
}

return symbol is { IsValueType: false } or INamedTypeSymbol { IsGenericType: true, ConstructedFrom.SpecialType: SpecialType.System_Nullable_T };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1765,6 +1765,68 @@ public partial class MyObject<T1, T2, T3, T4, T5> : DependencyObject
await CSharpAnalyzerTest<InvalidPropertyDefaultValueTypeAnalyzer>.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13);
}

[TestMethod]
[DataRow("where T : class", "null")]
[DataRow("where T : class", "default(T)")]
[DataRow("where T : TOther where TOther : class", "null")]
[DataRow("where T : class where TOther : class", "default(TOther)")]
[DataRow("where T : Delegate", "null")]
[DataRow("where T : Enum", "null")]
[DataRow("where T : DependencyObject", "null")]
[DataRow("where T : DependencyObject", "default(T)")]
[DataRow("where T : TOther where TOther : Delegate", "null")]
[DataRow("where T : TOther where TOther : Enum", "null")]
[DataRow("where T : TOther where TOther : DependencyObject", "null")]
[DataRow("where T : DependencyObject where TOther : class", "default(TOther)")]
public async Task InvalidPropertyDefaultValueTypeAnalyzer_TypeParameter_ConstrainedExplicitNull_DoesNotWarn(
string typeConstraints,
string defaultValue)
{
string source = $$"""
using System;
using CommunityToolkit.WinUI;
using Windows.UI.Xaml;
#nullable enable
namespace MyApp;
public partial class MyObject<T, TOther> : DependencyObject {{typeConstraints}}
{
[GeneratedDependencyProperty(DefaultValue = {{defaultValue}})]
public partial T {|CS9248:Name|} { get; set; }
}
""";

await CSharpAnalyzerTest<InvalidPropertyDefaultValueTypeAnalyzer>.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13);
}

[TestMethod]
[DataRow("where T : struct")]
[DataRow("where T : unmanaged")]
[DataRow("where T : struct, Enum")]
[DataRow("where T : unmanaged, Enum")]
public async Task InvalidPropertyDefaultValueTypeAnalyzer_TypeParameter_ConstrainedExplicitNull_Warns(string typeConstraints)
{
string source = $$"""
using System;
using CommunityToolkit.WinUI;
using Windows.UI.Xaml;
#nullable enable
namespace MyApp;
public partial class MyObject<T, TOther> : DependencyObject {{typeConstraints}}
{
[GeneratedDependencyProperty({|WCTDPG0010:DefaultValue = null|})]
public partial T {|CS9248:Name|} { get; set; }
}
""";

await CSharpAnalyzerTest<InvalidPropertyDefaultValueTypeAnalyzer>.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13);
}

[TestMethod]
[DataRow("string", "42")]
[DataRow("string", "3.14")]
Expand Down Expand Up @@ -3226,6 +3288,85 @@ public void Dispose()
await CSharpAnalyzerTest<UseGeneratedDependencyPropertyOnManualPropertyAnalyzer>.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13);
}

// Regression test for a case found in the Microsoft Store
[TestMethod]
[DataRow("where T : class", "null")]
[DataRow("where T : class", "default(T)")]
[DataRow("where T : TOther where TOther : class", "null")]
[DataRow("where T : class where TOther : class", "default(TOther)")]
[DataRow("where T : Delegate", "null")]
[DataRow("where T : Enum", "null")]
[DataRow("where T : DependencyObject", "null")]
[DataRow("where T : DependencyObject", "default(T)")]
[DataRow("where T : TOther where TOther : Delegate", "null")]
[DataRow("where T : TOther where TOther : Enum", "null")]
[DataRow("where T : TOther where TOther : DependencyObject", "null")]
[DataRow("where T : DependencyObject where TOther : class", "default(TOther)")]
public async Task UseGeneratedDependencyPropertyOnManualPropertyAnalyzer_ValidProperty_WithNullConstrainedGeneric_Warns(
string typeConstraints,
string defaultValue)
{
string source = $$"""
using System;
using Windows.UI.Xaml;
#nullable enable
namespace MyApp;
public partial class MyObject<T, TOther> : DependencyObject {{typeConstraints}}
{
public static readonly DependencyProperty NameProperty = DependencyProperty.Register(
name: "Name",
propertyType: typeof(T),
ownerType: typeof(MyObject<T, TOther>),
typeMetadata: new PropertyMetadata({{defaultValue}}));
public T {|WCTDPG0017:Name|}
{
get => (T?)GetValue(NameProperty);
set => SetValue(NameProperty, value);
}
}
""";

await CSharpAnalyzerTest<UseGeneratedDependencyPropertyOnManualPropertyAnalyzer>.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13);
}

[TestMethod]
[DataRow("where T : struct")]
[DataRow("where T : unmanaged")]
[DataRow("where T : struct, Enum")]
[DataRow("where T : unmanaged, Enum")]
public async Task UseGeneratedDependencyPropertyOnManualPropertyAnalyzer_ValidProperty_WithNullConstrainedGeneric_WCTDPG0031_DoesNotWarn(string typeConstraints)
{
string source = $$"""
using System;
using Windows.UI.Xaml;
#nullable enable
namespace MyApp;
public partial class MyObject<T, TOther> : DependencyObject {{typeConstraints}}
{
public static readonly DependencyProperty NameProperty = DependencyProperty.Register(
name: "Name",
propertyType: typeof(T),
ownerType: typeof(MyObject<T, TOther>),
typeMetadata: new PropertyMetadata({|WCTDPG0031:null|}));
public T {|WCTDPG0017:Name|}
{
get => (T)GetValue(NameProperty);
set => SetValue(NameProperty, value);
}
}
""";

await CSharpAnalyzerTest<UseGeneratedDependencyPropertyOnManualPropertyAnalyzer>.VerifyAnalyzerAsync(source, LanguageVersion.CSharp13);
}

[TestMethod]
[DataRow("private static readonly")]
[DataRow("public static")]
Expand Down

0 comments on commit fd9faea

Please sign in to comment.