fix(sourcegen): generate a data source factory for [InstanceMethodDataSource] (#7225)

src/TUnit.Core.SourceGenerator/Generators/TestMetadataGenerator.cs

@@ -22,6 +22,7 @@ namespace TUnit.Core.SourceGenerator.Generators;
 public sealed class TestMetadataGenerator : IIncrementalGenerator
 {
     private const string GeneratedNamespace = "TUnit.Generated";
+    private const string InstanceMethodDataSourceAttributeName = "global::TUnit.Core.InstanceMethodDataSourceAttribute";
 
     private static readonly DiagnosticDescriptor GenerationErrorDescriptor = new(
         "TUNIT0999",
@@ -2544,7 +2545,10 @@ private static void GenerateDataSourceAttribute(CodeWriter writer, CompilationCo
 
         var attrName = attrClass.GloballyQualifiedNonGeneric();
 
-        if (attrName == "global::TUnit.Core.MethodDataSourceAttribute")
+        // InstanceMethodDataSourceAttribute derives from MethodDataSourceAttribute and needs the
+        // same generated factory; without one, Native AOT falls back to a reflection lookup that
+        // finds no trimmed field or property metadata (#7225).
+        if (attrName is "global::TUnit.Core.MethodDataSourceAttribute" or InstanceMethodDataSourceAttributeName)
         {
             GenerateMethodDataSourceAttribute(writer, attr, typeSymbol, compilationContext.Compilation);
         }
@@ -2888,7 +2892,7 @@ private static void GenerateMethodDataSourceAttribute(CodeWriter writer, Attribu
 
         if (dataSourceMember == null
             || (dataSourceMethod == null && dataSourceProperty == null && dataSourceField == null)
-            || (dataSourceMethod == null && !CanReadDataSourceMember(dataSourceMember, compilation)))
+            || !CanReadDataSourceMember(dataSourceMember, compilation))
         {
             // Still generate the attribute even if method not found - it will fail at runtime with proper error
             // Use CodeGenerationHelpers to properly handle any generics on the attribute
@@ -2916,8 +2920,11 @@ private static void GenerateMethodDataSourceAttribute(CodeWriter writer, Attribu
             return;
         }
 
-        var attrTypeName = usesTestClassInstance
-            ? "global::TUnit.Core.InstanceMethodDataSourceAttribute"
+        // Keep a declared [InstanceMethodDataSource] as one even when its member is static, so the
+        // engine still sees IAccessesInstanceData as it does in reflection mode.
+        var isDeclaredInstanceAttribute = attr.AttributeClass?.GloballyQualifiedNonGeneric() == InstanceMethodDataSourceAttributeName;
+        var attrTypeName = usesTestClassInstance || isDeclaredInstanceAttribute
+            ? InstanceMethodDataSourceAttributeName
             : "global::TUnit.Core.MethodDataSourceAttribute";
 
         if (attr.ConstructorArguments is
@@ -2977,7 +2984,7 @@ private static void GenerateMethodDataSourceAttribute(CodeWriter writer, Attribu
         // a per-data-source compiler-generated state machine class (#6227). Only the per-test-unique
         // invocation is emitted, as a static lambda.
         writer.Append("Factory = static dataGeneratorMetadata => ");
-        EmitDataSourceFactoryInvocation(writer, dataSourceMember, targetType, usesTestClassInstance, hasArguments ? argumentsProperty.Value : (TypedConstant?)null, compilation);
+        EmitDataSourceFactoryInvocation(writer, dataSourceMember, targetType, usesTestClassInstance, hasArguments ? argumentsProperty.Value : (TypedConstant?)null, compilation, isDeclaredInstanceAttribute);
         writer.AppendLine(",");
 
         writer.Unindent();
@@ -2990,7 +2997,7 @@ private static void GenerateMethodDataSourceAttribute(CodeWriter writer, Attribu
     /// member's declared return type (mirroring the previous inline-iterator branch order)
     /// and whether the member is static or instance-based.
     /// </summary>
-    private static void EmitDataSourceFactoryInvocation(CodeWriter writer, ISymbol dataSourceMember, ITypeSymbol targetType, bool usesTestClassInstance, TypedConstant? arguments, Compilation compilation)
+    private static void EmitDataSourceFactoryInvocation(CodeWriter writer, ISymbol dataSourceMember, ITypeSymbol targetType, bool usesTestClassInstance, TypedConstant? arguments, Compilation compilation, bool decidesRowsFromRuntimeValue)
     {
         var dataSourceMethod = dataSourceMember as IMethodSymbol;
 
@@ -3069,30 +3076,52 @@ private static void EmitDataSourceFactoryInvocation(CodeWriter writer, ISymbol d
         // A declared type such as object does not say whether the value is one row or many. Reflection mode, and the
         // attribute's reflection fallback that read every field and every private or protected property before the
         // generator could, decide from the runtime value, so these factories hand the value to that same logic
-        // instead of FromValue. Accessible properties and methods keep FromValue, as their generated code always has
-        // (https://github.com/thomhurst/TUnit/issues/7178).
-        var isAccessibleMember = dataSourceMethod != null || IsAccessibleDataSourceMember(dataSourceMember, compilation);
-        if (kind == "Value" && (dataSourceMember is IFieldSymbol || !isAccessibleMember))
+        // instead of FromValue. Accessible properties and methods of [MethodDataSource] keep FromValue, as their
+        // generated code always has (https://github.com/thomhurst/TUnit/issues/7178). [InstanceMethodDataSource] had no
+        // generated factory before #7225, so every member it names keeps the runtime-value behavior it always had.
+        var isAccessibleMember = IsAccessibleDataSourceMember(dataSourceMember, compilation);
+        if (kind == "Value" && (decidesRowsFromRuntimeValue || dataSourceMember is IFieldSymbol || !isAccessibleMember))
         {
             helper = isStatic || !usesTestClassInstance
                 ? "global::TUnit.Core.Helpers.DataSourceFactories.FromRuntimeValue"
                 : "global::TUnit.Core.Helpers.DataSourceFactories.FromInstanceRuntimeValue";
             metadataArgument = "dataGeneratorMetadata, ";
         }
 
+        // [InstanceMethodDataSource] read its member by reflection before #7225, and that path, still used in
+        // reflection mode, treats a null value as a source with no rows, so SkipIfEmpty skips the test. The factory
+        // helpers would turn null into one row holding null, so a null value becomes an empty source here.
+        var nullAsEmpty = decidesRowsFromRuntimeValue && kind is "Value" or "Enumerable"
+            ? " ?? global::System.Array.Empty<object>()"
+            : "";
+
         if (!isAccessibleMember)
         {
-            // The generated class cannot name a private or protected field or property, so the factory reads it by
-            // reflection. GetField/GetProperty on a typeof() with a constant name and binding flags is understood
+            // The generated class cannot name a private or protected member, so the factory reads it by
+            // reflection. GetField/GetProperty/GetMethod on a typeof() with a constant name and binding flags is understood
             // by the trimmer and the AOT compiler, which keep that member, so this works under native AOT
             // without any trimming warning. The attribute's own reflection fallback finds the member from the
             // test class type at run time, which neither of them can see.
-            var lookup = dataSourceMember is IFieldSymbol ? "GetField" : "GetProperty";
             var bindingFlags = (isStatic ? "global::System.Reflection.BindingFlags.Static" : "global::System.Reflection.BindingFlags.Instance")
                 + " | global::System.Reflection.BindingFlags.Public | global::System.Reflection.BindingFlags.NonPublic | global::System.Reflection.BindingFlags.DeclaredOnly";
             var target = isStatic ? "null" : usesTestClassInstance ? "instance" : $"new {fullyQualifiedType}()";
+            // CanReadDataSourceMember only lets a parameterless, non-generic method through, so it is looked up with no
+            // parameter types, which also picks it out from any overloads. A parameterless generic overload of the same
+            // name would match that lookup too and make it ambiguous, so then the methods are filtered instead, without
+            // the genericParameterCount overload of GetMethod that .NET Framework and .NET Standard 2.0 lack.
+            var hasGenericParameterlessOverload = dataSourceMember is IMethodSymbol
+                && dataSourceMember.ContainingType.GetMembers(dataSourceMember.Name)
+                    .Any(static m => m is IMethodSymbol { IsGenericMethod: true, Parameters.Length: 0 });
+            var read = dataSourceMember switch
+            {
+                IFieldSymbol => $"GetField(\"{dataSourceMember.Name}\", {bindingFlags})!.GetValue({target})",
+                IPropertySymbol => $"GetProperty(\"{dataSourceMember.Name}\", {bindingFlags})!.GetValue({target})",
+                _ when hasGenericParameterlessOverload => $"GetMethods({bindingFlags}), static m => m.Name == \"{dataSourceMember.Name}\" && !m.IsGenericMethodDefinition && m.GetParameters().Length == 0)!.Invoke({target}, null)",
+                _ => $"GetMethod(\"{dataSourceMember.Name}\", {bindingFlags}, null, global::System.Type.EmptyTypes, null)!.Invoke({target}, null)"
+            };
             var cast = kind is "Value" or "Enumerable" ? "" : $"({returnType.GloballyQualified()})";
-            writer.Append($"{helper}({metadataArgument}{lambdaHeader} {cast}typeof({dataSourceMember.ContainingType.GloballyQualified()}).{lookup}(\"{dataSourceMember.Name}\", {bindingFlags})!.GetValue({target}))");
+            var find = hasGenericParameterlessOverload ? "global::System.Array.Find(" : "";
+            writer.Append($"{helper}({metadataArgument}{lambdaHeader} {cast}{find}typeof({dataSourceMember.ContainingType.GloballyQualified()}).{read}{nullAsEmpty})");
             return;
         }
 
@@ -3132,13 +3161,15 @@ private static void EmitDataSourceFactoryInvocation(CodeWriter writer, ISymbol d
                 writer.Append("global::System.Array.Empty<object?>()");
             }
             writer.AppendLine(";");
-            writer.AppendLine($"return {call};");
+            writer.AppendLine(nullAsEmpty.Length == 0 ? $"return {call};" : $"return (object?){call}{nullAsEmpty};");
             writer.Unindent();
             writer.Append("})");
         }
         else
         {
-            writer.Append($"{helper}({metadataArgument}{lambdaHeader} {call})");
+            writer.Append(nullAsEmpty.Length == 0
+                ? $"{helper}({metadataArgument}{lambdaHeader} {call})"
+                : $"{helper}({metadataArgument}{lambdaHeader} (object?){call}{nullAsEmpty})");
         }
     }
 
@@ -3172,7 +3203,7 @@ private static bool IsShadowedOnTargetType(IMethodSymbol declaringMethod, ITypeS
     }
 
     /// <summary>
-    /// Whether the generated class can read the data source field or property directly: the member and its getter
+    /// Whether the generated class can read the data source member directly: the member, and a property's getter,
     /// are accessible from the test assembly, so not private or protected.
     /// </summary>
     private static bool IsAccessibleDataSourceMember(ISymbol member, Compilation compilation)
@@ -3183,10 +3214,10 @@ private static bool IsAccessibleDataSourceMember(ISymbol member, Compilation com
     }
 
     /// <summary>
-    /// Whether a generated factory can read the data source field or property, directly or, for a private or
-    /// protected member, by reflection on its declaring type. That needs a declaring type that the generated class
-    /// can name without type parameters, a getter for a property, and a member type it can name when the factory
-    /// casts the value. Otherwise the attribute keeps its reflection fallback.
+    /// Whether a generated factory can read the data source member, directly or, for a private or protected member,
+    /// by reflection on its declaring type. That needs a declaring type that the generated class can name without
+    /// type parameters, a getter for a property, a method without parameters or type parameters, and a member type it
+    /// can name when the factory casts the value. Otherwise the attribute keeps its reflection fallback.
     /// </summary>
     private static bool CanReadDataSourceMember(ISymbol member, Compilation compilation)
     {
@@ -3200,8 +3231,18 @@ private static bool CanReadDataSourceMember(ISymbol member, Compilation compilat
             return true;
         }
 
+        if (member is IMethodSymbol { Parameters.Length: > 0 } or IMethodSymbol { IsGenericMethod: true })
+        {
+            return false;
+        }
+
         // EmitDataSourceFactoryInvocation casts the value for the async helpers, which take a typed delegate
-        var memberType = member is IPropertySymbol property ? property.Type : ((IFieldSymbol)member).Type;
+        var memberType = member switch
+        {
+            IPropertySymbol property => property.Type,
+            IMethodSymbol method => method.ReturnType,
+            _ => ((IFieldSymbol)member).Type
+        };
         var castsValue = IsAsyncEnumerable(memberType) || IsValueTask(memberType) || IsTask(memberType);
         return compilation.IsSymbolAccessibleWithin(member.ContainingType, compilation.Assembly)
             && !CodeGenerationHelpers.ContainsTypeParameter(member.ContainingType)

tests/TUnit.Core.SourceGenerator.Tests/Bugs/7225/Tests7225.cs

@@ -0,0 +1,49 @@
+namespace TUnit.Core.SourceGenerator.Tests.Bugs._7225;
+
+/// <summary>
+/// [InstanceMethodDataSource] gets the same generated Factory as [MethodDataSource], so Native AOT
+/// does not fall back to a reflection member lookup (#7225).
+/// </summary>
+internal class Tests7225 : TestsBase
+{
+    [Test]
+    public Task Test() => RunTest(Path.Combine(Git.TestsDirectory.FullName,
+            "TUnit.TestProject",
+            "Bugs",
+            "_7225",
+            "InstanceMethodDataSourceFieldTests.cs"),
+        async generatedFiles =>
+        {
+            var source = string.Join(Environment.NewLine, generatedFiles);
+            const string testClass = "global::TUnit.TestProject.Bugs._7225.InstanceMethodDataSourceFieldTests";
+            await Assert.That(source).Contains($"FromInstanceRuntimeValue(dataGeneratorMetadata, static instance => (object?)(({testClass})instance).Rows ?? global::System.Array.Empty<object>())");
+            await Assert.That(source).Contains($"FromInstanceEnumerable(dataGeneratorMetadata, static instance => (object?)(({testClass})instance).PropertyRows ?? global::System.Array.Empty<object>())");
+            await Assert.That(source).Contains($"FromEnumerable(static () => (object?){testClass}.StaticRows() ?? global::System.Array.Empty<object>())");
+            await Assert.That(source).DoesNotContain("new global::TUnit.Core.InstanceMethodDataSourceAttribute(\"Rows\"),");
+        });
+
+    /// <summary>
+    /// The factory keeps the behavior [InstanceMethodDataSource] had when it read members by reflection: an object
+    /// value decides its rows at run time, a null value is an empty source, and a private method is looked up
+    /// by a constant name that Native AOT keeps, as private fields and properties are.
+    /// </summary>
+    [Test]
+    public Task MemberShapes() => RunTest(Path.Combine(Git.TestsDirectory.FullName,
+            "TUnit.TestProject",
+            "Bugs",
+            "_7225",
+            "InstanceMethodDataSourceMemberShapeTests.cs"),
+        async generatedFiles =>
+        {
+            var source = string.Join(Environment.NewLine, generatedFiles);
+            const string testClass = "global::TUnit.TestProject.Bugs._7225.InstanceMethodDataSourceMemberShapeTests";
+            await Assert.That(source).Contains($"FromInstanceRuntimeValue(dataGeneratorMetadata, static instance => (object?)(({testClass})instance).ObjectProperty ?? global::System.Array.Empty<object>())");
+            await Assert.That(source).Contains($"FromInstanceRuntimeValue(dataGeneratorMetadata, static instance => (object?)(({testClass})instance).ObjectMethod() ?? global::System.Array.Empty<object>())");
+            await Assert.That(source).Contains($"FromInstanceEnumerable(dataGeneratorMetadata, static instance => (object?)(({testClass})instance).TupleRows ?? global::System.Array.Empty<object>())");
+            await Assert.That(source).Contains($"FromInstanceEnumerable(dataGeneratorMetadata, static instance => typeof({testClass}).GetMethod(\"PrivateRows\", ");
+            await Assert.That(source).DoesNotContain("new global::TUnit.Core.InstanceMethodDataSourceAttribute(\"PrivateRows\"),");
+            // A generic overload of the same name would make the GetMethod lookup ambiguous
+            await Assert.That(source).Contains($"global::System.Array.Find(typeof({testClass}).GetMethods(");
+            await Assert.That(source).Contains("m.Name == \"OverloadedRows\" && !m.IsGenericMethodDefinition && m.GetParameters().Length == 0)!.Invoke(instance, null)");
+        });
+}

tests/TUnit.Core.SourceGenerator.Tests/Tests1821.Test.verified.txt

@@ -95,7 +95,10 @@ internal static class TUnit_TestProject_Bugs__1821_Tests__TestSource
             hasDataSource: true,
             testDataSources: new global::TUnit.Core.IDataSourceAttribute[]
 {
-    new global::TUnit.Core.InstanceMethodDataSourceAttribute("DataSource"),
+    new global::TUnit.Core.InstanceMethodDataSourceAttribute("DataSource")
+    {
+        Factory = static dataGeneratorMetadata => global::TUnit.Core.Helpers.DataSourceFactories.FromInstanceEnumerable(dataGeneratorMetadata, static instance => (object?)((global::TUnit.TestProject.Bugs._1821.Tests)instance).DataSource() ?? global::System.Array.Empty<object>()),
+    },
 },
             classDataSources: new global::TUnit.Core.IDataSourceAttribute[]
 {