Skip to content

Warning AVLN2208: Item container in the data template #18132

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ avalonia_xaml_diagnostic.AVLN2205.severity = error
avalonia_xaml_diagnostic.AVLN2206.severity = info
# TemplatePartWrongType
avalonia_xaml_diagnostic.AVLN2207.severity = error
# ItemContainerInsideTemplate
avalonia_xaml_diagnostic.AVLN2208.severity = error
# Obsolete
avalonia_xaml_diagnostic.AVLN5001.severity = error

Expand Down
2 changes: 2 additions & 0 deletions build/WarnAsErrors.props
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,7 @@
<WarningsNotAsErrors>$(WarningsNotAsErrors);AVLN2205</WarningsNotAsErrors>
<!-- AVLN2207: TemplatePartWrongType -->
<WarningsNotAsErrors>$(WarningsNotAsErrors);AVLN2207</WarningsNotAsErrors>
<!-- AVLN2208: ItemContainerInsideTemplate -->
<WarningsNotAsErrors>$(WarningsNotAsErrors);AVLN2208</WarningsNotAsErrors>
</PropertyGroup>
</Project>
81 changes: 44 additions & 37 deletions src/Avalonia.Diagnostics/Diagnostics/Views/EventsPageView.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:vm="using:Avalonia.Diagnostics.ViewModels"
xmlns:controls="using:Avalonia.Diagnostics.Controls"
xmlns:models="using:Avalonia.Diagnostics.Models"
x:Class="Avalonia.Diagnostics.Views.EventsPageView"
Margin="2"
x:DataType="vm:EventsPageViewModel">
Expand Down Expand Up @@ -73,34 +74,37 @@

<ListBox Name="EventsList" ItemsSource="{Binding RecordedEvents}"
SelectedItem="{Binding SelectedEvent, Mode=TwoWay}">
<ListBox.Styles>
<Style Selector="ListBoxItem">
<Setter Property="Classes.handled" Value="{Binding IsHandled, DataType=vm:FiredEvent}" />
</Style>
</ListBox.Styles>

<ListBox.ItemTemplate>
<DataTemplate>
<ListBoxItem Classes.handled="{Binding IsHandled}">
<Grid ColumnDefinitions="Auto,Auto,Auto,*,Auto">
<Grid ColumnDefinitions="Auto,Auto,Auto,*,Auto">

<TextBlock Grid.Column="0" Text="{Binding TriggerTime, StringFormat={}{0:HH:mm:ss.fff}}"/>
<TextBlock Grid.Column="0" Text="{Binding TriggerTime, StringFormat={}{0:HH:mm:ss.fff}}"/>

<StackPanel Margin="10,0,0,0" Grid.Column="1" Spacing="2" Orientation="Horizontal" >
<TextBlock Tag="{Binding Event}" DoubleTapped="NavigateTo" Text="{Binding Event.Name}" FontWeight="Bold" Classes="nav" />
<TextBlock Text="on" />
<TextBlock Tag="{Binding Originator}" DoubleTapped="NavigateTo" Text="{Binding Originator.HandlerName}" Classes="nav" />
</StackPanel>
<StackPanel Margin="10,0,0,0" Grid.Column="1" Spacing="2" Orientation="Horizontal" >
<TextBlock Tag="{Binding Event}" DoubleTapped="NavigateTo" Text="{Binding Event.Name}" FontWeight="Bold" Classes="nav" />
<TextBlock Text="on" />
<TextBlock Tag="{Binding Originator}" DoubleTapped="NavigateTo" Text="{Binding Originator.HandlerName}" Classes="nav" />
</StackPanel>

<StackPanel Margin="2,0,0,0" Grid.Column="2" Spacing="2" Orientation="Horizontal" IsVisible="{Binding IsHandled}" >
<TextBlock Text="::" />
<TextBlock Text="Handled by" />
<TextBlock Tag="{Binding HandledBy}" DoubleTapped="NavigateTo" Text="{Binding HandledBy.HandlerName}" Classes="nav" />
</StackPanel>
<StackPanel Margin="2,0,0,0" Grid.Column="2" Spacing="2" Orientation="Horizontal" IsVisible="{Binding IsHandled}" >
<TextBlock Text="::" />
<TextBlock Text="Handled by" />
<TextBlock Tag="{Binding HandledBy}" DoubleTapped="NavigateTo" Text="{Binding HandledBy.HandlerName}" Classes="nav" />
</StackPanel>

