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

Code before:
#if !_MAUIXAML_SG_NAMESCOPE_DISABLE
		global::Microsoft.Maui.Controls.Internals.NameScope.SetNameScope(__root, iNameScope);
#endif
#if !_MAUIXAML_SG_NAMESCOPE_DISABLE
		label.transientNamescope = iNameScope;
#endif
#line 6 "{{testXamlFilePath}}"
		label.SetValue(global::Microsoft.Maui.Controls.Label.FontSizeProperty, staticExtension);
#line default
#line 6 "{{testXamlFilePath}}"
		__root.SetValue(global::Microsoft.Maui.Controls.ContentPage.ContentProperty, label);
#line default
	}
}

""";

		var additionalCode = """
namespace MyApp.Config
{
	public static class Settings
	{
		public static double DefaultFontSize => 14.0;
	}
}
""";

		var (result, generated) = RunGenerator(xaml, code, assertNoCompilationErrors: true, additionalCode: additionalCode);
		Assert.False(result.Diagnostics.Any());
		Assert.Equal(expected, generated, ignoreLineEndingDifferences: true);
	}
}


Review comment (suggestion):
@copilot I think I misunderstood the problem previously. This bit of code can be part of the `code` string value. Just don't use file scoped namespaces. Apply this change to all the other relevant locations in this PR.

Code after:
		var label = new global::Microsoft.Maui.Controls.Label();
		global::Microsoft.Maui.VisualDiagnostics.RegisterSourceInfo(label!, new global::System.Uri(@"Test.xaml;assembly=SourceGeneratorDriver.Generated", global::System.UriKind.Relative), 6, 6);
		var __root = this;
		global::Microsoft.Maui.VisualDiagnostics.RegisterSourceInfo(__root!, new global::System.Uri(@"Test.xaml;assembly=SourceGeneratorDriver.Generated", global::System.UriKind.Relative), 2, 2);
#if !_MAUIXAML_SG_NAMESCOPE_DISABLE
		global::Microsoft.Maui.Controls.Internals.INameScope iNameScope = global::Microsoft.Maui.Controls.Internals.NameScope.GetNameScope(__root) ?? new global::Microsoft.Maui.Controls.Internals.NameScope();
#endif
#if !_MAUIXAML_SG_NAMESCOPE_DISABLE
		global::Microsoft.Maui.Controls.Internals.NameScope.SetNameScope(__root, iNameScope);
#endif
#if !_MAUIXAML_SG_NAMESCOPE_DISABLE
		label.transientNamescope = iNameScope;
#endif
#line 6 "{{testXamlFilePath}}"
		label.SetValue(global::Microsoft.Maui.Controls.Label.FontSizeProperty, staticExtension);
#line default
#line 6 "{{testXamlFilePath}}"
		__root.SetValue(global::Microsoft.Maui.Controls.ContentPage.ContentProperty, label);
#line default
	}
}

""";

		var (result, generated) = RunGenerator(xaml, code, assertNoCompilationErrors: true);
		Assert.False(result.Diagnostics.Any());
		Assert.Equal(expected, generated, ignoreLineEndingDifferences: true);
	}
}
Repository: dotnet/maui
File: src/Controls/src/Core/AppThemeBinding.cs (C#)

Code before:
#nullable disable
using System;
using Microsoft.Maui.ApplicationModel;
using Microsoft.Maui.Controls.Internals;
using Microsoft.Maui.Controls.Xaml.Diagnostics;

namespace Microsoft.Maui.Controls
{
#if NET11_0_OR_GREATER
	public // TODO: add to PublicAPI.Unshipped.txt once we start targetting net11
#endif
	class AppThemeBinding : BindingBase
	{
		public const string AppThemeResource = "__MAUI_ApplicationTheme__";
		class AppThemeProxy : Element
		{
			public AppThemeProxy(Element parent, AppThemeBinding binding)
			{
				_parent = parent;
				Binding = binding;
				this.SetDynamicResource(AppThemeProperty, AppThemeResource);
				((IElementDefinition)parent)?.AddResourcesChangedListener(OnParentResourcesChanged);
			}

			public static BindableProperty AppThemeProperty = BindableProperty.Create("AppTheme", typeof(AppTheme), typeof(AppThemeBinding), AppTheme.Unspecified,
				propertyChanged: (bindable, oldValue, newValue) => ((AppThemeProxy)bindable).OnAppThemeChanged());
			readonly Element _parent;

			public void OnAppThemeChanged()
			{
				Binding.Apply(false);
			}

			public AppThemeBinding Binding { get; }

			public void Unsubscribe()
			{
				((IElementDefinition)_parent)?.RemoveResourcesChangedListener(OnParentResourcesChanged);
			}

Review comment (suggestion):
```suggestion
		internal const string AppThemeResource = "__MAUI_ApplicationTheme__";
```

Code after:
#nullable disable
using System;
using Microsoft.Maui.ApplicationModel;
using Microsoft.Maui.Controls.Internals;
using Microsoft.Maui.Controls.Xaml.Diagnostics;

namespace Microsoft.Maui.Controls
{
#if NET11_0_OR_GREATER
	public // TODO: add to PublicAPI.Unshipped.txt once we start targeting net11
#endif
	class AppThemeBinding : BindingBase
	{
		internal const string AppThemeResource = "__MAUI_ApplicationTheme__";
		class AppThemeProxy : Element
		{
			public AppThemeProxy(Element parent, AppThemeBinding binding)
			{
				_parent = parent;
				Binding = binding;
				this.SetDynamicResource(AppThemeProperty, AppThemeResource);
				((IElementDefinition)parent)?.AddResourcesChangedListener(OnParentResourcesChanged);
			}

			public static BindableProperty AppThemeProperty = BindableProperty.Create("AppTheme", typeof(AppTheme), typeof(AppThemeBinding), AppTheme.Unspecified,
				propertyChanged: (bindable, oldValue, newValue) => ((AppThemeProxy)bindable).OnAppThemeChanged());
			readonly Element _parent;

			public void OnAppThemeChanged()
			{
				Binding.Apply(false);
			}

			public AppThemeBinding Binding { get; }

			public void Unsubscribe()
			{
				((IElementDefinition)_parent)?.RemoveResourcesChangedListener(OnParentResourcesChanged);
			}
Repository: dotnet/maui
File: src/Controls/tests/SourceGen.UnitTests/InitializeComponent/SimplifyOnPlatform.cs (C#)

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

namespace Test;

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

		// Test with MacCatalyst where platform is not defined
		var (result, generated) = RunGenerator(xaml, code, targetFramework: "net10.0-maccatalyst");
		
		// Should not have any errors (no TargetInvocationException)
		Assert.False(result.Diagnostics.Any(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error));

		// Should generate code that creates the resource with default value
		// The resource key should still be present
		Assert.Contains("SizeMedium", generated, StringComparison.Ordinal);
		
		// The OnPlatform should be simplified to a default Double value
		Assert.DoesNotContain("OnPlatform", generated, StringComparison.Ordinal);
	}
}

Review comment (suggestion):
This assert does not tell us much. Let's assert that there is `.SetValue(global::....WidthRequestProperty, default(double))` here (please make sure to determine what exactly is in the generated code, I want it clearly visible in the test).

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

namespace Test;

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

		// Test with MacCatalyst where platform is not defined
		var (result, generated) = RunGenerator(xaml, code, targetFramework: "net10.0-maccatalyst");
		
		// Should not have any errors (no TargetInvocationException)
		Assert.False(result.Diagnostics.Any(d => d.Severity == Microsoft.CodeAnalysis.DiagnosticSeverity.Error));
		
		// Should generate SetValue with default(double) for the WidthRequest property
		// The generated code should look like: label.SetValue(global::Microsoft.Maui.Controls.VisualElement.WidthRequestProperty, double1);
		// where double1 is assigned from double0 which is: double double0 = default;
		Assert.Contains("double double0 = default;", generated, StringComparison.Ordinal);
		Assert.Contains("var double1 = double0;", generated, StringComparison.Ordinal);
		Assert.Contains("label.SetValue(global::Microsoft.Maui.Controls.VisualElement.WidthRequestProperty, double1);", generated, StringComparison.Ordinal);
	}
}
Repository: dotnet/maui
File: src/Controls/tests/BindingSourceGen.UnitTests/RelayCommandTests.cs (C#)

Code before:
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.Maui.Controls.BindingSourceGen;
using Xunit;

namespace BindingSourceGen.UnitTests;

public class RelayCommandTests
{
	// Note: C# lambda bindings (SetBinding with static lambda expressions) cannot work with
	// source-generated properties like those from [RelayCommand] because the semantic model
	// needs to resolve the lambda return type before source generators run.
	// 
	// The RelayCommand support is primarily for XAML string-based bindings which use
	// CompiledBindingMarkup.cs and the TryGetProperty extension method.
	//
	// These tests verify the behavior and document the limitation.

	[Fact]
	public void RelayCommandPropertyNotResolvedInLambdaBinding()
	{
		// This test documents that C# lambda bindings don't work with source-generated properties
		var source = """
			using Microsoft.Maui.Controls;
			using System.Threading.Tasks;

			namespace System.Windows.Input
			{
				public interface ICommand
				{
					event System.EventHandler CanExecuteChanged;
					bool CanExecute(object parameter);
					void Execute(object parameter);
				}
			}

			namespace CommunityToolkit.Mvvm.Input
			{

Review comment (suggestion):
@copilot Now this is not true. Expression-based bindings can work the same way the XAML compiled bindings work. The only problem is in tests, where the source generator will produce output that cannot be compiled, since we don't have also the code from the other source generator that actually produces the properties. What we should do it tests is to just assert that the generated interceptor code looks the way we expect it to without looking at the diagnostics/build errors.

Code after:
using System.Linq;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.Maui.Controls.BindingSourceGen;
using Xunit;

namespace BindingSourceGen.UnitTests;

public class RelayCommandTests
{
	// Note: Expression-based bindings work with RelayCommand through type inference in
	// GetLambdaReturnType. The generated interceptor code will be correct. In tests,
	// we'll see compilation errors because the actual SaveCommand property doesn't exist
	// (CommunityToolkit.Mvvm's source generator isn't running in the test environment),
	// but the interceptor code itself is generated correctly.

	[Fact]
	public void GenerateBindingToRelayCommandProperty()
	{
		var source = """
			using Microsoft.Maui.Controls;
			using System.Threading.Tasks;

			namespace System.Windows.Input
			{
				public interface ICommand
				{
					event System.EventHandler CanExecuteChanged;
					bool CanExecute(object parameter);
					void Execute(object parameter);
				}
			}

			namespace CommunityToolkit.Mvvm.Input
			{
				[System.AttributeUsage(System.AttributeTargets.Method)]
				public class RelayCommandAttribute : System.Attribute { }
			}
Repository: dotnet/maui
File: src/Controls/tests/BindingSourceGen.UnitTests/ObservablePropertyTests.cs (C#)

Code before:
				public class MyViewModel
				{
					[CommunityToolkit.Mvvm.ComponentModel.ObservableProperty]
					private ObservableCollection<Tag> _tags = new();
					// Tags property will be generated by CommunityToolkit.Mvvm
				}

				public class TestCode
				{
					public void Test()
					{
						var label = new Label();
						label.SetBinding(Label.BindingContextProperty, static (MyViewModel vm) => vm.Tags);
					}
				}
			}
			""";

		var result = SourceGenHelpers.Run(source);

		// The binding should be generated successfully with ObservableProperty inference
		Assert.NotNull(result.Binding);

		// Verify the generated interceptor code contains the Tags property access
		var allGeneratedCode = string.Join("\n\n", result.GeneratedFiles.Values);
		Assert.Contains("Tags", allGeneratedCode, System.StringComparison.Ordinal);
	}

	[Fact]
	public void DetectsObservablePropertyFromCamelCaseField()
	{
		// This test verifies that TryGetObservablePropertyType can detect ObservableProperty fields
		var source = """
			using System.Collections.ObjectModel;

			namespace CommunityToolkit.Mvvm.ComponentModel
			{
				[System.AttributeUsage(System.AttributeTargets.Field)]
				public class ObservablePropertyAttribute : System.Attribute { }
			}

			namespace TestApp
			{
				public class MyViewModel
				{
					[CommunityToolkit.Mvvm.ComponentModel.ObservableProperty]
					private string? name;
				}
			}
			""";


Review comment (bug):
Let's improve the assert to check for the full expected getter and setter: `"source => (source.Tags, true)"`, `source.Tags = value;`.

I might be wrong about the exact values of the expected generated expressions, so please double check the actual outputs.

Code after:
			}

			namespace TestApp
			{
				public class Tag { }

				public class MyViewModel
				{
					[CommunityToolkit.Mvvm.ComponentModel.ObservableProperty]
					private ObservableCollection<Tag> _tags = new();
					// Tags property will be generated by CommunityToolkit.Mvvm
				}

				public class TestCode
				{
					public void Test()
					{
						var label = new Label();
						label.SetBinding(Label.BindingContextProperty, static (MyViewModel vm) => vm.Tags);
					}
				}
			}
			""";

		var result = SourceGenHelpers.Run(source);

		// The binding should be generated successfully with ObservableProperty inference
		Assert.NotNull(result.Binding);

		// Verify the generated interceptor code contains the correct getter and setter references
		var allGeneratedCode = string.Join("\n\n", result.GeneratedFiles.Values);
		// Check that the handler contains the property access
		Assert.Contains("new(static source => source, \"Tags\")", allGeneratedCode, System.StringComparison.Ordinal);
		// Check that setter assigns to .Tags
		Assert.Contains("source.Tags = value;", allGeneratedCode, System.StringComparison.Ordinal);
	}
}
Repository: dotnet/maui
File: src/Graphics/src/Graphics/Color.cs (C#)

Code before:
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Diagnostics;
using System.Globalization;
using System.Numerics;

namespace Microsoft.Maui.Graphics
{
	/// <summary>
	/// Represents an RGBA color with floating-point components in the range of 0.0 to 1.0.
	/// </summary>
	[DebuggerDisplay("Red={Red}, Green={Green}, Blue={Blue}, Alpha={Alpha}")]
	[TypeConverter(typeof(Converters.ColorTypeConverter))]
	[ImmutableObject(true)]
	public class Color
	{
		/// <summary>
		/// The red component of the color, ranging from 0.0 to 1.0.
		/// </summary>
		public readonly float Red;

		/// <summary>
		/// The green component of the color, ranging from 0.0 to 1.0.
		/// </summary>
		public readonly float Green;

		/// <summary>
		/// The blue component of the color, ranging from 0.0 to 1.0.
		/// </summary>
		public readonly float Blue;

		/// <summary>
		/// The alpha (opacity) component of the color, ranging from 0.0 (transparent) to 1.0 (opaque).
		/// </summary>
		public readonly float Alpha = 1;

		/// <summary>
		/// Initializes a new instance of the <see cref="Color"/> class with default values (black).
		/// </summary>
		public Color()

Review comment (suggestion):
For Immutability wouldn't make sense to change this to be `record`? I'm not sure if it will be a binary breaking change thought

Code after:
using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Diagnostics;
using System.Globalization;
using System.Numerics;

namespace Microsoft.Maui.Graphics
{
	/// <summary>
	/// Represents an RGBA color with floating-point components in the range of 0.0 to 1.0.
	/// </summary>
	[DebuggerDisplay("Red={Red}, Green={Green}, Blue={Blue}, Alpha={Alpha}")]
	[TypeConverter(typeof(Converters.ColorTypeConverter))]
	[ImmutableObject(true)]
	public record class Color
	{
		/// <summary>
		/// The red component of the color, ranging from 0.0 to 1.0.
		/// </summary>
		public readonly float Red;

		/// <summary>
		/// The green component of the color, ranging from 0.0 to 1.0.
		/// </summary>
		public readonly float Green;

		/// <summary>
		/// The blue component of the color, ranging from 0.0 to 1.0.
		/// </summary>
		public readonly float Blue;

		/// <summary>
		/// The alpha (opacity) component of the color, ranging from 0.0 (transparent) to 1.0 (opaque).
		/// </summary>
		public readonly float Alpha = 1;

		/// <summary>
		/// Initializes a new instance of the <see cref="Color"/> class with default values (black).
		/// </summary>
		public Color()
Repository: dotnet/maui
File: src/Controls/tests/Xaml.UnitTests/Issues/Maui33867.xaml.cs (C#)

Code before:
		{
			// This test reproduces issue #33867:
			// XSG doesn't correctly parse inline StyleSheet elements with CDATA sections.
			if (inflator == XamlInflator.SourceGen)
			{
				var result = CreateMauiCompilation()
					.WithAdditionalSource(
"""
namespace Microsoft.Maui.Controls.Xaml.UnitTests;

public partial class Maui33867 : ContentPage
{
	public Maui33867() => InitializeComponent();
}
""")
					.RunMauiSourceGenerator(typeof(Maui33867));
				
				// Verify no compilation errors
				Assert.Empty(result.Diagnostics);
			}
			else
			{
				var page = new Maui33867(inflator);
				Assert.NotNull(page);
				Assert.NotNull(page.testLabel);
			}
		}
	}
}


Review comment (bug):
I believe we should actually inflate the page even with source gen, right? This `else` is incorrect.

Code after:
		{
			// This test reproduces issue #33867:
			// XSG doesn't correctly parse inline StyleSheet elements with CDATA sections.
			if (inflator == XamlInflator.SourceGen)
			{
				var result = CreateMauiCompilation()
					.WithAdditionalSource(
"""
namespace Microsoft.Maui.Controls.Xaml.UnitTests;

public partial class Maui33867 : ContentPage
{
	public Maui33867() => InitializeComponent();
}
""")
					.RunMauiSourceGenerator(typeof(Maui33867));
				
				// Verify no compilation errors
				Assert.Empty(result.Diagnostics);
			}

			var page = new Maui33867(inflator);
			Assert.NotNull(page);
			Assert.NotNull(page.testLabel);
		}
	}
}
Repository: dotnet/roslyn
File: src/Workspaces/MSBuild/Core/MSBuild/MSBuildProjectLoader.cs (C#)

Code before:

        public DefaultBinLogPathProvider(string logFilePath)
        {
            Contract.ThrowIfNull(logFilePath);

            _path = Path.GetDirectoryName(logFilePath) ?? ".";
            _filename = Path.GetFileNameWithoutExtension(logFilePath) is { Length: > 0 } fileName
                ? fileName
                : DefaultFileName;
            _extension = Path.GetExtension(logFilePath) is { Length: > 0 } extension
                ? extension
                : DefaultExtension;
        }

        public string? GetNewLogPath()
        {
            var newLogPath = _suffix == 0
                ? Path.Combine(_path, _filename + _extension)
                : Path.Combine(_path, $"{_filename}-{_suffix}{_extension}");

            _suffix++;
            return newLogPath;
        }
    }
}


Review comment (suggestion):
I think this could be called in parallel, right? Probably best to switch the increment to an Interlocked.Increment just to be sfae.

Code after:
    }

    internal sealed class BinLogPathProvider : IBinLogPathProvider
    {
        private const string DefaultFileName = "msbuild";
        private const string DefaultExtension = ".binlog";

        private readonly string _directory;
        private readonly string _filename;
        private readonly string _extension;
        private int _suffix = -1;

        public BinLogPathProvider(string? logFilePath)
        {
            logFilePath ??= DefaultFileName + DefaultExtension;

            _directory = Path.GetDirectoryName(logFilePath) ?? ".";
            _filename = Path.GetFileNameWithoutExtension(logFilePath) is { Length: > 0 } fileName
                ? fileName
                : DefaultFileName;
            _extension = Path.GetExtension(logFilePath) is { Length: > 0 } extension
                ? extension
                : DefaultExtension;
        }

        public string? GetNewLogPath()
        {
            var suffix = Interlocked.Increment(ref _suffix);

            var newPath = suffix == 0
                ? Path.Combine(_directory, _filename + _extension)
                : Path.Combine(_directory, $"{_filename}-{suffix}{_extension}");

            return Path.GetFullPath(newPath);
        }
    }
}
Repository: dotnet/roslyn
File: src/Compilers/CSharp/Portable/Syntax/ParameterSyntax.cs (C#)

Code before:
﻿// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

namespace Microsoft.CodeAnalysis.CSharp.Syntax
{
    public partial class ParameterSyntax
    {
        internal bool IsArgList
        {
            get
            {
                return this.Type == null && this.Identifier.ContextualKind() == SyntaxKind.ArgListKeyword;
            }
        }

        private partial void Validate()
        {
            if (Type is null && Identifier.IsKind(SyntaxKind.None))
            {
                throw new System.NotSupportedException(CSharpResources.ParameterRequiresTypeOrIdentifier);
            }
        }
    }
}


Review comment (suggestion):
>NotSupportedException [](http://example.com/codeflow?start=33&length=21)

It feels like this should be a flavor of an "invalid argument" exception. #Closed

Code after:
﻿// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

namespace Microsoft.CodeAnalysis.CSharp.Syntax
{
    public partial class ParameterSyntax
    {
        internal bool IsArgList
        {
            get
            {
                return this.Type == null && this.Identifier.ContextualKind() == SyntaxKind.ArgListKeyword;
            }
        }

        private partial void Validate()
        {
            if (Type is null && Identifier.IsKind(SyntaxKind.None))
            {
                throw new System.ArgumentException(CSharpResources.ParameterRequiresTypeOrIdentifier);
            }
        }
    }
}
Repository: dotnet/roslyn
File: src/Compilers/CSharp/Portable/Syntax/ParameterSyntax.cs (C#)

Code before:
﻿// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

namespace Microsoft.CodeAnalysis.CSharp.Syntax
{
    public partial class ParameterSyntax
    {
        internal bool IsArgList
        {
            get
            {
                return this.Type == null && this.Identifier.ContextualKind() == SyntaxKind.ArgListKeyword;
            }
        }

        private partial void Validate()
        {
            if (Type is null && Identifier.IsKind(SyntaxKind.None))
            {
                throw new System.NotSupportedException(CSharpResources.ParameterRequiresTypeOrIdentifier);
            }
        }
    }
}


Review comment (suggestion):
>throw new System.NotSupportedException(CSharpResources.ParameterRequiresTypeOrIdentifier); [](http://example.com/codeflow?start=16&length=90)

Is there any factory method that will always end-up here no matter what?  #Closed

Code after:
﻿// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

namespace Microsoft.CodeAnalysis.CSharp.Syntax
{
    public partial class ParameterSyntax
    {
        internal bool IsArgList
        {
            get
            {
                return this.Type == null && this.Identifier.ContextualKind() == SyntaxKind.ArgListKeyword;
            }
        }

        private partial void Validate()
        {
            if (Type is null && Identifier.IsKind(SyntaxKind.None))
            {
                throw new System.ArgumentException(CSharpResources.ParameterRequiresTypeOrIdentifier);
            }
        }
    }
}