Skip to content

Commit

Permalink
Allow DynamicData source to be on base types (#4482)
Browse files Browse the repository at this point in the history
  • Loading branch information
Youssef1313 authored Jan 4, 2025
1 parent 0b65aa2 commit 5f134f5
Show file tree
Hide file tree
Showing 10 changed files with 318 additions and 73 deletions.
46 changes: 41 additions & 5 deletions src/Adapter/MSTest.TestAdapter/DynamicDataOperations.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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);
}
Expand All @@ -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);
Expand Down Expand Up @@ -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;
}
}
5 changes: 3 additions & 2 deletions src/Adapter/MSTest.TestAdapter/Execution/TypeCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
59 changes: 51 additions & 8 deletions src/Analyzers/MSTest.Analyzers/DynamicDataShouldBeValidAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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",
Expand All @@ -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)",
Expand Down
Loading

0 comments on commit 5f134f5

Please sign in to comment.