Fix errors in diagnostic formats and log exceptions from future ones

mhutch · Jul 11, 2023 · 4d5d7b3 · 4d5d7b3
1 parent 5b61fd1
commit 4d5d7b3
Show file tree

Hide file tree

Showing 5 changed files with 103 additions and 8 deletions.
diff --git a/Core/Analysis/XmlCoreDiagnostics.cs b/Core/Analysis/XmlCoreDiagnostics.cs
@@ -8,7 +8,7 @@ class XmlCoreDiagnostics
 		public static XmlDiagnosticDescriptor IncompleteAttributeValue = new (
 			nameof (IncompleteAttributeValue),
 			"Incomplete attribute value",
-			"The value of attribute '{0}' ended unexpectedly.",
+			"The value of attribute '{0}' ended unexpectedly due to character '{1}'.",
 			XmlDiagnosticSeverity.Error
 		);
 
@@ -29,7 +29,7 @@ class XmlCoreDiagnostics
 		public static XmlDiagnosticDescriptor IncompleteAttribute = new (
 			nameof (IncompleteAttribute),
 			"Incomplete attribute",
-			"Attribute is incomplete due to unexpected character '{O}'.",
+			"Attribute is incomplete due to unexpected character '{0}'.",
 			XmlDiagnosticSeverity.Error
 		);
 
@@ -64,7 +64,7 @@ class XmlCoreDiagnostics
 		public static XmlDiagnosticDescriptor MalformedTagOpening = new (
 			nameof (MalformedTagOpening),
 			"Malformed tag",
-			"Tag is malformed due to unexpected character '{O}'.",
+			"Tag is malformed due to unexpected character '{0}'.",
 			XmlDiagnosticSeverity.Error
 		);
 

diff --git a/Core/Analysis/XmlDiagnosticDescriptor.cs b/Core/Analysis/XmlDiagnosticDescriptor.cs
@@ -3,17 +3,20 @@
 
 
 using System;
