Warning AVLN2208: Item container in the data template (#18132)

* Implement AVLN2208 diagnostic - ItemContainerInsideTemplate

* Add AVLN2208 tests

* Enable AVLN2208 as an error in the repository globally

* Fix invalid ListBoxItem inside of DataTemplate in devtools

.editorconfig

@@ -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
 

build/WarnAsErrors.props

@@ -17,5 +17,7 @@
     <WarningsNotAsErrors>$(WarningsNotAsErrors);AVLN2205</WarningsNotAsErrors>
     <!-- AVLN2207: TemplatePartWrongType -->
     <WarningsNotAsErrors>$(WarningsNotAsErrors);AVLN2207</WarningsNotAsErrors>
+    <!-- AVLN2208: ItemContainerInsideTemplate -->
+    <WarningsNotAsErrors>$(WarningsNotAsErrors);AVLN2208</WarningsNotAsErrors>
   </PropertyGroup>
 </Project>

src/Avalonia.Diagnostics/Diagnostics/Views/EventsPageView.xaml

@@ -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">
@@ -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" >
+              <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 Tag="{Binding Event}" DoubleTapped="NavigateTo" Text="{Binding Event.Name}" FontWeight="Bold" Classes="nav" />
-                  <TextBlock Text="on" />
+                <TextBlock Text="on" />
-                  <TextBlock Tag="{Binding Originator}" DoubleTapped="NavigateTo" Text="{Binding Originator.HandlerName}" Classes="nav" />
+                <TextBlock Tag="{Binding Originator}" DoubleTapped="NavigateTo" Text="{Binding Originator.HandlerName}" Classes="nav" />
-                </StackPanel>
+              </StackPanel>
 
-                <StackPanel Margin="2,0,0,0" Grid.Column="2" Spacing="2" Orientation="Horizontal" IsVisible="{Binding IsHandled}" >
+              <StackPanel Margin="2,0,0,0" Grid.Column="2" Spacing="2" Orientation="Horizontal" IsVisible="{Binding IsHandled}" >
-                  <TextBlock Text="::" />
+                <TextBlock Text="::" />
-                  <TextBlock Text="Handled by" />
+                <TextBlock Text="Handled by" />
-                  <TextBlock Tag="{Binding HandledBy}" DoubleTapped="NavigateTo" Text="{Binding HandledBy.HandlerName}" Classes="nav" />
+                <TextBlock Tag="{Binding HandledBy}" DoubleTapped="NavigateTo" Text="{Binding HandledBy.HandlerName}" Classes="nav" />
-                </StackPanel>
+              </StackPanel>
 
-                <StackPanel Grid.Column="4" Orientation="Horizontal" HorizontalAlignment="Right">
+              <StackPanel Grid.Column="4" Orientation="Horizontal" HorizontalAlignment="Right">
-                  <TextBlock Text="Routing (" />
+                <TextBlock Text="Routing (" />
-                  <TextBlock Text="{Binding Event.RoutingStrategies}"/>
+                <TextBlock Text="{Binding Event.RoutingStrategies}"/>
-                  <TextBlock Text=")"/>
+                <TextBlock Text=")"/>
-                </StackPanel>
+              </StackPanel>
 
-              </Grid>
+            </Grid>
-            </ListBoxItem>
           </DataTemplate>
         </ListBox.ItemTemplate>
       </ListBox>
@@ -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}"
+              <StackPanel Orientation="Vertical"
-                           PointerEntered="ListBoxItem_PointerEntered"
+                          Background="Transparent"
-                           PointerExited="ListBoxItem_PointerExited"
+                          PointerEntered="ListBoxItem_PointerEntered"
-                           >
+                          PointerExited="ListBoxItem_PointerExited">
-                <StackPanel Orientation="Vertical">
+
-
+                <Rectangle IsVisible="{Binding BeginsNewRoute}" StrokeDashArray="2,2" StrokeThickness="1" Stroke="Gray" />
-                  <Rectangle IsVisible="{Binding BeginsNewRoute}" StrokeDashArray="2,2" StrokeThickness="1" Stroke="Gray" />
+
-
+                <StackPanel Orientation="Horizontal" Spacing="2">
-                  <StackPanel Orientation="Horizontal" Spacing="2">
+                  <TextBlock Text="{Binding Route}" FontWeight="Bold" />
-                    <TextBlock Text="{Binding Route}" FontWeight="Bold" />
+                  <TextBlock Tag="{Binding}"
-                    <TextBlock Tag="{Binding}"
+                             DoubleTapped="NavigateTo"
-                               DoubleTapped="NavigateTo"
+                             Text="{Binding HandlerName}"
-                               Text="{Binding HandlerName}"
+                             Classes="nav" />
-                               Classes="nav" />
-                  </StackPanel>
-
                 </StackPanel>
-              </ListBoxItem>
+
+              </StackPanel>
             </DataTemplate>
           </ListBox.ItemTemplate>
         </ListBox>

src/Avalonia.Diagnostics/Diagnostics/Views/EventsPageView.xaml.cs

@@ -77,7 +77,7 @@ namespace Avalonia.Diagnostics.Views
         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)
             {

src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/AvaloniaXamlDiagnosticCodes.cs

@@ -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";

src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/AvaloniaXamlIlCompiler.cs

@@ -66,7 +66,8 @@ namespace Avalonia.Markup.Xaml.XamlIl.CompilerExtensions
                 new AvaloniaXamlIlConstructorServiceProviderTransformer(),
                 new AvaloniaXamlIlTransitionsTypeMetadataTransformer(),
                 new AvaloniaXamlIlResolveByNameMarkupExtensionReplacer(),
-                new AvaloniaXamlIlThemeVariantProviderTransformer()
+                new AvaloniaXamlIlThemeVariantProviderTransformer(),
+                new AvaloniaXamlIlDataTemplateWarningsTransformer()
             );
             InsertBefore<ConvertPropertyValuesToAssignmentsTransformer>(
                 new AvaloniaXamlIlOptionMarkupExtensionTransformer());

src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/Transformers/AvaloniaXamlIlDataTemplateWarningsTransformer.cs

@@ -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.
+        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
+    };
+}

src/Markup/Avalonia.Markup.Xaml.Loader/CompilerExtensions/Transformers/AvaloniaXamlIlWellKnownTypes.cs

@@ -63,6 +63,7 @@ namespace Avalonia.Markup.Xaml.XamlIl.CompilerExtensions.Transformers
         public IXamlType IDataTemplate { get; }
         public IXamlType ITemplateOfControl { get; }
         public IXamlType Control { get; }
+        public IXamlType ContentControl { get; }
         public IXamlType ItemsControl { get; }
         public IXamlType ReflectionBindingExtension { get; }
 
@@ -249,6 +250,7 @@ namespace Avalonia.Markup.Xaml.XamlIl.CompilerExtensions.Transformers
             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");

tests/Avalonia.Markup.Xaml.UnitTests/Xaml/XamlIlTests.cs

@@ -392,6 +392,72 @@ namespace Avalonia.Markup.Xaml.UnitTests
             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