Skip to content

Commit

Permalink
Fix ReadOnlyDictionary and improve DebuggerAttributes (#94581)
Browse files Browse the repository at this point in the history
* Fix ReadOnlyDictionary and improve DebuggerAttributes

* Fix debugger type proxy attributes for Key and Value collections of ReadOnlyDictionary
* Separate methods to test validity of the DebuggerTypeProxy attribute
  * ValidateDebuggerTypeProxyProperties - to verify debugger type proxy in most cases
  * CreateDebuggerTypeProxyWithNullArgument - to verify that the constructor of the debugger type proxy throws when its argument is null.
* Simplify and make more uniform debugger type proxy tests

A prerequisite for #94289

* Include ReadOnlyDictionary's Keys and Values in shared tests of debugger attributes

* Excuded some test cases because of bugs in .Net Framework 4.8

---------

Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>
  • Loading branch information
arturek and eiriktsarpalis authored Jan 26, 2024
1 parent 57ea59f commit bcc1d3d
Show file tree
Hide file tree
Showing 14 changed files with 58 additions and 102 deletions.
11 changes: 9 additions & 2 deletions src/libraries/Common/tests/System/Collections/DebugView.Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,14 @@ private static IEnumerable<object[]> TestDebuggerAttributes_ListInputs()
yield return new object[] { new Dictionary<float, double> { { 1.0f, 1.0 }, { 2.0f, 2.0 } }.Values };
yield return new object[] { new SortedDictionary<Guid, string> { { Guid.NewGuid(), "One" }, { Guid.NewGuid(), "Two" } }.Keys };
yield return new object[] { new SortedDictionary<long, Guid> { { 1L, Guid.NewGuid() }, { 2L, Guid.NewGuid() } }.Values };
#if !NETFRAMEWORK
// In .Net Framework 4.8 KeyCollection and ValueCollection from ReadOnlyDictionary are marked with
// an incorrect debugger type proxy attribute. Both classes have two template parameters, but
// ICollectionDebugView<> used there has only one. Neither VS nor this testing code is able to
// create a type proxy in such case.
yield return new object[] { new ReadOnlyDictionary<double, float>(new Dictionary<double, float> { { 1.0, 1.0f }, { 2.0, 2.0f } }).Keys };
yield return new object[] { new ReadOnlyDictionary<float, double>(new Dictionary<float, double> { { 1.0f, 1.0 }, { 2.0f, 2.0 } }).Values };
#endif
}

public static IEnumerable<object[]> TestDebuggerAttributes_InputsPresentedAsDictionary()
Expand Down Expand Up @@ -213,8 +221,7 @@ public static void TestDebuggerAttributes_List(object obj)
[MemberData(nameof(TestDebuggerAttributes_Inputs))]
public static void TestDebuggerAttributes_Null(object obj)
{
Type proxyType = DebuggerAttributes.GetProxyType(obj);
TargetInvocationException tie = Assert.Throws<TargetInvocationException>(() => Activator.CreateInstance(proxyType, (object)null));
TargetInvocationException tie = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.CreateDebuggerTypeProxyWithNullArgument(obj.GetType()));
Assert.IsType<ArgumentNullException>(tie.InnerException);
}

Expand Down
60 changes: 27 additions & 33 deletions src/libraries/Common/tests/System/Diagnostics/DebuggerAttributes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal static object GetFieldValue(object obj, string fieldName)

