[Proposal] Source generator for [QueryProperty]

[simonrozsival]

Motivation

ShellContent applies query properties via reflection. Since we can't fix this code just with [DynamicallyAccesseMebers(...)] attributes, we need a different solution.

The idea is to source-generate code equivalent to the reflection-based code. This would make this functionality not only trimmable and AOT-compatible, but also a bit faster.

Source generator specification

• The source generator should apply to these types:
  • any class which has at least one [QueryProperty] attribute applied to it
  • skip for classes that already implement IQueryAttributable
• The source generator will report a warning when:
  • the class is not partial
  • destination property is not defined on the type
  • destination property does not have a setter
    • note: current implementation requires the setter to be public but it's not clear why
  • repeated use of the same property name
  • the "query ID" contains characters which are invalid in an URL (for example & or ?)
    • optional, we currently don't have any validation of query ID on the [QueryProperty]
  • the class already contains a method with the same signature as IQueryAttributable.ApplyQueryAttributes
• The source generator will generate the following code:
  • add IQueryAttributable implementation to the class
  • generate the ApplyQueryAttributes method
    • generate an assignment per each [QueryProperty] attribute
    • mirror the logic from the current implementation
      • Note: can we optimize the conversion methods to avoid unnecessary calls to Convert.ChangeType?

Example

// app code
namespace MyNamespace
{
    [QueryProperty("id", "Id")]
    [QueryProperty("first_name", "FirstName")]
    [QueryProperty("last_name", "LastName")]
    [QueryProperty("image", "Image")]
    internal partial class MyViewModel
    {
        public int Id { get; private set; }
        public string FirstName { get; private set; } = null!;
        public string? LastName { get; private set; }
        public Image? Image { get; private set; }
    }
}

// <auto-generated />
namespace MyNamespace
{
    partial class MyViewModel : global::Microsoft.Maui.Controls.IQueryAttributable
    {
        public void ApplyQueryAttributes(global::System.Collections.Generic.IDictionary<string, object?> query)
        {
            if (query.TryGetValue("id", out object? id) && id is not null)
            {
                // Unclear: what to do when the input can't be converted to the target type?
                // - `ChangeType` should throw System.FormatException, that's identical to the current behavior
                // - `ChangeType` can lose information - for example it will allow changing double into int. Is that expected?
                this.Id = id is int _id ? _id : (int)global::System.Convert.ChangeType(id, typeof(int));
            }
            else
            {
                // Unclear: throw when not nullable? should we add overload with `oldQuery` to `IQueryAttributable?
                this.Id = default;
            }

            if (query.TryGetValue("first_name", out object? first_name) && first_name is not null)
            {
                this.FirstName = global::System.Net.WebUtility.UrlDecode((string)first_name);
            }
            else
            {
                // Unclear: Throw since it's a non-nullable reference type? Assign `null!` or `string.Empty`?
                this.FirstName = string.Empty;
            }

            if (query.TryGetValue("last_name", out object? last_name) && last_name is not null)
            {
                this.LastName = global::System.Net.WebUtility.UrlDecode((string)last_name);
            }
            else
            {
                this.LastName = null;
            }

            if (query.TryGetValue("image", out object? image) && image is not null)
            {
                this.Image = (Image)image;
            }
            else
            {
                this.Image = null;
            }
        }
    }
}

Notes and questions

• The reflection-based code which applies the query parameter doesn't work only with the current query parameters, but also with the previous query parameters.
  • When the new query doesn't contain a value for the given query ID but the old one did, it sets the value of the property to null (no matter what the actual type of the property is).
  • Is there some example where it is neccessary to avoid resetting the value of a query property when the value is missing in the dictionary? Why don't we always set it to a default value in that case?
  • If we need to compare the current query to the previous query, we'll need to extend the IQueryAttributable interface:

    interface IQueryAttributable
    {
        void ApplyQueryAttributes(IDictionary<string, object?> query);
        void ApplyQueryAttributes(IDictionary<string, object?> query, IDictionary<string, object?>? oldQuery) => ApplyQueryAttributes(query);
    }

• How should the code behave when there isn't a valid value for a required (non-nullable) property?
• How should the code behave when the value type is not compatible?
• We could generate a helper class QueryAttributableHelpers for the repeating code patterns.
  • Alternatively, we could introduce the QueryAttributableHelpers as a new public API and ship it with MAUI.
• There is an existing proposal for a source generator for shell navigation: Shell Routing through attributes on page (using source generators) #5312
• This would help with [iOS] Resolving Trimming Warnings for dotnet new maui #19397

[mattleibow]

One potential issue I see in:

interface IQueryAttributable
{
    void ApplyQueryAttributes(IDictionary<string, object?> query);
    void ApplyQueryAttributes(IDictionary<string, object?> query, IDictionary<string, object?>? oldQuery) => ApplyQueryAttributes(query);
}

If we add the new member, we don't break code now, but then we are going to always need both members implemented. If I want the new API with 2 params, we can implement it, but we will still need the old API.

I wonder if a new interface is better, and also not use a plain dictionary but rather some ApplyQueryAttributesEventArgs type thing:

class ApplyQueryAttributesEventArgs
{
    public IDictionary<string, object?> NewQuery { get; }
    public IDictionary<string, object?>? OldQuery { get; }

    // potentially new members to help
    public bool IsFirstBinding { get; }
}