ronantakizawa/codereview-bench huggingface dataset:
Repository: dotnet/maui
File: src/Controls/tests/SourceGen.UnitTests/InitializeComponent/CompiledBindings.cs (C#)

Code before:

public class TestViewModel
{
public override string ToString() => "ViewModel Value";
}
""";

var (result, generated) = RunGenerator(xaml, code);

// Verify it compiles without errors
Assert.Empty(result.Diagnostics);

// Verify generated code exists
Assert.NotNull(generated);

// Verify the generated code contains a TypedBinding for self-binding
Assert.Contains("TypedBinding<global::Test.TestViewModel, global::Test.TestViewModel>", generated, StringComparison.Ordinal);

// Verify the getter is the identity function for self-binding
Assert.Contains("getter: source => (source, true)", generated, StringComparison.Ordinal);

// Verify setter is null for self-bindings (not writable)
Assert.Contains("global::System.Action<global::Test.TestViewModel, global::Test.TestViewModel>? setter = null;", generated, StringComparison.Ordinal);

// Verify handlers array is empty for self-bindings
Assert.Contains("handlers: new global::System.Tuple<global::System.Func<global::Test.TestViewModel, object?>, string>[]", generated, StringComparison.Ordinal);
}
}


Review comment (suggestion):
@copilot I don't think this actually confirms the handlers array is empty -- this can follow by `{ ... }` syntax to initialize the array with items and BTW that's exactly what we would do in the case where there are any handlers in that array.

Code after:
	}
}

public class Product
{
	public string Name { get; set; } = "";
}
""";
		var testXamlFilePath = Path.Combine(Environment.CurrentDirectory, "Test.xaml");
		var expected = $$"""
//------------------------------------------------------------------------------
// <auto-generated>
//     This code was generated by a .NET MAUI source generator.
//
//     Changes to this file may cause incorrect behavior and will be lost if
//     the code is regenerated.
// </auto-generated>
//------------------------------------------------------------------------------
#nullable enable

namespace Test;

[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Maui.Controls.SourceGen, Version=10.0.0.0, Culture=neutral, PublicKeyToken=null", "10.0.0.0")]
public partial class TestPage
{
	private partial void InitializeComponent()
	{
		// Fallback to Runtime inflation if the page was updated by HotReload
		static string? getPathForType(global::System.Type type)
		{
			var assembly = type.Assembly;
			foreach (var xria in global::System.Reflection.CustomAttributeExtensions.GetCustomAttributes<global::Microsoft.Maui.Controls.Xaml.XamlResourceIdAttribute>(assembly))
			{
				if (xria.Type == type)
					return xria.Path;
			}
			return null;
		}

		var rlr = global::Microsoft.Maui.Controls.Internals.ResourceLoader.ResourceProvider2?.Invoke(new global::Microsoft.Maui.Controls.Internals.ResourceLoader.ResourceLoadingQuery
		{
			AssemblyName = typeof(global::Test.TestPage).Assembly.GetName(),
			ResourcePath = getPathForType(typeof(global::Test.TestPage)),
			Instance = this,
		});

		if (rlr?.ResourceContent != null)
		{
			this.InitializeComponentRuntime();
			return;
		}
Repository: dotnet/maui
File: src/Controls/tests/SourceGen.UnitTests/InitializeComponent/ExplicitPathBindingTest.cs (C#)

Code before:

namespace Test;

[XamlProcessing(XamlInflator.SourceGen)]
public partial class TestPage : ContentPage
{
	public string Name { get; set; } = "TestName";

	public TestPage()
	{
		InitializeComponent();
	}
}
""";

		var (result, generated) = RunGenerator(xaml, code);

		Assert.Empty(result.Diagnostics);
		Assert.NotNull(generated);

		// Should compile as a property binding
		Assert.Contains("TypedBinding<global::Test.TestPage, string>", generated, StringComparison.Ordinal);
	}

	[Fact]
	public void BindingWithExplicitPathEquals()
	{
		// This test verifies that {Binding Path=PropertyName} now works correctly after the fix
		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"
	Title="{Binding Path=Name}"/>
""";

		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

Review comment (suggestion):
These two tests are almost identical. Merge them into a single [Theory] with `"{Binding Name}"` and `"{Binding Path=Name}"` input values.

Code after:
using Microsoft.Maui.Controls;
using Microsoft.Maui.Controls.Xaml;

namespace Test;

[XamlProcessing(XamlInflator.SourceGen)]
public partial class TestPage : ContentPage
{
	public string Name { get; set; } = "TestName";

	public TestPage()
	{
		InitializeComponent();
	}
}
""";

		var (result, generated) = RunGenerator(xaml, code);

		Assert.Empty(result.Diagnostics);
		Assert.NotNull(generated);

		// Should compile as a property binding to Name property
		Assert.Contains("TypedBinding<global::Test.TestPage, string>", generated, StringComparison.Ordinal);
		Assert.Contains("source.Name", generated, StringComparison.Ordinal);
	}
}
Repository: dotnet/maui
File: src/Controls/tests/SourceGen.UnitTests/InitializeComponent/CompiledBindings.cs (C#)

Code before:
		var (result, generated) = RunGenerator(xaml, code);
		Assert.False(result.Diagnostics.Any());
		CodeIsEqual(expected, generated ?? string.Empty);
	}

	internal static void CodeIsEqual(string expectedCode, string actualCode)
	{
		var expectedLines = SplitCode(expectedCode);
		var actualLines = SplitCode(actualCode);

		foreach (var (expectedLine, actualLine) in expectedLines.Zip(actualLines))
		{
			Assert.Equal(expectedLine, actualLine);
		}

		Assert.Equal(expectedLines.Count(), actualLines.Count());

		static IEnumerable<string> SplitCode(string code)
			=> code.Split(Environment.NewLine)
				.Select(static line => line.Trim())
				.Where(static line => !string.IsNullOrWhiteSpace(line));
	}

	[Theory]
	[InlineData(".")]
	[InlineData("")]
	public void CanCompileSelfBinding(string bindingPath)
{
var bindingExpression = string.IsNullOrEmpty(bindingPath) ? "{Binding}" : $"{{Binding {bindingPath}}}";
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:TestViewModel">
<Label Text="{{bindingExpression}}"/>
</ContentPage>
""";

var code =
"""
#nullable enable
using System;
using Microsoft.Maui.Controls;
using Microsoft.Maui.Controls.Xaml;

namespace Test;


Review comment (suggestion):
Is there a valid way in XAML to express the self-referencing binding with explicilty stating Path? Is `"{Binding Path=.}"` valid? It seems it should be. Make sure that input is also one if the input data (please pass the full `"{Binding ...}"` in the InlineData value instead of deriving it later in code)

Code after:
		var (result, generated) = RunGenerator(xaml, code);
		Assert.False(result.Diagnostics.Any());
		CodeIsEqual(expected, generated ?? string.Empty);
	}

	internal static void CodeIsEqual(string expectedCode, string actualCode)
	{
		var expectedLines = SplitCode(expectedCode);
		var actualLines = SplitCode(actualCode);

		foreach (var (expectedLine, actualLine) in expectedLines.Zip(actualLines))
		{
			Assert.Equal(expectedLine, actualLine);
		}

		Assert.Equal(expectedLines.Count(), actualLines.Count());

		static IEnumerable<string> SplitCode(string code)
			=> code.Split(Environment.NewLine)
				.Select(static line => line.Trim())
				.Where(static line => !string.IsNullOrWhiteSpace(line));
	}

	[Fact]
	public void CorrectlyDetectsNullableReferenceTypes()
	{
		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 Product.Name}"/>
