From 489657c5458691f3a4c360d84b3a53a03b61b03f Mon Sep 17 00:00:00 2001 From: Stephane Delcroix Date: Thu, 28 Nov 2024 13:39:12 +0100 Subject: [PATCH] [XC] Add file info to exception reports (#25359) * add file info to exception reports * add test, generate full path, position * test for loggedErrors --- src/Controls/src/Build.Tasks/XamlCTask.cs | 13 ++++- src/Controls/src/Build.Tasks/XamlTask.cs | 2 +- src/Controls/src/Core/XamlParseException.cs | 2 +- .../src/Xaml/IXmlLineInfoExtensions.cs | 11 +++++ src/Controls/src/Xaml/XamlParser.cs | 8 ++-- .../CompiledTypeConverter.xaml.cs | 3 +- .../DefinitionCollectionTests.xaml.cs | 3 +- .../DuplicatePropertyElements.xaml.cs | 13 ++--- .../DuplicateXArgumentsElements.xaml.cs | 15 +++--- .../tests/Xaml.UnitTests/FontSize.xaml.cs | 3 +- .../Xaml.UnitTests/Issues/Maui21038.xaml | 10 ++++ .../Xaml.UnitTests/Issues/Maui21038.xaml.cs | 47 +++++++++++++++++++ .../{Maui21434.cs => Maui21434.xaml.cs} | 0 .../Xaml.UnitTests/Issues/Maui23711.xaml.cs | 3 +- .../Xaml.UnitTests/Issues/Maui25406.xaml.cs | 3 +- .../tests/Xaml.UnitTests/MockCompiler.cs | 18 +++++-- .../MultipleDataTemplateChildElements.xaml.cs | 15 +++--- .../OnPlatformOptimization.xaml.cs | 6 ++- .../tests/Xaml.UnitTests/XArray.xaml.cs | 3 +- 19 files changed, 141 insertions(+), 37 deletions(-) create mode 100644 src/Controls/src/Xaml/IXmlLineInfoExtensions.cs create mode 100644 src/Controls/tests/Xaml.UnitTests/Issues/Maui21038.xaml create mode 100644 src/Controls/tests/Xaml.UnitTests/Issues/Maui21038.xaml.cs rename src/Controls/tests/Xaml.UnitTests/Issues/{Maui21434.cs => Maui21434.xaml.cs} (100%) diff --git a/src/Controls/src/Build.Tasks/XamlCTask.cs b/src/Controls/src/Build.Tasks/XamlCTask.cs index 451b42ad8f2e..8bc45cf69c43 100644 --- a/src/Controls/src/Build.Tasks/XamlCTask.cs +++ b/src/Controls/src/Build.Tasks/XamlCTask.cs @@ -282,10 +282,19 @@ public override bool Execute(out IList thrownExceptions) LoggingHelper.LogMessage(Low, $"{new string(' ', 6)}Parsing Xaml"); - var rootnode = ParseXaml(resource.GetResourceStream(), typeDef); - if (rootnode == null) + ILRootNode rootnode = null; + try { + rootnode = ParseXaml(resource.GetResourceStream(), typeDef); + if (rootnode == null) + { + LoggingHelper.LogMessage(Low, $"{new string(' ', 8)}failed."); + continue; + } + } catch (XamlParseException xpe) { LoggingHelper.LogMessage(Low, $"{new string(' ', 8)}failed."); + xamlFilePath = LoggingHelper.GetXamlFilePath(xamlFilePath); + LoggingHelper.LogError("XamlC", null, xpe.HelpLink, xamlFilePath, xpe.XmlInfo.LineNumber, xpe.XmlInfo.LinePosition, 0, 0, xpe.UnformattedMessage); continue; } LoggingHelper.LogMessage(Low, $"{new string(' ', 8)}done."); diff --git a/src/Controls/src/Build.Tasks/XamlTask.cs b/src/Controls/src/Build.Tasks/XamlTask.cs index 032588eef344..1b0150150026 100644 --- a/src/Controls/src/Build.Tasks/XamlTask.cs +++ b/src/Controls/src/Build.Tasks/XamlTask.cs @@ -22,7 +22,7 @@ public abstract class XamlTask : MarshalByRefObject, ITask public bool DebugSymbols { get; set; } public string DebugType { get; set; } - protected TaskLoggingHelper LoggingHelper { get; } + internal TaskLoggingHelper LoggingHelper { get; } internal XamlTask() { diff --git a/src/Controls/src/Core/XamlParseException.cs b/src/Controls/src/Core/XamlParseException.cs index c4dfda9cdaa4..f289aec4dd20 100644 --- a/src/Controls/src/Core/XamlParseException.cs +++ b/src/Controls/src/Core/XamlParseException.cs @@ -52,7 +52,7 @@ public XamlParseException(string message, IXmlLineInfo xmlInfo, Exception innerE /// public IXmlLineInfo XmlInfo { get; private set; } internal string UnformattedMessage => _unformattedMessage ?? Message; - + static string FormatMessage(string message, IXmlLineInfo xmlinfo) { if (xmlinfo == null || !xmlinfo.HasLineInfo()) diff --git a/src/Controls/src/Xaml/IXmlLineInfoExtensions.cs b/src/Controls/src/Xaml/IXmlLineInfoExtensions.cs new file mode 100644 index 000000000000..7234a0b758dc --- /dev/null +++ b/src/Controls/src/Xaml/IXmlLineInfoExtensions.cs @@ -0,0 +1,11 @@ +using System.Xml; + +namespace Microsoft.Maui.Controls.Xaml; + +static class IXmlLineInfoExtensions +{ + public static IXmlLineInfo Clone(this IXmlLineInfo xmlLineInfo) + { + return new XmlLineInfo(xmlLineInfo.LineNumber, xmlLineInfo.LinePosition); + } +} \ No newline at end of file diff --git a/src/Controls/src/Xaml/XamlParser.cs b/src/Controls/src/Xaml/XamlParser.cs index fb00409662f6..06c36aa8c49c 100644 --- a/src/Controls/src/Xaml/XamlParser.cs +++ b/src/Controls/src/Xaml/XamlParser.cs @@ -77,7 +77,7 @@ static void ParseXamlElementFor(IElementNode node, XmlReader reader) name = new XmlName(reader.NamespaceURI, reader.LocalName); if (node.Properties.ContainsKey(name)) - throw new XamlParseException($"'{reader.Name}' is a duplicate property name.", (IXmlLineInfo)reader); + throw new XamlParseException($"'{reader.Name}' is a duplicate property name.", ((IXmlLineInfo)reader).Clone()); INode prop = null; if (reader.IsEmptyElement) @@ -92,7 +92,7 @@ static void ParseXamlElementFor(IElementNode node, XmlReader reader) else if (reader.NamespaceURI == X2009Uri && reader.LocalName == "Arguments") { if (node.Properties.ContainsKey(XmlName.xArguments)) - throw new XamlParseException($"'x:Arguments' is a duplicate directive name.", (IXmlLineInfo)reader); + throw new XamlParseException($"'x:Arguments' is a duplicate directive name.", ((IXmlLineInfo)reader).Clone()); var prop = ReadNode(reader); if (prop != null) @@ -103,7 +103,7 @@ static void ParseXamlElementFor(IElementNode node, XmlReader reader) (node.XmlType.Name == "DataTemplate" || node.XmlType.Name == "ControlTemplate")) { if (node.Properties.ContainsKey(XmlName._CreateContent)) - throw new XamlParseException($"Multiple child elements in {node.XmlType.Name}", (IXmlLineInfo)reader); + throw new XamlParseException($"Multiple child elements in {node.XmlType.Name}", ((IXmlLineInfo)reader).Clone()); var prop = ReadNode(reader, true); if (prop != null) @@ -189,7 +189,7 @@ static INode ReadNode(XmlReader reader, bool nested = false) break; } } - throw new XamlParseException("Closing PropertyElement expected", (IXmlLineInfo)reader); + throw new XamlParseException("Closing PropertyElement expected", ((IXmlLineInfo)reader).Clone()); } internal static IList GetTypeArguments(XmlReader reader) => GetTypeArguments(ParseXamlAttributes(reader, out _)); diff --git a/src/Controls/tests/Xaml.UnitTests/CompiledTypeConverter.xaml.cs b/src/Controls/tests/Xaml.UnitTests/CompiledTypeConverter.xaml.cs index 2b38b462a9fc..f822e7eb444c 100644 --- a/src/Controls/tests/Xaml.UnitTests/CompiledTypeConverter.xaml.cs +++ b/src/Controls/tests/Xaml.UnitTests/CompiledTypeConverter.xaml.cs @@ -119,7 +119,8 @@ public void CompiledTypeConverterAreInvoked(bool useCompiledXaml) [TestCase(typeof(Microsoft.Maui.Graphics.Converters.RectTypeConverter))] public void ConvertersAreReplaced(Type converterType) { - MockCompiler.Compile(typeof(CompiledTypeConverter), out var methodDef); + MockCompiler.Compile(typeof(CompiledTypeConverter), out var methodDef, out var hasLoggedErrors); + Assert.That(!hasLoggedErrors); Assert.That(!methodDef.Body.Instructions.Any(instr => HasConstructorForType(methodDef, instr, converterType)), $"This Xaml still generates a new {converterType}()"); } diff --git a/src/Controls/tests/Xaml.UnitTests/DefinitionCollectionTests.xaml.cs b/src/Controls/tests/Xaml.UnitTests/DefinitionCollectionTests.xaml.cs index 49256789ef3a..d19d1052f13c 100644 --- a/src/Controls/tests/Xaml.UnitTests/DefinitionCollectionTests.xaml.cs +++ b/src/Controls/tests/Xaml.UnitTests/DefinitionCollectionTests.xaml.cs @@ -49,7 +49,8 @@ public void DefinitionCollectionsParsedFromMarkup([Values(false, true)] bool use [Test] public void DefinitionCollectionsReplacedAtCompilation() { - MockCompiler.Compile(typeof(DefinitionCollectionTests), out var methodDef); + MockCompiler.Compile(typeof(DefinitionCollectionTests), out var methodDef, out var hasLoggedErrors); + Assert.That(!hasLoggedErrors); Assert.That(!methodDef.Body.Instructions.Any(instr => InstructionIsDefColConvCtor(methodDef, instr)), "This Xaml still generates [Row|Col]DefinitionCollectionTypeConverter ctor"); } diff --git a/src/Controls/tests/Xaml.UnitTests/DuplicatePropertyElements.xaml.cs b/src/Controls/tests/Xaml.UnitTests/DuplicatePropertyElements.xaml.cs index 769c8b2bd220..8810e6b9c043 100644 --- a/src/Controls/tests/Xaml.UnitTests/DuplicatePropertyElements.xaml.cs +++ b/src/Controls/tests/Xaml.UnitTests/DuplicatePropertyElements.xaml.cs @@ -13,13 +13,14 @@ public DuplicatePropertyElements(bool useCompiledXaml) [TestFixture] public static class Tests { - [TestCase(false)] - [TestCase(true)] - public static void ThrowXamlParseException(bool useCompiledXaml) + [Test] public static void ThrowXamlParseException([Values]bool useCompiledXaml) { - Assert.Throws(useCompiledXaml ? - (TestDelegate)(() => MockCompiler.Compile(typeof(DuplicatePropertyElements))) : - () => new DuplicatePropertyElements(useCompiledXaml)); + if (useCompiledXaml){ + MockCompiler.Compile(typeof(DuplicatePropertyElements), out var md, out var hasLoggedErrors); + Assert.That(hasLoggedErrors); + } + else + Assert.Throws(() => new DuplicatePropertyElements(useCompiledXaml)); } } } diff --git a/src/Controls/tests/Xaml.UnitTests/DuplicateXArgumentsElements.xaml.cs b/src/Controls/tests/Xaml.UnitTests/DuplicateXArgumentsElements.xaml.cs index 1c0a51c55bfd..cce6de2795ca 100644 --- a/src/Controls/tests/Xaml.UnitTests/DuplicateXArgumentsElements.xaml.cs +++ b/src/Controls/tests/Xaml.UnitTests/DuplicateXArgumentsElements.xaml.cs @@ -13,13 +13,16 @@ public DuplicateXArgumentsElements(bool useCompiledXaml) [TestFixture] public static class Tests { - [TestCase(false)] - [TestCase(true)] - public static void ThrowXamlParseException(bool useCompiledXaml) + [Test] + public static void ThrowXamlParseException([Values]bool useCompiledXaml) { - Assert.Throws(useCompiledXaml ? - (TestDelegate)(() => MockCompiler.Compile(typeof(DuplicateXArgumentsElements))) : - () => new DuplicateXArgumentsElements(useCompiledXaml)); + if (useCompiledXaml) + { + MockCompiler.Compile(typeof(DuplicateXArgumentsElements), out var md, out var hasLoggedErrors); + Assert.That(hasLoggedErrors); + } + else + Assert.Throws(() => new DuplicateXArgumentsElements(useCompiledXaml)); } } } diff --git a/src/Controls/tests/Xaml.UnitTests/FontSize.xaml.cs b/src/Controls/tests/Xaml.UnitTests/FontSize.xaml.cs index 79830cc8a4e7..b852f77894e7 100644 --- a/src/Controls/tests/Xaml.UnitTests/FontSize.xaml.cs +++ b/src/Controls/tests/Xaml.UnitTests/FontSize.xaml.cs @@ -25,7 +25,8 @@ public class Tests [Test] public void FontSizeExtensionsAreReplaced() { - MockCompiler.Compile(typeof(FontSize), out var methodDef); + MockCompiler.Compile(typeof(FontSize), out var methodDef, out var hasLoggedErrors); + Assert.That(!hasLoggedErrors); Assert.That(!methodDef.Body.Instructions.Any(instr => InstructionIsFontSizeConverterCtor(methodDef, instr)), "This Xaml still generates a new FontSizeConverter()"); } diff --git a/src/Controls/tests/Xaml.UnitTests/Issues/Maui21038.xaml b/src/Controls/tests/Xaml.UnitTests/Issues/Maui21038.xaml new file mode 100644 index 000000000000..fa934dfa1609 --- /dev/null +++ b/src/Controls/tests/Xaml.UnitTests/Issues/Maui21038.xaml @@ -0,0 +1,10 @@ + + + + diff --git a/src/Controls/tests/Xaml.UnitTests/Issues/Maui21038.xaml.cs b/src/Controls/tests/Xaml.UnitTests/Issues/Maui21038.xaml.cs new file mode 100644 index 000000000000..4fb6dc1492cd --- /dev/null +++ b/src/Controls/tests/Xaml.UnitTests/Issues/Maui21038.xaml.cs @@ -0,0 +1,47 @@ +using System; +using Microsoft.Maui.ApplicationModel; +using Microsoft.Maui.Controls.Core.UnitTests; +using Microsoft.Maui.Dispatching; + +using Microsoft.Maui.UnitTests; +using NUnit.Framework; + +namespace Microsoft.Maui.Controls.Xaml.UnitTests; +[XamlCompilation(XamlCompilationOptions.Skip)] +public partial class Maui21038 +{ + public Maui21038() + { + InitializeComponent(); + } + + public Maui21038(bool useCompiledXaml) + { + //this stub will be replaced at compile time + } + + [TestFixture] + class Test + { + [SetUp] + public void Setup() + { + Application.SetCurrentApplication(new MockApplication()); + DispatcherProvider.SetCurrent(new DispatcherProviderStub()); + } + + [TearDown] public void TearDown() => AppInfo.SetCurrent(null); + + [Test] + public void XamlParseErrorsHaveFileInfo([Values(false, true)] bool useCompiledXaml) + { + if (useCompiledXaml) + { + MockCompiler.Compile(typeof(Maui21038), out var md, out var hasLoggedErrors); + Assert.That(hasLoggedErrors); + } + else + Assert.Throws(() => new Maui21038(useCompiledXaml)); + } + } +} diff --git a/src/Controls/tests/Xaml.UnitTests/Issues/Maui21434.cs b/src/Controls/tests/Xaml.UnitTests/Issues/Maui21434.xaml.cs similarity index 100% rename from src/Controls/tests/Xaml.UnitTests/Issues/Maui21434.cs rename to src/Controls/tests/Xaml.UnitTests/Issues/Maui21434.xaml.cs diff --git a/src/Controls/tests/Xaml.UnitTests/Issues/Maui23711.xaml.cs b/src/Controls/tests/Xaml.UnitTests/Issues/Maui23711.xaml.cs index f74b354b3002..888e399d0606 100644 --- a/src/Controls/tests/Xaml.UnitTests/Issues/Maui23711.xaml.cs +++ b/src/Controls/tests/Xaml.UnitTests/Issues/Maui23711.xaml.cs @@ -47,7 +47,8 @@ public void TearDown() [Test] public void UsesReflectionBasedBindingsWhenCompilationOfBindingsWithSourceIsDisabled([Values(false, true)] bool compileBindingsWithSource) { - MockCompiler.Compile(typeof(Maui23711), out MethodDefinition methodDefinition, compileBindingsWithSource: compileBindingsWithSource); + MockCompiler.Compile(typeof(Maui23711), out MethodDefinition methodDefinition, out bool hasLoggedErrors, compileBindingsWithSource: compileBindingsWithSource); + Assert.That(!hasLoggedErrors); Assert.AreEqual(compileBindingsWithSource, ContainsTypedBindingInstantiation(methodDefinition)); } diff --git a/src/Controls/tests/Xaml.UnitTests/Issues/Maui25406.xaml.cs b/src/Controls/tests/Xaml.UnitTests/Issues/Maui25406.xaml.cs index 9b74a2698e2a..b07b6ea72cc0 100644 --- a/src/Controls/tests/Xaml.UnitTests/Issues/Maui25406.xaml.cs +++ b/src/Controls/tests/Xaml.UnitTests/Issues/Maui25406.xaml.cs @@ -40,7 +40,8 @@ public void TearDown() [Test] public void WhenBindingIsCompiledBindingExtensionDoesNotReceiveServiceProviderWithXamlTypeResolver() { - MockCompiler.Compile(typeof(Maui25406), out var md, generateFullIl: false); + MockCompiler.Compile(typeof(Maui25406), out var md, out bool hasLoggedErrors, generateFullIl: false); + Assert.That(!hasLoggedErrors); Assert.That(!md.Body.Instructions.Any(static i => i.OpCode == OpCodes.Newobj && i.Operand.ToString().Contains("XamlServiceProvider", StringComparison.Ordinal))); } } diff --git a/src/Controls/tests/Xaml.UnitTests/MockCompiler.cs b/src/Controls/tests/Xaml.UnitTests/MockCompiler.cs index 30323b8ee322..ad918c83b7ab 100644 --- a/src/Controls/tests/Xaml.UnitTests/MockCompiler.cs +++ b/src/Controls/tests/Xaml.UnitTests/MockCompiler.cs @@ -9,18 +9,29 @@ namespace Microsoft.Maui.Controls.Xaml.UnitTests { public static class MockCompiler { + public static void Compile( + Type type, + out bool hasLoggedErrors, + string targetFramework = null, + bool treatWarningsAsErrors = false, + bool compileBindingsWithSource = true) + => Compile(type, out _, out hasLoggedErrors, targetFramework, treatWarningsAsErrors, compileBindingsWithSource); + public static void Compile( Type type, string targetFramework = null, bool treatWarningsAsErrors = false, bool compileBindingsWithSource = true) - { - Compile(type, out _, targetFramework, treatWarningsAsErrors, compileBindingsWithSource); - } + { + Compile(type, out _, out var hasLoggedErrors, targetFramework, treatWarningsAsErrors, compileBindingsWithSource); + if (hasLoggedErrors) + throw new Exception("XamlC failed"); + } public static void Compile( Type type, out MethodDefinition methodDefinition, + out bool hasLoggedErrors, string targetFramework = null, bool treatWarningsAsErrors = false, bool compileBindingsWithSource = true, @@ -51,6 +62,7 @@ public static void Compile( if (xamlc.Execute(out IList exceptions) || exceptions == null || !exceptions.Any()) { methodDefinition = xamlc.InitCompForType; + hasLoggedErrors = xamlc.LoggingHelper.HasLoggedErrors; return; } if (exceptions.Count > 1) diff --git a/src/Controls/tests/Xaml.UnitTests/MultipleDataTemplateChildElements.xaml.cs b/src/Controls/tests/Xaml.UnitTests/MultipleDataTemplateChildElements.xaml.cs index f48862e6cbb7..9f73c26f680b 100644 --- a/src/Controls/tests/Xaml.UnitTests/MultipleDataTemplateChildElements.xaml.cs +++ b/src/Controls/tests/Xaml.UnitTests/MultipleDataTemplateChildElements.xaml.cs @@ -13,13 +13,16 @@ public MultipleDataTemplateChildElements(bool useCompiledXaml) [TestFixture] public static class Tests { - [TestCase(false)] - [TestCase(true)] - public static void ThrowXamlParseException(bool useCompiledXaml) + [Test] + public static void ThrowXamlParseException([Values]bool useCompiledXaml) { - Assert.Throws(useCompiledXaml ? - (TestDelegate)(() => MockCompiler.Compile(typeof(MultipleDataTemplateChildElements))) : - () => new MultipleDataTemplateChildElements(useCompiledXaml)); + if (useCompiledXaml) + { + MockCompiler.Compile(typeof(MultipleDataTemplateChildElements), out var md, out var hasLoggedErrors); + Assert.That(hasLoggedErrors); + } + else + Assert.Throws(() => new MultipleDataTemplateChildElements(useCompiledXaml)); } } } diff --git a/src/Controls/tests/Xaml.UnitTests/OnPlatformOptimization.xaml.cs b/src/Controls/tests/Xaml.UnitTests/OnPlatformOptimization.xaml.cs index 4fb6b01d0a0a..fd43309d77e7 100644 --- a/src/Controls/tests/Xaml.UnitTests/OnPlatformOptimization.xaml.cs +++ b/src/Controls/tests/Xaml.UnitTests/OnPlatformOptimization.xaml.cs @@ -28,7 +28,8 @@ public class Tests [Test] public void OnPlatformExtensionsAreSimplified([Values("net7.0-ios", "net7.0-android")] string targetFramework) { - MockCompiler.Compile(typeof(OnPlatformOptimization), out var methodDef, targetFramework); + MockCompiler.Compile(typeof(OnPlatformOptimization), out var methodDef, out var hasLoggedErrors, targetFramework); + Assert.That(!hasLoggedErrors); Assert.That(!methodDef.Body.Instructions.Any(instr => InstructionIsOnPlatformExtensionCtor(methodDef, instr)), "This Xaml still generates a new OnPlatformExtension()"); var expected = targetFramework.EndsWith("-ios") ? "bar" : "foo"; @@ -48,7 +49,8 @@ public void ValuesAreSet(bool useCompiledXaml) [Ignore("capability disabled for now")] public void OnPlatformAreSimplified([Values("net6.0-ios", "net6.0-android")] string targetFramework) { - MockCompiler.Compile(typeof(OnPlatformOptimization), out var methodDef, targetFramework); + MockCompiler.Compile(typeof(OnPlatformOptimization), out var methodDef, out bool hasLoggedErrors, targetFramework); + Assert.That(!hasLoggedErrors); Assert.That(!methodDef.Body.Instructions.Any(instr => InstructionIsOnPlatformCtor(methodDef, instr)), "This Xaml still generates a new OnPlatform()"); Assert.That(methodDef.Body.Instructions.Any(instr => InstructionIsLdstr(methodDef.Module, instr, "Mobile, eventually ?")), $"This Xaml doesn't generate content for {targetFramework}"); Assert.That(!methodDef.Body.Instructions.Any(instr => InstructionIsLdstr(methodDef.Module, instr, "Desktop, maybe ?")), $"This Xaml generates content not required for {targetFramework}"); diff --git a/src/Controls/tests/Xaml.UnitTests/XArray.xaml.cs b/src/Controls/tests/Xaml.UnitTests/XArray.xaml.cs index 749e17399749..e232ca18fb4a 100644 --- a/src/Controls/tests/Xaml.UnitTests/XArray.xaml.cs +++ b/src/Controls/tests/Xaml.UnitTests/XArray.xaml.cs @@ -42,7 +42,8 @@ public void SupportsXArray(bool useCompiledXaml) [Test] public void ArrayExtensionNotPresentInGeneratedCode([Values(false)] bool useCompiledXaml) { - MockCompiler.Compile(typeof(XArray), out var methodDef); + MockCompiler.Compile(typeof(XArray), out var methodDef, out var hasLoggedErrors); + Assert.That(!hasLoggedErrors); Assert.That(!methodDef.Body.Instructions.Any(instr => InstructionIsArrayExtensionCtor(methodDef, instr)), "This Xaml still generates a new ArrayExtension()"); }