From 28dac233b07e686f7d841a4bd411515cc51b7e9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 12 Mar 2026 15:59:10 +0100 Subject: [PATCH 1/3] Simplify DynamicDataOperations: use FlattenHierarchy instead of manual hierarchy walk Replace the manual type hierarchy walk with DeclaredOnly + loop with a single reflection call using BindingFlags.FlattenHierarchy. This lets the .NET runtime handle inheritance search for static members with its own built-in caching, removing the need for custom ConcurrentDictionary caches and the TypeMemberKey struct. --- .../DataSource/DynamicDataOperations.cs | 53 ++----------------- 1 file changed, 4 insertions(+), 49 deletions(-) diff --git a/src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs b/src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs index 4ce72a026f..ac6b0d87f0 100644 --- a/src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs +++ b/src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs @@ -5,7 +5,7 @@ namespace Microsoft.VisualStudio.TestTools.UnitTesting; internal static class DynamicDataOperations { - private const BindingFlags DeclaredOnlyLookup = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly; + private const BindingFlags MemberLookup = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | BindingFlags.FlattenHierarchy; public static IEnumerable GetData(Type? dynamicDataDeclaringType, DynamicDataSourceType dynamicDataSourceType, string dynamicDataSourceName, object?[] dynamicDataSourceArguments, MethodInfo methodInfo) { @@ -167,56 +167,11 @@ private static bool TryGetData(object dataSource, [NotNullWhen(true)] out IEnume } private static FieldInfo? GetFieldConsideringInheritance(Type type, string fieldName) - { - // NOTE: Don't use GetRuntimeField. It considers inheritance only for instance fields. - Type? currentType = type; - while (currentType is not null) - { - FieldInfo? field = currentType.GetField(fieldName, DeclaredOnlyLookup); - if (field is not null) - { - return field; - } - - currentType = currentType.BaseType; - } - - return null; - } + => type.GetField(fieldName, MemberLookup); private static PropertyInfo? GetPropertyConsideringInheritance(Type type, string propertyName) - { - // NOTE: Don't use GetRuntimeProperty. It considers inheritance only for instance properties. - Type? currentType = type; - while (currentType is not null) - { - PropertyInfo? property = currentType.GetProperty(propertyName, DeclaredOnlyLookup); - if (property is not null) - { - return property; - } - - currentType = currentType.BaseType; - } - - return null; - } + => type.GetProperty(propertyName, MemberLookup); private static MethodInfo? GetMethodConsideringInheritance(Type type, string methodName) - { - // NOTE: Don't use GetRuntimeMethod. It considers inheritance only for instance methods. - Type? currentType = type; - while (currentType is not null) - { - MethodInfo? method = currentType.GetMethod(methodName, DeclaredOnlyLookup); - if (method is not null) - { - return method; - } - - currentType = currentType.BaseType; - } - - return null; - } + => type.GetMethod(methodName, MemberLookup); } From d3408308df13f9cc99f60b39f29fe96c01c08d35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 12 Mar 2026 21:48:54 +0100 Subject: [PATCH 2/3] Fix --- .../DataSource/DynamicDataOperations.cs | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs b/src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs index ac6b0d87f0..5ff922182b 100644 --- a/src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs +++ b/src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs @@ -19,15 +19,15 @@ public static IEnumerable GetData(Type? dynamicDataDeclaringType, Dyna { case DynamicDataSourceType.AutoDetect: #pragma warning disable IDE0045 // Convert to conditional expression - it becomes less readable. - if (GetPropertyConsideringInheritance(dynamicDataDeclaringType, dynamicDataSourceName) is { } dynamicDataPropertyInfo) + if (dynamicDataDeclaringType.GetProperty(dynamicDataSourceName, MemberLookup) is { } dynamicDataPropertyInfo) { obj = GetDataFromProperty(dynamicDataPropertyInfo); } - else if (GetMethodConsideringInheritance(dynamicDataDeclaringType, dynamicDataSourceName) is { } dynamicDataMethodInfo) + else if (dynamicDataDeclaringType.GetMethod(dynamicDataSourceName, MemberLookup) is { } dynamicDataMethodInfo) { obj = GetDataFromMethod(dynamicDataMethodInfo, dynamicDataSourceArguments); } - else if (GetFieldConsideringInheritance(dynamicDataDeclaringType, dynamicDataSourceName) is { } dynamicDataFieldInfo) + else if (dynamicDataDeclaringType.GetField(dynamicDataSourceName, MemberLookup) is { } dynamicDataFieldInfo) { obj = GetDataFromField(dynamicDataFieldInfo); } @@ -39,21 +39,21 @@ public static IEnumerable GetData(Type? dynamicDataDeclaringType, Dyna break; case DynamicDataSourceType.Property: - PropertyInfo property = GetPropertyConsideringInheritance(dynamicDataDeclaringType, dynamicDataSourceName) + PropertyInfo property = dynamicDataDeclaringType.GetProperty(dynamicDataSourceName, MemberLookup) ?? throw new ArgumentNullException($"{DynamicDataSourceType.Property} {dynamicDataSourceName}"); obj = GetDataFromProperty(property); break; case DynamicDataSourceType.Method: - MethodInfo method = GetMethodConsideringInheritance(dynamicDataDeclaringType, dynamicDataSourceName) + MethodInfo method = dynamicDataDeclaringType.GetMethod(dynamicDataSourceName, MemberLookup) ?? throw new ArgumentNullException($"{DynamicDataSourceType.Method} {dynamicDataSourceName}"); obj = GetDataFromMethod(method, dynamicDataSourceArguments); break; case DynamicDataSourceType.Field: - FieldInfo field = GetFieldConsideringInheritance(dynamicDataDeclaringType, dynamicDataSourceName) + FieldInfo field = dynamicDataDeclaringType.GetField(dynamicDataSourceName, MemberLookup) ?? throw new ArgumentNullException($"{DynamicDataSourceType.Field} {dynamicDataSourceName}"); obj = GetDataFromField(field); @@ -166,12 +166,4 @@ private static bool TryGetData(object dataSource, [NotNullWhen(true)] out IEnume return false; } - private static FieldInfo? GetFieldConsideringInheritance(Type type, string fieldName) - => type.GetField(fieldName, MemberLookup); - - private static PropertyInfo? GetPropertyConsideringInheritance(Type type, string propertyName) - => type.GetProperty(propertyName, MemberLookup); - - private static MethodInfo? GetMethodConsideringInheritance(Type type, string methodName) - => type.GetMethod(methodName, MemberLookup); } From 690d5cd6a1aafd0ff678ceade3e4d67c35488522 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Thu, 23 Apr 2026 13:19:40 +0200 Subject: [PATCH 3/3] Address review comments: remove trailing blank line, update analyzer to disallow private base members - Remove trailing blank line in DynamicDataOperations.cs (Thread 5) - Update DynamicDataShouldBeValidAnalyzer.TryGetMemberCore to filter out private members from base types, matching the FlattenHierarchy runtime behavior (Thread 4) - Add test WhenPrivateMemberIsFromBase_Diagnostic to cover the new behavior --- .../DynamicDataShouldBeValidAnalyzer.cs | 14 +++++-- .../DataSource/DynamicDataOperations.cs | 1 - .../DynamicDataShouldBeValidAnalyzerTests.cs | 37 +++++++++++++++++++ 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/src/Analyzers/MSTest.Analyzers/DynamicDataShouldBeValidAnalyzer.cs b/src/Analyzers/MSTest.Analyzers/DynamicDataShouldBeValidAnalyzer.cs index 95b858d033..59555f1239 100644 --- a/src/Analyzers/MSTest.Analyzers/DynamicDataShouldBeValidAnalyzer.cs +++ b/src/Analyzers/MSTest.Analyzers/DynamicDataShouldBeValidAnalyzer.cs @@ -155,9 +155,10 @@ private static void AnalyzeAttribute(SymbolAnalysisContext context, AttributeDat private static (ISymbol? Member, bool AreTooMany) TryGetMember(INamedTypeSymbol declaringType, string memberName) { INamedTypeSymbol? currentType = declaringType; + bool disallowPrivate = false; while (currentType is not null) { - (ISymbol? Member, bool AreTooMany) result = TryGetMemberCore(currentType, memberName); + (ISymbol? Member, bool AreTooMany) result = TryGetMemberCore(currentType, memberName, disallowPrivate); if (result.Member is not null || result.AreTooMany) { return result; @@ -165,14 +166,21 @@ private static (ISymbol? Member, bool AreTooMany) TryGetMember(INamedTypeSymbol // Only continue to look at base types if the member is not found on the current type and we are not hit by "too many methods" rule. currentType = currentType.BaseType; + disallowPrivate = true; } return (null, false); - static (ISymbol? Member, bool AreTooMany) TryGetMemberCore(INamedTypeSymbol declaringType, string memberName) + static (ISymbol? Member, bool AreTooMany) TryGetMemberCore(INamedTypeSymbol declaringType, string memberName, bool disallowPrivate) { + ImmutableArray potentialMembers = declaringType.GetMembers(memberName); + if (disallowPrivate) + { + potentialMembers = potentialMembers.RemoveAll(m => m.DeclaredAccessibility == Accessibility.Private); + } + // If we cannot find the member on the given type, report a diagnostic. - if (declaringType.GetMembers(memberName) is { Length: 0 } potentialMembers) + if (potentialMembers is { Length: 0 }) { return (null, false); } diff --git a/src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs b/src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs index 35463c73be..be9c634584 100644 --- a/src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs +++ b/src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs @@ -170,5 +170,4 @@ private static bool TryGetData(object dataSource, [NotNullWhen(true)] out IEnume data = null; return false; } - } diff --git a/test/UnitTests/MSTest.Analyzers.UnitTests/DynamicDataShouldBeValidAnalyzerTests.cs b/test/UnitTests/MSTest.Analyzers.UnitTests/DynamicDataShouldBeValidAnalyzerTests.cs index f4f236ae00..e74adb0abb 100644 --- a/test/UnitTests/MSTest.Analyzers.UnitTests/DynamicDataShouldBeValidAnalyzerTests.cs +++ b/test/UnitTests/MSTest.Analyzers.UnitTests/DynamicDataShouldBeValidAnalyzerTests.cs @@ -1204,6 +1204,43 @@ public void TestMethod2(object[] o) await VerifyCS.VerifyAnalyzerAsync(code); } + [TestMethod] + public async Task WhenPrivateMemberIsFromBase_Diagnostic() + { + string code = """ + using System; + using System.Collections.Generic; + using Microsoft.VisualStudio.TestTools.UnitTesting; + + public abstract class MyTestClassBase + { + private static IEnumerable Data => new List(); + private static IEnumerable GetData() => new List(); + } + + [TestClass] + public class MyTestClass : MyTestClassBase + { + [{|#0:DynamicData("Data")|}] + [TestMethod] + public void TestMethod1(object[] o) + { + } + + [{|#1:DynamicData("GetData", DynamicDataSourceType.Method)|}] + [TestMethod] + public void TestMethod2(object[] o) + { + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync( + code, + VerifyCS.Diagnostic(DynamicDataShouldBeValidAnalyzer.MemberNotFoundRule).WithLocation(0).WithArguments("MyTestClass", "Data"), + VerifyCS.Diagnostic(DynamicDataShouldBeValidAnalyzer.MemberNotFoundRule).WithLocation(1).WithArguments("MyTestClass", "GetData")); + } + [TestMethod] public async Task WhenMethodHasParameters_Diagnostic() {