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

fix(codegen): markup-ext improper cast for nullable value-type dp #18971

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
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)
jeromelaban marked this conversation as resolved.
Show resolved Hide resolved
{
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;
}
Loading