+using System.Diagnostics.CodeAnalysis;
 
 namespace MonoDevelop.Xml.Analysis
 {
 	public class XmlDiagnosticDescriptor
 	{
 		public string Id { get; }
 		public string Title { get; }
+
+		[StringSyntax (StringSyntaxAttribute.CompositeFormat)]
 		public string? Message { get; }
 		public XmlDiagnosticSeverity Severity { get; }
 
-		public XmlDiagnosticDescriptor (string id, string title, string? message, XmlDiagnosticSeverity severity)
+		public XmlDiagnosticDescriptor (string id, string title, [StringSyntax (StringSyntaxAttribute.CompositeFormat)] string? message, XmlDiagnosticSeverity severity)
 		{
 			Title = title ?? throw new ArgumentNullException (nameof (title));
 			Id = id ?? throw new ArgumentNullException (nameof (id));
@@ -28,9 +31,15 @@ public XmlDiagnosticDescriptor (string id, string title, XmlDiagnosticSeverity s
 
 		internal string GetFormattedMessage (object[]? args)
 		{
-			combinedMsg ??= (combinedMsg = Title + Environment.NewLine + Message);
-			if (args != null && args.Length > 0) {
-				return string.Format (combinedMsg, args);
+			try {
+				combinedMsg ??= (combinedMsg = Title + Environment.NewLine + Message);
+				if (args != null && args.Length > 0) {
+					return string.Format (combinedMsg, args);
+				}
+			} catch (FormatException ex) {
+				// this is likely to be called from somewhere other than where the diagnostic was constructed
+				// so ensure the error has enough info to track it down
+				throw new FormatException ($"Error formatting message for diagnostic {Id}", ex);
 			}
 			return combinedMsg;
 		}

diff --git a/Core/StringSyntaxAttribute.cs b/Core/StringSyntaxAttribute.cs
@@ -0,0 +1,71 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace System.Diagnostics.CodeAnalysis
+{
+#if !NET7_0_OR_GREATER
+	/// <summary>Specifies the syntax used in a string.</summary>
+	[AttributeUsage (AttributeTargets.Parameter | AttributeTargets.Field | AttributeTargets.Property, AllowMultiple = false, Inherited = false)]
+    sealed class StringSyntaxAttribute : Attribute
+    {
+        /// <summary>Initializes the <see cref="StringSyntaxAttribute"/> with the identifier of the syntax used.</summary>
+        /// <param name="syntax">The syntax identifier.</param>
+        public StringSyntaxAttribute(string syntax)
+        {
+            Syntax = syntax;
+            Arguments = Array.Empty<object?>();
+        }
+
+        /// <summary>Initializes the <see cref="StringSyntaxAttribute"/> with the identifier of the syntax used.</summary>
+        /// <param name="syntax">The syntax identifier.</param>
+        /// <param name="arguments">Optional arguments associated with the specific syntax employed.</param>
+        public StringSyntaxAttribute(string syntax, params object?[] arguments)
+        {
+            Syntax = syntax;
+            Arguments = arguments;
+        }
+
+        /// <summary>Gets the identifier of the syntax used.</summary>
+        public string Syntax { get; }
+
+        /// <summary>Optional arguments associated with the specific syntax employed.</summary>
+        public object?[] Arguments { get; }
+
+        /// <summary>The syntax identifier for strings containing composite formats for string formatting.</summary>
+        public const string CompositeFormat = nameof(CompositeFormat);
+
+        /// <summary>The syntax identifier for strings containing date format specifiers.</summary>
+        public const string DateOnlyFormat = nameof(DateOnlyFormat);
+
+        /// <summary>The syntax identifier for strings containing date and time format specifiers.</summary>
+        public const string DateTimeFormat = nameof(DateTimeFormat);
+
+        /// <summary>The syntax identifier for strings containing <see cref="Enum"/> format specifiers.</summary>
+        public const string EnumFormat = nameof(EnumFormat);
+
+        /// <summary>The syntax identifier for strings containing <see cref="Guid"/> format specifiers.</summary>
+        public const string GuidFormat = nameof(GuidFormat);
+
+        /// <summary>The syntax identifier for strings containing JavaScript Object Notation (JSON).</summary>
+        public const string Json = nameof(Json);
+
+        /// <summary>The syntax identifier for strings containing numeric format specifiers.</summary>
+        public const string NumericFormat = nameof(NumericFormat);
+
+        /// <summary>The syntax identifier for strings containing regular expressions.</summary>
+        public const string Regex = nameof(Regex);
+
+        /// <summary>The syntax identifier for strings containing time format specifiers.</summary>
+        public const string TimeOnlyFormat = nameof(TimeOnlyFormat);
+
+        /// <summary>The syntax identifier for strings containing <see cref="TimeSpan"/> format specifiers.</summary>
+        public const string TimeSpanFormat = nameof(TimeSpanFormat);
+
+        /// <summary>The syntax identifier for strings containing URIs.</summary>
+        public const string Uri = nameof(Uri);
+
+        /// <summary>The syntax identifier for strings containing XML.</summary>
+        public const string Xml = nameof(Xml);
+    }
+#endif
+}
diff --git a/Editor/Tagging/XmlSyntaxValidationTagger.cs b/Editor/Tagging/XmlSyntaxValidationTagger.cs
@@ -13,20 +13,25 @@
 
 using MonoDevelop.Xml.Analysis;
 using MonoDevelop.Xml.Editor.Parsing;
+using MonoDevelop.Xml.Editor.Logging;
+using Microsoft.Extensions.Logging;
+using MonoDevelop.Xml.Logging;
 
 namespace MonoDevelop.MSBuild.Editor
 {
 	class XmlSyntaxValidationTagger : ITagger<IErrorTag>, IDisposable
 	{
 		readonly XmlBackgroundParser parser;
 		readonly JoinableTaskContext joinableTaskContext;
+		readonly ILogger<XmlSyntaxValidationTagger> logger;
 		ParseCompletedEventArgs<XmlParseResult>? lastArgs;
 
 		public XmlSyntaxValidationTagger (ITextBuffer buffer, XmlSyntaxValidationTaggerProvider provider)
 		{
 			parser = provider.ParserProvider.GetParser (buffer);
 			parser.ParseCompleted += ParseCompleted;
 			joinableTaskContext = provider.JoinableTaskContext;
+			logger = provider.LoggerFactory.GetLogger<XmlSyntaxValidationTagger> (buffer);
 		}
 
 		void ParseCompleted (object? sender, ParseCompletedEventArgs<XmlParseResult> args)
@@ -48,6 +53,9 @@ public void Dispose ()
 		}
 
 		public IEnumerable<ITagSpan<IErrorTag>> GetTags (NormalizedSnapshotSpanCollection spans)
+			=> logger.InvokeAndLogExceptions (() => GetTagsInternal (spans));
+
+		IEnumerable<ITagSpan<IErrorTag>> GetTagsInternal (NormalizedSnapshotSpanCollection spans)
 		{
 			//this may be assigned from another thread so capture a consistent value
 			var args = lastArgs;
@@ -84,6 +92,10 @@ static string GetErrorTypeName (XmlDiagnosticSeverity severity)
 				return PredefinedErrorTypeNames.SyntaxError;
 			case XmlDiagnosticSeverity.Warning:
 				return PredefinedErrorTypeNames.Warning;
+			case XmlDiagnosticSeverity.Suggestion:
+				return PredefinedErrorTypeNames.HintedSuggestion;
+			case XmlDiagnosticSeverity.None:
+				return PredefinedErrorTypeNames.Suggestion;
 			}
 			throw new ArgumentException ($"Unknown DiagnosticSeverity value {severity}", nameof (severity));
 		}

diff --git a/Editor/Tagging/XmlSyntaxValidationTaggerProvider.cs b/Editor/Tagging/XmlSyntaxValidationTaggerProvider.cs
@@ -12,6 +12,7 @@
 using Microsoft.VisualStudio.Utilities;
 
 using MonoDevelop.Xml.Editor;
+using MonoDevelop.Xml.Editor.Logging;
 using MonoDevelop.Xml.Editor.Parsing;
 
 namespace MonoDevelop.MSBuild.Editor
@@ -25,12 +26,14 @@ class XmlSyntaxValidationTaggerProvider : ITaggerProvider
 	{
 		public JoinableTaskContext JoinableTaskContext { get; }
 		public XmlParserProvider ParserProvider { get; }
+		public IEditorLoggerFactory LoggerFactory { get; }
 
 		[ImportingConstructor]
-		public XmlSyntaxValidationTaggerProvider (JoinableTaskContext joinableTaskContext, XmlParserProvider parserProvider)
+		public XmlSyntaxValidationTaggerProvider (JoinableTaskContext joinableTaskContext, XmlParserProvider parserProvider, IEditorLoggerFactory loggerFactory)
 		{
 			JoinableTaskContext = joinableTaskContext;
 			ParserProvider = parserProvider;
+			LoggerFactory = loggerFactory;
 		}
 
 		public ITagger<T> CreateTagger<T> (ITextBuffer buffer) where T : ITag