From 7d65ea17110bb265e410ca58916374a49fb2ebe3 Mon Sep 17 00:00:00 2001 From: Sergio Pedri Date: Fri, 13 Dec 2024 11:53:18 -0800 Subject: [PATCH] Fix handling of defaulted custom structs in analyzer --- .../DependencyPropertyGenerator.Execute.cs | 59 ++------------- ...endencyPropertyOnManualPropertyAnalyzer.cs | 14 +++- .../Extensions/WinRTExtensions.cs | 71 ++++++++++++++++++ .../Models/DependencyPropertyDefaultValue.cs | 5 ++ .../Models/TypedConstantInfo.cs | 5 ++ .../Test_Analyzers.cs | 32 ++++---- ...ndencyPropertyOnManualPropertyCodeFixer.cs | 74 ++++++++++++++++++- 7 files changed, 189 insertions(+), 71 deletions(-) create mode 100644 components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Extensions/WinRTExtensions.cs diff --git a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/DependencyPropertyGenerator.Execute.cs b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/DependencyPropertyGenerator.Execute.cs index f3dc16bc8..566094d56 100644 --- a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/DependencyPropertyGenerator.Execute.cs +++ b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/DependencyPropertyGenerator.Execute.cs @@ -27,11 +27,6 @@ partial class DependencyPropertyGenerator /// private static partial class Execute { - /// - /// Placeholder for . - /// - private static readonly DependencyPropertyDefaultValue.Null NullInfo = new(); - /// /// Generates the sources for the embedded types, for PrivateAssets="all" scenarios. /// @@ -274,7 +269,7 @@ public static DependencyPropertyDefaultValue GetDefaultValue( } // Invalid callback, the analyzer will emit an error - return NullInfo; + return DependencyPropertyDefaultValue.Null.Instance; } token.ThrowIfCancellationRequested(); @@ -313,7 +308,7 @@ public static DependencyPropertyDefaultValue GetDefaultValue( } // Otherwise, the value has been explicitly set to 'null', so let's respect that - return NullInfo; + return DependencyPropertyDefaultValue.Null.Instance; } token.ThrowIfCancellationRequested(); @@ -322,54 +317,14 @@ public static DependencyPropertyDefaultValue GetDefaultValue( // First we need to special case non nullable values, as for those we need 'default'. if (!propertySymbol.Type.IsDefaultValueNull()) { - string fullyQualifiedTypeName = propertySymbol.Type.GetFullyQualifiedName(); - - // There is a special case for this: if the type of the property is a built-in WinRT - // projected enum type or struct type (ie. some projected value type in general, except - // for 'Nullable' values), then we can just use 'null' and bypass creating the property - // metadata. The WinRT runtime will automatically instantiate a default value for us. - if (propertySymbol.Type.IsContainedInNamespace(WellKnownTypeNames.XamlNamespace(useWindowsUIXaml)) || - propertySymbol.Type.IsContainedInNamespace("System.Numerics")) - { - return new DependencyPropertyDefaultValue.Default(fullyQualifiedTypeName, IsProjectedType: true); - } - - // Special case a few more well known value types that are mapped for WinRT - if (propertySymbol.Type.Name is "Point" or "Rect" or "Size" && - propertySymbol.Type.IsContainedInNamespace("Windows.Foundation")) - { - return new DependencyPropertyDefaultValue.Default(fullyQualifiedTypeName, IsProjectedType: true); - } - - // Special case two more system types - if (propertySymbol.Type is INamedTypeSymbol { MetadataName: "TimeSpan" or "DateTimeOffset", ContainingNamespace.MetadataName: "System" }) - { - return new DependencyPropertyDefaultValue.Default(fullyQualifiedTypeName, IsProjectedType: true); - } - - // Lastly, special case the well known primitive types - if (propertySymbol.Type.SpecialType is - SpecialType.System_Int32 or - SpecialType.System_Byte or - SpecialType.System_SByte or - SpecialType.System_Int16 or - SpecialType.System_UInt16 or - SpecialType.System_UInt32 or - SpecialType.System_Int64 or - SpecialType.System_UInt64 or - SpecialType.System_Char or - SpecialType.System_Single or - SpecialType.System_Double) - { - return new DependencyPropertyDefaultValue.Default(fullyQualifiedTypeName, IsProjectedType: true); - } - - // In all other cases, just use 'default(T)' here - return new DependencyPropertyDefaultValue.Default(fullyQualifiedTypeName, IsProjectedType: false); + // For non nullable types, we return 'default(T)', unless we can optimize for projected types + return new DependencyPropertyDefaultValue.Default( + TypeName: propertySymbol.Type.GetFullyQualifiedName(), + IsProjectedType: propertySymbol.Type.IsWellKnownWinRTProjectedValueType(useWindowsUIXaml)); } // For all other ones, we can just use the 'null' placeholder again - return NullInfo; + return DependencyPropertyDefaultValue.Null.Instance; } /// diff --git a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/Analyzers/UseGeneratedDependencyPropertyOnManualPropertyAnalyzer.cs b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/Analyzers/UseGeneratedDependencyPropertyOnManualPropertyAnalyzer.cs index 2e898dc22..10881e9f5 100644 --- a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/Analyzers/UseGeneratedDependencyPropertyOnManualPropertyAnalyzer.cs +++ b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Diagnostics/Analyzers/UseGeneratedDependencyPropertyOnManualPropertyAnalyzer.cs @@ -65,6 +65,7 @@ public override void Initialize(AnalysisContext context) context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); context.EnableConcurrentExecution(); + context.RegisterCompilationStartAction(static context => { // Get the XAML mode to use @@ -393,7 +394,18 @@ void HandleSetAccessor(IPropertySymbol propertySymbol, PropertyFlags propertyFla } // First, check if the metadata is 'null' (simplest case) - if (propertyMetadataArgument.Value.ConstantValue is not { HasValue: true, Value: null }) + if (propertyMetadataArgument.Value.ConstantValue is { HasValue: true, Value: null }) + { + // Here we need to special case non nullable value types that are not well known WinRT projected types. + // In this case, we cannot rely on XAML calling their default constructor. Rather, we need to preserve + // the explicit 'null' value that users had in their code. The analyzer will then warn on these cases + if (!propertyTypeSymbol.IsDefaultValueNull() && + !propertyTypeSymbol.IsWellKnownWinRTProjectedValueType(useWindowsUIXaml)) + { + fieldFlags.DefaultValue = TypedConstantInfo.Null.Instance; + } + } + else { // Next, check if the argument is 'new PropertyMetadata(...)' with the default value for the property type if (propertyMetadataArgument.Value is not IObjectCreationOperation { Arguments: [{ } defaultValueArgument] } objectCreationOperation) diff --git a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Extensions/WinRTExtensions.cs b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Extensions/WinRTExtensions.cs new file mode 100644 index 000000000..de7983612 --- /dev/null +++ b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Extensions/WinRTExtensions.cs @@ -0,0 +1,71 @@ +// 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 CommunityToolkit.GeneratedDependencyProperty.Constants; +using Microsoft.CodeAnalysis; + +namespace CommunityToolkit.GeneratedDependencyProperty.Extensions; + +/// +/// Extension methods for WinRT scenarios. +/// +internal static class WinRTExtensions +{ + /// + /// Checks whether a given type is a well known WinRT projected value type (ie. a type that XAML can default). + /// + /// The input instance to check. + /// Whether to use the UWP XAML or WinUI 3 XAML namespaces. + /// Whether is a well known WinRT projected value type.. + public static bool IsWellKnownWinRTProjectedValueType(this ITypeSymbol symbol, bool useWindowsUIXaml) + { + // This method only cares about non nullable value types + if (symbol.IsDefaultValueNull()) + { + return false; + } + + // There is a special case for this: if the type of the property is a built-in WinRT + // projected enum type or struct type (ie. some projected value type in general, except + // for 'Nullable' values), then we can just use 'null' and bypass creating the property + // metadata. The WinRT runtime will automatically instantiate a default value for us. + if (symbol.IsContainedInNamespace(WellKnownTypeNames.XamlNamespace(useWindowsUIXaml)) || + symbol.IsContainedInNamespace("System.Numerics")) + { + return true; + } + + // Special case a few more well known value types that are mapped for WinRT + if (symbol.Name is "Point" or "Rect" or "Size" && + symbol.IsContainedInNamespace("Windows.Foundation")) + { + return true; + } + + // Special case two more system types + if (symbol is INamedTypeSymbol { MetadataName: "TimeSpan" or "DateTimeOffset", ContainingNamespace.MetadataName: "System" }) + { + return true; + } + + // Lastly, special case the well known primitive types + if (symbol.SpecialType is + SpecialType.System_Int32 or + SpecialType.System_Byte or + SpecialType.System_SByte or + SpecialType.System_Int16 or + SpecialType.System_UInt16 or + SpecialType.System_UInt32 or + SpecialType.System_Int64 or + SpecialType.System_UInt64 or + SpecialType.System_Char or + SpecialType.System_Single or + SpecialType.System_Double) + { + return true; + } + + return false; + } +} diff --git a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Models/DependencyPropertyDefaultValue.cs b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Models/DependencyPropertyDefaultValue.cs index 29469028e..993c9d717 100644 --- a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Models/DependencyPropertyDefaultValue.cs +++ b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Models/DependencyPropertyDefaultValue.cs @@ -16,6 +16,11 @@ internal abstract partial record DependencyPropertyDefaultValue /// public sealed record Null : DependencyPropertyDefaultValue { + /// + /// The shared instance (the type is stateless). + /// + public static Null Instance { get; } = new(); + /// public override string ToString() { diff --git a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Models/TypedConstantInfo.cs b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Models/TypedConstantInfo.cs index 48accd4e1..427f25813 100644 --- a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Models/TypedConstantInfo.cs +++ b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.SourceGenerators/Models/TypedConstantInfo.cs @@ -23,6 +23,11 @@ internal abstract partial record TypedConstantInfo /// public sealed record Null : TypedConstantInfo { + /// + /// The shared instance (the type is stateless). + /// + public static Null Instance { get; } = new(); + /// public override string ToString() { diff --git a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_Analyzers.cs b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_Analyzers.cs index db6972429..b073ab716 100644 --- a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_Analyzers.cs +++ b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_Analyzers.cs @@ -1382,7 +1382,6 @@ public string? Name [TestMethod] [DataRow("global::System.TimeSpan", "global::System.TimeSpan", "global::System.TimeSpan.FromSeconds(1)")] [DataRow("global::System.TimeSpan?", "global::System.TimeSpan?", "global::System.TimeSpan.FromSeconds(1)")] - [DataRow("global::Windows.UI.Xaml.Visibility", "global::Windows.UI.Xaml.Visibility", "global::Windows.UI.Xaml.Visibility.Collapsed")] public async Task UseGeneratedDependencyPropertyOnManualPropertyAnalyzer_ValidProperty_ExplicitDefaultValue_DoesNotWarn( string dependencyPropertyType, string propertyType, @@ -1425,21 +1424,21 @@ public enum MyEnum { A, B, C } [DataRow("object", "object?")] [DataRow("int", "int")] [DataRow("int?", "int?")] - [DataRow("global::System.TimeSpan", "global::System.TimeSpan", "null")] - [DataRow("global::System.TimeSpan?", "global::System.TimeSpan?", "default(global::System.TimeSpan?)")] - [DataRow("global::System.DateTimeOffset", "global::System.DateTimeOffset", "null")] - [DataRow("global::System.DateTimeOffset?", "global::System.DateTimeOffset?", "default(global::System.DateTimeOffset?)")] - [DataRow("global::System.Guid?", "global::System.Guid?", "default(global::System.Guid?)")] - [DataRow("global::System.Collections.Generic.KeyValuePair?", "global::System.Collections.Generic.KeyValuePair?", "default(global::System.Collections.Generic.KeyValuePair?)")] - [DataRow("global::System.Collections.Generic.KeyValuePair?", "global::System.Collections.Generic.KeyValuePair?", "null")] - [DataRow("global::MyApp.MyStruct", "global::MyApp.MyStruct", "default(global::MyApp.MyStruct)")] - [DataRow("global::MyApp.MyStruct?", "global::MyApp.MyStruct?", "null")] - [DataRow("global::MyApp.MyStruct?", "global::MyApp.MyStruct?", "default(global::MyApp.MyStruct?)")] - [DataRow("global::MyApp.MyEnum", "global::MyApp.MyEnum", "default(global::MyApp.MyEnum)")] - [DataRow("global::MyApp.MyEnum?", "global::MyApp.MyEnum?", "null")] - [DataRow("global::MyApp.MyEnum?", "global::MyApp.MyEnum?", "default(global::MyApp.MyEnum?)")] - [DataRow("global::MyApp.MyClass", "global::MyApp.MyClass", "null")] - [DataRow("global::MyApp.MyClass", "global::MyApp.MyClass", "default(global::MyApp.MyClass)")] + [DataRow("global::System.TimeSpan", "global::System.TimeSpan")] + [DataRow("global::System.TimeSpan?", "global::System.TimeSpan?")] + [DataRow("global::System.DateTimeOffset", "global::System.DateTimeOffset")] + [DataRow("global::System.DateTimeOffset?", "global::System.DateTimeOffset?")] + [DataRow("global::System.Guid?", "global::System.Guid?")] + [DataRow("global::System.Collections.Generic.KeyValuePair?", "global::System.Collections.Generic.KeyValuePair?")] + [DataRow("global::System.Collections.Generic.KeyValuePair?", "global::System.Collections.Generic.KeyValuePair?" )] + [DataRow("global::MyApp.MyStruct", "global::MyApp.MyStruct")] + [DataRow("global::MyApp.MyStruct?", "global::MyApp.MyStruct?")] + [DataRow("global::MyApp.MyStruct?", "global::MyApp.MyStruct?")] + [DataRow("global::MyApp.MyEnum", "global::MyApp.MyEnum")] + [DataRow("global::MyApp.MyEnum?", "global::MyApp.MyEnum?")] + [DataRow("global::MyApp.MyEnum?", "global::MyApp.MyEnum?")] + [DataRow("global::MyApp.MyClass", "global::MyApp.MyClass")] + [DataRow("global::MyApp.MyClass", "global::MyApp.MyClass")] public async Task UseGeneratedDependencyPropertyOnManualPropertyAnalyzer_ValidProperty_Warns( string dependencyPropertyType, string propertyType) @@ -1504,6 +1503,7 @@ public class MyClass { } [DataRow("global::Windows.Foundation.Size", "global::Windows.Foundation.Size", "default(global::Windows.Foundation.Size)")] [DataRow("global::Windows.UI.Xaml.Visibility", "global::Windows.UI.Xaml.Visibility", "default(global::Windows.UI.Xaml.Visibility)")] [DataRow("global::Windows.UI.Xaml.Visibility", "global::Windows.UI.Xaml.Visibility", "global::Windows.UI.Xaml.Visibility.Visible")] + [DataRow("global::Windows.UI.Xaml.Visibility", "global::Windows.UI.Xaml.Visibility", "global::Windows.UI.Xaml.Visibility.Collapsed")] [DataRow("global::System.TimeSpan", "global::System.TimeSpan", "default(System.TimeSpan)")] [DataRow("global::System.DateTimeOffset", "global::System.DateTimeOffset", "default(global::System.DateTimeOffset)")] [DataRow("global::System.DateTimeOffset?", "global::System.DateTimeOffset?", "null")] diff --git a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_UseGeneratedDependencyPropertyOnManualPropertyCodeFixer.cs b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_UseGeneratedDependencyPropertyOnManualPropertyCodeFixer.cs index 01cf0971a..5b7badbb1 100644 --- a/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_UseGeneratedDependencyPropertyOnManualPropertyCodeFixer.cs +++ b/components/DependencyPropertyGenerator/CommunityToolkit.DependencyPropertyGenerator.Tests/Test_UseGeneratedDependencyPropertyOnManualPropertyCodeFixer.cs @@ -57,10 +57,10 @@ public class Test_UseGeneratedDependencyPropertyOnManualPropertyCodeFixer [DataRow("global::System.TimeSpan?", "global::System.TimeSpan?")] [DataRow("global::System.Guid?", "global::System.Guid?")] [DataRow("global::System.Collections.Generic.KeyValuePair?", "global::System.Collections.Generic.KeyValuePair?")] - [DataRow("global::MyApp.MyStruct", "global::MyApp.MyStruct")] [DataRow("global::MyApp.MyStruct?", "global::MyApp.MyStruct?")] - [DataRow("global::MyApp.MyEnum", "global::MyApp.MyEnum")] [DataRow("global::MyApp.MyEnum?", "global::MyApp.MyEnum?")] + [DataRow("global::MyApp.MyClass", "global::MyApp.MyClass")] + [DataRow("global::MyApp.MyClass", "global::MyApp.MyClass?")] public async Task SimpleProperty(string dependencyPropertyType, string propertyType) { string original = $$""" @@ -86,6 +86,7 @@ public class MyControl : Control public struct MyStruct { public string X { get; set; } } public enum MyEnum { A, B, C } + public class MyClass { } """; string @fixed = $$""" @@ -101,6 +102,75 @@ public partial class MyControl : Control public partial {{propertyType}} {|CS9248:Name|} { get; set; } } + public struct MyStruct { public string X { get; set; } } + public enum MyEnum { A, B, C } + public class MyClass { } + """; + + CSharpCodeFixTest test = new(LanguageVersion.Preview) + { + TestCode = original, + FixedCode = @fixed, + ReferenceAssemblies = ReferenceAssemblies.Net.Net80, + TestState = { AdditionalReferences = + { + MetadataReference.CreateFromFile(typeof(Point).Assembly.Location), + MetadataReference.CreateFromFile(typeof(ApplicationView).Assembly.Location), + MetadataReference.CreateFromFile(typeof(DependencyProperty).Assembly.Location), + MetadataReference.CreateFromFile(typeof(GeneratedDependencyPropertyAttribute).Assembly.Location) + }} + }; + + await test.RunAsync(); + } + + // These are custom value types, on properties where the metadata was set to 'null'. In this case, the + // default value would just be 'null', as XAML can't default initialize them. To preserve behavior, + // we must include an explicit default value. This will warn when the code is recompiled, but that + // is expected, because this specific scenario was (1) niche, and (2) kinda busted already anyway. + [TestMethod] + [DataRow("global::MyApp.MyStruct", "global::MyApp.MyStruct")] + [DataRow("global::MyApp.MyEnum", "global::MyApp.MyEnum")] + public async Task SimpleProperty_ExplicitNull(string dependencyPropertyType, string propertyType) + { + string original = $$""" + using Windows.UI.Xaml; + using Windows.UI.Xaml.Controls; + + namespace MyApp; + + public class MyControl : Control + { + public static readonly DependencyProperty NameProperty = DependencyProperty.Register( + name: nameof(Name), + propertyType: typeof({{dependencyPropertyType}}), + ownerType: typeof(MyControl), + typeMetadata: null); + + public {{propertyType}} [|Name|] + { + get => ({{propertyType}})GetValue(NameProperty); + set => SetValue(NameProperty, value); + } + } + + public struct MyStruct { public string X { get; set; } } + public enum MyEnum { A, B, C } + """; + + string @fixed = $$""" + using CommunityToolkit.WinUI; + using Windows.UI.Xaml; + using Windows.UI.Xaml.Controls; + + namespace MyApp; + + public partial class MyControl : Control + { + [GeneratedDependencyProperty(DefaultValue = null)] + public partial {{propertyType}} {|CS9248:Name|} { get; set; } + } + public struct MyStruct { public string X { get; set; } } public enum MyEnum { A, B, C } """;