From 7d2a9d8e1a1c644ff742f56847ac777e55f3a61f Mon Sep 17 00:00:00 2001 From: xiaoy312 Date: Fri, 29 Nov 2024 15:25:32 -0500 Subject: [PATCH] fix(codegen): markup-ext improper cast for nullable value-type dp --- .../Helpers/SymbolExtensions.cs | 19 +++++++------- .../XamlGenerator/XamlFileGenerator.cs | 5 ++-- .../CustomMarkupExtensions.xaml | 8 ++++-- .../CustomMarkupExtensions.xaml.cs | 5 ++++ .../Given_MarkupExtension.cs | 10 +++++++ .../MarkupExtension_CodegenTypeCast.xaml | 11 ++++++++ .../MarkupExtension_CodegenTypeCast.xaml.cs | 26 +++++++++++++++++++ 7 files changed, 71 insertions(+), 13 deletions(-) create mode 100644 src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Markup/MarkupExtension_CodegenTypeCast.xaml create mode 100644 src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Markup/MarkupExtension_CodegenTypeCast.xaml.cs diff --git a/src/SourceGenerators/SourceGeneratorHelpers/Helpers/SymbolExtensions.cs b/src/SourceGenerators/SourceGeneratorHelpers/Helpers/SymbolExtensions.cs index fcd38071e58c..791b8c9aba9e 100644 --- a/src/SourceGenerators/SourceGeneratorHelpers/Helpers/SymbolExtensions.cs +++ b/src/SourceGenerators/SourceGeneratorHelpers/Helpers/SymbolExtensions.cs @@ -599,20 +599,21 @@ private static string GetFullyQualifiedType(this ITypeSymbol type, bool includeG /// /// The dependency-property or the attached dependency-property setter /// The property type - public static INamedTypeSymbol? FindDependencyPropertyType(this ISymbol propertyOrSetter) + public static INamedTypeSymbol? FindDependencyPropertyType(this ISymbol propertyOrSetter, bool unwrapNullable = true) { - if (propertyOrSetter is IPropertySymbol dependencyProperty) + var type = propertyOrSetter switch { - return dependencyProperty.Type.OriginalDefinition is { SpecialType: SpecialType.System_Nullable_T } - ? (dependencyProperty.Type as INamedTypeSymbol)?.TypeArguments[0] as INamedTypeSymbol - : dependencyProperty.Type as INamedTypeSymbol; - } - else if (propertyOrSetter is IMethodSymbol { IsStatic: true, Parameters.Length: 2 } attachedPropertySetter) + IPropertySymbol dp => dp.Type, + IMethodSymbol { IsStatic: true, Parameters.Length: 2 } adpSetter => adpSetter.Parameters[1].Type, + + _ => null, + }; + if (unwrapNullable && type?.IsNullable(out var innerType) == true) { - return attachedPropertySetter.Parameters[1].Type as INamedTypeSymbol; + type = innerType; } - return null; + return type as INamedTypeSymbol; } } } diff --git a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs index c1e23e45dce8..9ea8b183341f 100644 --- a/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs +++ b/src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs @@ -4629,7 +4629,7 @@ private string GetCustomMarkupExtensionValue(XamlMemberDefinition member, string var property = FindProperty(member.Member); var declaringType = property?.ContainingType; - var propertyType = property?.FindDependencyPropertyType(); + var propertyType = property?.FindDependencyPropertyType(unwrapNullable: false); if (declaringType == null || propertyType == null) { @@ -4675,7 +4675,8 @@ private string GetCustomMarkupExtensionValue(XamlMemberDefinition member, string var provider = $"{globalized.MarkupHelper}.CreateParserContext({providerDetails.JoinBy(", ")})"; var provideValue = $"(({globalized.IMarkupExtensionOverrides})new {globalized.MarkupType}{markupInitializer}).ProvideValue({provider})"; - if (IsImplementingInterface(propertyType, Generation.IConvertibleSymbol.Value)) + var unwrappedPropertyType = propertyType.IsNullable(out var nullableInnerType) ? nullableInnerType as INamedTypeSymbol : propertyType; + if (IsImplementingInterface(unwrappedPropertyType, Generation.IConvertibleSymbol.Value)) { provideValue = $"{globalized.XamlBindingHelper}.ConvertValue(typeof({globalized.PvtpType}), {provideValue})"; } diff --git a/src/SourceGenerators/XamlGenerationTests/CustomMarkupExtensions.xaml b/src/SourceGenerators/XamlGenerationTests/CustomMarkupExtensions.xaml index 28a847ff9a9d..4f0a0f22c8af 100644 --- a/src/SourceGenerators/XamlGenerationTests/CustomMarkupExtensions.xaml +++ b/src/SourceGenerators/XamlGenerationTests/CustomMarkupExtensions.xaml @@ -5,7 +5,8 @@ xmlns:local="using:XamlGenerationTests.Shared.MarkupExtensions" xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" xmlns:ext="clr-namespace:XamlGenerationTests.Shared.MarkupExtensions" - mc:Ignorable="d"> + xmlns:void="There is no mistake so great that it cannot be undone." + mc:Ignorable="d void"> @@ -72,7 +73,10 @@ - + + + + diff --git a/src/SourceGenerators/XamlGenerationTests/CustomMarkupExtensions.xaml.cs b/src/SourceGenerators/XamlGenerationTests/CustomMarkupExtensions.xaml.cs index 6a9d4a3d8166..3cc9e32e6f01 100644 --- a/src/SourceGenerators/XamlGenerationTests/CustomMarkupExtensions.xaml.cs +++ b/src/SourceGenerators/XamlGenerationTests/CustomMarkupExtensions.xaml.cs @@ -111,6 +111,11 @@ protected override object ProvideValue() } } + public class ReturnNullExtension : MarkupExtension + { + protected override object ProvideValue() => null; + } + public class ComplexObject { public string StringProp { get; set; } diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Markup/Given_MarkupExtension.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Markup/Given_MarkupExtension.cs index b2939cf7f8c2..023d76c16773 100644 --- a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Markup/Given_MarkupExtension.cs +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Markup/Given_MarkupExtension.cs @@ -94,6 +94,16 @@ public void When_MarkupExtension_Enum() Assert.AreEqual(Orientation.Horizontal, page.EnumMarkupExtension_Horizontal.Orientation); Assert.AreEqual(Orientation.Vertical, page.EnumMarkupExtension_Vertical.Orientation); } + + [TestMethod] + public void When_MarkupExtension_ReturnNullForNullableStructType() + { + // it also shouldn't throw here + var page = new MarkupExtension_CodegenTypeCast(); + + Assert.IsTrue(page.Control.IsChecked, "sanity check failed: Control.IsChecked is not true"); + Assert.IsNull(page.SUT.IsChecked, "Property value should be set to null by the markup-extension"); + } #endif } } diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Markup/MarkupExtension_CodegenTypeCast.xaml b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Markup/MarkupExtension_CodegenTypeCast.xaml new file mode 100644 index 000000000000..031e067641d2 --- /dev/null +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Markup/MarkupExtension_CodegenTypeCast.xaml @@ -0,0 +1,11 @@ + + + + + + diff --git a/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Markup/MarkupExtension_CodegenTypeCast.xaml.cs b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Markup/MarkupExtension_CodegenTypeCast.xaml.cs new file mode 100644 index 000000000000..3edcf665e1d5 --- /dev/null +++ b/src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Markup/MarkupExtension_CodegenTypeCast.xaml.cs @@ -0,0 +1,26 @@ +using Microsoft.UI.Xaml; +using Microsoft.UI.Xaml.Controls; +using Microsoft.UI.Xaml.Markup; + +namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Markup; + +public sealed partial class MarkupExtension_CodegenTypeCast : Page +{ + public MarkupExtension_CodegenTypeCast() + { + this.InitializeComponent(); + } +} +public partial class AlreadyCheckedBox : CheckBox +{ + public AlreadyCheckedBox() + { + // setting a default value, so we may observe ReturnNullExtension working or not. + this.IsChecked = true; + } +} + +public class ReturnNullExtension : MarkupExtension +{ + protected override object ProvideValue() => null; +}