Skip to content

Commit

Permalink
fix(codegen): markup-ext improper cast for nullable value-type dp
Browse files Browse the repository at this point in the history
  • Loading branch information
Xiaoy312 committed Nov 29, 2024
1 parent 58e477b commit 7d2a9d8
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -599,20 +599,21 @@ private static string GetFullyQualifiedType(this ITypeSymbol type, bool includeG
/// </summary>
/// <param name="propertyOrSetter">The dependency-property or the attached dependency-property setter</param>
/// <returns>The property type</returns>
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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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})";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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">

<UserControl.Resources>
<ext:InverseBoolMarkup x:Key="Inverse" />
Expand Down Expand Up @@ -72,7 +73,10 @@
<!-- XAML markup extension without Suffix Extension work on bindin an attached property using a binding and converter -->
<TextBlock ext:MarkupExtensionTestBehavior.CustomText="{Binding IsRightTapEnabled, ElementName=RootGrid, Converter={ext:TestMarkupSuffix}}" />


<!-- #18819: null-value markup return can safely convert to nullable-struct -->
<!-- {x:Null} is a special case, it doesn't use the custom markup-ext code-gen -->
<CheckBox IsChecked="{ext:ReturnNull}" />
</StackPanel>
</Grid>

</UserControl>
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<Page x:Class="Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Markup.MarkupExtension_CodegenTypeCast"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:local="using:Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Markup">
<StackPanel>
<local:AlreadyCheckedBox x:Name="SUT"
x:FieldModifier="Public"
IsChecked="{local:ReturnNull}" />
<local:AlreadyCheckedBox x:Name="Control" x:FieldModifier="Public" />
</StackPanel>
</Page>
Original file line number Diff line number Diff line change
@@ -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;
}

0 comments on commit 7d2a9d8

Please sign in to comment.