ArgumentOutOfRange exception in ResolveContentPropertyTransformer.Transform

ds1709 commented 7 months ago

Describe the bug

There'e multiple scenarios when compilling axaml throws internal compiller error (ArgumentOutOfRangeException). This exception occures when you have in source axaml file any xml elements of types which has no Content property and has no any Add method, but contains any nested elements. I faced with this trying to add objects into DataGrid, but it reproduces for other object types.

To Reproduce

Scenario 1 (DataGrid):

<DataGrid>
    <sys:Int32>1<sys:Int32>
</DataGrid>

Scenario 2 (Object):

<sys:Object>
    <sys:Int32>1<sys:Int32>
</sys:Object>

In both scenarios on compilling:

XamlTransformException: Internal compiler error: Index was out of range. Must be non-negative and less than the size of the collection.
Parameter name: index (ResolveContentPropertyTransformer) Line 37, position 6.

ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection.
Parameter name: index
   at System.ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument argument, ExceptionResource resource)
   at System.Collections.Generic.List`1.get_Item(Int32 index)
   at XamlX.Transform.Transformers.ResolveContentPropertyTransformer.Transform(AstTransformationContext context, IXamlAstNode node) in D:\Git\AvaloniaUI\Avalonia\src\Markup\Avalonia.Markup.Xaml.Loader\xamlil.github\src\XamlX\Transform\Transformers\ResolveContentPropertyTransformer.cs
   ... <truncated>

Expected behavior

Copmpilation error No Content property or any Add methods found for type <type name>.

Avalonia version

11.1.0-beta1

OS

No response

Additional context

This error occures if object in xaml which has no Content property and has no any Add method has nested element(s) and whitespaces. The problem is here (body of Transform method):

var child = ni.Children[c];
if (child is IXamlAstValueNode valueNode)
{
    if (propertyNode == null)
    {
        var contentProperty = context.Configuration.FindContentProperty(ni.Type.GetClrType());
        if (contentProperty != null)
            propertyNode = new XamlAstXamlPropertyValueNode(ni,
                new XamlAstClrProperty(ni, contentProperty, context.Configuration),
                Array.Empty<IXamlAstValueNode>(), false);
        else
        {
            var adders = XamlTransformHelpers.FindPossibleAdders(context, ni.Type.GetClrType());
            if (adders.Count == 0)
            {
                // If there's no content property, strip all whitespace-only nodes and continue
                WhitespaceNormalization.RemoveWhitespaceNodes(ni.Children); // << !!! removes all whitespaces and breaks loop iterator
                if (!ni.Children.Contains(child))
                {
                    continue;
                }

                throw new XamlTransformException(
                    $"No Content property or any Add methods found for type {ni.Type.GetClrType().GetFqn()}",
                    child);
            }

            propertyNode = new XamlAstXamlPropertyValueNode(ni, new XamlAstClrProperty(ni,
                    "Content", ni.Type.GetClrType(), null,
                    adders.Select(a => new XamlDirectCallPropertySetter(a)
                    {
                        BinderParameters = {AllowMultiple = true}
                    })),
                Array.Empty<IXamlAstValueNode>(),
                false);
        }
    }
    // We are going in reverse order, so insert at the beginning
    propertyNode.Values.Insert(0, valueNode);
    ni.Children.RemoveAt(c);
}

In reproduce scenarios, nested elemnt <sys:Int32>1<sys:Int32> is surrounded by indent spaces, so owner element contains three child elements (children count is 3, c value is 2). In first loop step call of WhitespaceNormalization.RemoveWhitespaceNodes removes spaces, so now child count is 1, but loop iterator after decrement is 1. So, next loop step fails on ni.Children[c].

ds1709 commented 7 months ago

In my local fork I created three unit tests for various scenarios. But I had to add InternalsVisibleTo attribute in Avalonia.Build.Tasks project to get access to it's internal classes. Here's the code:

[Fact]
public void Transform_Does_Not_Fail_For_Object_With_Whitespaces()
{
    // <Object>\n\t\n\t\n\t</Object>

    var doc = XDocumentXamlParser.Parse("<root/>");

    var typeSystem = new CecilTypeSystem(new[]
        {
            GetType().Assembly.Location,
            typeof(void).Assembly.Location,
        });

    var context = CreateContext(typeSystem, doc);

    var objectType = typeSystem.GetType("System.Object");

    var node = new XamlAstObjectNode(doc.Root, new XamlAstClrTypeReference(doc.Root, objectType, false));
    node.Children.Add(new XamlAstTextNode(doc.Root, "\n\t"));
    node.Children.Add(new XamlAstTextNode(doc.Root, "\n\t"));
    node.Children.Add(new XamlAstTextNode(doc.Root, "\n\t"));

    var transformer = new ResolveContentPropertyTransformer();
    transformer.Transform(context, node);

    Assert.True(true);
}

[Fact]
public void Transform_Fail_For_Object_With_Content()
{
    // <Object><Int32>1<Int32></Object>

    var doc = XDocumentXamlParser.Parse("<root/>");

    var typeSystem = new CecilTypeSystem(new[]
        {
            GetType().Assembly.Location,
            typeof(void).Assembly.Location,
        });

    var context = CreateContext(typeSystem, doc);

    var objectType = typeSystem.GetType("System.Object");
    var intType = typeSystem.GetType("System.Int32");

    var node = new XamlAstObjectNode(doc.Root, new XamlAstClrTypeReference(doc.Root, objectType, false));
    node.Children.Add(new XamlConstantNode(doc.Root, intType, 1));

    var transformer = new ResolveContentPropertyTransformer();
    var ex = Assert.Throws<XamlTransformException>(() => transformer.Transform(context, node));
}

[Fact]
public void Transform_Fail_For_Object_With_Content_And_Whitespaces()
{
    // <Object>\n\t<Int32>1<Int32>\n\t</Object>

    var doc = XDocumentXamlParser.Parse("<root/>");

    var typeSystem = new CecilTypeSystem(new[]
        {
            GetType().Assembly.Location,
            typeof(void).Assembly.Location,
        });

    var context = CreateContext(typeSystem, doc);

    var objectType = typeSystem.GetType("System.Object");
    var intType = typeSystem.GetType("System.Int32");

    var node = new XamlAstObjectNode(doc.Root, new XamlAstClrTypeReference(doc.Root, objectType, false));
    node.Children.Add(new XamlAstTextNode(doc.Root, "\n\t"));
    node.Children.Add(new XamlConstantNode(doc.Root, intType, 1));
    node.Children.Add(new XamlAstTextNode(doc.Root, "\n\t"));

    var transformer = new ResolveContentPropertyTransformer();
    var ex = Assert.Throws<XamlTransformException>(() => transformer.Transform(context, node));
}

private AstTransformationContext CreateContext(IXamlTypeSystem typeSystem, XamlDocument doc)
{
    var thisAssembly = typeSystem.FindAssembly(typeof(ResolveContentPropertyTransformerTest).Assembly.GetName().Name);
    var mapping = new XamlLanguageTypeMappings(typeSystem, false);
    var compilerConfig = new TransformerConfiguration(typeSystem, thisAssembly, mapping);
    return new AstTransformationContext(compilerConfig, doc);
}

Transform_Does_Not_Fail_For_Object_With_Whitespaces: fails. Transform_Fail_For_Object_With_Content: success. Transform_Fail_For_Object_With_Content_And_Whitespaces: fails.

ds1709 commented 7 months ago

A little more on the topic.

I don't see any reason not to throw XamlTransformException right on if (adders.Count == 0). Why should we continue to handle element in this case?
Why are we trying to find content property on each loop step, why can't we find it once before loop? Same for creating content property node. If it's some kind of optimisation if is there any no whitespace child node, then it's counterintuitively and must be simplified.
Whole method body is tricky and must be simplified for better readability.

AvaloniaUI / Avalonia