[XC] Add file info to exception reports (dotnet#25359)

* add file info to exception reports * add test, generate full path, position * test for loggedErrors
Glepooek · Nov 28, 2024 · 489657c · 489657c
1 parent ccc35cd
commit 489657c
Show file tree

Hide file tree

Showing 19 changed files with 141 additions and 37 deletions.
diff --git 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<Exception> 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
@@ -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
@@ -52,7 +52,7 @@ public XamlParseException(string message, IXmlLineInfo xmlInfo, Exception innerE
 		/// <include file="../../docs/Microsoft.Maui.Controls.Xaml/XamlParseException.xml" path="//Member[@MemberName='XmlInfo']/Docs/*" />
 		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
@@ -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);
+    }
+}
diff --git 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<XmlType> 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
@@ -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
@@ -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
@@ -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<XamlParseException>(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<XamlParseException>(() => new DuplicatePropertyElements(useCompiledXaml));
 			}
 		}
 	}

diff --git 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<XamlParseException>(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<XamlParseException>(() => new DuplicateXArgumentsElements(useCompiledXaml));
 			}
 		}
 	}

diff --git 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
@@ -0,0 +1,10 @@
+<?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"
+                    xmlns:local="clr-namespace:Microsoft.Maui.Controls.Xaml.UnitTests"
+                    x:Class="Microsoft.Maui.Controls.Xaml.UnitTests.Maui21038">
+    <Label>
+        <Label.Text>foo</Label.Text>
+        <Label.Text>bar</Label.Text>
+    </Label>
+</ContentPage>
diff --git a/src/Controls/tests/Xaml.UnitTests/Issues/Maui21038.xaml.cs 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<XamlParseException>(() => new Maui21038(useCompiledXaml));
+        }
+    }
+}
diff --git a/.../tests/Xaml.UnitTests/Issues/Maui21434.cs → ...s/Xaml.UnitTests/Issues/Maui21434.xaml.cs b/.../tests/Xaml.UnitTests/Issues/Maui21434.cs → ...s/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
@@ -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
@@ -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
@@ -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<Exception> 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
@@ -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<XamlParseException>(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<XamlParseException>(() => new MultipleDataTemplateChildElements(useCompiledXaml));
 			}
 		}
 	}

diff --git 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
@@ -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()");
 			}