[Trimming] Use typed bindings internally

[simonrozsival]

Description of Change

This is a follow-up to #20415. Since all bindings now can be typed bindings, I started replacing some bindings declared in code with typed bindings to fix trimming warnings in dotnet new maui on iOS. I haven't even tried to replace any bindings in Tizen, Windows, or Android specific code.

ShellSearchResultRenderer changes

The default template of ShellSearchResultRenderer uses SearchHandler.DisplayMemberName as the path for the binding. I don't see a good way to reimplement this in a way which would work well with typed bindings (would we need Func<object, string> DisplayMember in addition to DisplayMemberName? would that be a good API?). Instead, I decided to propose a new feature switch that will disable the default template and required to always define custom ItemTemplate.

Issues Fixed

Contributes to #19397 - fixes 9 trimming warnings.

[StephaneDelcroix]

I know we've merged that API already, but TypedBinding.ForSingleNestingLevel looks odd. couldn't we go with the original plan of exposing, but internally, a generic SetBinding overload that takes an Expression, so we're future proof ? Even if we fail on multi-level ?

Or is it a performance issue ? can we address that ?

[simonrozsival]

@StephaneDelcroix Yes, the problem with the expressions-based bindings (#19995) was performance. I agree that the name isn't the best and there aren't any checks that the delegates access just the single level so it can be misused. I proposed a different way to approach this via source generators (#20574) and if that is approved in some form, we could use it instead of this internal API and drop it.

[jonathanpeppers]

It looks like we should make the feature flag around DisplayMemberName and the default template can still work.

TODO: how many DisplayMemberName-like features exist out there? Telerik, third party, etc.?

[simonrozsival]

I updated the feature switch. It will now always just call ToString() on the item (the default behavior of bindings that set string properties) + added a warning if somebody uses DisplayMemberName.

[jonathanpeppers]

Should this throw? and then we put [RequiresUnreferencedCode] on the setter of the DisplayMemberName property?

[simonrozsival]

I wasn't sure if this should throw or not. When I'm running the app from the terminal, I don't see any stack trace or the exception message when the app crashes. Maybe do both?

The problem with [RequiresUnreferencedCode] on DisplayMemberName is that it causes ~50 trimming warnings similar to this one:

/.../maui/src/Controls/src/Core/Shell/SearchHandler.cs(398): Trim analysis warning IL2026: Microsoft.Maui.Controls.SearchHandler..cctor(): Using member 'Microsoft.Maui.Controls.SearchHandler.DisplayMemberName.set' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. DisplayMemberName is not supported. Consider implementing custom ItemTemplate instead. Alternatively, enable DisplayMemberName by setting the $(MauiShellSearchResultsRendererDisplayMemberNameSupported) MSBuild property to true. Note: DisplayMemberName is not trimming-safe and it might not work as expected in NativeAOT or fully trimmed apps. [/.../MyMauiApp/MyMauiApp.csproj::TargetFramework=net9.0-maccatalyst]

I am not sure why the static constructor would cause such a warning, but unfortunately, I don't know what to do about it.

[jonathanpeppers]

So if we have:

public static readonly BindableProperty DisplayMemberNameProperty =
	BindableProperty.Create(nameof(DisplayMemberName), typeof(string), typeof(SearchHandler), null, BindingMode.OneTime);

//...

public string DisplayMemberName
{
	get { return (string)GetValue(DisplayMemberNameProperty); }
	[RequiresUnreferencedCode]
	set { SetValue(DisplayMemberNameProperty, value); }
}

Was it warning about nameof(DisplayMemberName)? I think we can probably leave the BindableProperty alone and only add the attribute to the setter of DisplayMemberName , if that is possible?

[simonrozsival]

This is exactly the code that I had locally, and it produced the flood of trimming warnings (we can't apply the attribute to fields BTW). Maybe the warnings are a bug in ILC? @vitek-karas might know what's going on when he's back from vacation.

[simonrozsival]

Ok, it's probably not a bug, but it's still strange. We get 2 exactly same warnings on each line where we use typeof(SearchHandler), for example:

// /.../maui/src/Controls/src/Core/Shell/SearchHandler.cs(145): Trim analysis warning IL2026: Microsoft.Maui.Controls.SearchHandler..cctor(): Using member 'Microsoft.Maui.Controls.SearchHandler.DisplayMemberName.set' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Use ItemTemplate instead. [/.../MyMauiApp/MyMauiApp.csproj::TargetFramework=net9.0-maccatalyst]
// /.../maui/src/Controls/src/Core/Shell/SearchHandler.cs(145): Trim analysis warning IL2026: Microsoft.Maui.Controls.SearchHandler..cctor(): Using member 'Microsoft.Maui.Controls.SearchHandler.DisplayMemberName.set' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. Use ItemTemplate instead. [/.../MyMauiApp/MyMauiApp.csproj::TargetFramework=net9.0-maccatalyst]
145: public static readonly BindableProperty CancelButtonColorProperty = BindableProperty.Create(nameof(CancelButtonColor), typeof(Color), typeof(SearchHandler), default(Color));

At the same time, the compiled XAML doesn't produce any warnings. That is because XamlC doesn't use the setter method, but it sets the value using BindableObject.SetValue:

<Shell.SearchHandler>
    <local:AnimalSearchHandler Placeholder="Enter search term"
                               ShowsResults="true"
                               DisplayMemberName="Name" />
</Shell.SearchHandler>

// decompiled IL
animalSearchHandler.SetValue(SearchHandler.DisplayMemberNameProperty, "Name");

[vitek-karas]

This is very likely a problem in the trimmer - and possibly even in NativeAOT, but that I'm not sure of. The root cause is that the parameter to which we're passing typeof(SearchHandler) has DAM(PublicProperties | PublicMethods).

The PublicProperties will mark the DisplayMemberName property as reflection accessible, which in turn marks the getter of it as reflection accessible -> warning.
The PublicMethods will mark the getter direcly as reflection accessible -> warning.

@sbomer as FYI.

[simonrozsival]

@jonathanpeppers I think this PR is now ready to be merged