Merge pull request #1755 from 0xced/fix-generic-exception-formatting

spectreconsole · Feb 5, 2025 · c1eb94c · c1eb94c
2 parents a6b96e9 + 9d8d3c1
commit c1eb94c
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 23 deletions.
diff --git a/src/Spectre.Console/Widgets/Exceptions/ExceptionFormatter.cs b/src/Spectre.Console/Widgets/Exceptions/ExceptionFormatter.cs
@@ -33,11 +33,12 @@ private static Markup GetMessage(Exception ex, ExceptionSettings settings)
     {
         var shortenTypes = (settings.Format & ExceptionFormats.ShortenTypes) != 0;
         var exceptionType = ex.GetType();
-        var exceptionTypeFullName = exceptionType.FullName ?? exceptionType.Name;
-        var type = Emphasize(exceptionTypeFullName, new[] { '.' }, settings.Style.Exception, shortenTypes, settings);
+        var exceptionTypeName = TypeNameHelper.GetTypeDisplayName(exceptionType, fullName: !shortenTypes, includeSystemNamespace: true);
+        var type = new StringBuilder();
+        Emphasize(type, exceptionTypeName, new[] { '.' }, settings.Style.Exception, shortenTypes, settings, limit: '<');
 
         var message = $"[{settings.Style.Message.ToMarkup()}]{ex.Message.EscapeMarkup()}[/]";
-        return new Markup(string.Concat(type, ": ", message));
+        return new Markup($"{type}: {message}");
     }
 
     private static Grid GetStackFrames(Exception ex, ExceptionSettings settings)
@@ -101,7 +102,7 @@ private static Grid GetStackFrames(Exception ex, ExceptionSettings settings)
                 builder.Append(' ');
             }
 
-            builder.Append(Emphasize(methodName, new[] { '.' }, styles.Method, shortenMethods, settings));
+            Emphasize(builder, methodName, new[] { '.' }, styles.Method, shortenMethods, settings);
             builder.AppendWithStyle(styles.Parenthesis, "(");
             AppendParameters(builder, method, settings);
             builder.AppendWithStyle(styles.Parenthesis, ")");
@@ -168,7 +169,7 @@ private static void AppendPath(StringBuilder builder, string path, ExceptionSett
         void AppendPath()
         {
             var shortenPaths = (settings.Format & ExceptionFormats.ShortenPaths) != 0;
-            builder.Append(Emphasize(path, new[] { '/', '\\' }, settings.Style.Path, shortenPaths, settings));
+            Emphasize(builder, path, new[] { '/', '\\' }, settings.Style.Path, shortenPaths, settings);
         }
 
         if ((settings.Format & ExceptionFormats.ShowLinks) != 0)
@@ -192,32 +193,25 @@ void AppendPath()
         }
     }
 
-    private static string Emphasize(string input, char[] separators, Style color, bool compact,
-        ExceptionSettings settings)
+    private static void Emphasize(StringBuilder builder, string input, char[] separators, Style color, bool compact,
+        ExceptionSettings settings, char? limit = null)
     {
-        var builder = new StringBuilder();
+        var limitIndex = limit.HasValue ? input.IndexOf(limit.Value) : -1;
 
-        var type = input;
-        var index = type.LastIndexOfAny(separators);
+        var index = limitIndex != -1 ? input[..limitIndex].LastIndexOfAny(separators) : input.LastIndexOfAny(separators);
         if (index != -1)
         {
             if (!compact)
             {
-                builder.AppendWithStyle(
-                    settings.Style.NonEmphasized,
-                    type.Substring(0, index + 1));
+                builder.AppendWithStyle(settings.Style.NonEmphasized, input[..(index + 1)]);
             }
 
-            builder.AppendWithStyle(
-                color,
-                type.Substring(index + 1, type.Length - index - 1));
+            builder.AppendWithStyle(color, input[(index + 1)..]);
         }
         else
         {
-            builder.Append(type.EscapeMarkup());
+            builder.AppendWithStyle(color, input);
         }
-
-        return builder.ToString();
     }
 
     private static bool ShowInStackTrace(StackFrame frame)