<StackPanel Grid.Column="4" Orientation="Horizontal" HorizontalAlignment="Right">
<TextBlock Text="Routing (" />
<TextBlock Text="{Binding Event.RoutingStrategies}"/>
<TextBlock Text=")"/>
</StackPanel>
<StackPanel Grid.Column="4" Orientation="Horizontal" HorizontalAlignment="Right">
<TextBlock Text="Routing (" />
<TextBlock Text="{Binding Event.RoutingStrategies}"/>
<TextBlock Text=")"/>
</StackPanel>

</Grid>
</ListBoxItem>
</Grid>
</DataTemplate>
</ListBox.ItemTemplate>
</ListBox>
Expand All @@ -111,26 +115,29 @@
<TextBlock DockPanel.Dock="Top" FontSize="16" Text="Event chain:" />

<ListBox ItemsSource="{Binding SelectedEvent.EventChain}">
<ListBox.Styles>
<Style Selector="ListBoxItem">
<Setter Property="Classes.handled" Value="{Binding Handled, DataType=models:EventChainLink}" />
</Style>
</ListBox.Styles>
<ListBox.ItemTemplate>
<DataTemplate>
<ListBoxItem Classes.handled="{Binding Handled}"
PointerEntered="ListBoxItem_PointerEntered"
PointerExited="ListBoxItem_PointerExited"
>
<StackPanel Orientation="Vertical">

<Rectangle IsVisible="{Binding BeginsNewRoute}" StrokeDashArray="2,2" StrokeThickness="1" Stroke="Gray" />

<StackPanel Orientation="Horizontal" Spacing="2">
<TextBlock Text="{Binding Route}" FontWeight="Bold" />
<TextBlock Tag="{Binding}"
DoubleTapped="NavigateTo"
Text="{Binding HandlerName}"
Classes="nav" />
</StackPanel>

<StackPanel Orientation="Vertical"
Background="Transparent"
PointerEntered="ListBoxItem_PointerEntered"
PointerExited="ListBoxItem_PointerExited">

<Rectangle IsVisible="{Binding BeginsNewRoute}" StrokeDashArray="2,2" StrokeThickness="1" Stroke="Gray" />

<StackPanel Orientation="Horizontal" Spacing="2">
<TextBlock Text="{Binding Route}" FontWeight="Bold" />
<TextBlock Tag="{Binding}"
DoubleTapped="NavigateTo"
Text="{Binding HandlerName}"
Classes="nav" />
</StackPanel>
</ListBoxItem>

</StackPanel>
</DataTemplate>
</ListBox.ItemTemplate>
</ListBox>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ private void InitializeComponent()
private void ListBoxItem_PointerEntered(object? sender, PointerEventArgs e)
{
if (DataContext is EventsPageViewModel vm
&& sender is ListBoxItem control
&& sender is Control control
&& control.DataContext is EventChainLink chainLink
&& chainLink.Handler is Visual visual)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ internal static class AvaloniaXamlDiagnosticCodes
public const string RequiredTemplatePartMissing = "AVLN2205";
public const string OptionalTemplatePartMissing = "AVLN2206";
public const string TemplatePartWrongType = "AVLN2207";
public const string ItemContainerInsideTemplate = "AVLN2208";

// XAML emit errors 3000-3999.
public const string EmitError = "AVLN3000";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ void InsertBeforeMany(Type[] types, params IXamlAstTransformer[] t)
new AvaloniaXamlIlConstructorServiceProviderTransformer(),
new AvaloniaXamlIlTransitionsTypeMetadataTransformer(),
new AvaloniaXamlIlResolveByNameMarkupExtensionReplacer(),
new AvaloniaXamlIlThemeVariantProviderTransformer()
new AvaloniaXamlIlThemeVariantProviderTransformer(),
new AvaloniaXamlIlDataTemplateWarningsTransformer()
);
InsertBefore<ConvertPropertyValuesToAssignmentsTransformer>(
new AvaloniaXamlIlOptionMarkupExtensionTransformer());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using Avalonia.Markup.Xaml.XamlIl.CompilerExtensions.Transformers;
using XamlX;
using XamlX.Ast;
using XamlX.Transform;
using XamlX.TypeSystem;

