Skip to content

Commit

Permalink
[XC] Produce warning when x:DataType is inherited from outer scope of…
Browse files Browse the repository at this point in the history
… `DataTemplate` (#22803)

* Add tests

* Add build exception

* Detect leaving DataTemplate scope
  • Loading branch information
simonrozsival committed Jun 21, 2024
1 parent 78eb023 commit 4ca773f
Show file tree
Hide file tree
Showing 7 changed files with 135 additions and 8 deletions.
1 change: 1 addition & 0 deletions src/Controls/src/Build.Tasks/BuildException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class BuildExceptionCode
public static BuildExceptionCode BPMissingGetter = new BuildExceptionCode("XC", 0021, nameof(BPMissingGetter), "");
public static BuildExceptionCode BindingWithoutDataType = new BuildExceptionCode("XC", 0022, nameof(BindingWithoutDataType), "https://learn.microsoft.com/dotnet/maui/fundamentals/data-binding/compiled-bindings"); //warning
public static BuildExceptionCode BindingWithNullDataType = new BuildExceptionCode("XC", 0023, nameof(BindingWithNullDataType), "https://learn.microsoft.com/dotnet/maui/fundamentals/data-binding/compiled-bindings"); //warning
public static BuildExceptionCode BindingWithXDataTypeFromOuterScope = new BuildExceptionCode("XC", 0024, nameof(BindingWithXDataTypeFromOuterScope), "https://learn.microsoft.com/dotnet/maui/fundamentals/data-binding/compiled-bindings"); //warning

//Bindings, conversions
public static BuildExceptionCode Conversion = new BuildExceptionCode("XC", 0040, nameof(Conversion), "");
Expand Down
7 changes: 5 additions & 2 deletions src/Controls/src/Build.Tasks/ErrorMessages.resx
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,13 @@
</data>
<data name="BindingWithoutDataType" xml:space="preserve">
<value>Binding could be compiled to improve runtime performance if x:DataType is specified. See https://learn.microsoft.com/dotnet/maui/fundamentals/data-binding/compiled-bindings for more information.</value>
</data>
</data>
<data name="BindingWithNullDataType" xml:space="preserve">
<value>Binding could be compiled to improve runtime performance if x:DataType is not explicitly null. See https://learn.microsoft.com/dotnet/maui/fundamentals/data-binding/compiled-bindings for more information.</value>
</data>
</data>
<data name="BindingWithXDataTypeFromOuterScope" xml:space="preserve">
<value>Binding might be compiled incorrectly since the x:DataType annotation comes from an outer scope. Make sure you annotate all DataTemplate XAML elements with the correct x:DataType. See https://learn.microsoft.com/dotnet/maui/fundamentals/data-binding/compiled-bindings for more information.</value>
</data>
<data name="BindingPropertyNotFound" xml:space="preserve">
<value>Binding: Property "{0}" not found on "{1}".</value>
<comment>0 is property name, 1 is type name</comment>
Expand Down
13 changes: 13 additions & 0 deletions src/Controls/src/Build.Tasks/SetPropertiesVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -417,13 +417,20 @@ static IEnumerable<Instruction> CompileBindingPath(ElementNode node, ILContext c
skipNode = GetParent(node);
}

bool xDataTypeIsInOuterScope = false;
while (n != null)
{
if (n != skipNode && n.Properties.TryGetValue(XmlName.xDataType, out dataTypeNode))
{
break;
}

if (n.XmlType.Name == nameof(Microsoft.Maui.Controls.DataTemplate)
&& n.XmlType.NamespaceUri == XamlParser.MauiUri)
{
xDataTypeIsInOuterScope = true;
}

n = GetParent(n);
}

Expand All @@ -434,6 +441,12 @@ static IEnumerable<Instruction> CompileBindingPath(ElementNode node, ILContext c
yield break;
}

if (xDataTypeIsInOuterScope)
{
context.LoggingHelper.LogWarningOrError(BuildExceptionCode.BindingWithXDataTypeFromOuterScope, context.XamlFilePath, node.LineNumber, node.LinePosition, 0, 0, null);
// continue compilation
}

if (dataTypeNode is ElementNode enode
&& enode.XmlType.NamespaceUri == XamlParser.X2009Uri
&& enode.XmlType.Name == nameof(Microsoft.Maui.Controls.Xaml.NullExtension))
Expand Down
16 changes: 13 additions & 3 deletions src/Controls/src/Build.Tasks/XamlCTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ class LoggingHelperContext
public IList<int> WarningsAsErrors { get; set; }
public IList<int> WarningsNotAsErrors { get; set; }
public IList<int> NoWarn { get; set; }
public bool HasLoggedError { get; set; }
public string PathPrefix { get; set; }
}

static LoggingHelperContext Context { get; set; }
internal static List<BuildException> LoggedErrors { get; set; }