</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
{
Repository: dotnet/maui
File: src/Controls/tests/SourceGen.UnitTests/InitializeComponent/SetterCompiledConverters.cs (C#)

Code before:
using System.Linq;
using Xunit;

namespace Microsoft.Maui.Controls.SourceGen.UnitTests;

public class SetterCompiledConverters : SourceGenXamlInitializeComponentTestBase
{
	[Fact]
	public void SetterWithCompiledConverters_DoesNotGenerateDeadCode()
	{
		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" 
	x:Class="Test.TestPage">
	<ContentPage.Resources>
		<ResourceDictionary>
			<Style x:Key="testStyle" TargetType="Label">
				<Setter Property="FontSize" Value="16" />
				<Setter Property="TextColor" Value="Red" />
			</Style>
		</ResourceDictionary>
	</ContentPage.Resources>
	<Label x:Name="testLabel" Style="{StaticResource testStyle}" Text="Test" />
</ContentPage>
""";

		var code =
"""
using Microsoft.Maui.Controls;
using Microsoft.Maui.Controls.Xaml;

namespace Test;

[XamlProcessing(XamlInflator.SourceGen)]
public partial class TestPage : ContentPage
{
	public TestPage()
	{
		InitializeComponent();
	}
}
""";

		var testXamlFilePath = Path.Combine(Environment.CurrentDirectory, "Test.xaml");
		var expected =
$$"""
//------------------------------------------------------------------------------
// <auto-generated>

Review comment (suggestion):
@copilot Let's drop this Label. The `StaticResourceExtension` complicates the generated code and it's not really relevant to the original issue.

Code after:
using System.Linq;
using Xunit;

namespace Microsoft.Maui.Controls.SourceGen.UnitTests;

public class SetterCompiledConverters : SourceGenXamlInitializeComponentTestBase
{
	[Fact]
	public void SetterWithCompiledConverters_DoesNotGenerateDeadCode()
	{
		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" 
	x:Class="Test.TestPage">
	<ContentPage.Resources>
		<ResourceDictionary>
			<Style x:Key="testStyle" TargetType="Label">
				<Setter Property="FontSize" Value="16" />
				<Setter Property="TextColor" Value="Red" />
			</Style>
		</ResourceDictionary>
	</ContentPage.Resources>
</ContentPage>
""";

		var code =
"""
using Microsoft.Maui.Controls;
using Microsoft.Maui.Controls.Xaml;

namespace Test;

[XamlProcessing(XamlInflator.SourceGen)]
public partial class TestPage : ContentPage
{
	public TestPage()
	{
		InitializeComponent();
	}
}
""";

		var testXamlFilePath = Path.Combine(Environment.CurrentDirectory, "Test.xaml");
		var expected =
$$"""
//------------------------------------------------------------------------------
// <auto-generated>
//     This code was generated by a .NET MAUI source generator.
Repository: dotnet/maui
File: src/Controls/src/SourceGen/SetterValueProvider.cs (C#)

Code before:
			!node.Properties.TryGetValue(new XmlName(XamlParser.MauiUri, "Value"), out valueNode) &&
			node.CollectionItems.Count == 1)
			valueNode = node.CollectionItems[0];

		// Value must be a simple ValueNode (not a markup extension or element)
		if (valueNode is not ValueNode)
			return false;

		// TargetName (if present) must be a simple value
		if (node.Properties.TryGetValue(new XmlName("", "TargetName"), out var targetNode) &&
			targetNode is not ValueNode)
			return false;

		// All properties must be simple values (no ElementNode or MarkupNode)
		foreach (var prop in node.Properties.Values)
		{
			if (prop is not ValueNode)
				return false;
		}

		return true;
	}

	public bool TryProvideValue(ElementNode node, IndentedTextWriter writer, SourceGenContext context, NodeSGExtensions.GetNodeValueDelegate? getNodeValue, out ITypeSymbol? returnType, out string value)
	{
		return KnownMarkups.ProvideValueForSetter(node, writer, context, getNodeValue, out returnType, out value);
	}
}


Review comment (suggestion):
@copilot move the `KnownMarkups.ProvideValueForSetter` method here, so all the relevant code is here. If possbile, shere as much code as possible between `CanProvideValue` and `TryProvideValue`.

Code after:
		foreach (var prop in node.Properties.Values)
		{
			if (prop is MarkupNode or ElementNode)
				return false;
		}

		return true;
	}

	public bool TryProvideValue(ElementNode node, IndentedTextWriter writer, SourceGenContext context, NodeSGExtensions.GetNodeValueDelegate? getNodeValue, out ITypeSymbol? returnType, out string value)
	{
		returnType = context.Compilation.GetTypeByMetadataName("Microsoft.Maui.Controls.Setter")!;

		// Get the value node (shared logic with CanProvideValue)
		var valueNode = GetValueNode(node);
		if (valueNode == null)
		{
			value = string.Empty;
			return false;
		}

		var bpNode = (ValueNode)node.Properties[new XmlName("", "Property")];
		var bpRef = bpNode.GetBindableProperty(context);

		string targetsetter;
		if (node.Properties.TryGetValue(new XmlName("", "TargetName"), out var targetNode))
			targetsetter = $"TargetName = \"{((ValueNode)targetNode).Value}\", ";
		else
			targetsetter = "";

		if (valueNode is ValueNode vn)
		{
			value = $"new global::Microsoft.Maui.Controls.Setter {{{targetsetter}Property = {bpRef.ToFQDisplayString()}, Value = {vn.ConvertTo(bpRef, writer, context)}}}";
			return true;
		}
		else if (getNodeValue != null)
		{
			var lvalue = getNodeValue(valueNode, bpRef.Type);
			value = $"new global::Microsoft.Maui.Controls.Setter {{{targetsetter}Property = {bpRef.ToFQDisplayString()}, Value = {lvalue.ValueAccessor}}}";
			return true;
		}
		else if (context.Variables.TryGetValue(valueNode, out var variable))
		{
			value = $"new global::Microsoft.Maui.Controls.Setter {{{targetsetter}Property = {bpRef.ToFQDisplayString()}, Value = {variable.ValueAccessor}}}";
			return true;
		}

		value = string.Empty;
		//FIXME context.ReportDiagnostic
		return false;
	}
Repository: dotnet/maui
File: src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.Android.cs (C#)

Code before:
			}
			void OnAnimationEnded(object? sender, AAnimation.AnimationEndEventArgs e)
			{
				if (sender is not AAnimation animation)
					return;

				animation.AnimationEnd -= OnAnimationEnded;
				FireAnimationEnded();
			}

			public override void OnCreate(Bundle? savedInstanceState)
			{
				base.OnCreate(savedInstanceState);
				SetStyle(DialogFragment.StyleNormal, Resource.Style.Maui_MainTheme_NoActionBar);
			}

			public override void OnStart()
			{
				base.OnStart();

				var dialog = Dialog;

				if (dialog is null || dialog.Window is null || View is null)
				{
					// SAFETY: Fire event even on early return to prevent deadlock
					FirePresentationCompleted();
					return;
				}

				int width = ViewGroup.LayoutParams.MatchParent;
				int height = ViewGroup.LayoutParams.MatchParent;
				dialog.Window.SetLayout(width, height);

				// Signal that the modal is fully presented and ready
				FirePresentationCompleted();
			}

			public override void OnDismiss(IDialogInterface dialog)
			{
				_modal.PropertyChanged -= OnModalPagePropertyChanged;
				_modal.HandlerChanged -= OnPageHandlerChanged;

				if (_modal.Toolbar?.Handler is not null)
				{
					_modal.Toolbar.Handler = null;
				}

				_modal.Handler = null;
				_modal = null!;
				_mauiWindowContext = null!;
				_navigationRootManager?.Disconnect();

Review comment (bug):
I would say we don't need this here. I added those `null` check here to make the nullable analyser happy. If this results in `true`, something is very wrong.

Code after:
			}
			void OnAnimationEnded(object? sender, AAnimation.AnimationEndEventArgs e)
			{
				if (sender is not AAnimation animation)
					return;

				animation.AnimationEnd -= OnAnimationEnded;
				FireAnimationEnded();
			}

			public override void OnCreate(Bundle? savedInstanceState)
			{
				base.OnCreate(savedInstanceState);
				SetStyle(DialogFragment.StyleNormal, Resource.Style.Maui_MainTheme_NoActionBar);
			}

			public override void OnStart()
			{
				base.OnStart();

				var dialog = Dialog;

				if (dialog is null || dialog.Window is null || View is null)
				{
					// SAFETY: Fire event even on early return to prevent deadlock
					FirePresentationCompleted();
					return;
				}

				int width = ViewGroup.LayoutParams.MatchParent;
				int height = ViewGroup.LayoutParams.MatchParent;
				dialog.Window.SetLayout(width, height);
			}

			public override void OnResume()
			{
				base.OnResume();

				// Signal that the modal is fully presented and ready
				FirePresentationCompleted();
			}

			public override void OnDismiss(IDialogInterface dialog)
			{
				_modal.PropertyChanged -= OnModalPagePropertyChanged;
				_modal.HandlerChanged -= OnPageHandlerChanged;

				if (_modal.Toolbar?.Handler is not null)
				{
					_modal.Toolbar.Handler = null;
				}
Repository: dotnet/maui
File: src/Controls/src/Core/Platform/ModalNavigationManager/ModalNavigationManager.Android.cs (C#)

Code before:
			}

			public override void OnStart()
			{
				base.OnStart();

				var dialog = Dialog;

				if (dialog is null || dialog.Window is null || View is null)
				{
					// SAFETY: Fire event even on early return to prevent deadlock
					FirePresentationCompleted();
					return;
				}

				int width = ViewGroup.LayoutParams.MatchParent;
				int height = ViewGroup.LayoutParams.MatchParent;
				dialog.Window.SetLayout(width, height);

				// Signal that the modal is fully presented and ready
				FirePresentationCompleted();
			}

			public override void OnDismiss(IDialogInterface dialog)
			{
				_modal.PropertyChanged -= OnModalPagePropertyChanged;
				_modal.HandlerChanged -= OnPageHandlerChanged;

				if (_modal.Toolbar?.Handler is not null)
				{
					_modal.Toolbar.Handler = null;
				}

				_modal.Handler = null;
				_modal = null!;
				_mauiWindowContext = null!;
				_navigationRootManager?.Disconnect();
				_navigationRootManager = null;
				base.OnDismiss(dialog);
			}

			public override void OnDestroy()
			{
				base.OnDestroy();
				FireAnimationEnded();

				// SAFETY: If destroyed before OnStart completed, fire PresentationCompleted to prevent deadlock
				FirePresentationCompleted();
			}

			void FireAnimationEnded()

Review comment (suggestion):

From what I investigated, the best place to add Fire it is on `OnResume`. `OnStart` will complete before the page is presented on the screen.

Code after:
			}

			public override void OnStart()
			{
				base.OnStart();

				var dialog = Dialog;

				if (dialog is null || dialog.Window is null || View is null)
				{
					// SAFETY: Fire event even on early return to prevent deadlock
					FirePresentationCompleted();
					return;
				}

				int width = ViewGroup.LayoutParams.MatchParent;
				int height = ViewGroup.LayoutParams.MatchParent;
				dialog.Window.SetLayout(width, height);
			}

			public override void OnResume()
			{
				base.OnResume();

				// Signal that the modal is fully presented and ready
				FirePresentationCompleted();
			}

			public override void OnDismiss(IDialogInterface dialog)
			{
				_modal.PropertyChanged -= OnModalPagePropertyChanged;
				_modal.HandlerChanged -= OnPageHandlerChanged;

				if (_modal.Toolbar?.Handler is not null)
				{
					_modal.Toolbar.Handler = null;
				}

				_modal.Handler = null;
				_modal = null!;
				_mauiWindowContext = null!;
				_navigationRootManager?.Disconnect();
				_navigationRootManager = null;
				base.OnDismiss(dialog);
			}

			public override void OnDestroy()
			{
				base.OnDestroy();
				FireAnimationEnded();
Repository: dotnet/maui
File: src/Controls/tests/BindingSourceGen.UnitTests/IntegrationTests.cs (C#)

Code before:

							};
						bindableObject.SetBinding(bindableProperty, binding);
					}
				}
			}
			""",
			result.GeneratedFiles["Path-To-Program.cs-GeneratedBindingInterceptors-14-11.g.cs"]);
	}

	[Fact]
	public void GenerateBindingWithNullableObjectAndNullableReferenceTypeProperty_SetBinding()
	{
		var source = """
		using Microsoft.Maui.Controls;
		var label = new Label();
		label.SetBinding(Label.TextProperty, static (Foo f) => f.NullableObject?.StringProperty);

		class Foo
		{
			public Bar? NullableObject { get; set; }
		}

		class Bar
		{
			public string? StringProperty { get; set; }
		}
		""";

		var result = SourceGenHelpers.Run(source);
		Assert.NotNull(result.Binding);

		var id = Math.Abs(result.Binding.SimpleLocation!.GetHashCode());

		// The key assertion: no CS8603 errors should be present
		AssertExtensions.AssertNoDiagnostics(result);
		AssertExtensions.CodeIsEqual(
			$$"""
			//------------------------------------------------------------------------------
			// <auto-generated>
			//     This code was generated by a .NET MAUI source generator.
			//
			//     Changes to this file may cause incorrect behavior and will be lost if
			//     the code is regenerated.
			// </auto-generated>
			//------------------------------------------------------------------------------
			#nullable enable

			namespace System.Runtime.CompilerServices
			{
			    using System;

Review comment (suggestion):
I think that to make this test capture the reported issue, `StringProperty` should not be nullable. The nullability of the getter return type comes from `NullableObject` being nullable.

Code after:

							};
						bindableObject.SetBinding(bindableProperty, binding);
					}
				}
			}
			""",
			result.GeneratedFiles["Path-To-Program.cs-GeneratedBindingInterceptors-14-11.g.cs"]);
	}

	[Fact]
	public void GenerateBindingWithNullableObjectAndNullableReferenceTypeProperty_SetBinding()
	{
		var source = """
		using Microsoft.Maui.Controls;
		var label = new Label();
		label.SetBinding(Label.TextProperty, static (Foo f) => f.NullableObject?.StringProperty);

		class Foo
		{
			public Bar? NullableObject { get; set; }
		}

		class Bar
		{
			public string StringProperty { get; set; } = "";
		}
		""";

		var result = SourceGenHelpers.Run(source);
		Assert.NotNull(result.Binding);

		var id = Math.Abs(result.Binding.SimpleLocation!.GetHashCode());

		// The key assertion: no CS8603 errors should be present
		AssertExtensions.AssertNoDiagnostics(result);
		AssertExtensions.CodeIsEqual(
			$$"""
			//------------------------------------------------------------------------------
			// <auto-generated>
			//     This code was generated by a .NET MAUI source generator.
			//
			//     Changes to this file may cause incorrect behavior and will be lost if
			//     the code is regenerated.
			// </auto-generated>
			//------------------------------------------------------------------------------
			#nullable enable

			namespace System.Runtime.CompilerServices
			{
			    using System;