Allow DynamicData source to be on base types (#4482)

Youssef1313 · web-flow · commit 5f134f587bb0 · 2025-01-04T10:43:39.000+01:00
diff --git a/src/Adapter/MSTest.TestAdapter/DynamicDataOperations.cs b/src/Adapter/MSTest.TestAdapter/DynamicDataOperations.cs
@@ -1,4 +1,4 @@
-﻿// Copyright (c) Microsoft Corporation. All rights reserved.
+// Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using Microsoft.VisualStudio.TestTools.UnitTesting;
@@ -19,11 +19,11 @@ public IEnumerable<object[]> GetData(Type? _dynamicDataDeclaringType, DynamicDat
         {
             case DynamicDataSourceType.AutoDetect:
 #pragma warning disable IDE0045 // Convert to conditional expression - it becomes less readable.
-                if (PlatformServiceProvider.Instance.ReflectionOperations.GetDeclaredProperty(_dynamicDataDeclaringType, _dynamicDataSourceName) is { } dynamicDataPropertyInfo)
+                if (GetPropertyConsideringInheritance(_dynamicDataDeclaringType, _dynamicDataSourceName) is { } dynamicDataPropertyInfo)
                 {
                     obj = GetDataFromProperty(dynamicDataPropertyInfo);
                 }
-                else if (PlatformServiceProvider.Instance.ReflectionOperations.GetDeclaredMethod(_dynamicDataDeclaringType, _dynamicDataSourceName) is { } dynamicDataMethodInfo)
+                else if (GetMethodConsideringInheritance(_dynamicDataDeclaringType, _dynamicDataSourceName) is { } dynamicDataMethodInfo)
                 {
                     obj = GetDataFromMethod(dynamicDataMethodInfo);
                 }
@@ -35,14 +35,14 @@ public IEnumerable<object[]> GetData(Type? _dynamicDataDeclaringType, DynamicDat
 
                 break;
             case DynamicDataSourceType.Property:
-                PropertyInfo property = PlatformServiceProvider.Instance.ReflectionOperations.GetDeclaredProperty(_dynamicDataDeclaringType, _dynamicDataSourceName)
+                PropertyInfo property = GetPropertyConsideringInheritance(_dynamicDataDeclaringType, _dynamicDataSourceName)
                     ?? throw new ArgumentNullException($"{DynamicDataSourceType.Property} {_dynamicDataSourceName}");
 
                 obj = GetDataFromProperty(property);
                 break;
 
             case DynamicDataSourceType.Method:
-                MethodInfo method = PlatformServiceProvider.Instance.ReflectionOperations.GetDeclaredMethod(_dynamicDataDeclaringType, _dynamicDataSourceName)
+                MethodInfo method = GetMethodConsideringInheritance(_dynamicDataDeclaringType, _dynamicDataSourceName)
                     ?? throw new ArgumentNullException($"{DynamicDataSourceType.Method} {_dynamicDataSourceName}");
 
                 obj = GetDataFromMethod(method);
@@ -251,4 +251,40 @@ private static bool IsTupleOrValueTuple(Type type, out int tupleSize)
         return false;
     }
 #endif
+
+    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 = PlatformServiceProvider.Instance.ReflectionOperations.GetDeclaredProperty(currentType, propertyName);
+            if (property is not null)
+            {
+                return property;
+            }
+
+            currentType = currentType.BaseType;
+        }
+
+        return null;
+    }
+
+    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 = PlatformServiceProvider.Instance.ReflectionOperations.GetDeclaredMethod(currentType, methodName);
+            if (method is not null)
+            {
+                return method;
+            }
+
+            currentType = currentType.BaseType;
+        }
+
+        return null;
+    }
 }