namespace Avalonia.Markup.Xaml.XamlIl.CompilerExtensions.Transformers;

#if !XAMLX_INTERNAL
public
#endif
class AvaloniaXamlIlDataTemplateWarningsTransformer : IXamlAstTransformer
{
public IXamlAstNode Transform(AstTransformationContext context, IXamlAstNode node)
{
var avaloniaTypes = context.GetAvaloniaTypes();
var contentControl = context.GetAvaloniaTypes().ContentControl;

// This transformers only looks for ContentControl delivered objects inside of DataTemplate
if ((node is not XamlAstObjectNode objectNode)
|| !contentControl.IsAssignableFrom(objectNode.Type.GetClrType())
|| context.ParentNodes().FirstOrDefault() is not XamlAstObjectNode parentNode
|| !avaloniaTypes.IDataTemplate.IsAssignableFrom(parentNode.Type.GetClrType()))
{
return node;
}

// And only inside of ItemTemplate or DataTemplates property value.
if (context.ParentNodes().OfType<XamlAstXamlPropertyValueNode>().FirstOrDefault() is not { } valueNode
|| valueNode.Property.GetClrProperty() is not { } clrProperty
|| !((clrProperty.Name == "ItemTemplate" && clrProperty.DeclaringType == avaloniaTypes.ItemsControl)
|| (clrProperty.Name == "DataTemplates" && clrProperty.DeclaringType == avaloniaTypes.Control)))
{
return node;
}

// And only inside of ItemsControl
if (context.ParentNodes().SkipWhile(p => p != valueNode)
.OfType<XamlAstObjectNode>().FirstOrDefault() is not { } itemsControlNode
|| !avaloniaTypes.ItemsControl.IsAssignableFrom(itemsControlNode.Type.GetClrType()))
{
return node;
}

// Avalonia doesn't have any reliable way to determine container type from the API.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add an attribute for this at some point? It could be useful. (Not as part of this PR of course.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about it.
And there was a PR for that #13104
The problem is that such attribute won't be followed by third party controls, and it gives a little of benefit otherwise, for built-in controls.

Let's add it to next api review, I suppose. If we will find a better idea.

if (GetKnownItemContainerTypeFullName(itemsControlNode.Type.GetClrType()) is not { } knownItemContainerTypeName
|| itemsControlNode.Type.GetClrType().Assembly?.FindType(knownItemContainerTypeName) is not { } knownItemContainerType)
{
return node;
}

if (knownItemContainerType.IsAssignableFrom(objectNode.Type.GetClrType()))
{
context.ReportDiagnostic(new XamlDiagnostic(
AvaloniaXamlDiagnosticCodes.ItemContainerInsideTemplate,
XamlDiagnosticSeverity.Warning,
$"Unexpected '{knownItemContainerType.Name}' inside of '{itemsControlNode.Type.GetClrType().Name}.{clrProperty.Name}'. "
+ $"'{itemsControlNode.Type.GetClrType().Name}.{clrProperty.Name}' defines template of the container content, not the container itself.", node));
}

return node;
}

private static string? GetKnownItemContainerTypeFullName(IXamlType itemsControlType) => itemsControlType.FullName switch
{
"Avalonia.Controls.ListBox" => "Avalonia.Controls.ListBoxItem",
"Avalonia.Controls.ComboBox" => "Avalonia.Controls.ComboBoxItem",
"Avalonia.Controls.Menu" => "Avalonia.Controls.MenuItem",
"Avalonia.Controls.MenuItem" => "Avalonia.Controls.MenuItem",
"Avalonia.Controls.Primitives.TabStrip" => "Avalonia.Controls.Primitives.TabStripItem",
"Avalonia.Controls.TabControl" => "Avalonia.Controls.TabItem",
"Avalonia.Controls.TreeView" => "Avalonia.Controls.TreeViewItem",
_ => null
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ sealed class AvaloniaXamlIlWellKnownTypes
public IXamlType IDataTemplate { get; }
public IXamlType ITemplateOfControl { get; }
public IXamlType Control { get; }
public IXamlType ContentControl { get; }
public IXamlType ItemsControl { get; }
public IXamlType ReflectionBindingExtension { get; }

Expand Down Expand Up @@ -249,6 +250,7 @@ public AvaloniaXamlIlWellKnownTypes(TransformerConfiguration cfg)
DataTemplate = cfg.TypeSystem.GetType("Avalonia.Markup.Xaml.Templates.DataTemplate");
IDataTemplate = cfg.TypeSystem.GetType("Avalonia.Controls.Templates.IDataTemplate");
Control = cfg.TypeSystem.GetType("Avalonia.Controls.Control");
ContentControl = cfg.TypeSystem.GetType("Avalonia.Controls.ContentControl");
ITemplateOfControl = cfg.TypeSystem.GetType("Avalonia.Controls.ITemplate`1").MakeGenericType(Control);
ItemsControl = cfg.TypeSystem.GetType("Avalonia.Controls.ItemsControl");
ReflectionBindingExtension = cfg.TypeSystem.GetType("Avalonia.Markup.Xaml.MarkupExtensions.ReflectionBindingExtension");
Expand Down
66 changes: 66 additions & 0 deletions tests/Avalonia.Markup.Xaml.UnitTests/Xaml/XamlIlTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,72 @@ public void Control_Theme_Parser_Throws_For_Duplicate_Setter()
Assert.Equal(RuntimeXamlDiagnosticSeverity.Warning, warning.Severity);
Assert.StartsWith("Duplicate setter encountered for property 'Height'", warning.Title);
}

[Fact]
public void Item_Container_Inside_Of_ItemTemplate_Should_Be_Warned()
{
using var _ = UnitTestApplication.Start(TestServices.StyledWindow);

var xaml = new RuntimeXamlLoaderDocument(@"
<ListBox xmlns='https://github.com/avaloniaui'
xmlns:x='http://schemas.microsoft.com/winfx/2006/xaml'>
<ListBox.ItemTemplate>
<DataTemplate>
<ListBoxItem />
</DataTemplate>
</ListBox.ItemTemplate>
</ListBox>");
var diagnostics = new List<RuntimeXamlDiagnostic>();
// We still have a runtime check in the StyleInstance class, but in this test we only care about compile warnings.
var listBox = (ListBox)AvaloniaRuntimeXamlLoader.Load(xaml, new RuntimeXamlLoaderConfiguration
{
DiagnosticHandler = diagnostic =>
{
diagnostics.Add(diagnostic);
return diagnostic.Severity;
}
});
// ItemTemplate should still work as before, creating whatever object user put inside
Assert.IsType<ListBoxItem>(listBox.ItemTemplate!.Build(null));

// But invalid usage should be warned:
var warning = Assert.Single(diagnostics);
Assert.Equal(RuntimeXamlDiagnosticSeverity.Warning, warning.Severity);
Assert.Equal("AVLN2208", warning.Id);
}

[Fact]
public void Item_Container_Inside_Of_DataTemplates_Should_Be_Warned()
{
using var _ = UnitTestApplication.Start(TestServices.StyledWindow);

var xaml = new RuntimeXamlLoaderDocument(@"
<TabControl xmlns='https://github.com/avaloniaui'
xmlns:x='http://schemas.microsoft.com/winfx/2006/xaml'>
<TabControl.DataTemplates>
<DataTemplate x:DataType='x:Object'>
<TabItem />
</DataTemplate>
</TabControl.DataTemplates>
</TabControl>");
var diagnostics = new List<RuntimeXamlDiagnostic>();
// We still have a runtime check in the StyleInstance class, but in this test we only care about compile warnings.
var tabControl = (TabControl)AvaloniaRuntimeXamlLoader.Load(xaml, new RuntimeXamlLoaderConfiguration
{
DiagnosticHandler = diagnostic =>
{
diagnostics.Add(diagnostic);
return diagnostic.Severity;
}
});
// ItemTemplate should still work as before, creating whatever object user put inside
Assert.IsType<TabItem>(tabControl.DataTemplates[0]!.Build(null));

// But invalid usage should be warned:
var warning = Assert.Single(diagnostics);
Assert.Equal(RuntimeXamlDiagnosticSeverity.Warning, warning.Severity);
Assert.Equal("AVLN2208", warning.Id);
}
}

public class XamlIlBugTestsEventHandlerCodeBehind : Window
Expand Down