Skip to content

Commit 0ab2847

Browse files
Xiaoy312Xiaoy312
committed
fix(codegen): markup-ext improper cast for nullable value-type dp
1 parent 58e477b commit 0ab2847

File tree

7 files changed

+71
-13
lines changed

7 files changed

+71
-13
lines changed

‎src/SourceGenerators/SourceGeneratorHelpers/Helpers/SymbolExtensions.cs

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -599,20 +599,21 @@ private static string GetFullyQualifiedType(this ITypeSymbol type, bool includeG
599599
/// </summary>
600600
/// <param name="propertyOrSetter">The dependency-property or the attached dependency-property setter</param>
601601
/// <returns>The property type</returns>
602-
public static INamedTypeSymbol? FindDependencyPropertyType(this ISymbol propertyOrSetter)
602+
public static INamedTypeSymbol? FindDependencyPropertyType(this ISymbol propertyOrSetter, bool unwrapNullable = true)
603603
{
604-
if (propertyOrSetter is IPropertySymbol dependencyProperty)
604+
var type = propertyOrSetter switch
605605
{
606-
return dependencyProperty.Type.OriginalDefinition is { SpecialType: SpecialType.System_Nullable_T }
607-
? (dependencyProperty.Type as INamedTypeSymbol)?.TypeArguments[0] as INamedTypeSymbol
608-
: dependencyProperty.Type as INamedTypeSymbol;
609-
}
610-
else if (propertyOrSetter is IMethodSymbol { IsStatic: true, Parameters.Length: 2 } attachedPropertySetter)
606+
IPropertySymbol dp => dp.Type,
607+
IMethodSymbol { IsStatic: true, Parameters.Length: 2 } adpSetter => adpSetter.Parameters[1].Type,
608+
609+
_ => null,
610+
};
611+
if (unwrapNullable && type?.IsNullable(out var innerType) == true)
611612
{
612-
return attachedPropertySetter.Parameters[1].Type as INamedTypeSymbol;
613+
type = innerType;
613614
}
614615

615-
return null;
616+
return type as INamedTypeSymbol;
616617
}
617618
}
618619
}

‎src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4629,7 +4629,7 @@ private string GetCustomMarkupExtensionValue(XamlMemberDefinition member, string
46294629

46304630
var property = FindProperty(member.Member);
46314631
var declaringType = property?.ContainingType;
4632-
var propertyType = property?.FindDependencyPropertyType();
4632+
var propertyType = property?.FindDependencyPropertyType(unwrapNullable: false);
46334633

46344634
if (declaringType == null || propertyType == null)
46354635
{
@@ -4675,7 +4675,8 @@ private string GetCustomMarkupExtensionValue(XamlMemberDefinition member, string
46754675
var provider = $"{globalized.MarkupHelper}.CreateParserContext({providerDetails.JoinBy(", ")})";
46764676

46774677
var provideValue = $"(({globalized.IMarkupExtensionOverrides})new {globalized.MarkupType}{markupInitializer}).ProvideValue({provider})";
4678-
if (IsImplementingInterface(propertyType, Generation.IConvertibleSymbol.Value))
4678+
var unwrappedPropertyType = propertyType.IsNullable(out var nullableInnerType) ? nullableInnerType as INamedTypeSymbol : propertyType;
4679+
if (IsImplementingInterface(unwrappedPropertyType, Generation.IConvertibleSymbol.Value))
46794680
{
46804681
provideValue = $"{globalized.XamlBindingHelper}.ConvertValue(typeof({globalized.PvtpType}), {provideValue})";
46814682
}

‎src/SourceGenerators/XamlGenerationTests/CustomMarkupExtensions.xaml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
xmlns:local="using:XamlGenerationTests.Shared.MarkupExtensions"
66
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
77
xmlns:ext="clr-namespace:XamlGenerationTests.Shared.MarkupExtensions"
8-
mc:Ignorable="d">
8+
xmlns:void="There is no mistake so great that it cannot be undone."
9+
mc:Ignorable="d void">
910

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

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

‎src/SourceGenerators/XamlGenerationTests/CustomMarkupExtensions.xaml.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,11 @@ protected override object ProvideValue()
111111
}
112112
}
113113

114+
public class ReturnNullExtension : MarkupExtension
115+
{
116+
protected override object ProvideValue() => null;
117+
}
118+
114119
public class ComplexObject
115120
{
116121
public string StringProp { get; set; }

‎src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Markup/Given_MarkupExtension.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,16 @@ public void When_MarkupExtension_Enum()
9494
Assert.AreEqual(Orientation.Horizontal, page.EnumMarkupExtension_Horizontal.Orientation);
9595
Assert.AreEqual(Orientation.Vertical, page.EnumMarkupExtension_Vertical.Orientation);
9696
}
97+
98+
[TestMethod]
99+
public void When_MarkupExtension_ReturnNullForNullableStructType()
100+
{
101+
// it also shouldn't throw here
102+
var page = new MarkupExtension_CodegenTypeCast();
103+
104+
Assert.IsTrue(page.Control.IsChecked, "sanity check failed: Control.IsChecked is not true");
105+
Assert.IsNull(page.SUT.IsChecked, "Property value should be set to null by the markup-extension");
106+
}
97107
#endif
98108
}
99109
}

‎src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Markup/MarkupExtension_CodegenTypeCast.xaml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<Page x:Class="Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Markup.MarkupExtension_CodegenTypeCast"
2+
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
3+
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
4+
xmlns:local="using:Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Markup">
5+
<StackPanel>
6+
<local:AlreadyCheckedBox x:Name="SUT"
7+
x:FieldModifier="public"
8+
IsChecked="{local:ReturnNull}" />
9+
<local:AlreadyCheckedBox x:Name="Control" x:FieldModifier="public" />
10+
</StackPanel>
11+
</Page>

‎src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Markup/MarkupExtension_CodegenTypeCast.xaml.cs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
using Microsoft.UI.Xaml;
2+
using Microsoft.UI.Xaml.Controls;
3+
using Microsoft.UI.Xaml.Markup;
4+
5+
namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Markup;
6+
7+
public sealed partial class MarkupExtension_CodegenTypeCast : Page
8+
{
9+
public MarkupExtension_CodegenTypeCast()
10+
{
11+
this.InitializeComponent();
12+
}
13+
}
14+
public partial class AlreadyCheckedBox : CheckBox
15+
{
16+
public AlreadyCheckedBox()
17+
{
18+
// setting a default value, so we may observe ReturnNullExtension working or not.
19+
this.IsChecked = true;
20+
}
21+
}
22+
23+
public class ReturnNullExtension : MarkupExtension
24+
{
25+
protected override object ProvideValue() => null;
26+
}

0 commit comments

Comments
 (0)