public static void SetContext(
this TaskLoggingHelper loggingHelper,
Expand Down Expand Up @@ -98,7 +98,8 @@ public static void LogWarningOrError(this TaskLoggingHelper loggingHelper, Build
|| (Context.WarningsAsErrors != null && Context.WarningsAsErrors.Contains(code.CodeCode)))
{
loggingHelper.LogError("XamlC", $"{code.CodePrefix}{code.CodeCode:0000}", code.HelpLink, xamlFilePath, lineNumber, linePosition, endLineNumber, endLinePosition, ErrorMessages.ResourceManager.GetString(code.ErrorMessageKey), messageArgs);
Context.HasLoggedError = true;
LoggedErrors ??= new();
LoggedErrors.Add(new BuildException(code, new XmlLineInfo(lineNumber, linePosition), innerException: null));
}
else
{
Expand Down Expand Up @@ -309,7 +310,16 @@ public override bool Execute(out IList<Exception> thrownExceptions)
}
else
{
success &= !LoggingHelper.HasLoggedErrors;
if (LoggingHelperExtensions.LoggedErrors is List<BuildException> errors)
{
foreach (var error in errors)
{
success = false;
(thrownExceptions = thrownExceptions ?? new List<Exception>()).Add(error);
}

LoggingHelperExtensions.LoggedErrors = null;
}
}

if (initComp.HasCustomAttributes)
Expand Down
28 changes: 28 additions & 0 deletions src/Controls/tests/Xaml.UnitTests/Issues/Maui22714.xaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?xml version="1.0" encoding="UTF-8"?>
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
x:Class="Microsoft.Maui.Controls.Xaml.UnitTests.Maui22714"
xmlns:local="clr-namespace:Microsoft.Maui.Controls.Xaml.UnitTests">

<StackLayout>
<ScrollView x:DataType="local:PageViewModel22714">
<StackLayout BindableLayout.ItemsSource="{Binding Items}">
<BindableLayout.ItemTemplate>
<DataTemplate x:DataType="local:ItemViewModel22714">
<Label Text="{Binding Title}" />
</DataTemplate>
</BindableLayout.ItemTemplate>
</StackLayout>
</ScrollView>

<ScrollView x:DataType="local:PageViewModel22714">
<StackLayout BindableLayout.ItemsSource="{Binding Items}">
<BindableLayout.ItemTemplate>
<DataTemplate>
<Label Text="{Binding Title}" />
</DataTemplate>
</BindableLayout.ItemTemplate>
</StackLayout>
</ScrollView>
</StackLayout>
</ContentPage>
71 changes: 71 additions & 0 deletions src/Controls/tests/Xaml.UnitTests/Issues/Maui22714.xaml.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
using Microsoft.Maui.ApplicationModel;
using Microsoft.Maui.Dispatching;

using Microsoft.Maui.Controls.Core.UnitTests;
using Microsoft.Maui.UnitTests;
using NUnit.Framework;

namespace Microsoft.Maui.Controls.Xaml.UnitTests;

[XamlCompilation(XamlCompilationOptions.Skip)]
public partial class Maui22714
{
public Maui22714()
{
InitializeComponent();
}

public Maui22714(bool useCompiledXaml)
{
//this stub will be replaced at compile time
}

[TestFixture]
public class Test
{
[SetUp]
public void Setup()
{
Application.SetCurrentApplication(new MockApplication());
DispatcherProvider.SetCurrent(new DispatcherProviderStub());
}

[TearDown] public void TearDown() => AppInfo.SetCurrent(null);

[Test]
public void TestNonCompiledResourceDictionary(
[Values(false, true)] bool useCompiledXaml,
[Values(false, true)] bool treatWarningsAsErrors)
{
if (useCompiledXaml)
{
if (treatWarningsAsErrors)
{
Assert.Throws(
new BuildExceptionConstraint(22, 32, static msg => msg.Contains(" XC0024 ", System.StringComparison.Ordinal)),
() => MockCompiler.Compile(typeof(Maui22714), treatWarningsAsErrors: true));
}
else
{
MockCompiler.Compile(typeof(Maui22714), treatWarningsAsErrors: false);
}
}
else
{
// This will never affect non-compiled builds
_ = new Maui22714(useCompiledXaml);
}
}
}
}

public class PageViewModel22714
{
public string Title { get; set; }
public ItemViewModel22714[] Items { get; set; }
}

public class ItemViewModel22714
{
public string Title { get; set; }
}
7 changes: 4 additions & 3 deletions src/Controls/tests/Xaml.UnitTests/MockCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ namespace Microsoft.Maui.Controls.Xaml.UnitTests
{
public static class MockCompiler
{
public static void Compile(Type type, string targetFramework = null)
public static void Compile(Type type, string targetFramework = null, bool treatWarningsAsErrors = false)
{
Compile(type, out _, targetFramework);
Compile(type, out _, targetFramework, treatWarningsAsErrors);
}

public static void Compile(Type type, out MethodDefinition methodDefinition, string targetFramework = null)
public static void Compile(Type type, out MethodDefinition methodDefinition, string targetFramework = null, bool treatWarningsAsErrors = false)
{
methodDefinition = null;
var assembly = type.Assembly.Location;
Expand All @@ -32,6 +32,7 @@ public static void Compile(Type type, out MethodDefinition methodDefinition, str
ValidateOnly = true,
Type = type.FullName,
TargetFramework = targetFramework,
TreatWarningsAsErrors = treatWarningsAsErrors,
BuildEngine = new MSBuild.UnitTests.DummyBuildEngine()
};

Expand Down

0 comments on commit 4ca773f

Please sign in to comment.