internal static void InvokeDebuggerTypeProxyProperties(object obj)
{
DebuggerAttributeInfo info = ValidateDebuggerTypeProxyProperties(obj.GetType(), obj);
DebuggerAttributeInfo info = ValidateDebuggerTypeProxyProperties(obj);
foreach (PropertyInfo pi in info.Properties)
{
pi.GetValue(info.Instance, null);
Expand All @@ -40,17 +40,7 @@ internal static void InvokeDebuggerTypeProxyProperties(object obj)

internal static DebuggerAttributeInfo ValidateDebuggerTypeProxyProperties(object obj)
{
return ValidateDebuggerTypeProxyProperties(obj.GetType(), obj);
}

internal static DebuggerAttributeInfo ValidateDebuggerTypeProxyProperties(Type type, object obj)
{
return ValidateDebuggerTypeProxyProperties(type, type.GenericTypeArguments, obj);
}

internal static DebuggerAttributeInfo ValidateDebuggerTypeProxyProperties(Type type, Type[] genericTypeArguments, object obj)
{
Type proxyType = GetProxyType(type, genericTypeArguments);
Type proxyType = GetProxyType(obj);

// Create an instance of the proxy type, and make sure we can access all of the instance properties
// on the type without exception
Expand All @@ -63,35 +53,54 @@ internal static DebuggerAttributeInfo ValidateDebuggerTypeProxyProperties(Type t
};
}

public static DebuggerBrowsableState? GetDebuggerBrowsableState(MemberInfo info)
internal static void CreateDebuggerTypeProxyWithNullArgument(Type type)
{
Type proxyType = GetProxyType(type);
Activator.CreateInstance(proxyType, [null]);
}

internal static DebuggerBrowsableState? GetDebuggerBrowsableState(MemberInfo info)
{
CustomAttributeData debuggerBrowsableAttribute = info.CustomAttributes
.SingleOrDefault(a => a.AttributeType == typeof(DebuggerBrowsableAttribute));
// Enums in attribute constructors are boxed as ints, so cast to int? first.
return (DebuggerBrowsableState?)(int?)debuggerBrowsableAttribute?.ConstructorArguments.Single().Value;
}

public static IEnumerable<FieldInfo> GetDebuggerVisibleFields(Type debuggerAttributeType)
internal static IEnumerable<FieldInfo> GetDebuggerVisibleFields(Type debuggerAttributeType)
{
// The debugger doesn't evaluate non-public members of type proxies.
IEnumerable<FieldInfo> visibleFields = debuggerAttributeType.GetFields()
.Where(fi => fi.IsPublic && GetDebuggerBrowsableState(fi) != DebuggerBrowsableState.Never);
return visibleFields;
}

public static IEnumerable<PropertyInfo> GetDebuggerVisibleProperties(Type debuggerAttributeType)
internal static IEnumerable<PropertyInfo> GetDebuggerVisibleProperties(Type debuggerAttributeType)
{
// The debugger doesn't evaluate non-public members of type proxies. GetGetMethod returns null if the getter is non-public.
IEnumerable<PropertyInfo> visibleProperties = debuggerAttributeType.GetProperties()
.Where(pi => pi.GetGetMethod() != null && GetDebuggerBrowsableState(pi) != DebuggerBrowsableState.Never);
return visibleProperties;
}

public static object GetProxyObject(object obj) => Activator.CreateInstance(GetProxyType(obj), obj);
internal static object GetProxyObject(object obj) => Activator.CreateInstance(GetProxyType(obj), obj);

internal static Type GetProxyType(object obj) => GetProxyType(obj.GetType());

internal static Type GetProxyType(Type type)
{
CustomAttributeData cad = FindAttribute(type, attributeType: typeof(DebuggerTypeProxyAttribute));

public static Type GetProxyType(object obj) => GetProxyType(obj.GetType());
Type proxyType = cad.ConstructorArguments[0].ArgumentType == typeof(Type) ?
(Type)cad.ConstructorArguments[0].Value :
Type.GetType((string)cad.ConstructorArguments[0].Value);
if (type.GenericTypeArguments.Length > 0)
{
proxyType = proxyType.MakeGenericType(type.GenericTypeArguments);
}

public static Type GetProxyType(Type type) => GetProxyType(type, type.GenericTypeArguments);
return proxyType;
}

internal static DebuggerDisplayResult ValidateFullyDebuggerDisplayReferences(object obj)
{
Expand Down Expand Up @@ -136,21 +145,6 @@ private static CustomAttributeData FindAttribute(Type type, Type attributeType)
throw new InvalidOperationException($"Expected one {attributeType.Name} on {type}.");
}

private static Type GetProxyType(Type type, Type[] genericTypeArguments)
{
CustomAttributeData cad = FindAttribute(type, attributeType: typeof(DebuggerTypeProxyAttribute));

Type proxyType = cad.ConstructorArguments[0].ArgumentType == typeof(Type) ?
(Type)cad.ConstructorArguments[0].Value :
Type.GetType((string)cad.ConstructorArguments[0].Value);
if (genericTypeArguments.Length > 0)
{
proxyType = proxyType.MakeGenericType(genericTypeArguments);
}

return proxyType;
}

private static string FormatDebuggerDisplayNamedArgument(string argumentName, CustomAttributeData debuggerDisplayAttributeData, object obj)
{
CustomAttributeNamedArgument namedAttribute = debuggerDisplayAttributeData.NamedArguments.FirstOrDefault(na => na.MemberName == argumentName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,17 +84,8 @@ public static void DebuggerAttribute()
DebuggerAttributes.ValidateDebuggerDisplayReferences(new ArrayList());
DebuggerAttributes.ValidateDebuggerTypeProxyProperties(new ArrayList() { "a", 1, "b", 2 });

bool threwNull = false;
try
{
DebuggerAttributes.ValidateDebuggerTypeProxyProperties(typeof(ArrayList), null);
}
catch (TargetInvocationException ex)
{
threwNull = ex.InnerException is ArgumentNullException;
}

Assert.True(threwNull);
TargetInvocationException tie = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.CreateDebuggerTypeProxyWithNullArgument(typeof(ArrayList)));
Assert.IsType<ArgumentNullException>(tie.InnerException);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,19 +356,10 @@ public void DebuggerAttribute()

var hash = new Hashtable() { { "a", 1 }, { "b", 2 } };
DebuggerAttributes.ValidateDebuggerTypeProxyProperties(hash);
DebuggerAttributes.ValidateDebuggerTypeProxyProperties(typeof(Hashtable), Hashtable.Synchronized(hash));
DebuggerAttributes.ValidateDebuggerTypeProxyProperties(Hashtable.Synchronized(hash));

bool threwNull = false;
try
{
DebuggerAttributes.ValidateDebuggerTypeProxyProperties(typeof(Hashtable), null);
}
catch (TargetInvocationException ex)
{
threwNull = ex.InnerException is ArgumentNullException;
}

Assert.True(threwNull);
TargetInvocationException tie = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.CreateDebuggerTypeProxyWithNullArgument(typeof(Hashtable)));
Assert.IsType<ArgumentNullException>(tie.InnerException);
}

[Fact]
Expand Down
13 changes: 2 additions & 11 deletions src/libraries/System.Collections.NonGeneric/tests/QueueTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -112,17 +112,8 @@ public static void DebuggerAttribute()
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsDebuggerTypeProxyAttributeSupported))]
public static void DebuggerAttribute_NullQueue_ThrowsArgumentNullException()
{
bool threwNull = false;
try
{
DebuggerAttributes.ValidateDebuggerTypeProxyProperties(typeof(Queue), null);
}
catch (TargetInvocationException ex)
{
threwNull = ex.InnerException is ArgumentNullException;
}

Assert.True(threwNull);
TargetInvocationException tie = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.CreateDebuggerTypeProxyWithNullArgument(typeof(Queue)));
Assert.IsType<ArgumentNullException>(tie.InnerException);
}

