[XSG] better AppThemeBinding

[StephaneDelcroix]

Description of Change

bette AppThemeBinding for non-valueNodes

Issues Fixed

[Copilot]

Pull Request Overview

This pull request enhances the XAML source generator to support type conversion for StaticResource and AppThemeBinding markup extensions. The changes enable proper handling of value nodes and element nodes within these extensions, improving type safety and conversion accuracy.

Key changes:

• Added support for type conversion in StaticResource when the resource is a string value that needs conversion to the target property type
• Implemented proper handling of AppThemeBinding with Light, Dark, and Default properties, including type conversion for value nodes
• Extended the GetNodeValueDelegate parameter through the markup extension processing pipeline

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/Controls/tests/SourceGen.UnitTests/InitializeComponent/StaticResourceWithTypeConversion.cs	New test file validating StaticResource type conversion for Grid row/column definitions
src/Controls/tests/SourceGen.UnitTests/InitializeComponent/AppThemeBinding.cs	New comprehensive test suite for AppThemeBinding with Light, Dark, and Default theme combinations
src/Controls/src/SourceGen/Visitors/SetPropertiesVisitor.cs	Updated to pass getNodeValue delegate when calling TryProvideValue
src/Controls/src/SourceGen/NodeSGExtensions.cs	Added overload for TryProvideValue accepting GetNodeValueDelegate parameter
src/Controls/src/SourceGen/KnownMarkups.cs	Major enhancements: added TypeConverter attribute lookup for StaticResource, refactored AppThemeBinding to support both ValueNode and ElementNode with proper type conversion

[Copilot]

Variable shadowing issue: The variable pv is declared twice within the same scope using the pattern context.Variables.TryGetValue(node.Parent, out var pv). While this compiles, it creates unnecessary variable declarations. Consider declaring pv once before the if-else chain and reusing it.

Suggested change

	if (targetNode is ValueNode vn)
	{
	// For ValueNode, convert directly using the property type
	if (bpFieldSymbol is not null)
	return vn.ConvertTo(bpFieldSymbol, writer, context, context.Variables.TryGetValue(node.Parent, out var pv) ? pv : null);
	else if (propertyType is not null)
	return vn.ConvertTo(propertyType, writer, context, context.Variables.TryGetValue(node.Parent, out var pv) ? pv : null);
	ILocalValue? pv = null;
	if (targetNode is ValueNode vn)
	{
	// For ValueNode, convert directly using the property type
	context.Variables.TryGetValue(node.Parent, out pv);
	if (bpFieldSymbol is not null)
	return vn.ConvertTo(bpFieldSymbol, writer, context, pv);
	else if (propertyType is not null)
	return vn.ConvertTo(propertyType, writer, context, pv);

[Copilot]

Inconsistent assertion style: This test uses Assert.Empty(result.Diagnostics) while most other tests in the same directory use Assert.False(result.Diagnostics.Any()). For consistency with the existing codebase, consider using Assert.False(result.Diagnostics.Any()) instead.

Suggested change

	Assert.Empty(result.Diagnostics);
	Assert.False(result.Diagnostics.Any());

[Copilot]

Potential null reference exception: On line 693, propertyType could be null if both typeandconverter?.type and propertySymbol?.Type are null. The check on line 697 only verifies that propertySymbol is not null, but doesn't guarantee that propertyType (i.e., propertySymbol.Type) is not null. Line 699 then uses propertyType!.GetAttributes() which could throw a NullReferenceException. Consider adding a null check for propertyType before accessing its members, e.g., if (converter == null && propertyType != null).

Suggested change

	if (converter == null && propertySymbol != null)
	{
	List<AttributeData> attributes = [.. propertySymbol.GetAttributes(), .. propertyType!.GetAttributes()];
	if (converter == null && propertySymbol != null && propertyType != null)
	{
	List<AttributeData> attributes = [.. propertySymbol.GetAttributes(), .. propertyType.GetAttributes()];

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.