Repository navigation
fix(sourcegen): close generic methods of generic classes on every class source #7152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9d29869
5e9976d
b1a7aac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,7 +23,7 @@ | |
| private const string GeneratedNamespace = "TUnit.Generated"; | ||
|
|
||
| private static readonly DiagnosticDescriptor GenerationErrorDescriptor = new( | ||
| "TUNIT0999", | ||
|
Check warning on line 26 in src/TUnit.Core.SourceGenerator/Generators/TestMetadataGenerator.cs
|
||
| "Source Generation Error", | ||
| "Failed to generate test metadata for {0}.{1}: {2}", | ||
| "TUnit", | ||
|
|
@@ -840,8 +840,19 @@ | |
| AttributeData? violationSource = null, | ||
| bool isClassRow = false, | ||
| AttributeData? methodRow = null, | ||
| bool fromGenerateGenericTest = false) | ||
| bool fromGenerateGenericTest = false, | ||
| AttributeData? methodViolationRowSource = null, | ||
| bool methodViolationAllMethodDataSources = false) | ||
| { | ||
| // A generic method of a generic class needs both the class and the method type arguments. | ||
| // A source that supplies only one of them (a method row or [GenerateGenericTest] without a | ||
| // class source) would emit an entry that names the open type parameters, which does not | ||
| // compile (#7117). Those sources are paired with each class source further down instead. | ||
| if (classTypeArgCount > 0 && methodTypeArgCount > 0 && typeArguments.Length != classTypeArgCount + methodTypeArgCount) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| var classTypeArgs = classTypeArgCount > 0 ? typeArguments.Take(classTypeArgCount).ToArray() : Array.Empty<ITypeSymbol>(); | ||
| var methodTypeArgs = methodTypeArgCount > 0 ? typeArguments.Skip(classTypeArgCount).ToArray() : Array.Empty<ITypeSymbol>(); | ||
|
|
||
|
|
@@ -928,8 +939,10 @@ | |
| violation = new MethodConstraintViolation( | ||
| methodConstraintViolation, | ||
| // [GenerateGenericTest] (the violation source) has no rows: it reports one failure. | ||
| RowSource: violationSource is null ? specificAttr : null, | ||
| AllMethodDataSources: specificAttr is null && violationSource is null); | ||
| // A method data source paired with a class row reports each of its own rows | ||
| // (methodViolationRowSource), and IInfersType<T> each row of every method data source. | ||
| RowSource: methodViolationRowSource ?? (violationSource is null ? specificAttr : null), | ||
| AllMethodDataSources: methodViolationAllMethodDataSources || (specificAttr is null && violationSource is null)); | ||
|
|
||
| // All violations of one concrete class share one entry, so that each failing row gets | ||
| // its own data source index, and so its own test id. | ||
|
|
@@ -996,14 +1009,58 @@ | |
| // A class row combined with a generic method row. A class violation is reported once per | ||
| // class row; a method violation once per method row, as reflection mode reports it when | ||
| // the row's inferred method type arguments fail to resolve. | ||
| // A method data source other than an [Arguments] row (such as [MethodDataSource]) may yield | ||
| // several rows, so a method violation is reported once per row it yields instead. | ||
| void TryAddCombinedArgumentsInstantiation(ITypeSymbol[] classTypes, ITypeSymbol[] combinedTypes, AttributeData classAttr, AttributeData methodAttr) | ||
| { | ||
| TryAddInstantiation(combinedTypes, classAttr, | ||
| reportMethodConstraintViolation: true, | ||
| reportClassViolationAs: ClassViolationSource.ClassArguments, | ||
| violationSource: ValidateClassTypeConstraints(testMethod.TypeSymbol, classTypes, compilation) ? methodAttr : classAttr, | ||
| isClassRow: true, | ||
| methodRow: methodAttr); | ||
| methodRow: methodAttr, | ||
| methodViolationRowSource: methodAttr.AttributeClass?.Name == "ArgumentsAttribute" ? null : methodAttr); | ||
| } | ||
|
|
||
| // A class row paired with method type arguments that no single method data source inferred: | ||
| // a method [GenerateGenericTest] type (methodSource) or IInfersType<T> (null). The class row | ||
| // scopes the entry to itself, and every method data source runs on it, as on a non-generic | ||
| // class (#7117). | ||
| void TryAddClassRowInstantiationWithEveryMethodSource(ITypeSymbol[] classTypes, ITypeSymbol[] combinedTypes, AttributeData classAttr, AttributeData? methodSource, List<AttributeData>? methodDataSources) | ||
| { | ||
| var countBefore = results.Count; | ||
| TryAddInstantiation(combinedTypes, classAttr, | ||
| reportMethodConstraintViolation: true, | ||
| reportClassViolationAs: ClassViolationSource.ClassArguments, | ||
| violationSource: methodSource is not null && ValidateClassTypeConstraints(testMethod.TypeSymbol, classTypes, compilation) ? methodSource : classAttr, | ||
| isClassRow: true, | ||
| methodViolationAllMethodDataSources: methodSource is null); | ||
|
|
||
| if (results.Count > countBefore | ||
| && results[results.Count - 1] is { MethodConstraintViolations: null } added | ||
| && methodDataSources is not null) | ||
| { | ||
| foreach (var dataSource in methodDataSources) | ||
| { | ||
| added.AddRow(dataSource, isClassRow: false); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // The class and method type arguments of a generic method of a generic class. A method type | ||
| // argument inferred from a member of the open class (a [MethodDataSource] returning | ||
| // IEnumerable<Func<(T, int)>>) may name a class type parameter, which is replaced by this class | ||
| // type argument, as reflection mode reads the member on the closed class (#7117). | ||
| ITypeSymbol[] CloseOverClass(ITypeSymbol[] classTypes, ITypeSymbol[] methodTypes) | ||
| { | ||
| var combined = new ITypeSymbol[classTypes.Length + methodTypes.Length]; | ||
| Array.Copy(classTypes, combined, classTypes.Length); | ||
| for (var i = 0; i < methodTypes.Length; i++) | ||
| { | ||
| combined[classTypes.Length + i] = SubstituteTypeParameters(methodTypes[i], testMethod.TypeSymbol.TypeParameters, classTypes, compilation); | ||
| } | ||
|
|
||
| return combined; | ||
| } | ||
|
|
||
| // Single-pass classification of method attributes into named buckets, replacing the | ||
|
|
@@ -1238,8 +1295,78 @@ | |
| .ToList() | ||
| : []; | ||
|
|
||
| if (testMethod is { IsGenericType: true, IsGenericMethod: true } && classGenericTestAttrs.Count > 0 && methodGenericTestAttrs.Count > 0) | ||
| if (testMethod is { IsGenericType: true, IsGenericMethod: true }) | ||
| { | ||
| // A generic method of a generic class pairs every class source with every method | ||
| // source (#7117), as reflection mode closes the method on each concrete class. Method | ||
| // data sources come first, in reflection mode's order, so a type that one of them | ||
| // infers runs only that source. Any other [GenerateGenericTest] type runs every method | ||
| // data source. A null source (IInfersType<T>) also runs every method data source. | ||
| var methodSources = new List<(ITypeSymbol[] Types, AttributeData? Source)>(); | ||
| foreach (var methodArgAttr in methodArgumentsAttributes) | ||
| { | ||
| if (InferTypesFromArgumentsAttribute(testMethod.MethodSymbol, methodArgAttr, compilation) is { } types && types.Length == methodTypeArgCount) | ||
| { | ||
| methodSources.Add((types, methodArgAttr)); | ||
| } | ||
| } | ||
|
|
||
| // A typed data source (DataSourceGeneratorAttribute<T>) infers no method type arguments | ||
| // on a generic class in either mode (InferTypesFromDataSourceAttribute here, and | ||
| // ReflectionTestDataCollector.InferMethodInstantiationsFromDataSources), so it is not a | ||
| // method source: it runs on every instantiation like any other method data source. | ||
| if (InferTypesFromTypeInferringAttributes(testMethod.MethodSymbol) is { } inferringTypes && inferringTypes.Length == methodTypeArgCount) | ||
| { | ||
| methodSources.Add((inferringTypes, null)); | ||
| } | ||
|
|
||
| foreach (var mdsAttr in methodDataSourceBucket ?? Enumerable.Empty<AttributeData>()) | ||
| { | ||
| if (InferTypesFromMethodDataSource(testMethod, mdsAttr) is { } types && types.Length == methodTypeArgCount) | ||
| { | ||
| methodSources.Add((types, mdsAttr)); | ||
| } | ||
| } | ||
|
|
||
| foreach (var classGenericTest in classGenericTestAttrs) | ||
| { | ||
| var classIsValid = ValidateClassTypeConstraints(testMethod.TypeSymbol, classGenericTest.TypeArgs!, compilation); | ||
| foreach (var (methodTypes, source) in methodSources) | ||
| { | ||
| TryAddInstantiation(CloseOverClass(classGenericTest.TypeArgs!, methodTypes), source, | ||
| reportMethodConstraintViolation: true, | ||
| reportClassViolationAs: ClassViolationSource.GenerateGenericTest, | ||
| violationSource: classIsValid ? null : classGenericTest.Attribute); | ||
| } | ||
| } | ||
|
|
||
| foreach (var classAttr in classArgumentsAttributes) | ||
| { | ||
| var classTypes = InferTypesFromClassArgumentsAttribute(testMethod.TypeSymbol, classAttr, compilation); | ||
| if (classTypes is not { Length: > 0 }) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| // Method rows were already paired with this class row above; they join their entries. | ||
| foreach (var (methodTypes, source) in methodSources) | ||
| { | ||
| if (source is not null) | ||
| { | ||
| TryAddCombinedArgumentsInstantiation(classTypes, CloseOverClass(classTypes, methodTypes), classAttr, source); | ||
| } | ||
| else | ||
| { | ||
| TryAddClassRowInstantiationWithEveryMethodSource(classTypes, CloseOverClass(classTypes, methodTypes), classAttr, methodSource: null, typedDataSourceBucket); | ||
| } | ||
| } | ||
|
|
||
| foreach (var methodGenericTest in methodGenericTestAttrs) | ||
| { | ||
| TryAddClassRowInstantiationWithEveryMethodSource(classTypes, classTypes.Concat(methodGenericTest.TypeArgs!).ToArray(), classAttr, methodGenericTest.Attribute, typedDataSourceBucket); | ||
| } | ||
| } | ||
|
|
||
| // Cartesian product of class and method type args | ||
| foreach (var classGenericTest in classGenericTestAttrs) | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| using Microsoft.CodeAnalysis; | ||
| using Microsoft.CodeAnalysis.CSharp; | ||
| using TUnit.Core.SourceGenerator.Generators; | ||
|
|
||
| namespace TUnit.Core.SourceGenerator.Tests.Bugs._7117; | ||
|
|
||
| /// <summary> | ||
| /// Regression test for https://github.com/thomhurst/TUnit/issues/7117. | ||
| /// | ||
| /// <c>[GenerateGenericTest]</c> on a generic method of a generic class whose type arguments come from | ||
| /// a class-level data source (not a class-level <c>[GenerateGenericTest]</c>) used to emit generated | ||
| /// code that referenced the open method type parameter, which fails with CS0246. The snapshot | ||
| /// harness only inspects generator diagnostics, so this test compiles the generated output. | ||
| /// </summary> | ||
| internal class GenerateGenericTestOnMethodOfGenericClassTests | ||
| { | ||
| private const string Source = | ||
| """ | ||
| global using global::System; | ||
| global using global::System.Collections.Generic; | ||
| global using global::System.Linq; | ||
| global using global::System.Threading.Tasks; | ||
| global using global::TUnit.Core; | ||
|
|
||
| namespace MyTests; | ||
|
|
||
| [Arguments(1)] | ||
| public class GenericClass<T>(T value) | ||
| { | ||
| [Test] | ||
| [GenerateGenericTest(typeof(int))] | ||
| [Arguments(1)] | ||
| public void Run<TMethod>(TMethod arg) { } | ||
|
|
||
| [Test] | ||
| [GenerateGenericTest(typeof(string))] | ||
| public void NoMethodRows<TMethod>() { } | ||
|
|
||
| [Test] | ||
| [GenerateGenericTest(typeof(long))] | ||
| [Arguments(1)] | ||
| public void UsesBoth<TMethod>(T classArg) { } | ||
| } | ||
|
|
||
| [Arguments("a")] | ||
| [Arguments(2)] | ||
| public class TwoClassRows<T>(T value) | ||
| { | ||
| [Test] | ||
| [GenerateGenericTest(typeof(int))] | ||
| [GenerateGenericTest(typeof(string))] | ||
| [Arguments(1)] | ||
| public void Run<TMethod>(TMethod arg) { } | ||
| } | ||
|
|
||
| [GenerateGenericTest(typeof(string))] | ||
| public class ClassGenerateGenericTest<T> | ||
| { | ||
| [Test] | ||
| [GenerateGenericTest(typeof(int))] | ||
| [Arguments(1)] | ||
| public void Run<TMethod>(TMethod arg) { } | ||
| } | ||
|
|
||
| [Arguments(1)] | ||
| public class ClassRowMethodDataSource<T>(T value) | ||
| { | ||
| public static IEnumerable<Func<(string, int)>> Rows() { yield return () => ("a", 1); } | ||
|
|
||
| [Test] | ||
| [MethodDataSource(nameof(Rows))] | ||
| public void FromMethodDataSource<TMethod>(TMethod arg, int count) { } | ||
| } | ||
|
|
||
| [GenerateGenericTest(typeof(long))] | ||
| public class ClassGenerateGenericTestMethodDataSource<T> | ||
| { | ||
| public static IEnumerable<Func<(string, int)>> Rows() { yield return () => ("a", 1); } | ||
|
|
||
| [Test] | ||
| [MethodDataSource(nameof(Rows))] | ||
| public void FromMethodDataSource<TMethod>(TMethod arg, int count) { } | ||
| } | ||
|
|
||
| [GenerateGenericTest(typeof(long))] | ||
| public class ClassTypedMethodDataSource<T> | ||
| { | ||
| public static IEnumerable<Func<(T, int)>> Rows() { yield return () => (default(T)!, 1); } | ||
|
|
||
| [Test] | ||
| [MethodDataSource(nameof(Rows))] | ||
| public void FromClassTypedSource<TMethod>(TMethod arg, int count) { } | ||
| } | ||
|
|
||
| public class MethodRowInfersClass<T> | ||
| { | ||
| [Test] | ||
| [GenerateGenericTest(typeof(int))] | ||
| [Arguments("x", 1)] | ||
| public void Run<TMethod>(T classValue, TMethod arg) { } | ||
| } | ||
| """; | ||
|
|
||
| [Test] | ||
| public async Task Generated_code_compiles() | ||
| { | ||
| var parseOptions = new CSharpParseOptions(LanguageVersion.Preview); | ||
|
|
||
| var compilation = CSharpCompilation.Create( | ||
| "Issue7117", | ||
| [CSharpSyntaxTree.ParseText(Source, parseOptions)], | ||
| ReferencesHelper.References, | ||
| new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); | ||
|
|
||
| GeneratorDriver driver = CSharpGeneratorDriver.Create( | ||
| [new TestMetadataGenerator().AsSourceGenerator()], | ||
| parseOptions: parseOptions); | ||
| driver.RunGeneratorsAndUpdateCompilation(compilation, out var output, out _); | ||
|
|
||
| var errors = output.GetDiagnostics() | ||
| .Where(d => d.Severity == DiagnosticSeverity.Error) | ||
| .ToArray(); | ||
|
|
||
| var generated = string.Join( | ||
| Environment.NewLine + "----" + Environment.NewLine, | ||
| output.SyntaxTrees.Skip(1).Select(t => t.GetText().ToString())); | ||
|
|
||
| await Assert.That(errors) | ||
| .IsEmpty() | ||
| .Because($"generated code must compile. Errors:{Environment.NewLine}" | ||
| + string.Join(Environment.NewLine, errors.Select(e => e.ToString())) | ||
| + Environment.NewLine + generated); | ||
|
|
||
| // Both the class and the method are closed over their own type arguments. | ||
| await Assert.That(generated).Contains(".Run<int>("); | ||
| await Assert.That(generated).Contains(".NoMethodRows<string>("); | ||
| await Assert.That(generated).Contains(".UsesBoth<long>("); | ||
| await Assert.That(generated).Contains(".FromMethodDataSource<string>("); | ||
| await Assert.That(generated).Contains(".FromClassTypedSource<long>("); | ||
| await Assert.That(generated).DoesNotContain("<TMethod>"); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| using Shouldly; | ||
| using TUnit.Engine.Tests.Enums; | ||
|
|
||
| namespace TUnit.Engine.Tests; | ||
|
|
||
| /// <summary> | ||
| /// https://github.com/thomhurst/TUnit/issues/7117: [GenerateGenericTest] on a generic method of a | ||
| /// generic class closes the method on every class the class sources produce, in both modes. | ||
| /// </summary> | ||
| public class Issue7117Tests(TestMode testMode) : InvokableTestBase(testMode) | ||
| { | ||
| [Test] | ||
| [Arguments("MethodGenerateGenericTestOnClassRow", new[] { "Run<int>(2)" })] | ||
| [Arguments("MethodGenerateGenericTestWithClassRows", new[] { "Run<bool>", "Run<bool>" })] | ||
| [Arguments("ClassGenerateGenericTestWithMethodRow", new[] { "Run<int>(1)" })] | ||
| [Arguments("MethodGenerateGenericTestTypeNotInferredByRow", new[] { "Run<int>(2)", "Run<long>(2)" })] | ||
| [Arguments("MethodDataSourceOnClassRow", new[] { "Run<string>(a, 1)" })] | ||
| [Arguments("MethodDataSourceOnClassGenerateGenericTest", new[] { "Run<string>(a, 1)" })] | ||
| [Arguments("ClassTypedMethodDataSourceOnClassGenerateGenericTest", new[] { "Run<long>(0, 1)" })] | ||
| [Arguments("TypedDataSourceOnClassGenerateGenericTest", new[] { "Run<long>(42)" })] | ||
| public async Task Generic_Method_Of_Generic_Class_Runs_Each_Instantiation(string className, string[] expectedTestNames) | ||
| { | ||
| await RunTestsWithFilter( | ||
| $"/*/TUnit.TestProject.Bugs._7117/{className}*/*", | ||
| [ | ||
| result => result.ResultSummary.Outcome.ShouldBe("Completed"), | ||
| result => result.ResultSummary.Counters.Total.ShouldBe(expectedTestNames.Length), | ||
| result => result.ResultSummary.Counters.Passed.ShouldBe(expectedTestNames.Length), | ||
| result => result.ResultSummary.Counters.Failed.ShouldBe(0), | ||
| result => result.Results.Select(x => x.TestName).OrderBy(x => x, StringComparer.Ordinal) | ||
| .ShouldBe(expectedTestNames.OrderBy(x => x, StringComparer.Ordinal)) | ||
| ]); | ||
| } | ||
|
|
||
| // A method constraint violation paired with a class row fails once per row of the method source | ||
| // ([MethodDataSource] rows, or every method data source for IInfersType<T>), not once in total. | ||
| [Test] | ||
| [Arguments("MethodDataSourceConstraintViolationOnClassRow")] | ||
| [Arguments("MatrixConstraintViolationOnClassRow")] | ||
| public async Task Method_Constraint_Violation_On_Class_Row_Fails_Once_Per_Row(string className) | ||
| { | ||
| await RunTestsWithFilter( | ||
| $"/*/TUnit.TestProject.Bugs._7117/{className}/*", | ||
| [ | ||
| result => result.ResultSummary.Outcome.ShouldBe("Failed"), | ||
| result => result.ResultSummary.Counters.Total.ShouldBe(2), | ||
| result => result.ResultSummary.Counters.Failed.ShouldBe(2), | ||
| result => result.Results.ShouldAllBe(x => x.TestName == "Run"), | ||
| result => result.Results.ShouldAllBe(x => x.Output!.ErrorInfo!.Message!.Contains("TMethod")) | ||
| ]); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.