[Theory]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,17 +243,8 @@ public void DebuggerAttribute_SynchronizedList()
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsDebuggerTypeProxyAttributeSupported))]
public void DebuggerAttribute_NullSortedList_ThrowsArgumentNullException()
{
bool threwNull = false;
try
{
DebuggerAttributes.ValidateDebuggerTypeProxyProperties(typeof(SortedList), null);
}
catch (TargetInvocationException ex)
{
threwNull = ex.InnerException is ArgumentNullException;
}

Assert.True(threwNull);
TargetInvocationException tie = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.CreateDebuggerTypeProxyWithNullArgument(typeof(ArrayList)));
Assert.IsType<ArgumentNullException>(tie.InnerException);
}

[Fact]
Expand Down
13 changes: 2 additions & 11 deletions src/libraries/System.Collections.NonGeneric/tests/StackTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,8 @@ public static void DebuggerAttribute()
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsDebuggerTypeProxyAttributeSupported))]
public static void DebuggerAttribute_NullStack_ThrowsArgumentNullException()
{
bool threwNull = false;
try
{
DebuggerAttributes.ValidateDebuggerTypeProxyProperties(typeof(Stack), null);
}
catch (TargetInvocationException ex)
{
threwNull = ex.InnerException is ArgumentNullException;
}

Assert.True(threwNull);
TargetInvocationException tie = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.CreateDebuggerTypeProxyWithNullArgument(typeof(Stack)));
Assert.IsType<ArgumentNullException>(tie.InnerException);
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public static void DebuggerAttributeTests()
[ActiveIssue("https://github.com/dotnet/runtime/issues/57588", typeof(PlatformDetection), nameof(PlatformDetection.IsBuiltWithAggressiveTrimming), nameof(PlatformDetection.IsBrowser))]
public static void DebuggerAttribute_NullCollection_ThrowsArgumentNullException()
{
TargetInvocationException ex = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.ValidateDebuggerTypeProxyProperties(typeof(ObservableCollection<int>), null));
TargetInvocationException ex = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.CreateDebuggerTypeProxyWithNullArgument(typeof(ObservableCollection<int>)));
ArgumentNullException argumentNullException = Assert.IsType<ArgumentNullException>(ex.InnerException);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,13 +243,13 @@ public static void DebuggerAttributeTests()
Assert.Equal(dict.Count, itemArray.Length);

