From 54f4b0c24bc1622464e7c94db8a1efc7613a75a8 Mon Sep 17 00:00:00 2001 From: Sergey Tepliakov Date: Tue, 20 Feb 2018 14:59:05 -0800 Subject: [PATCH] Fix stack trace for tuples type as part of a generic type. Fix for #57 --- .../EnhancedStackTrace.Frames.cs | 38 +++++++--- .../Internal/ReflectionHelper.cs | 30 ++++++++ .../Internal/TypeNameHelper.cs | 16 ++++ .../AggregateException.cs | 5 +- .../DynamicCompilation.cs | 4 +- .../Ben.Demystifier.Test/LineEndingsHelper.cs | 14 ++++ .../NonThrownException.cs | 8 +- .../ReflectionHelperTests.cs | 27 +++++++ test/Ben.Demystifier.Test/TuplesTests.cs | 76 +++++++++++++++++++ 9 files changed, 196 insertions(+), 22 deletions(-) create mode 100644 src/Ben.Demystifier/Internal/ReflectionHelper.cs create mode 100644 test/Ben.Demystifier.Test/LineEndingsHelper.cs create mode 100644 test/Ben.Demystifier.Test/ReflectionHelperTests.cs create mode 100644 test/Ben.Demystifier.Test/TuplesTests.cs diff --git a/src/Ben.Demystifier/EnhancedStackTrace.Frames.cs b/src/Ben.Demystifier/EnhancedStackTrace.Frames.cs index 5b6e1d7..d2acf07 100644 --- a/src/Ben.Demystifier/EnhancedStackTrace.Frames.cs +++ b/src/Ben.Demystifier/EnhancedStackTrace.Frames.cs @@ -19,7 +19,6 @@ namespace System.Diagnostics { public partial class EnhancedStackTrace { - private static List GetFrames(Exception exception) { if (exception == null) @@ -516,7 +515,7 @@ namespace System.Diagnostics { foreach (var attrib in attribs) { - if (attrib is Attribute att && att.GetType().Namespace == "System.Runtime.CompilerServices" && att.GetType().Name == "IsReadOnlyAttribute") + if (attrib is Attribute att && att.GetType().IsReadOnlyAttribute()) { return "in"; } @@ -579,6 +578,32 @@ namespace System.Diagnostics } private static ResolvedParameter GetValueTupleParameter(IList tupleNames, string prefix, string name, Type parameterType) + { + string typeName; + + if (parameterType.IsValueTuple()) + { + typeName = GetValueTupleParameterName(tupleNames, parameterType); + + } + else + { + // Need to unwrap the first generic argument first. + var genericTypeName = TypeNameHelper.GetTypeNameForGenericType(parameterType); + var valueTupleFullName = GetValueTupleParameterName(tupleNames, parameterType.GetGenericArguments()[0]); + typeName = $"{genericTypeName}<{valueTupleFullName}>"; + } + + return new ResolvedParameter + { + Prefix = prefix, + Name = name, + Type = typeName, + ResolvedType = parameterType, + }; + } + + private static string GetValueTupleParameterName(IList tupleNames, Type parameterType) { var sb = new StringBuilder(); sb.Append("("); @@ -608,14 +633,7 @@ namespace System.Diagnostics } sb.Append(")"); - - return new ResolvedParameter - { - Prefix = prefix, - Name = name, - Type = sb.ToString(), - ResolvedType = parameterType, - }; + return sb.ToString(); } private static bool ShowInStackTrace(MethodBase method) diff --git a/src/Ben.Demystifier/Internal/ReflectionHelper.cs b/src/Ben.Demystifier/Internal/ReflectionHelper.cs new file mode 100644 index 0000000..9306532 --- /dev/null +++ b/src/Ben.Demystifier/Internal/ReflectionHelper.cs @@ -0,0 +1,30 @@ +// Copyright (c) Ben A Adams. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Reflection; +using System.Reflection.Emit; + +namespace System.Diagnostics.Internal +{ + /// + /// A helper class that contains utilities methods for dealing with reflection. + /// + public static class ReflectionHelper + { + /// + /// Returns true if is System.Runtime.CompilerServices.IsReadOnlyAttribute. + /// + public static bool IsReadOnlyAttribute(this Type type) + { + return type.Namespace == "System.Runtime.CompilerServices" && type.Name == "IsReadOnlyAttribute"; + } + + /// + /// Returns true if the is a value tuple type. + /// + public static bool IsValueTuple(this Type type) + { + return type.Namespace == "System" && type.Name.Contains("ValueTuple`"); + } + } +} diff --git a/src/Ben.Demystifier/Internal/TypeNameHelper.cs b/src/Ben.Demystifier/Internal/TypeNameHelper.cs index 217b183..fadce4e 100644 --- a/src/Ben.Demystifier/Internal/TypeNameHelper.cs +++ b/src/Ben.Demystifier/Internal/TypeNameHelper.cs @@ -43,6 +43,22 @@ namespace System.Diagnostics.Internal return builder.ToString(); } + /// + /// Returns a name of given generic type without '`'. + /// + public static string GetTypeNameForGenericType(Type type) + { + if (!type.IsGenericType) + { + throw new ArgumentException("The given type should be generic", nameof(type)); + } + + var genericPartIndex = type.Name.IndexOf('`'); + Debug.Assert(genericPartIndex >= 0); + + return type.Name.Substring(0, genericPartIndex); + } + private static void ProcessType(StringBuilder builder, Type type, DisplayNameOptions options) { if (type.IsGenericType) diff --git a/test/Ben.Demystifier.Test/AggregateException.cs b/test/Ben.Demystifier.Test/AggregateException.cs index b4d66df..3c0034a 100644 --- a/test/Ben.Demystifier.Test/AggregateException.cs +++ b/test/Ben.Demystifier.Test/AggregateException.cs @@ -33,7 +33,7 @@ namespace Ben.Demystifier.Test // Assert var stackTrace = demystifiedException.ToString(); - stackTrace = ReplaceLineEndings.Replace(stackTrace, ""); + stackTrace = LineEndingsHelper.RemoveLineEndings(stackTrace); var trace = string.Join("", stackTrace.Split(new[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries) // Remove items that vary between test runners .Where(s => @@ -80,8 +80,5 @@ namespace Ben.Demystifier.Test await Task.Delay(1).ConfigureAwait(false); throw new InvalidOperationException(); } - - - private Regex ReplaceLineEndings = new Regex(" in [^\n\r]+"); } } diff --git a/test/Ben.Demystifier.Test/DynamicCompilation.cs b/test/Ben.Demystifier.Test/DynamicCompilation.cs index f5dc588..3731011 100644 --- a/test/Ben.Demystifier.Test/DynamicCompilation.cs +++ b/test/Ben.Demystifier.Test/DynamicCompilation.cs @@ -37,7 +37,7 @@ namespace Ben.Demystifier.Test // Assert var stackTrace = demystifiedException.ToString(); - stackTrace = ReplaceLineEndings.Replace(stackTrace, ""); + stackTrace = LineEndingsHelper.RemoveLineEndings(stackTrace); var trace = stackTrace.Split(new[] { Environment.NewLine }, StringSplitOptions.None) // Remove items that vary between test runners .Where(s => @@ -53,7 +53,5 @@ namespace Ben.Demystifier.Test " at async Task Ben.Demystifier.Test.DynamicCompilation.DoesNotPreventStackTrace()"}, trace); } - - private Regex ReplaceLineEndings = new Regex(" in [^\n\r]+"); } } diff --git a/test/Ben.Demystifier.Test/LineEndingsHelper.cs b/test/Ben.Demystifier.Test/LineEndingsHelper.cs new file mode 100644 index 0000000..8a475f2 --- /dev/null +++ b/test/Ben.Demystifier.Test/LineEndingsHelper.cs @@ -0,0 +1,14 @@ +using System.Text.RegularExpressions; + +namespace Ben.Demystifier.Test +{ + internal static class LineEndingsHelper + { + private static readonly Regex ReplaceLineEndings = new Regex(" in [^\n\r]+"); + + public static string RemoveLineEndings(string original) + { + return ReplaceLineEndings.Replace(original, ""); + } + } +} diff --git a/test/Ben.Demystifier.Test/NonThrownException.cs b/test/Ben.Demystifier.Test/NonThrownException.cs index d3c96db..ef5abf2 100644 --- a/test/Ben.Demystifier.Test/NonThrownException.cs +++ b/test/Ben.Demystifier.Test/NonThrownException.cs @@ -28,7 +28,7 @@ namespace Ben.Demystifier.Test // Assert var stackTrace = demystifiedException.ToString(); - stackTrace = ReplaceLineEndings.Replace(stackTrace, ""); + stackTrace = LineEndingsHelper.RemoveLineEndings(stackTrace); var trace = stackTrace.Split(new[]{Environment.NewLine}, StringSplitOptions.None); Assert.Equal( @@ -51,7 +51,7 @@ namespace Ben.Demystifier.Test // Assert stackTrace = demystifiedException.ToString(); - stackTrace = ReplaceLineEndings.Replace(stackTrace, ""); + stackTrace = LineEndingsHelper.RemoveLineEndings(stackTrace); trace = stackTrace.Split(new[] { Environment.NewLine }, StringSplitOptions.None); Assert.Equal( @@ -76,7 +76,7 @@ namespace Ben.Demystifier.Test // Assert var stackTrace = est.ToString(); - stackTrace = ReplaceLineEndings.Replace(stackTrace, ""); + stackTrace = LineEndingsHelper.RemoveLineEndings(stackTrace); var trace = stackTrace.Split(new[] { Environment.NewLine }, StringSplitOptions.None) // Remove Full framework entries .Where(s => !s.StartsWith(" at bool System.Threading._ThreadPoolWaitCallbac") && @@ -88,7 +88,5 @@ namespace Ben.Demystifier.Test " at bool System.Threading.ThreadPoolWorkQueue.Dispatch()"}, trace); } - - private Regex ReplaceLineEndings = new Regex(" in [^\n\r]+"); } } diff --git a/test/Ben.Demystifier.Test/ReflectionHelperTests.cs b/test/Ben.Demystifier.Test/ReflectionHelperTests.cs new file mode 100644 index 0000000..5773e6b --- /dev/null +++ b/test/Ben.Demystifier.Test/ReflectionHelperTests.cs @@ -0,0 +1,27 @@ +using System; +using System.Diagnostics.Internal; +using Xunit; + +namespace Ben.Demystifier.Test +{ + public class ReflectionHelperTest + { + [Fact] + public void IsValueTupleReturnsTrueForTupleWith1Element() + { + Assert.True(typeof(ValueTuple).IsValueTuple()); + } + + [Fact] + public void IsValueTupleReturnsTrueForTupleWith1ElementWithOpenedType() + { + Assert.True(typeof(ValueTuple<>).IsValueTuple()); + } + + [Fact] + public void IsValueTupleReturnsTrueForTupleWith6ElementsWithOpenedType() + { + Assert.True(typeof(ValueTuple<,,,,,>).IsValueTuple()); + } + } +} diff --git a/test/Ben.Demystifier.Test/TuplesTests.cs b/test/Ben.Demystifier.Test/TuplesTests.cs new file mode 100644 index 0000000..698336e --- /dev/null +++ b/test/Ben.Demystifier.Test/TuplesTests.cs @@ -0,0 +1,76 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Threading.Tasks; +using Xunit; + +namespace Ben.Demystifier.Test +{ + public class TuplesTests + { + [Fact] + public void DemistifiesAsyncMethodWithTuples() + { + Exception demystifiedException = null; + + try + { + AsyncThatReturnsTuple().GetAwaiter().GetResult(); + } + catch (Exception ex) + { + demystifiedException = ex.Demystify(); + } + + // Assert + var stackTrace = demystifiedException.ToString(); + stackTrace = LineEndingsHelper.RemoveLineEndings(stackTrace); + var trace = string.Join("", stackTrace.Split(new[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries)); + + var expected = string.Join("", new[] { + "System.ArgumentException: Value does not fall within the expected range.", + " at async Task<(int left, int right)> Ben.Demystifier.Test.TuplesTests.AsyncThatReturnsTuple()", + " at void Ben.Demystifier.Test.TuplesTests.DemistifiesAsyncMethodWithTuples()"}); + + Assert.Equal(expected, trace); + } + + [Fact] + public void DemistifiesListOfTuples() + { + Exception demystifiedException = null; + + try + { + ListOfTuples(); + } + catch (Exception ex) + { + demystifiedException = ex.Demystify(); + } + + // Assert + var stackTrace = demystifiedException.ToString(); + stackTrace = LineEndingsHelper.RemoveLineEndings(stackTrace); + var trace = string.Join("", stackTrace.Split(new[] { Environment.NewLine }, StringSplitOptions.RemoveEmptyEntries)); + + var expected = string.Join("", new[] { + "System.ArgumentException: Value does not fall within the expected range.", + " at List<(int left, int right)> Ben.Demystifier.Test.TuplesTests.ListOfTuples()", + " at void Ben.Demystifier.Test.TuplesTests.DemistifiesListOfTuples()"}); + + Assert.Equal(expected, trace); + } + + async Task<(int left, int right)> AsyncThatReturnsTuple() + { + await Task.Delay(1).ConfigureAwait(false); + throw new ArgumentException(); + } + + List<(int left, int right)> ListOfTuples() + { + throw new ArgumentException(); + } + } +}