[XSG] Fix CS8601 error in XAML SourceGen setter for nullable reference types (#32940)

StephaneDelcroix · Copilot · simonrozsival · web-flow · commit aae0d33c335b · 2025-12-03T22:56:42.000+01:00
* Fix CS8601 error in XAML SourceGen setter for nullable reference types When binding path contains nullable access (e.g., Child.Message where Child is nullable), the generated setter was assigning a nullable value to a non-nullable property, causing CS8601 'Possible null reference assignment'. The fix adds an early return for null values in the setter when: - PropertyType.IsNullable is true (the binding path involves nullable access) - SetterOptions.AcceptsNullValue is false (the target property doesn't accept null) This mirrors the existing logic in BindingCodeWriter.cs used for C# binding interceptors. Fixes #32893 * Add test for nullable value types through nullable paths The test verifies that binding to non-nullable value type properties through nullable paths works correctly. In XAML SourceGen, value types accessed through nullable paths remain non-nullable (using ?? default fallback in getter) rather than becoming Nullable<T>, so the setter uses pattern matching instead of HasValue checks. * [XSG] Add comprehensive test coverage for nullable types in conditional access paths (#32951) * Initial plan * Add comprehensive nullable type tests for conditional access paths Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> * Refactor test assertions to use regex matching and full expressions Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: simonrozsival <374616+simonrozsival@users.noreply.github.com>
diff --git a/src/Controls/src/SourceGen/CompiledBindingMarkup.cs b/src/Controls/src/SourceGen/CompiledBindingMarkup.cs
@@ -488,7 +488,27 @@ void AppendSetterLambda(IndentedTextWriter code, BindingInvocationDescription bi
 		code.WriteLine("{");
 		code.Indent++;
 		
-		var setter = Setter.From(binding.Path, "source", "value");
+		var assignedValueExpression = "value";
+
+		// early return for nullable values if the setter doesn't accept them
+		if (binding.PropertyType.IsNullable && !binding.SetterOptions.AcceptsNullValue)
+		{
+			if (binding.PropertyType.IsValueType)
+			{
+				code.WriteLine("if (!value.HasValue)");
+				assignedValueExpression = "value.Value";
+			}
+			else
+			{
+				code.WriteLine("if (value is null)");
+			}
+			using (PrePost.NewBlock(code))
+			{
+				code.WriteLine("return;");
+			}
+		}
+
+		var setter = Setter.From(binding.Path, "source", assignedValueExpression);
 		if (setter.PatternMatchingExpressions.Length > 0)
 		{
 			code.Write("if (");
diff --git a/src/Controls/tests/SourceGen.UnitTests/InitializeComponent/CompiledBindings.cs b/src/Controls/tests/SourceGen.UnitTests/InitializeComponent/CompiledBindings.cs
@@ -421,6 +421,10 @@ private partial void InitializeComponent()
 		{
 			global::System.Action<global::Test.TestPage, string?>? setter = static (source, value) =>
 			{
+				if (value is null)
+				{
+					return;
+				}
 				if (source.Product is {} p0)
 				{
 					p0.Name = value;
@@ -497,6 +501,63 @@ public class Product
 		Assert.False(result.Diagnostics.Any(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error));
 	}
 
+	[Fact]
+	public void CorrectlyHandlesNullableValueTypesWithNonNullableTarget()
+	{
+		// This test covers binding to a non-nullable value type property through a nullable path
+		// Note: In XAML SourceGen, value types accessed through nullable paths remain non-nullable
+		// (using ?? default fallback in getter) rather than becoming Nullable<T>
+		var xaml =
+"""
+<?xml version="1.0" encoding="UTF-8"?>
+<ContentPage
+	xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
+	xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
+	xmlns:test="clr-namespace:Test"
+	x:Class="Test.TestPage"
+	x:DataType="test:TestPage">
+	<Slider Value="{Binding Product.Quantity}"/>
+</ContentPage>
+""";
+
+		var code =
+"""
+#nullable enable
+#pragma warning disable CS0219 // Variable is assigned but its value is never used
+using System;
+using Microsoft.Maui.Controls;
+using Microsoft.Maui.Controls.Xaml;
+
+namespace Test;
+
+[XamlProcessing(XamlInflator.SourceGen)]
+public partial class TestPage : ContentPage
+{
+	public Product? Product { get; set; } = null;
+
+	public TestPage()
+	{
+		InitializeComponent();
+	}
+}
+
+public class Product
+{
+	public double Quantity { get; set; } = 0;  // Non-nullable double
+}
+""";
+		var (result, generated) = RunGenerator(xaml, code);
+		
+		// Verify no errors are present - value types through nullable paths don't need null checks
+		// because they use ?? default fallback in the getter and keep the non-nullable type
+		Assert.False(result.Diagnostics.Any(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error));
+		Assert.NotNull(generated);
+		
+		// The setter for value types should NOT have HasValue check because PropertyType stays as double, not double?
+		// The conditional access is handled by pattern matching (source.Product is {} p0)
+		Assert.Contains("if (source.Product is {} p0)", generated, StringComparison.Ordinal);
+	}
+
 	[Fact]
 	public void TargetNullValueWithNullablePathGeneratesValidCode()
 	{
@@ -715,4 +776,225 @@ public class TestViewModel
 		var emptyHandlersPattern = @"handlers:\s*new\s+global::System\.Tuple<global::System\.Func<global::Test\.TestViewModel,\s*object\?>,\s*string>\[\]\s*\{\s*\}";
 		Assert.Matches(emptyHandlersPattern, generated);
 	}
+
+	[Fact]
+	public void ValueTypeAtEndOfConditionalAccessPath()
+	{
+		// Test value type property at the end of a binding path with conditional access in the middle
+		// The setter should use pattern matching and NOT have HasValue check since PropertyType is non-nullable
+		var xaml =
+"""
+<?xml version="1.0" encoding="UTF-8"?>
+<ContentPage
+	xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
+	xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
+	xmlns:test="clr-namespace:Test"
+	x:Class="Test.TestPage"
+	x:DataType="test:TestPage">
+	<Label Text="{Binding Container.Count}"/>
+</ContentPage>
+""";
+
+		var code =
+"""
+#nullable enable
+using System;
+using Microsoft.Maui.Controls;
+using Microsoft.Maui.Controls.Xaml;
+
+namespace Test;
+
+[XamlProcessing(XamlInflator.SourceGen)]
+public partial class TestPage : ContentPage
+{
+	public Container? Container { get; set; }
+}
+
+public class Container
+{
+	public int Count { get; set; }
+}
+""";
+
+		var (result, generated) = RunGenerator(xaml, code);
+		
+		Assert.False(result.Diagnostics.Any(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error));
+		Assert.NotNull(generated);
+		
+		// Verify setter uses pattern matching for conditional access
+		Assert.Contains("if (source.Container is {} p0)", generated, StringComparison.Ordinal);
+		
+		// Should NOT have HasValue check since Count is int, not int?
+		Assert.DoesNotContain("HasValue", generated, StringComparison.Ordinal);
+		
+		// Getter should have ?? default for value type through nullable path
+		Assert.Contains("getter: source => (source.Container?.Count ?? default, true)", generated, StringComparison.Ordinal);
+	}
+
+	[Fact]
+	public void NonNullableReferenceTypeAtEndOfConditionalAccessPath()
+	{
+		// Test non-nullable reference type property at the end of a binding path with conditional access
+		// The setter should have an early return for null since the target property doesn't accept null
+		var xaml =
+"""
+<?xml version="1.0" encoding="UTF-8"?>
+<ContentPage
+	xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
+	xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
+	xmlns:test="clr-namespace:Test"
+	x:Class="Test.TestPage"
+	x:DataType="test:TestPage">
+	<Entry Text="{Binding Container.Label}"/>
+</ContentPage>
+""";
+
+		var code =
+"""
+#nullable enable
+using System;
+using Microsoft.Maui.Controls;
+using Microsoft.Maui.Controls.Xaml;
+
+namespace Test;
+
+[XamlProcessing(XamlInflator.SourceGen)]
+public partial class TestPage : ContentPage
+{
+	public Container? Container { get; set; }
+}
+
+public class Container
+{
+	public string Label { get; set; } = "";  // Non-nullable string
+}
+""";
+
+		var (result, generated) = RunGenerator(xaml, code);
+		
+		Assert.False(result.Diagnostics.Any(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error));
+		Assert.NotNull(generated);
+		
+		// Verify setter has early return for null since Label doesn't accept null
+		Assert.Contains("if (value is null)", generated, StringComparison.Ordinal);
+		Assert.Contains("return;", generated, StringComparison.Ordinal);
+		
+		// Verify setter uses pattern matching for conditional access
+		Assert.Contains("if (source.Container is {} p0)", generated, StringComparison.Ordinal);
+	}
+
+	[Fact]
+	public void NullableReferenceTypeAtEndOfConditionalAccessPath()
+	{
+		// Test nullable reference type property at the end of a binding path with conditional access
+		// The setter should NOT have an early return since the target property accepts null
+		var xaml =
+"""
+<?xml version="1.0" encoding="UTF-8"?>
+<ContentPage
+	xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
+	xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
+	xmlns:test="clr-namespace:Test"
+	x:Class="Test.TestPage"
+	x:DataType="test:TestPage">
+	<Entry Text="{Binding Container.Description}"/>
+</ContentPage>
+""";
+
+		var code =
+"""
+#nullable enable
+using System;
+using Microsoft.Maui.Controls;
+using Microsoft.Maui.Controls.Xaml;
+
+namespace Test;
+
+[XamlProcessing(XamlInflator.SourceGen)]
+public partial class TestPage : ContentPage
+{
+	public Container? Container { get; set; }
+}
+
+public class Container
+{
+	public string? Description { get; set; }  // Nullable string
+}
+""";
+
+		var (result, generated) = RunGenerator(xaml, code);
+		
+		Assert.False(result.Diagnostics.Any(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error));
+		Assert.NotNull(generated);
+		
+		// Verify setter does NOT have early return since Description accepts null
+		// The setter should just use pattern matching without null check on value
+		Assert.Contains("if (source.Container is {} p0)", generated, StringComparison.Ordinal);
+		
+		// Check the setter signature accepts nullable
+		Assert.Contains("global::System.Action<global::Test.TestPage, string?>", generated, StringComparison.Ordinal);
+		
+		// Verify the pattern "if (value is null)" followed by "return;" does NOT appear
+		// This would indicate an early return that we don't want for nullable target properties
+		var earlyReturnPattern = @"if\s*\(\s*value\s+is\s+null\s*\)\s*\{\s*return\s*;";
+		Assert.DoesNotMatch(earlyReturnPattern, generated);
+	}
+
+	[Fact]
+	public void NullableValueTypeAtEndOfConditionalAccessPath()
+	{
+		// Test nullable value type property (int?) at the end of a binding path with conditional access
+		// The setter should NOT have an early return since the target property accepts null
+		var xaml =
+"""
+<?xml version="1.0" encoding="UTF-8"?>
+<ContentPage
+	xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
+	xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
+	xmlns:test="clr-namespace:Test"
+	x:Class="Test.TestPage"
+	x:DataType="test:TestPage">
+	<Label Text="{Binding Container.OptionalCount}"/>
+</ContentPage>
+""";
+
+		var code =
+"""
+#nullable enable
+using System;
+using Microsoft.Maui.Controls;
+using Microsoft.Maui.Controls.Xaml;
+
+namespace Test;
+
+[XamlProcessing(XamlInflator.SourceGen)]
+public partial class TestPage : ContentPage
+{
+	public Container? Container { get; set; }
+}
+
+public class Container
+{
+	public int? OptionalCount { get; set; }  // Nullable int
+}
+""";
+
+		var (result, generated) = RunGenerator(xaml, code);
+		
+		Assert.False(result.Diagnostics.Any(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error));
+		Assert.NotNull(generated);
+		
+		// Verify setter uses pattern matching for conditional access
+		Assert.Contains("if (source.Container is {} p0)", generated, StringComparison.Ordinal);
+		
+		// Check the setter signature accepts nullable value type
+		Assert.Contains("global::System.Action<global::Test.TestPage, int?>", generated, StringComparison.Ordinal);
+		
+		// Verify no early return patterns for nullable value types
+		// Should NOT have "if (!value.HasValue) { return;" or "if (value is null) { return;"
+		var hasValueEarlyReturnPattern = @"if\s*\(\s*!\s*value\s*\.\s*HasValue\s*\)\s*\{\s*return\s*;";
+		var isNullEarlyReturnPattern = @"if\s*\(\s*value\s+is\s+null\s*\)\s*\{\s*return\s*;";
+		Assert.DoesNotMatch(hasValueEarlyReturnPattern, generated);
+		Assert.DoesNotMatch(isNullEarlyReturnPattern, generated);
+	}
 }