diff --git a/src/Adapter/MSTest.TestAdapter/Execution/TypeCache.cs b/src/Adapter/MSTest.TestAdapter/Execution/TypeCache.cs
@@ -370,7 +370,7 @@ private TestClassInfo CreateClassInfo(Type classType, TestMethod testMethod)
     {
         try
         {
-            PropertyInfo? testContextProperty = PlatformServiceProvider.Instance.ReflectionOperations.GetRuntimeProperty(classType, TestContextPropertyName);
+            PropertyInfo? testContextProperty = PlatformServiceProvider.Instance.ReflectionOperations.GetRuntimeProperty(classType, TestContextPropertyName, includeNonPublic: false);
             if (testContextProperty == null)
             {
                 // that's okay may be the property was not defined
@@ -810,7 +810,8 @@ private MethodInfo GetMethodInfoForTestMethod(TestMethod testMethod, TestClassIn
         else if (methodBase != null)
         {
             Type[] parameters = methodBase.GetParameters().Select(i => i.ParameterType).ToArray();
-            testMethodInfo = PlatformServiceProvider.Instance.ReflectionOperations.GetRuntimeMethod(methodBase.DeclaringType!, methodBase.Name, parameters);
+            // TODO: Should we pass true for includeNonPublic?
+            testMethodInfo = PlatformServiceProvider.Instance.ReflectionOperations.GetRuntimeMethod(methodBase.DeclaringType!, methodBase.Name, parameters, includeNonPublic: false);
         }
 
         return testMethodInfo is null
diff --git a/src/Adapter/MSTest.TestAdapter/SourceGeneration/SourceGeneratedReflectionOperations.cs b/src/Adapter/MSTest.TestAdapter/SourceGeneration/SourceGeneratedReflectionOperations.cs
@@ -61,22 +61,33 @@ public PropertyInfo[] GetDeclaredProperties(Type type)
         => ReflectionDataProvider.TypeProperties[type];
 
     public PropertyInfo? GetDeclaredProperty(Type type, string propertyName)
-        => GetRuntimeProperty(type, propertyName);
+        => GetRuntimeProperty(type, propertyName, includeNonPublic: true);
 
     public Type[] GetDefinedTypes(Assembly assembly)
         => ReflectionDataProvider.Types;
 
     public MethodInfo[] GetRuntimeMethods(Type type)
         => ReflectionDataProvider.TypeMethods[type];
 
-    public MethodInfo? GetRuntimeMethod(Type declaringType, string methodName, Type[] parameters) => throw new NotImplementedException();
+    public MethodInfo? GetRuntimeMethod(Type declaringType, string methodName, Type[] parameters, bool includeNonPublic)
+    {
+        IEnumerable<MethodInfo> runtimeMethods = GetRuntimeMethods(declaringType)
+            .Where(
+                m => m.Name == methodName &&
+                m.GetParameters().Select(pi => pi.ParameterType).SequenceEqual(parameters) &&
+                (includeNonPublic || m.IsPublic));
+        return runtimeMethods.SingleOrDefault();
+    }
 
-    public PropertyInfo? GetRuntimeProperty(Type classType, string propertyName)
+    public PropertyInfo? GetRuntimeProperty(Type classType, string propertyName, bool includeNonPublic)
     {
         Dictionary<string, PropertyInfo> type = ReflectionDataProvider.TypePropertiesByName[classType];
 
         // We as asking for TestContext here, it may not be there.
-        return type.TryGetValue(propertyName, out PropertyInfo? propertyInfo) ? propertyInfo : null;
+        PropertyInfo? property = type.TryGetValue(propertyName, out PropertyInfo? propertyInfo) ? propertyInfo : null;
+        return !includeNonPublic && (property?.GetMethod?.IsPublic == true || property?.SetMethod?.IsPublic == true)
+            ? null
+            : property;
     }
 
     public Type? GetType(string typeName)
diff --git a/src/Adapter/MSTestAdapter.PlatformServices/Interfaces/IReflectionOperations2.cs b/src/Adapter/MSTestAdapter.PlatformServices/Interfaces/IReflectionOperations2.cs
@@ -19,9 +19,9 @@ internal interface IReflectionOperations2 : IReflectionOperations
 
     MethodInfo[] GetRuntimeMethods(Type type);
 
-    MethodInfo? GetRuntimeMethod(Type declaringType, string methodName, Type[] parameters);
+    MethodInfo? GetRuntimeMethod(Type declaringType, string methodName, Type[] parameters, bool includeNonPublic);
 
-    PropertyInfo? GetRuntimeProperty(Type classType, string propertyName);
+    PropertyInfo? GetRuntimeProperty(Type classType, string propertyName, bool includeNonPublic);
 
     Type? GetType(string typeName);
 
diff --git a/src/Adapter/MSTestAdapter.PlatformServices/Services/ReflectionOperations2.cs b/src/Adapter/MSTestAdapter.PlatformServices/Services/ReflectionOperations2.cs
@@ -45,11 +45,15 @@ public Type[] GetDefinedTypes(Assembly assembly)
     public MethodInfo[] GetRuntimeMethods(Type type)
         => type.GetMethods(Everything);
 
-    public MethodInfo? GetRuntimeMethod(Type declaringType, string methodName, Type[] parameters)
-        => declaringType.GetRuntimeMethod(methodName, parameters);
-
-    public PropertyInfo? GetRuntimeProperty(Type classType, string testContextPropertyName)
-        => classType.GetProperty(testContextPropertyName);
+    public MethodInfo? GetRuntimeMethod(Type declaringType, string methodName, Type[] parameters, bool includeNonPublic)
+        => includeNonPublic
+            ? declaringType.GetMethod(methodName, Everything, null, parameters, null)
+            : declaringType.GetMethod(methodName, parameters);
+
+    public PropertyInfo? GetRuntimeProperty(Type classType, string testContextPropertyName, bool includeNonPublic)
+        => includeNonPublic
+            ? classType.GetProperty(testContextPropertyName, Everything)
+            : classType.GetProperty(testContextPropertyName);
 
     public Type? GetType(string typeName)
         => Type.GetType(typeName);
diff --git a/src/Analyzers/MSTest.Analyzers/DynamicDataShouldBeValidAnalyzer.cs b/src/Analyzers/MSTest.Analyzers/DynamicDataShouldBeValidAnalyzer.cs
@@ -138,6 +138,48 @@ private static void AnalyzeAttribute(SymbolAnalysisContext context, AttributeDat
         AnalyzeDisplayNameSource(context, attributeData, attributeSyntax, methodSymbol, methodInfoTypeSymbol);
     }
 
+    private static (ISymbol? Member, bool AreTooMany) TryGetMember(INamedTypeSymbol declaringType, string memberName)
+    {
+        INamedTypeSymbol? currentType = declaringType;
+        while (currentType is not null)
+        {
+            (ISymbol? Member, bool AreTooMany) result = TryGetMemberCore(currentType, memberName);
+            if (result.Member is not null || result.AreTooMany)
+            {
+                return result;
+            }
+
+            // 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;
+        }
+
+        return (null, false);
+
+        static (ISymbol? Member, bool AreTooMany) TryGetMemberCore(INamedTypeSymbol declaringType, string memberName)
+        {
+            // If we cannot find the member on the given type, report a diagnostic.
+            if (declaringType.GetMembers(memberName) is { Length: 0 } potentialMembers)
+            {
+                return (null, false);
+            }
+
+            ISymbol? potentialProperty = potentialMembers.FirstOrDefault(m => m.Kind == SymbolKind.Property);
+            if (potentialProperty is not null)
+            {
+                return (potentialProperty, false);
+            }
+
+            IEnumerable<ISymbol> candidateMethods = potentialMembers.Where(m => m.Kind == SymbolKind.Method);
+            if (candidateMethods.Count() > 1)
+            {
+                // If there are multiple methods with the same name, report a diagnostic. This is not a supported scenario.
+                return (null, true);
+            }
+
+            return (candidateMethods.FirstOrDefault() ?? potentialMembers[0], false);
+        }
+    }
+
     private static void AnalyzeDataSource(SymbolAnalysisContext context, AttributeData attributeData, SyntaxNode attributeSyntax,
         IMethodSymbol methodSymbol, INamedTypeSymbol dynamicDataSourceTypeSymbol, INamedTypeSymbol ienumerableTypeSymbol)
     {
@@ -173,22 +215,23 @@ private static void AnalyzeDataSource(SymbolAnalysisContext context, AttributeDa
             return;
         }
 
-        // If we cannot find the member on the given type, report a diagnostic.
-        if (declaringType.GetMembers(memberName) is { Length: 0 } potentialMembers)
+        (ISymbol? member, bool areTooMany) = TryGetMember(declaringType, memberName);
+
+        if (areTooMany)
         {
-            context.ReportDiagnostic(attributeSyntax.CreateDiagnostic(MemberNotFoundRule, declaringType.Name, memberName));
+            // If there are multiple methods with the same name and all are parameterless, report a diagnostic. This is not a supported scenario.
+            // Note: This is likely to happen only when they differ in arity (for example, one is non-generic and the other is generic).
+            context.ReportDiagnostic(attributeSyntax.CreateDiagnostic(FoundTooManyMembersRule, declaringType.Name, memberName));
             return;
         }
 
-        // If there are multiple members with the same name, report a diagnostic. This is not a supported scenario.
-        if (potentialMembers.Length > 1)
+        if (member is null)
         {
-            context.ReportDiagnostic(attributeSyntax.CreateDiagnostic(FoundTooManyMembersRule, declaringType.Name, memberName));
+            // If we cannot find the member on the given type, report a diagnostic.
+            context.ReportDiagnostic(attributeSyntax.CreateDiagnostic(MemberNotFoundRule, declaringType.Name, memberName));
             return;
         }
 
-        ISymbol member = potentialMembers[0];
-
         switch (member.Kind)
         {
             case SymbolKind.Property:
diff --git a/test/IntegrationTests/MSTest.IntegrationTests/Parameterized tests/DynamicDataTests.cs b/test/IntegrationTests/MSTest.IntegrationTests/Parameterized tests/DynamicDataTests.cs
@@ -1,4 +1,4 @@
-// Copyright (c) Microsoft Corporation. All rights reserved.
+﻿// Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System.Collections.Immutable;
@@ -25,14 +25,22 @@ public void ExecuteDynamicDataTests()
         VerifyE2E.TestsPassed(
             testResults,
             "DynamicDataTest_SourceProperty (\"John;Doe\",LibProjectReferencedByDataSourceTest.User)",
+            "DynamicDataTest_SourcePropertyFromBase (\"John;Doe\",LibProjectReferencedByDataSourceTest.User)",
+            "DynamicDataTest_SourcePropertyShadowingBase (\"John;Doe\",LibProjectReferencedByDataSourceTest.User)",
             "DynamicDataTest_SourcePropertyAuto (\"John;Doe\",LibProjectReferencedByDataSourceTest.User)",
+            "DynamicDataTest_SourcePropertyAutoFromBase (\"John;Doe\",LibProjectReferencedByDataSourceTest.User)",
+            "DynamicDataTest_SourcePropertyAutoShadowingBase (\"John;Doe\",LibProjectReferencedByDataSourceTest.User)",
             "Custom DynamicDataTestMethod DynamicDataTest_SourcePropertyOtherType_CustomDisplayName with 2 parameters",
             "Custom DynamicDataTestMethod DynamicDataTest_SourceMethodOtherType_CustomDisplayName with 2 parameters",
             "UserDynamicDataTestMethod DynamicDataTest_SourcePropertyOtherType_CustomDisplayNameOtherType with 2 parameters",
             "Custom DynamicDataTestMethod DynamicDataTest_SourceMethod_CustomDisplayName with 2 parameters",
             "UserDynamicDataTestMethod DynamicDataTest_SourceMethodOtherType_CustomDisplayNameOtherType with 2 parameters",
             "DynamicDataTest_SourceMethod (\"Jane;Doe\",LibProjectReferencedByDataSourceTest.User)",
+            "DynamicDataTest_SourceMethodFromBase (\"Jane;Doe\",LibProjectReferencedByDataSourceTest.User)",
+            "DynamicDataTest_SourceMethodShadowingBase (\"Jane;Doe\",LibProjectReferencedByDataSourceTest.User)",
             "DynamicDataTest_SourceMethodAuto (\"Jane;Doe\",LibProjectReferencedByDataSourceTest.User)",
+            "DynamicDataTest_SourceMethodAutoFromBase (\"Jane;Doe\",LibProjectReferencedByDataSourceTest.User)",
+            "DynamicDataTest_SourceMethodAutoShadowingBase (\"Jane;Doe\",LibProjectReferencedByDataSourceTest.User)",
             "UserDynamicDataTestMethod DynamicDataTest_SourcePropertyOtherType_CustomDisplayNameOtherType with 2 parameters",
             "StackOverflowException_Example (DataSourceTestProject.DynamicDataTests+ExampleTestCase)",
             "Custom DynamicDataTestMethod DynamicDataTest_SourceProperty_CustomDisplayName with 2 parameters",
@@ -48,11 +56,19 @@ public void ExecuteDynamicDataTests()
             "UserDynamicDataTestMethod DynamicDataTest_SourceProperty_CustomDisplayNameOtherType with 2 parameters",
             "UserDynamicDataTestMethod DynamicDataTest_SourceProperty_CustomDisplayNameOtherType with 2 parameters",
             "DynamicDataTest_SourceMethod (\"John;Doe\",LibProjectReferencedByDataSourceTest.User)",
+            "DynamicDataTest_SourceMethodFromBase (\"John;Doe\",LibProjectReferencedByDataSourceTest.User)",
+            "DynamicDataTest_SourceMethodShadowingBase (\"John;Doe\",LibProjectReferencedByDataSourceTest.User)",
             "DynamicDataTest_SourceMethodAuto (\"John;Doe\",LibProjectReferencedByDataSourceTest.User)",
+            "DynamicDataTest_SourceMethodAutoFromBase (\"John;Doe\",LibProjectReferencedByDataSourceTest.User)",
+            "DynamicDataTest_SourceMethodAutoShadowingBase (\"John;Doe\",LibProjectReferencedByDataSourceTest.User)",
             "Custom DynamicDataTestMethod DynamicDataTest_SourcePropertyOtherType_CustomDisplayName with 2 parameters",
             "UserDynamicDataTestMethod DynamicDataTest_SourceMethodOtherType_CustomDisplayNameOtherType with 2 parameters",
             "DynamicDataTest_SourceProperty (\"Jane;Doe\",LibProjectReferencedByDataSourceTest.User)",
+            "DynamicDataTest_SourcePropertyFromBase (\"Jane;Doe\",LibProjectReferencedByDataSourceTest.User)",
+            "DynamicDataTest_SourcePropertyShadowingBase (\"Jane;Doe\",LibProjectReferencedByDataSourceTest.User)",
             "DynamicDataTest_SourcePropertyAuto (\"Jane;Doe\",LibProjectReferencedByDataSourceTest.User)",
+            "DynamicDataTest_SourcePropertyAutoFromBase (\"Jane;Doe\",LibProjectReferencedByDataSourceTest.User)",
+            "DynamicDataTest_SourcePropertyAutoShadowingBase (\"Jane;Doe\",LibProjectReferencedByDataSourceTest.User)",
             "DynamicDataTest_SourcePropertyOtherType (\"Jane;Doe\",LibProjectReferencedByDataSourceTest.User)",
             "Custom DynamicDataTestMethod DynamicDataTest_SourceMethod_CustomDisplayName with 2 parameters",
             "MethodWithOverload (\"1\",1)",
diff --git a/test/IntegrationTests/MSTest.VstestConsoleWrapper.IntegrationTests/Parameterized tests/DynamicDataTests.cs b/test/IntegrationTests/MSTest.VstestConsoleWrapper.IntegrationTests/Parameterized tests/DynamicDataTests.cs
diff --git a/test/IntegrationTests/TestAssets/DynamicDataTestProject/DynamicDataTests.cs b/test/IntegrationTests/TestAssets/DynamicDataTestProject/DynamicDataTests.cs
diff --git a/test/UnitTests/MSTest.Analyzers.UnitTests/DynamicDataShouldBeValidAnalyzerTests.cs b/test/UnitTests/MSTest.Analyzers.UnitTests/DynamicDataShouldBeValidAnalyzerTests.cs

Original file line number	Diff line number	Diff line change
`@@ -370,7 +370,7 @@ private TestClassInfo CreateClassInfo(Type classType, TestMethod testMethod)`
`370`	`370`	`{`
`371`	`371`	`try`
`372`	`372`	`{`
`373`		`- PropertyInfo? testContextProperty = PlatformServiceProvider.Instance.ReflectionOperations.GetRuntimeProperty(classType, TestContextPropertyName);`
	`373`	`+ PropertyInfo? testContextProperty = PlatformServiceProvider.Instance.ReflectionOperations.GetRuntimeProperty(classType, TestContextPropertyName, includeNonPublic: false);`
`374`	`374`	`if (testContextProperty == null)`
`375`	`375`	`{`
`376`	`376`	`// that's okay may be the property was not defined`
`@@ -810,7 +810,8 @@ private MethodInfo GetMethodInfoForTestMethod(TestMethod testMethod, TestClassIn`
`810`	`810`	`else if (methodBase != null)`
`811`	`811`	`{`
`812`	`812`	`Type[] parameters = methodBase.GetParameters().Select(i => i.ParameterType).ToArray();`
`813`		`- testMethodInfo = PlatformServiceProvider.Instance.ReflectionOperations.GetRuntimeMethod(methodBase.DeclaringType!, methodBase.Name, parameters);`
	`813`	`+ // TODO: Should we pass true for includeNonPublic?`
	`814`	`+ testMethodInfo = PlatformServiceProvider.Instance.ReflectionOperations.GetRuntimeMethod(methodBase.DeclaringType!, methodBase.Name, parameters, includeNonPublic: false);`
`814`	`815`	`}`
`815`	`816`
`816`	`817`	`return testMethodInfo is null`