diff --git a/src/Spectre.Console/Widgets/Exceptions/TypeNameHelper.cs b/src/Spectre.Console/Widgets/Exceptions/TypeNameHelper.cs
@@ -39,11 +39,12 @@ internal static class TypeNameHelper
     /// <param name="type">The <see cref="Type"/>.</param>
     /// <param name="fullName"><c>true</c> to print a fully qualified name.</param>
     /// <param name="includeGenericParameterNames"><c>true</c> to include generic parameter names.</param>
+    /// <param name="includeSystemNamespace"><c>true</c> to include the <c>System</c> namespace.</param>
     /// <returns>The pretty printed type name.</returns>
-    public static string GetTypeDisplayName(Type type, bool fullName = false, bool includeGenericParameterNames = true)
+    public static string GetTypeDisplayName(Type type, bool fullName = false, bool includeGenericParameterNames = true, bool includeSystemNamespace = false)
     {
         var builder = new StringBuilder();
-        ProcessType(builder, type, new DisplayNameOptions(fullName, includeGenericParameterNames));
+        ProcessType(builder, type, new DisplayNameOptions(fullName, includeGenericParameterNames, includeSystemNamespace));
         return builder.ToString();
     }
 
@@ -71,7 +72,7 @@ private static void ProcessType(StringBuilder builder, Type type, DisplayNameOpt
         {
             builder.Append(builtInName);
         }
-        else if (type.Namespace == nameof(System))
+        else if (type.Namespace == nameof(System) && !options.IncludeSystemNamespace)
         {
             builder.Append(type.Name);
         }
@@ -181,14 +182,17 @@ private static void ProcessGenericType(StringBuilder builder, Type type, Type[]
 
     private struct DisplayNameOptions
     {
-        public DisplayNameOptions(bool fullName, bool includeGenericParameterNames)
+        public DisplayNameOptions(bool fullName, bool includeGenericParameterNames, bool includeSystemNamespace)
         {
             FullName = fullName;
             IncludeGenericParameterNames = includeGenericParameterNames;
+            IncludeSystemNamespace = includeSystemNamespace;
         }
 
         public bool FullName { get; }
 
         public bool IncludeGenericParameterNames { get; }
+
+        public bool IncludeSystemNamespace { get; }
     }
 }
diff --git a/src/Tests/Spectre.Console.Tests/Data/Exceptions.cs b/src/Tests/Spectre.Console.Tests/Data/Exceptions.cs
@@ -6,6 +6,8 @@ public static class TestExceptions
 
     public static bool GenericMethodThatThrows<T0, T1, TRet>(int? number) => throw new InvalidOperationException("Throwing!");
 
+    public static bool MethodThatThrowsGenericException<T>() => throw new GenericException<T>("Throwing!", default);
+
     public static void ThrowWithInnerException()
     {
         try
@@ -42,3 +44,6 @@ public static (string Key, List<T> Values) GetTuplesWithInnerException<T>((int F
         return ("key", new List<T>());
     }
 }
+
+#pragma warning disable CS9113 // Parameter is unread.
+public class GenericException<T>(string message, T value) : Exception(message);
diff --git a/...ests/Expectations/Exception/GenericException.Output_exceptionFormats=Default.verified.txt b/...ests/Expectations/Exception/GenericException.Output_exceptionFormats=Default.verified.txt
@@ -0,0 +1,4 @@
+[38;5;7mSpectre.Console.Tests.Data.[0m[38;5;15mGenericException<Spectre.Console.IAnsiConsole>[0m: [1;38;5;9mThrowing![0m
+  [38;5;8mat[0m [38;5;12mbool[0m [38;5;7mSpectre.Console.Tests.Data.TestExceptions.[0m[38;5;11mMethodThatThrowsGenericException<T>[0m[38;5;7m()[0m [38;5;8min[0m [38;5;7m{ProjectDirectory}Data/[0m[1;38;5;11mExceptions.cs[0m[38;5;8m:[0m[38;5;12m9[0m
+  [38;5;8mat[0m [38;5;12mvoid[0m [38;5;7mSpectre.Console.Tests.Unit.ExceptionTests.<>c.[0m[38;5;11m<Should_Write_GenericException>b__8_0[0m[38;5;7m()[0m [38;5;8min[0m [38;5;7m{ProjectDirectory}Unit/[0m[1;38;5;11mExceptionTests.cs[0m[38;5;8m:[0m[38;5;12m134[0m
+  [38;5;8mat[0m [38;5;12mException[0m [38;5;7mSpectre.Console.Tests.Unit.ExceptionTests.[0m[38;5;11mGetException[0m[38;5;7m([0m[38;5;12mAction[0m [38;5;7maction)[0m [38;5;8min[0m [38;5;7m{ProjectDirectory}Unit/[0m[1;38;5;11mExceptionTests.cs[0m[38;5;8m:[0m[38;5;12m147[0m
diff --git a/...Expectations/Exception/GenericException.Output_exceptionFormats=ShortenTypes.verified.txt b/...Expectations/Exception/GenericException.Output_exceptionFormats=ShortenTypes.verified.txt
@@ -0,0 +1,4 @@
+[38;5;15mGenericException<IAnsiConsole>[0m: [1;38;5;9mThrowing![0m
+  [38;5;8mat[0m [38;5;12mbool[0m [38;5;7mSpectre.Console.Tests.Data.TestExceptions.[0m[38;5;11mMethodThatThrowsGenericException<T>[0m[38;5;7m()[0m [38;5;8min[0m [38;5;7m{ProjectDirectory}Data/[0m[1;38;5;11mExceptions.cs[0m[38;5;8m:[0m[38;5;12m9[0m
+  [38;5;8mat[0m [38;5;12mvoid[0m [38;5;7mSpectre.Console.Tests.Unit.ExceptionTests.<>c.[0m[38;5;11m<Should_Write_GenericException>b__8_0[0m[38;5;7m()[0m [38;5;8min[0m [38;5;7m{ProjectDirectory}Unit/[0m[1;38;5;11mExceptionTests.cs[0m[38;5;8m:[0m[38;5;12m134[0m
+  [38;5;8mat[0m [38;5;12mException[0m [38;5;7mSpectre.Console.Tests.Unit.ExceptionTests.[0m[38;5;11mGetException[0m[38;5;7m([0m[38;5;12mAction[0m [38;5;7maction)[0m [38;5;8min[0m [38;5;7m{ProjectDirectory}Unit/[0m[1;38;5;11mExceptionTests.cs[0m[38;5;8m:[0m[38;5;12m147[0m
diff --git a/src/Tests/Spectre.Console.Tests/Unit/ExceptionTests.cs b/src/Tests/Spectre.Console.Tests/Unit/ExceptionTests.cs
@@ -123,6 +123,23 @@ public Task Should_Write_Exception_With_No_StackTrace()
         return Verifier.Verify(result);
     }
 
+    [Theory]
+    [InlineData(ExceptionFormats.Default)]
+    [InlineData(ExceptionFormats.ShortenTypes)]
+    [Expectation("GenericException")]
+    public Task Should_Write_GenericException(ExceptionFormats exceptionFormats)
+    {
+        // Given
+        var console = new TestConsole { EmitAnsiSequences = true }.Width(1024);
+        var dex = GetException(() => TestExceptions.MethodThatThrowsGenericException<IAnsiConsole>());
+
+        // When
+        var result = console.WriteNormalizedException(dex, exceptionFormats);
+
+        // Then
+        return Verifier.Verify(result).UseParameters(exceptionFormats);
+    }
+
     public static Exception GetException(Action action)
     {
         try