DebuggerAttributes.ValidateDebuggerDisplayReferences(dict.Keys);
info = DebuggerAttributes.ValidateDebuggerTypeProxyProperties(typeof(ReadOnlyDictionary<int, int>.KeyCollection), new Type[] { typeof(int) }, dict.Keys);
info = DebuggerAttributes.ValidateDebuggerTypeProxyProperties(dict.Keys);
itemProperty = info.Properties.Single(pr => pr.GetCustomAttribute<DebuggerBrowsableAttribute>().State == DebuggerBrowsableState.RootHidden);
int[] items = itemProperty.GetValue(info.Instance) as int[];
Assert.Equal(dict.Keys, items);

DebuggerAttributes.ValidateDebuggerDisplayReferences(dict.Values);
info = DebuggerAttributes.ValidateDebuggerTypeProxyProperties(typeof(ReadOnlyDictionary<int, int>.KeyCollection), new Type[] { typeof(int) }, dict.Values);
info = DebuggerAttributes.ValidateDebuggerTypeProxyProperties(dict.Values);
itemProperty = info.Properties.Single(pr => pr.GetCustomAttribute<DebuggerBrowsableAttribute>().State == DebuggerBrowsableState.RootHidden);
items = itemProperty.GetValue(info.Instance) as int[];
Assert.Equal(dict.Values, items);
Expand All @@ -259,7 +259,7 @@ public static void DebuggerAttributeTests()
[ActiveIssue("https://github.com/dotnet/runtime/issues/57588", typeof(PlatformDetection), nameof(PlatformDetection.IsBuiltWithAggressiveTrimming), nameof(PlatformDetection.IsBrowser))]
public static void DebuggerAttribute_NullDictionary_ThrowsArgumentNullException()
{
TargetInvocationException ex = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.ValidateDebuggerTypeProxyProperties(typeof(ReadOnlyDictionary<int, int>), null));
TargetInvocationException ex = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.CreateDebuggerTypeProxyWithNullArgument(typeof(ReadOnlyDictionary<int, int>)));
ArgumentNullException argumentNullException = Assert.IsType<ArgumentNullException>(ex.InnerException);
Assert.Equal("dictionary", argumentNullException.ParamName);
}
Expand All @@ -268,7 +268,7 @@ public static void DebuggerAttribute_NullDictionary_ThrowsArgumentNullException(
[ActiveIssue("https://github.com/dotnet/runtime/issues/57588", typeof(PlatformDetection), nameof(PlatformDetection.IsBuiltWithAggressiveTrimming), nameof(PlatformDetection.IsBrowser))]
public static void DebuggerAttribute_NullDictionaryKeys_ThrowsArgumentNullException()
{
TargetInvocationException ex = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.ValidateDebuggerTypeProxyProperties(typeof(ReadOnlyDictionary<int, int>.KeyCollection), new Type[] { typeof(int) }, null));
TargetInvocationException ex = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.CreateDebuggerTypeProxyWithNullArgument(typeof(ReadOnlyDictionary<int, int>.KeyCollection)));
ArgumentNullException argumentNullException = Assert.IsType<ArgumentNullException>(ex.InnerException);
Assert.Equal("collection", argumentNullException.ParamName);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ public static void DebuggerAttribute_Tests()
[ActiveIssue("https://github.com/dotnet/runtime/issues/57588", typeof(PlatformDetection), nameof(PlatformDetection.IsBuiltWithAggressiveTrimming), nameof(PlatformDetection.IsBrowser))]
public static void DebuggerAttribute_NullCollection_ThrowsArgumentNullException()
{
TargetInvocationException ex = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.ValidateDebuggerTypeProxyProperties(typeof(ReadOnlyObservableCollection<int>), null));
TargetInvocationException ex = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.CreateDebuggerTypeProxyWithNullArgument(typeof(ReadOnlyObservableCollection<int>)));
ArgumentNullException argumentNullException = Assert.IsType<ArgumentNullException>(ex.InnerException);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ public DictionaryEntry Entry

