Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

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;
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change switches from a most-derived-first, per-type lookup (via DeclaredOnly + manual BaseType walk) to a single reflection lookup using FlattenHierarchy. That is not behavior-equivalent: (1) BindingFlags.FlattenHierarchy will not return private static members declared on base types, whereas the previous implementation would find them; and (2) the single-call lookup can surface multiple matches across the inheritance chain and throw AmbiguousMatchException in cases where the previous implementation would have returned the derived member. If the intent is purely perf, consider keeping the previous resolution semantics (derived-first, include non-public base members) and introducing caching as described in the PR, rather than changing the BindingFlags behavior.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR title/description mention adding ConcurrentDictionary caches (and a TypeMemberKey struct for net462), but this diff instead removes the custom hierarchy-walk helpers and performs direct reflection lookups with FlattenHierarchy—no caching is introduced. Please update the PR description/title to reflect the actual approach, or include the intended caching changes.

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with the behavior change. Let's ensure that the analyzer that validates dynamic data handles it correctly though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In DynamicDataShouldBeValidAnalyzer, we have TryGetMemberCore local function. We can possibly add bool disallowPrivate there, and we pass false in the first iteration, and true on subsequent iterations.


public static IEnumerable<object[]> GetData(Type? dynamicDataDeclaringType, DynamicDataSourceType dynamicDataSourceType, string dynamicDataSourceName, object?[] dynamicDataSourceArguments, MethodInfo methodInfo)
{
Expand All @@ -19,15 +19,15 @@
{
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);
}
Expand All @@ -39,21 +39,21 @@

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);
Expand Down Expand Up @@ -166,57 +166,4 @@
return false;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

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;
}

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;
}

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;
}
}

Check failure on line 169 in src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx (Build Linux Debug)

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs#L169

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs(169,1): error SA1508: (NETCORE_ENGINEERING_TELEMETRY=Build) A closing brace should not be preceded by a blank line (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1508.md)

Check failure on line 169 in src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx (Build Linux Debug)

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs#L169

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs(169,1): error IDE2002: (NETCORE_ENGINEERING_TELEMETRY=Build) Consecutive braces must not have blank line between them (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide2002)

Check failure on line 169 in src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx (Build Linux Debug)

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs#L169

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs(169,1): error SA1508: (NETCORE_ENGINEERING_TELEMETRY=Build) A closing brace should not be preceded by a blank line (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1508.md)

Check failure on line 169 in src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx (Build Linux Debug)

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs#L169

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs(169,1): error SA1508: (NETCORE_ENGINEERING_TELEMETRY=Build) A closing brace should not be preceded by a blank line (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1508.md)

Check failure on line 169 in src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx (Build Linux Debug)

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs#L169

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs(169,1): error IDE2002: (NETCORE_ENGINEERING_TELEMETRY=Build) Consecutive braces must not have blank line between them (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide2002)

Check failure on line 169 in src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx (Build Linux Release)

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs#L169

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs(169,1): error SA1508: (NETCORE_ENGINEERING_TELEMETRY=Build) A closing brace should not be preceded by a blank line (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1508.md)

Check failure on line 169 in src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx (Build Linux Release)

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs#L169

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs(169,1): error IDE2002: (NETCORE_ENGINEERING_TELEMETRY=Build) Consecutive braces must not have blank line between them (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide2002)

Check failure on line 169 in src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx (Build MacOS Release)

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs#L169

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs(169,1): error SA1508: (NETCORE_ENGINEERING_TELEMETRY=Build) A closing brace should not be preceded by a blank line (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1508.md)

Check failure on line 169 in src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx (Build MacOS Debug)

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs#L169

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs(169,1): error IDE2002: (NETCORE_ENGINEERING_TELEMETRY=Build) Consecutive braces must not have blank line between them (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide2002)

Check failure on line 169 in src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx (Build MacOS Debug)

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs#L169

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs(169,1): error SA1508: (NETCORE_ENGINEERING_TELEMETRY=Build) A closing brace should not be preceded by a blank line (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1508.md)

Check failure on line 169 in src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx (Build MacOS Debug)

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs#L169

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs(169,1): error SA1508: (NETCORE_ENGINEERING_TELEMETRY=Build) A closing brace should not be preceded by a blank line (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1508.md)

Check failure on line 169 in src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx (Build MacOS Debug)

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs#L169

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs(169,1): error IDE2002: (NETCORE_ENGINEERING_TELEMETRY=Build) Consecutive braces must not have blank line between them (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide2002)

Check failure on line 169 in src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx (Build MacOS Debug)

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs#L169

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs(169,1): error SA1508: (NETCORE_ENGINEERING_TELEMETRY=Build) A closing brace should not be preceded by a blank line (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1508.md)

Check failure on line 169 in src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs#L169

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs(169,1): error IDE2002: (NETCORE_ENGINEERING_TELEMETRY=Build) Consecutive braces must not have blank line between them (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide2002)

Check failure on line 169 in src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs#L169

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs(169,1): error SA1508: (NETCORE_ENGINEERING_TELEMETRY=Build) A closing brace should not be preceded by a blank line (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1508.md)

Check failure on line 169 in src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs#L169

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs(169,1): error SA1508: (NETCORE_ENGINEERING_TELEMETRY=Build) A closing brace should not be preceded by a blank line (https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1508.md)

Check failure on line 169 in src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs

View check run for this annotation

Azure Pipelines / microsoft.testfx

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs#L169

src/TestFramework/TestFramework/Attributes/DataSource/DynamicDataOperations.cs(169,1): error IDE2002: (NETCORE_ENGINEERING_TELEMETRY=Build) Consecutive braces must not have blank line between them (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide2002)
Loading