IEnumerable<TValue> IReadOnlyDictionary<TKey, TValue>.Values => Values;

[DebuggerTypeProxy(typeof(ICollectionDebugView<>))]
[DebuggerTypeProxy(typeof(DictionaryKeyCollectionDebugView<,>))]
[DebuggerDisplay("Count = {Count}")]
public sealed class KeyCollection : ICollection<TKey>, ICollection, IReadOnlyCollection<TKey>
{
Expand Down Expand Up @@ -303,7 +303,7 @@ void ICollection.CopyTo(Array array, int index)
object ICollection.SyncRoot => (_collection is ICollection coll) ? coll.SyncRoot : this;
}

[DebuggerTypeProxy(typeof(ICollectionDebugView<>))]
[DebuggerTypeProxy(typeof(DictionaryValueCollectionDebugView<,>))]
[DebuggerDisplay("Count = {Count}")]
public sealed class ValueCollection : ICollection<TValue>, ICollection, IReadOnlyCollection<TValue>
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ public static void DebuggerAttributeTests()
[Fact]
public static void DebuggerAttributeTests_Null()
{
TargetInvocationException ex = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.ValidateDebuggerTypeProxyProperties(typeof(CaptureCollection), null));
TargetInvocationException ex = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.CreateDebuggerTypeProxyWithNullArgument(typeof(CaptureCollection)));
Assert.IsType<ArgumentNullException>(ex.InnerException);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ public static void DebuggerAttributeTests()
[Fact]
public static void DebuggerAttributeTests_Null()
{
TargetInvocationException ex = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.ValidateDebuggerTypeProxyProperties(typeof(GroupCollection), null));
TargetInvocationException ex = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.CreateDebuggerTypeProxyWithNullArgument(typeof(GroupCollection)));
Assert.IsType<ArgumentNullException>(ex.InnerException);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public static void DebuggerAttributeTests()
[Fact]
public static void DebuggerAttributeTests_Null()
{
TargetInvocationException ex = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.ValidateDebuggerTypeProxyProperties(typeof(MatchCollection), null));
TargetInvocationException ex = Assert.Throws<TargetInvocationException>(() => DebuggerAttributes.CreateDebuggerTypeProxyWithNullArgument(typeof(MatchCollection)));
Assert.IsType<ArgumentNullException>(ex.InnerException);
}
}
Expand Down

0 comments on commit bcc1d3d

Please sign in to comment.