diff --git a/JustDummies.Analyzers.UnitTests/AnalyzerTestHarness.cs b/JustDummies.Analyzers.UnitTests/AnalyzerTestHarness.cs index 7a90b64f..2d142299 100644 --- a/JustDummies.Analyzers.UnitTests/AnalyzerTestHarness.cs +++ b/JustDummies.Analyzers.UnitTests/AnalyzerTestHarness.cs @@ -49,6 +49,11 @@ private static ImmutableArray BuildReferences() { // The JustDummies core, so Any.Reproducibly / Any.ReproduciblyAsync resolve inside the snippet. references.Add(MetadataReference.CreateFromFile(typeof(Any).Assembly.Location)); + // The xUnit adapter and xUnit itself, so [Reproducible], [Fact], [Theory] and TheoryData resolve in the + // snippets the lifecycle rules are tested against. + references.Add(MetadataReference.CreateFromFile(typeof(JustDummies.Xunit.ReproducibleAttribute).Assembly.Location)); + references.Add(MetadataReference.CreateFromFile(typeof(FactAttribute).Assembly.Location)); + return references.ToImmutableArray(); } diff --git a/JustDummies.Analyzers.UnitTests/Jd007DrawOutsideThePinnedScopeTests.cs b/JustDummies.Analyzers.UnitTests/Jd007DrawOutsideThePinnedScopeTests.cs new file mode 100644 index 00000000..b337632e --- /dev/null +++ b/JustDummies.Analyzers.UnitTests/Jd007DrawOutsideThePinnedScopeTests.cs @@ -0,0 +1,184 @@ +using System.Collections.Immutable; + +using Microsoft.CodeAnalysis; + +using NFluent; + +namespace JustDummies.Analyzers.UnitTests; + +public class Jd007DrawOutsideThePinnedScopeTests { + + [Fact] + public async Task Reports_a_draw_in_the_constructor_of_a_reproducible_class() { + const string source = """ + using JustDummies; + using JustDummies.Xunit; + using Xunit; + + [Reproducible] + public class Sample { + private readonly string _reference; + + public Sample() { + _reference = Any.String().NonEmpty().Generate(); + } + + [Fact] + public void T() { } + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new DrawOutsideThePinnedScopeAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(1); + Check.That(diagnostics[0].Id).IsEqualTo("JD007"); + Check.That(diagnostics[0].GetMessage()).Contains("constructor"); + } + + [Fact] + public async Task Reports_a_draw_in_a_field_initializer() { + const string source = """ + using JustDummies; + using JustDummies.Xunit; + using Xunit; + + [Reproducible] + public class Sample { + private readonly string _reference = Any.String().NonEmpty().Generate(); + + [Fact] + public void T() { } + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new DrawOutsideThePinnedScopeAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(1); + Check.That(diagnostics[0].Id).IsEqualTo("JD007"); + Check.That(diagnostics[0].GetMessage()).Contains("field initializer"); + } + + [Fact] + public async Task Reports_a_draw_in_InitializeAsync() { + const string source = """ + using System.Threading.Tasks; + using JustDummies; + using JustDummies.Xunit; + using Xunit; + + [Reproducible] + public class Sample : IAsyncLifetime { + private string _reference = ""; + + public ValueTask InitializeAsync() { + _reference = Any.String().NonEmpty().Generate(); + + return default; + } + + public ValueTask DisposeAsync() => default; + + [Fact] + public void T() { } + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new DrawOutsideThePinnedScopeAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(1); + Check.That(diagnostics[0].Id).IsEqualTo("JD007"); + Check.That(diagnostics[0].GetMessage()).Contains("InitializeAsync"); + } + + [Fact] + public async Task Does_not_report_a_draw_in_the_test_body() { + // The body IS inside the pinned scope — verified against xunit.v3 by probe before this rule was written. + const string source = """ + using JustDummies; + using JustDummies.Xunit; + using Xunit; + + [Reproducible] + public class Sample { + [Fact] + public void T() { + string reference = Any.String().NonEmpty().Generate(); + } + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new DrawOutsideThePinnedScopeAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(0); + } + + [Fact] + public async Task Does_not_report_a_class_without_the_attribute() { + const string source = """ + using JustDummies; + using Xunit; + + public class Sample { + private readonly string _reference = Any.String().NonEmpty().Generate(); + + [Fact] + public void T() { } + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new DrawOutsideThePinnedScopeAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(0); + } + + [Fact] + public async Task Does_not_report_a_draw_from_an_isolated_context() { + // Any.WithSeed is isolated by design: the ambient scope does not govern it, so the diagnostic would be wrong. + const string source = """ + using JustDummies; + using JustDummies.Xunit; + using Xunit; + + [Reproducible] + public class Sample { + private readonly string _reference; + + public Sample() { + AnyContext context = Any.WithSeed(1234); + _reference = context.String().NonEmpty().Generate(); + } + + [Fact] + public void T() { } + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new DrawOutsideThePinnedScopeAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(0); + } + + [Fact] + public async Task Reports_under_an_assembly_level_attribute() { + const string source = """ + using JustDummies; + using JustDummies.Xunit; + using Xunit; + + [assembly: Reproducible] + + public class Sample { + private readonly string _reference = Any.String().NonEmpty().Generate(); + + [Fact] + public void T() { } + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new DrawOutsideThePinnedScopeAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(1); + Check.That(diagnostics[0].Id).IsEqualTo("JD007"); + } + +} diff --git a/JustDummies.Analyzers.UnitTests/Jd008ArbitraryValueInTheoryDataTests.cs b/JustDummies.Analyzers.UnitTests/Jd008ArbitraryValueInTheoryDataTests.cs new file mode 100644 index 00000000..2d5fe091 --- /dev/null +++ b/JustDummies.Analyzers.UnitTests/Jd008ArbitraryValueInTheoryDataTests.cs @@ -0,0 +1,149 @@ +using System.Collections.Immutable; + +using Microsoft.CodeAnalysis; + +using NFluent; + +namespace JustDummies.Analyzers.UnitTests; + +public class Jd008ArbitraryValueInTheoryDataTests { + + [Fact] + public async Task Reports_a_draw_in_a_TheoryData_property() { + const string source = """ + using JustDummies; + using Xunit; + + public class Sample { + public static TheoryData Cases => new() { Any.String().NonEmpty().Generate() }; + + [Theory] + [MemberData(nameof(Cases))] + public void T(string reference) { } + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new ArbitraryValueInTheoryDataAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(1); + Check.That(diagnostics[0].Id).IsEqualTo("JD008"); + Check.That(diagnostics[0].GetMessage()).Contains("discovery"); + } + + [Fact] + public async Task Reports_a_draw_in_a_member_named_by_MemberData() { + const string source = """ + using System.Collections.Generic; + using JustDummies; + using Xunit; + + public class Sample { + public static IEnumerable Cases() { + yield return new object[] { Any.Int32().Positive().Generate() }; + } + + [Theory] + [MemberData(nameof(Cases))] + public void T(int quantity) { } + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new ArbitraryValueInTheoryDataAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(1); + Check.That(diagnostics[0].Id).IsEqualTo("JD008"); + } + + [Fact] + public async Task Reports_a_draw_in_a_ClassData_provider() { + const string source = """ + using System.Collections; + using System.Collections.Generic; + using JustDummies; + using Xunit; + + public class Cases : IEnumerable { + public IEnumerator GetEnumerator() { + yield return new object[] { Any.Int32().Positive().Generate() }; + } + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + } + + public class Sample { + [Theory] + [ClassData(typeof(Cases))] + public void T(int quantity) { } + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new ArbitraryValueInTheoryDataAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(1); + Check.That(diagnostics[0].Id).IsEqualTo("JD008"); + } + + [Fact] + public async Task Does_not_report_a_provider_that_yields_the_generator() { + // The compliant shape: the provider hands over the recipe, the body draws inside the pinned scope. + const string source = """ + using JustDummies; + using Xunit; + + public class Sample { + public static TheoryData> Cases => new() { Any.String().NonEmpty() }; + + [Theory] + [MemberData(nameof(Cases))] + public void T(IAny reference) { + string value = reference.Generate(); + } + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new ArbitraryValueInTheoryDataAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(0); + } + + [Fact] + public async Task Does_not_report_a_draw_in_an_ordinary_test_body() { + const string source = """ + using JustDummies; + using Xunit; + + public class Sample { + [Fact] + public void T() { + string reference = Any.String().NonEmpty().Generate(); + } + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new ArbitraryValueInTheoryDataAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(0); + } + + [Fact] + public async Task Does_not_report_an_ordinary_collection_of_object_arrays() { + // A member returning object[] sequences is only a provider when xUnit is told so; without a [MemberData] + // naming it and without the TheoryData shape, the rule must stay quiet here — this one IS named, so the + // sibling test covers the positive. Here nothing references it. + const string source = """ + using System.Collections.Generic; + using JustDummies; + + public class Sample { + public static List Values() { + return new List { Any.String().NonEmpty().Generate() }; + } + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new ArbitraryValueInTheoryDataAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(0); + } + +} diff --git a/JustDummies.Analyzers.UnitTests/Jd009DrawInStaticInitializerTests.cs b/JustDummies.Analyzers.UnitTests/Jd009DrawInStaticInitializerTests.cs new file mode 100644 index 00000000..d378fbc6 --- /dev/null +++ b/JustDummies.Analyzers.UnitTests/Jd009DrawInStaticInitializerTests.cs @@ -0,0 +1,97 @@ +using System.Collections.Immutable; + +using Microsoft.CodeAnalysis; + +using NFluent; + +namespace JustDummies.Analyzers.UnitTests; + +public class Jd009DrawInStaticInitializerTests { + + [Fact] + public async Task Reports_a_draw_in_a_static_field_initializer() { + const string source = """ + using JustDummies; + + public static class Sample { + private static readonly string Reference = Any.String().NonEmpty().Generate(); + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new DrawInStaticInitializerAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(1); + Check.That(diagnostics[0].Id).IsEqualTo("JD009"); + Check.That(diagnostics[0].GetMessage()).Contains("once for the whole suite"); + } + + [Fact] + public async Task Reports_a_draw_in_a_static_constructor() { + const string source = """ + using JustDummies; + + public static class Sample { + private static readonly string Reference; + + static Sample() { + Reference = Any.String().NonEmpty().Generate(); + } + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new DrawInStaticInitializerAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(1); + Check.That(diagnostics[0].Id).IsEqualTo("JD009"); + } + + [Fact] + public async Task Does_not_report_a_static_field_holding_the_generator() { + // The compliant shape: the recipe is shared, the draw happens per read. RandomSource resolves the source at + // Generate() time, so a shared generator is safe — only a shared value is not. + const string source = """ + using JustDummies; + + public static class Sample { + private static readonly IAny Reference = Any.String().NonEmpty(); + + public static string Next() => Reference.Generate(); + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new DrawInStaticInitializerAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(0); + } + + [Fact] + public async Task Does_not_report_a_draw_in_an_instance_initializer() { + const string source = """ + using JustDummies; + + public class Sample { + private readonly string _reference = Any.String().NonEmpty().Generate(); + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new DrawInStaticInitializerAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(0); + } + + [Fact] + public async Task Does_not_report_a_draw_in_an_ordinary_method() { + const string source = """ + using JustDummies; + + public static class Sample { + public static string Next() => Any.String().NonEmpty().Generate(); + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new DrawInStaticInitializerAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(0); + } + +} diff --git a/JustDummies.Analyzers.UnitTests/Jd010ReproducibleOnNonTestMethodTests.cs b/JustDummies.Analyzers.UnitTests/Jd010ReproducibleOnNonTestMethodTests.cs new file mode 100644 index 00000000..bd787733 --- /dev/null +++ b/JustDummies.Analyzers.UnitTests/Jd010ReproducibleOnNonTestMethodTests.cs @@ -0,0 +1,104 @@ +using System.Collections.Immutable; + +using Microsoft.CodeAnalysis; + +using NFluent; + +namespace JustDummies.Analyzers.UnitTests; + +public class Jd010ReproducibleOnNonTestMethodTests { + + [Fact] + public async Task Reports_the_attribute_on_a_helper_method() { + const string source = """ + using JustDummies; + using JustDummies.Xunit; + + public class Sample { + [Reproducible] + private string Arrange() => Any.String().NonEmpty().Generate(); + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new ReproducibleOnNonTestMethodAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(1); + Check.That(diagnostics[0].Id).IsEqualTo("JD010"); + Check.That(diagnostics[0].GetMessage()).Contains("Arrange"); + } + + [Fact] + public async Task Does_not_report_the_attribute_on_a_Fact() { + const string source = """ + using JustDummies; + using JustDummies.Xunit; + using Xunit; + + public class Sample { + [Fact] + [Reproducible] + public void T() { + string reference = Any.String().NonEmpty().Generate(); + } + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new ReproducibleOnNonTestMethodAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(0); + } + + [Fact] + public async Task Does_not_report_the_attribute_on_a_Theory() { + const string source = """ + using JustDummies.Xunit; + using Xunit; + + public class Sample { + [Theory] + [InlineData(1)] + [Reproducible] + public void T(int value) { } + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new ReproducibleOnNonTestMethodAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(0); + } + + [Fact] + public async Task Does_not_report_the_attribute_on_a_class() { + // The class and assembly levels are exactly where xUnit does collect it. + const string source = """ + using JustDummies.Xunit; + using Xunit; + + [Reproducible] + public class Sample { + [Fact] + public void T() { } + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new ReproducibleOnNonTestMethodAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(0); + } + + [Fact] + public async Task Does_not_report_a_method_without_the_attribute() { + const string source = """ + using JustDummies; + + public class Sample { + private string Arrange() => Any.String().NonEmpty().Generate(); + } + """; + + ImmutableArray diagnostics = await AnalyzerTestHarness.GetDiagnosticsAsync(new ReproducibleOnNonTestMethodAnalyzer(), source); + + Check.That(diagnostics.Length).IsEqualTo(0); + } + +} diff --git a/JustDummies.Analyzers.UnitTests/JustDummies.Analyzers.UnitTests.csproj b/JustDummies.Analyzers.UnitTests/JustDummies.Analyzers.UnitTests.csproj index 292ec2b2..d56c1670 100644 --- a/JustDummies.Analyzers.UnitTests/JustDummies.Analyzers.UnitTests.csproj +++ b/JustDummies.Analyzers.UnitTests/JustDummies.Analyzers.UnitTests.csproj @@ -28,6 +28,9 @@ + + diff --git a/JustDummies.Analyzers/AnalyzerReleases.Unshipped.md b/JustDummies.Analyzers/AnalyzerReleases.Unshipped.md index cea25194..73cf7173 100644 --- a/JustDummies.Analyzers/AnalyzerReleases.Unshipped.md +++ b/JustDummies.Analyzers/AnalyzerReleases.Unshipped.md @@ -11,3 +11,7 @@ JD003 | JustDummies.Reproducibility | Error | AwaitableBodyPassedToReproduc JD004 | JustDummies.Reproducibility | Error | DiscardedSeedingResultAnalyzer JD005 | JustDummies.Usage | Error | GeneratorRenderedAsTextAnalyzer JD006 | JustDummies.Usage | Warning | DiscardedGeneratorResultAnalyzer +JD007 | JustDummies.Reproducibility | Warning | DrawOutsideThePinnedScopeAnalyzer +JD008 | JustDummies.Reproducibility | Warning | ArbitraryValueInTheoryDataAnalyzer +JD009 | JustDummies.Reproducibility | Warning | DrawInStaticInitializerAnalyzer +JD010 | JustDummies.Reproducibility | Warning | ReproducibleOnNonTestMethodAnalyzer diff --git a/JustDummies.Analyzers/ArbitraryValueInTheoryDataAnalyzer.cs b/JustDummies.Analyzers/ArbitraryValueInTheoryDataAnalyzer.cs new file mode 100644 index 00000000..c2922998 --- /dev/null +++ b/JustDummies.Analyzers/ArbitraryValueInTheoryDataAnalyzer.cs @@ -0,0 +1,48 @@ +using System.Collections.Immutable; + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace JustDummies.Analyzers; + +/// +/// JD008 — reports a value drawn inside a theory's data provider. xUnit evaluates providers at discovery, +/// before any test case runs: the draw happens once for the whole run, outside every seed scope, and every case of +/// the theory shares the one value. The theory reads as if it enumerated arbitrary cases and enumerates a constant. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class ArbitraryValueInTheoryDataAnalyzer : DiagnosticAnalyzer { + + /// + public override ImmutableArray SupportedDiagnostics { get; } = + ImmutableArray.Create(Descriptors.ArbitraryValueInTheoryData); + + /// + public override void Initialize(AnalysisContext context) { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterCompilationStartAction(OnCompilationStart); + } + + private static void OnCompilationStart(CompilationStartAnalysisContext context) { + KnownSymbols symbols = KnownSymbols.From(context.Compilation); + if (symbols.Any is null || symbols.IAny is null) { return; } + + // Nothing to reason about without xUnit: a data provider is an xUnit concept. + if (symbols.MemberDataAttribute is null) { return; } + + context.RegisterOperationAction(operationContext => Analyze(operationContext, symbols), OperationKind.Invocation); + } + + private static void Analyze(OperationAnalysisContext context, KnownSymbols symbols) { + IInvocationOperation invocation = (IInvocationOperation)context.Operation; + + if (!GeneratorFacts.IsGenerateCall(invocation, symbols.IAny!)) { return; } + if (!GeneratorFacts.RootsAtAmbientAny(invocation, symbols.Any!)) { return; } + if (!XunitFacts.IsTheoryDataProvider(context.ContainingSymbol, symbols)) { return; } + + context.ReportDiagnostic(Diagnostic.Create(Descriptors.ArbitraryValueInTheoryData, invocation.Syntax.GetLocation())); + } + +} diff --git a/JustDummies.Analyzers/Descriptors.cs b/JustDummies.Analyzers/Descriptors.cs index b19998b0..58996eab 100644 --- a/JustDummies.Analyzers/Descriptors.cs +++ b/JustDummies.Analyzers/Descriptors.cs @@ -67,4 +67,44 @@ internal static class Descriptors { description: "Every constraint returns a new generator rather than mutating the receiver. A discarded result therefore silently drops the invariant the arrangement declared, and the generator keeps drawing from the wider domain — so the test passes on most runs and fails on the one that draws outside it, with a value nobody can reproduce.", helpLinkUri: HelpLinks.For(DiagnosticIds.DiscardedGeneratorResult)); + public static readonly DiagnosticDescriptor DrawOutsideThePinnedScope = new( + id: DiagnosticIds.DrawOutsideThePinnedScope, + title: "An arbitrary value is drawn before [Reproducible] pins the seed", + messageFormat: "Draw this value inside the test body: {0} runs before [Reproducible] opens the seed scope, so the seed the failure reports does not replay it", + category: DiagnosticCategories.Reproducibility, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "xUnit constructs the test-class instance, and awaits IAsyncLifetime.InitializeAsync, before running the hooks the adapter pins the seed from. A value drawn there comes from the unseeded ambient source, so the test advertises full reproducibility while part of its arrangement is unpinned: pinning the reported seed does not bring the failure back.", + helpLinkUri: HelpLinks.For(DiagnosticIds.DrawOutsideThePinnedScope)); + + public static readonly DiagnosticDescriptor ArbitraryValueInTheoryData = new( + id: DiagnosticIds.ArbitraryValueInTheoryData, + title: "A theory's data provider draws an arbitrary value", + messageFormat: "Draw this value in the test body, or let the provider yield the generator: theory data is produced at discovery, before any seed is pinned, and every case shares the one value", + category: DiagnosticCategories.Reproducibility, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "xUnit evaluates a theory's data provider at discovery time, once for the whole run and outside every seed scope. The drawn value is therefore shared by every case of the theory, replayable from no reported seed, and constant where the theory reads as if it enumerated arbitrary cases.", + helpLinkUri: HelpLinks.For(DiagnosticIds.ArbitraryValueInTheoryData)); + + public static readonly DiagnosticDescriptor DrawInStaticInitializer = new( + id: DiagnosticIds.DrawInStaticInitializer, + title: "An arbitrary value is drawn in a static initializer", + messageFormat: "Hold the generator rather than the value: a static initializer draws once for the whole suite, under whichever test happened to run first", + category: DiagnosticCategories.Reproducibility, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "A type initializer runs once, lazily, when the first test touches the type. The value is drawn under whatever ambient context that test had pinned, is shared by every other test in the class, and is replayable from none of their reported seeds — so the tests become order-dependent and stop varying between runs. Store the generator in the static field and call Generate() per test.", + helpLinkUri: HelpLinks.For(DiagnosticIds.DrawInStaticInitializer)); + + public static readonly DiagnosticDescriptor ReproducibleOnNonTestMethod = new( + id: DiagnosticIds.ReproducibleOnNonTestMethod, + title: "[Reproducible] is applied to a method that is not a test", + messageFormat: "Remove [Reproducible] from '{0}' or make it a test: xUnit collects the attribute from the test method, its class and the assembly only, so here it pins nothing", + category: DiagnosticCategories.Reproducibility, + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "The adapter's hooks are collected from a test method, its declaring class and the assembly. On a helper — or on a method whose [Fact] was removed during a refactor — the attribute is never read: it pins no seed and reports none. Because a working [Reproducible] is silent on a passing test by design, nothing else distinguishes the inert form from the working one.", + helpLinkUri: HelpLinks.For(DiagnosticIds.ReproducibleOnNonTestMethod)); + } diff --git a/JustDummies.Analyzers/DiagnosticIds.cs b/JustDummies.Analyzers/DiagnosticIds.cs index 594b29e2..f04a3bf9 100644 --- a/JustDummies.Analyzers/DiagnosticIds.cs +++ b/JustDummies.Analyzers/DiagnosticIds.cs @@ -16,4 +16,10 @@ internal static class DiagnosticIds { public const string GeneratorRenderedAsText = "JD005"; public const string DiscardedGeneratorResult = "JD006"; + // Category: Reproducibility — draws that escape the pinned seed scope + public const string DrawOutsideThePinnedScope = "JD007"; + public const string ArbitraryValueInTheoryData = "JD008"; + public const string DrawInStaticInitializer = "JD009"; + public const string ReproducibleOnNonTestMethod = "JD010"; + } diff --git a/JustDummies.Analyzers/DrawInStaticInitializerAnalyzer.cs b/JustDummies.Analyzers/DrawInStaticInitializerAnalyzer.cs new file mode 100644 index 00000000..79c14d1d --- /dev/null +++ b/JustDummies.Analyzers/DrawInStaticInitializerAnalyzer.cs @@ -0,0 +1,54 @@ +using System.Collections.Immutable; + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace JustDummies.Analyzers; + +/// +/// JD009 — reports a value drawn in a static field initializer or a static constructor. The type initializer runs +/// once, lazily, on whichever test first touches the type: one value is shared by every test in the class, drawn +/// under whatever seed that first test happened to pin, and replayable from none of them. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class DrawInStaticInitializerAnalyzer : DiagnosticAnalyzer { + + /// + public override ImmutableArray SupportedDiagnostics { get; } = + ImmutableArray.Create(Descriptors.DrawInStaticInitializer); + + /// + public override void Initialize(AnalysisContext context) { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterCompilationStartAction(OnCompilationStart); + } + + private static void OnCompilationStart(CompilationStartAnalysisContext context) { + KnownSymbols symbols = KnownSymbols.From(context.Compilation); + if (symbols.Any is null || symbols.IAny is null) { return; } + + context.RegisterOperationAction(operationContext => Analyze(operationContext, symbols), OperationKind.Invocation); + } + + private static void Analyze(OperationAnalysisContext context, KnownSymbols symbols) { + IInvocationOperation invocation = (IInvocationOperation)context.Operation; + + if (!GeneratorFacts.IsGenerateCall(invocation, symbols.IAny!)) { return; } + if (!GeneratorFacts.RootsAtAmbientAny(invocation, symbols.Any!)) { return; } + if (!IsStaticInitialization(context.ContainingSymbol)) { return; } + + context.ReportDiagnostic(Diagnostic.Create(Descriptors.DrawInStaticInitializer, invocation.Syntax.GetLocation())); + } + + private static bool IsStaticInitialization(ISymbol symbol) { + return symbol switch { + IFieldSymbol { IsStatic: true } => true, + IPropertySymbol { IsStatic: true } => true, + IMethodSymbol { IsStatic: true, MethodKind: MethodKind.StaticConstructor } => true, + _ => false, + }; + } + +} diff --git a/JustDummies.Analyzers/DrawOutsideThePinnedScopeAnalyzer.cs b/JustDummies.Analyzers/DrawOutsideThePinnedScopeAnalyzer.cs new file mode 100644 index 00000000..27a8c6e6 --- /dev/null +++ b/JustDummies.Analyzers/DrawOutsideThePinnedScopeAnalyzer.cs @@ -0,0 +1,79 @@ +using System.Collections.Immutable; + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace JustDummies.Analyzers; + +/// +/// JD007 — reports a value drawn during a [Reproducible] test class's construction, which xUnit runs +/// before the adapter opens the seed scope. The draw comes from the unseeded ambient source, so the seed the +/// failure reports replays the body and not the arrangement: the reader pins it, the run still differs, and the +/// test looks unreplayable while advertising that it is not. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class DrawOutsideThePinnedScopeAnalyzer : DiagnosticAnalyzer { + + private const string InitializeAsyncMethodName = "InitializeAsync"; + + /// + public override ImmutableArray SupportedDiagnostics { get; } = + ImmutableArray.Create(Descriptors.DrawOutsideThePinnedScope); + + /// + public override void Initialize(AnalysisContext context) { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterCompilationStartAction(OnCompilationStart); + } + + private static void OnCompilationStart(CompilationStartAnalysisContext context) { + KnownSymbols symbols = KnownSymbols.From(context.Compilation); + if (symbols.Any is null || symbols.IAny is null || symbols.ReproducibleAttribute is null) { return; } + + context.RegisterOperationAction(operationContext => Analyze(operationContext, symbols), OperationKind.Invocation); + } + + private static void Analyze(OperationAnalysisContext context, KnownSymbols symbols) { + IInvocationOperation invocation = (IInvocationOperation)context.Operation; + + if (!GeneratorFacts.IsGenerateCall(invocation, symbols.IAny!)) { return; } + if (!GeneratorFacts.RootsAtAmbientAny(invocation, symbols.Any!)) { return; } + + ISymbol containing = context.ContainingSymbol; + if (!RunsBeforeTheScopeOpens(containing, out string? phase)) { return; } + if (!XunitFacts.IsCoveredByReproducible(containing, symbols.ReproducibleAttribute!)) { return; } + + context.ReportDiagnostic(Diagnostic.Create(Descriptors.DrawOutsideThePinnedScope, invocation.Syntax.GetLocation(), phase)); + } + + // xUnit constructs the test-class instance, and awaits IAsyncLifetime.InitializeAsync, before it runs the + // BeforeAfterTestAttribute hooks the adapter pins the seed from. Everything drawn there is outside the scope. + private static bool RunsBeforeTheScopeOpens(ISymbol symbol, out string? phase) { + phase = null; + + if (symbol is IFieldSymbol { IsStatic: false } or IPropertySymbol { IsStatic: false }) { + phase = "a field initializer"; + + return true; + } + + if (symbol is not IMethodSymbol method || method.IsStatic) { return false; } + + if (method.MethodKind == MethodKind.Constructor) { + phase = "the test class constructor"; + + return true; + } + + if (method.Name == InitializeAsyncMethodName) { + phase = "InitializeAsync"; + + return true; + } + + return false; + } + +} diff --git a/JustDummies.Analyzers/GeneratorFacts.cs b/JustDummies.Analyzers/GeneratorFacts.cs index 60751ea5..d6d4ad9a 100644 --- a/JustDummies.Analyzers/GeneratorFacts.cs +++ b/JustDummies.Analyzers/GeneratorFacts.cs @@ -9,6 +9,8 @@ namespace JustDummies.Analyzers; /// internal static class GeneratorFacts { + private const string GenerateMethodName = "Generate"; + /// /// Whether is a JustDummies generator — the IAny<T> interface itself, or /// any type implementing it. Matching the interface rather than a list of concrete builders keeps the rules @@ -26,6 +28,41 @@ public static bool IsGenerator(ITypeSymbol? type, INamedTypeSymbol iAnyType) { return false; } + /// + /// Whether is a materializing Generate() call — the single member of + /// IAny<T>, and the only thing that turns a recipe into a value. + /// + public static bool IsGenerateCall(IInvocationOperation invocation, INamedTypeSymbol iAnyType) { + IMethodSymbol method = invocation.TargetMethod; + + return method.Name == GenerateMethodName + && method.Parameters.IsEmpty + && IsGenerator(method.ContainingType, iAnyType); + } + + /// + /// Whether the chain the Generate() call sits on provably starts at a static JustDummies.Any + /// factory — that is, whether the value is drawn from the ambient random source that a seed scope pins. + /// + /// + /// Deliberately conservative: it answers "yes" only for a chain written inline from Any. A generator + /// reached through a local, a field or a parameter answers "no" and is not reported, which under-reports rather + /// than misfiring on a draw from an isolated AnyContext — that context is unaffected by the ambient + /// scope, so reporting it would be plainly wrong. + /// + public static bool RootsAtAmbientAny(IInvocationOperation invocation, INamedTypeSymbol anyType) { + for (IOperation? current = invocation; current is IInvocationOperation call;) { + if (call.Instance is null) { + // A static call: ambient only when it is one of Any's own factories. + return SymbolEqualityComparer.Default.Equals(call.TargetMethod.ContainingType, anyType); + } + + current = Unwrap(call.Instance); + } + + return false; + } + /// /// Strips the implicit conversions Roslyn inserts around a generator when it flows into an object or /// string position, so the rule sees the recipe rather than the conversion wrapping it. diff --git a/JustDummies.Analyzers/KnownSymbols.cs b/JustDummies.Analyzers/KnownSymbols.cs index 242af8e2..04f2616d 100644 --- a/JustDummies.Analyzers/KnownSymbols.cs +++ b/JustDummies.Analyzers/KnownSymbols.cs @@ -3,18 +3,32 @@ namespace JustDummies.Analyzers; /// -/// Resolves the JustDummies types an analyzer matches against, by metadata name. Analyzers never reference the -/// JustDummies assembly directly — they are loaded into the consumer's compiler — so a null field simply means the -/// library is not part of the analyzed compilation, in which case the analyzer stays silent. +/// Resolves the types an analyzer matches against, by metadata name. Analyzers never reference the JustDummies +/// assembly directly — they are loaded into the consumer's compiler — so a null field simply means the type is not +/// part of the analyzed compilation, in which case the rule that needs it stays silent. /// +/// +/// The xUnit and adapter lookups are what let a rule reason about a test's lifecycle without JustDummies ever +/// depending on either: a consumer who uses neither gets a compilation where those fields are null, and the rules +/// that need them never register. +/// internal sealed class KnownSymbols { - public const string AnyMetadataName = "JustDummies.Any"; - public const string IAnyMetadataName = "JustDummies.IAny`1"; + public const string AnyMetadataName = "JustDummies.Any"; + public const string IAnyMetadataName = "JustDummies.IAny`1"; + public const string AnyContextMetadataName = "JustDummies.AnyContext"; + + public const string ReproducibleAttributeMetadataName = "JustDummies.Xunit.ReproducibleAttribute"; + public const string FactAttributeMetadataName = "Xunit.v3.IFactAttribute"; + public const string MemberDataAttributeMetadataName = "Xunit.MemberDataAttribute"; private KnownSymbols(Compilation compilation) { - Any = compilation.GetTypeByMetadataName(AnyMetadataName); - IAny = compilation.GetTypeByMetadataName(IAnyMetadataName); + Any = compilation.GetTypeByMetadataName(AnyMetadataName); + IAny = compilation.GetTypeByMetadataName(IAnyMetadataName); + AnyContext = compilation.GetTypeByMetadataName(AnyContextMetadataName); + ReproducibleAttribute = compilation.GetTypeByMetadataName(ReproducibleAttributeMetadataName); + FactAttribute = compilation.GetTypeByMetadataName(FactAttributeMetadataName); + MemberDataAttribute = compilation.GetTypeByMetadataName(MemberDataAttributeMetadataName); } /// The JustDummies.Any façade, or null when the compilation does not reference JustDummies. @@ -23,6 +37,18 @@ private KnownSymbols(Compilation compilation) { /// The unbound JustDummies.IAny<T> generator interface, or null as above. public INamedTypeSymbol? IAny { get; } + /// The isolated JustDummies.AnyContext, whose draws are unaffected by the ambient seed scope. + public INamedTypeSymbol? AnyContext { get; } + + /// The xUnit adapter's [Reproducible], or null when the adapter is not referenced. + public INamedTypeSymbol? ReproducibleAttribute { get; } + + /// The interface every xUnit test attribute implements — [Fact], [Theory] and derivatives. + public INamedTypeSymbol? FactAttribute { get; } + + /// xUnit's [MemberData], which names the member producing a theory's cases. + public INamedTypeSymbol? MemberDataAttribute { get; } + public static KnownSymbols From(Compilation compilation) { return new KnownSymbols(compilation); } diff --git a/JustDummies.Analyzers/ReproducibleOnNonTestMethodAnalyzer.cs b/JustDummies.Analyzers/ReproducibleOnNonTestMethodAnalyzer.cs new file mode 100644 index 00000000..39191ac3 --- /dev/null +++ b/JustDummies.Analyzers/ReproducibleOnNonTestMethodAnalyzer.cs @@ -0,0 +1,51 @@ +using System.Collections.Immutable; + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; + +namespace JustDummies.Analyzers; + +/// +/// JD010 — reports [Reproducible] on a method xUnit never treats as a test. The adapter's hooks are +/// collected from the test method, its class and the assembly only, so an attribute on a helper — or on a method +/// whose [Fact] was removed during a refactor — pins nothing and reports nothing. It is invisible when it +/// works (a passing test stays silent by design), so nothing else can tell the two apart. +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class ReproducibleOnNonTestMethodAnalyzer : DiagnosticAnalyzer { + + /// + public override ImmutableArray SupportedDiagnostics { get; } = + ImmutableArray.Create(Descriptors.ReproducibleOnNonTestMethod); + + /// + public override void Initialize(AnalysisContext context) { + context.EnableConcurrentExecution(); + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.RegisterCompilationStartAction(OnCompilationStart); + } + + private static void OnCompilationStart(CompilationStartAnalysisContext context) { + KnownSymbols symbols = KnownSymbols.From(context.Compilation); + if (symbols.ReproducibleAttribute is null || symbols.FactAttribute is null) { return; } + + context.RegisterSymbolAction(symbolContext => Analyze(symbolContext, symbols), SymbolKind.Method); + } + + private static void Analyze(SymbolAnalysisContext context, KnownSymbols symbols) { + IMethodSymbol method = (IMethodSymbol)context.Symbol; + + foreach (AttributeData attribute in method.GetAttributes()) { + if (!SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, symbols.ReproducibleAttribute)) { continue; } + if (XunitFacts.IsTestMethod(method, symbols.FactAttribute!)) { return; } + + Location location = attribute.ApplicationSyntaxReference?.GetSyntax(context.CancellationToken).GetLocation() + ?? method.Locations[0]; + + context.ReportDiagnostic(Diagnostic.Create(Descriptors.ReproducibleOnNonTestMethod, location, method.Name)); + + return; + } + } + +} diff --git a/JustDummies.Analyzers/XunitFacts.cs b/JustDummies.Analyzers/XunitFacts.cs new file mode 100644 index 00000000..a3ba2313 --- /dev/null +++ b/JustDummies.Analyzers/XunitFacts.cs @@ -0,0 +1,133 @@ +using System.Collections.Generic; +using System.Linq; + +using Microsoft.CodeAnalysis; + +namespace JustDummies.Analyzers; + +/// +/// Facts about an xUnit test's shape and lifecycle, needed by the rules that reason about when a value is +/// drawn relative to the seed scope [Reproducible] opens. +/// +/// +/// Every lookup flows through , so a compilation that references neither xUnit nor the +/// JustDummies adapter simply never reaches these rules — JustDummies stays standalone and error-agnostic. +/// +internal static class XunitFacts { + + /// + /// Whether is covered by [Reproducible] — declared on the member itself, on + /// its containing type or a base type, or on the assembly. These are the three levels the adapter honours. + /// + public static bool IsCoveredByReproducible(ISymbol symbol, INamedTypeSymbol reproducibleAttribute) { + if (HasAttribute(symbol, reproducibleAttribute)) { return true; } + + for (INamedTypeSymbol? type = symbol.ContainingType; type is not null; type = type.BaseType) { + if (HasAttribute(type, reproducibleAttribute)) { return true; } + } + + return HasAttribute(symbol.ContainingAssembly, reproducibleAttribute); + } + + /// + /// Whether carries an attribute xUnit treats as a test — anything implementing + /// IFactAttribute, which covers [Fact], [Theory] and third-party derivatives alike. + /// + public static bool IsTestMethod(IMethodSymbol method, INamedTypeSymbol factAttribute) { + foreach (AttributeData attribute in method.GetAttributes()) { + if (attribute.AttributeClass is not null && Implements(attribute.AttributeClass, factAttribute)) { return true; } + } + + return false; + } + + /// + /// Whether produces a theory's cases — a member xUnit evaluates at discovery, + /// before any test runs and outside every seed scope. + /// + /// + /// Four shapes are recognised, because a provider is written in four ways: a member named by a + /// [MemberData] in the same type; a member returning TheoryData; a member returning a sequence of + /// object arrays; and a type implementing that sequence, which is the [ClassData] shape. + /// + public static bool IsTheoryDataProvider(ISymbol symbol, KnownSymbols symbols) { + // A draw inside a property's body reports the accessor (get_Cases), not the property, so normalize first — + // otherwise a [MemberData(nameof(Cases))] never matches the member it names. + ISymbol member = symbol is IMethodSymbol { AssociatedSymbol: not null } accessor ? accessor.AssociatedSymbol! : symbol; + + if (IsNamedByMemberData(member, symbols)) { return true; } + + ITypeSymbol? returnType = member switch { + IMethodSymbol method => method.ReturnType, + IPropertySymbol property => property.Type, + _ => null, + }; + + if (returnType is not null && (IsTheoryData(returnType) || IsObjectArraySequence(returnType))) { return true; } + + // The [ClassData] shape: the containing type is itself the sequence of cases. + return member.ContainingType is not null && IsObjectArraySequence(member.ContainingType); + } + + private static bool IsNamedByMemberData(ISymbol symbol, KnownSymbols symbols) { + if (symbols.MemberDataAttribute is null || symbol.ContainingType is null) { return false; } + + IEnumerable attributes = symbol.ContainingType.GetMembers() + .OfType() + .SelectMany(member => member.GetAttributes()); + + foreach (AttributeData attribute in attributes) { + if (!SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, symbols.MemberDataAttribute)) { continue; } + if (attribute.ConstructorArguments.Length == 0) { continue; } + if (attribute.ConstructorArguments[0].Value as string == symbol.Name) { return true; } + } + + return false; + } + + // Matched by name rather than by symbol: xUnit v3 roots TheoryData<...> in TheoryDataBase<,>, and which generic + // base carries which arity has moved between releases. The namespace plus the TheoryData prefix is the stable part. + private static bool IsTheoryData(ITypeSymbol type) { + for (ITypeSymbol? current = type; current is not null; current = current.BaseType) { + bool inXunit = current.ContainingNamespace is { IsGlobalNamespace: false } ns && ns.ToDisplayString() == "Xunit"; + if (inXunit && current.Name.StartsWith("TheoryData", System.StringComparison.Ordinal)) { return true; } + } + + return false; + } + + // IEnumerable — the raw shape xUnit ultimately consumes, and what a [ClassData] type implements. + private static bool IsObjectArraySequence(ITypeSymbol type) { + IEnumerable candidates = type is INamedTypeSymbol named + ? named.AllInterfaces.Concat(new[] { named }) + : type.AllInterfaces; + + foreach (INamedTypeSymbol candidate in candidates) { + if (candidate.OriginalDefinition.SpecialType != SpecialType.System_Collections_Generic_IEnumerable_T) { continue; } + if (candidate.TypeArguments.Length == 1 && candidate.TypeArguments[0] is IArrayTypeSymbol { ElementType.SpecialType: SpecialType.System_Object }) { return true; } + } + + return false; + } + + private static bool HasAttribute(ISymbol? symbol, INamedTypeSymbol attributeType) { + if (symbol is null) { return false; } + + foreach (AttributeData attribute in symbol.GetAttributes()) { + if (SymbolEqualityComparer.Default.Equals(attribute.AttributeClass, attributeType)) { return true; } + } + + return false; + } + + private static bool Implements(INamedTypeSymbol type, INamedTypeSymbol @interface) { + if (SymbolEqualityComparer.Default.Equals(type, @interface)) { return true; } + + foreach (INamedTypeSymbol implemented in type.AllInterfaces) { + if (SymbolEqualityComparer.Default.Equals(implemented, @interface)) { return true; } + } + + return false; + } + +} diff --git a/doc/handwritten/for-users/analyzers/JD007.en.md b/doc/handwritten/for-users/analyzers/JD007.en.md new file mode 100644 index 00000000..4899d3b6 --- /dev/null +++ b/doc/handwritten/for-users/analyzers/JD007.en.md @@ -0,0 +1,58 @@ +# JD007: DrawOutsideThePinnedScope + +🌍 **Languages:** +🇬🇧 English (this file) | 🇫🇷 [Français](./JD007.fr.md) + +| | | +|---|---| +| **Category** | Reproducibility (`JustDummies.Reproducibility`) | +| **Severity** | 🟠 Warning | +| **Enabled by default** | Yes | + +`[Reproducible]` pins the seed from an xUnit *before/after* hook. xUnit constructs the test-class instance — and awaits `IAsyncLifetime.InitializeAsync` — **before** running those hooks, so anything drawn during construction comes from the unseeded ambient source. + +The result is worse than no reproducibility at all: the test advertises it. The failure prints *"Reproduce this run with `[Reproducible(Seed = 1234)]`"*, the reader pins 1234, the arrangement still differs, the failure does not come back, and the seed looks broken. + +Draw inside the test body, or hold the **generator** in the field and materialize it per test. + +## Noncompliant + +```csharp +[Reproducible] +public class OrderTests { + private readonly string _reference; + + public OrderTests() { + _reference = Any.String().StartingWith("ORD-").Generate(); // JD007: drawn before the seed is pinned + } + + [Fact] + public void It_is_accepted() { /* uses _reference */ } +} +``` + +## Compliant + +```csharp +[Reproducible] +public class OrderTests { + // The recipe is safe to share: the random source is resolved at Generate(), not at construction. + private readonly IAny _reference = Any.String().StartingWith("ORD-"); + + [Fact] + public void It_is_accepted() { + string reference = _reference.Generate(); // drawn inside the pinned scope + } +} +``` + +## What it does not flag + +* A draw in the **test body**, which is inside the scope — verified against xunit.v3 by probe before this rule was written. +* A draw from an isolated `Any.WithSeed(...)` context: that context is unaffected by the ambient scope by design, so reporting it would be wrong. +* A generator reached through a local, a field or a parameter rather than written inline from `Any`. The rule only reports a chain it can prove starts at the ambient source, which under-reports rather than misfiring. +* An `IClassFixture` constructor. The fixture type does not itself carry `[Reproducible]`, so the rule cannot see the link; the draw is equally unpinned, and the help page is the only warning you get. + +--- + +[← All analyzer rules](README.md) diff --git a/doc/handwritten/for-users/analyzers/JD007.fr.md b/doc/handwritten/for-users/analyzers/JD007.fr.md new file mode 100644 index 00000000..34d5d218 --- /dev/null +++ b/doc/handwritten/for-users/analyzers/JD007.fr.md @@ -0,0 +1,58 @@ +# JD007 : DrawOutsideThePinnedScope + +🌍 **Langues :** +🇫🇷 Français (ce fichier) | 🇬🇧 [English](./JD007.en.md) + +| | | +|---|---| +| **Catégorie** | Reproductibilité (`JustDummies.Reproducibility`) | +| **Sévérité** | 🟠 Avertissement | +| **Activée par défaut** | Oui | + +`[Reproducible]` épingle la graine depuis un hook *before/after* d'xUnit. Or xUnit construit l'instance de la classe de test — et attend `IAsyncLifetime.InitializeAsync` — **avant** d'exécuter ces hooks : tout ce qui est tiré pendant la construction provient donc de la source ambiante non ensemencée. + +Le résultat est pire qu'une absence de reproductibilité : le test l'annonce. L'échec affiche « *Reproduce this run with `[Reproducible(Seed = 1234)]`* », le lecteur épingle 1234, l'arrangement diffère quand même, l'échec ne revient pas, et la graine paraît cassée. + +Tirez dans le corps du test, ou conservez le **générateur** dans le champ et matérialisez-le à chaque test. + +## Non conforme + +```csharp +[Reproducible] +public class OrderTests { + private readonly string _reference; + + public OrderTests() { + _reference = Any.String().StartingWith("ORD-").Generate(); // JD007 : tiré avant l'épinglage + } + + [Fact] + public void It_is_accepted() { /* utilise _reference */ } +} +``` + +## Conforme + +```csharp +[Reproducible] +public class OrderTests { + // La recette est partageable sans risque : la source aléatoire est résolue à Generate(), pas à la construction. + private readonly IAny _reference = Any.String().StartingWith("ORD-"); + + [Fact] + public void It_is_accepted() { + string reference = _reference.Generate(); // tiré dans la portée épinglée + } +} +``` + +## Ce qui n'est pas signalé + +* Un tirage dans le **corps du test**, qui est bien dans la portée — vérifié par sonde contre xunit.v3 avant l'écriture de cette règle. +* Un tirage depuis un contexte isolé `Any.WithSeed(...)` : ce contexte échappe par conception à la portée ambiante, le signaler serait faux. +* Un générateur atteint par une variable locale, un champ ou un paramètre plutôt qu'écrit en ligne depuis `Any`. La règle ne signale qu'une chaîne dont elle peut prouver qu'elle part de la source ambiante : elle sous-signale plutôt que de se tromper. +* Un constructeur d'`IClassFixture`. Le type de fixture ne porte pas lui-même `[Reproducible]`, donc la règle ne peut pas voir le lien ; le tirage y est tout aussi non épinglé, et cette page est le seul avertissement dont vous disposez. + +--- + +[← Toutes les règles d'analyse](README.fr.md) diff --git a/doc/handwritten/for-users/analyzers/JD008.en.md b/doc/handwritten/for-users/analyzers/JD008.en.md new file mode 100644 index 00000000..327685b7 --- /dev/null +++ b/doc/handwritten/for-users/analyzers/JD008.en.md @@ -0,0 +1,60 @@ +# JD008: ArbitraryValueInTheoryData + +🌍 **Languages:** +🇬🇧 English (this file) | 🇫🇷 [Français](./JD008.fr.md) + +| | | +|---|---| +| **Category** | Reproducibility (`JustDummies.Reproducibility`) | +| **Severity** | 🟠 Warning | +| **Enabled by default** | Yes | + +xUnit evaluates a theory's data provider at **discovery**, before any test case runs. A value drawn there is therefore drawn once for the whole run, outside every seed scope — `Any.Reproducibly` and `[Reproducible]` alike. + +Three defects ride on that one shape, and all three are silent: + +* every case of the theory receives the **same** value, so a theory that reads as if it enumerated arbitrary cases enumerates a constant; +* the value is replayable from no reported seed, because no seed was pinned when it was drawn; +* the draw happens even for cases that are filtered out, and its position in the sequence shifts when cases are added or removed. + +Draw in the test body, or let the provider yield the **generator** and materialize it inside the test. + +## Noncompliant + +```csharp +public static TheoryData Cases => new() { + Any.String().NonEmpty().Generate(), // JD008: drawn at discovery, shared by every case +}; + +[Theory] +[MemberData(nameof(Cases))] +public void It_is_accepted(string reference) { /* ... */ } +``` + +## Compliant + +```csharp +public static TheoryData> Cases => new() { + Any.String().NonEmpty(), // the recipe travels, not the value +}; + +[Theory, Reproducible] +[MemberData(nameof(Cases))] +public void It_is_accepted(IAny reference) { + string value = reference.Generate(); // drawn per case, inside the pinned scope +} +``` + +## What it recognises as a provider + +A member named by a `[MemberData]` in the same type; a member returning `TheoryData` or `TheoryData<...>`; a member returning a sequence of `object[]`; and a type implementing that sequence, which is the `[ClassData]` shape. + +## What it does not flag + +* A provider that yields generators rather than values — the compliant shape above. +* A draw in an ordinary test body, which is the point of the rule. +* A draw from an isolated `Any.WithSeed(...)` context, or a generator reached through a local or field rather than written inline from `Any`. + +--- + +[← All analyzer rules](README.md) diff --git a/doc/handwritten/for-users/analyzers/JD008.fr.md b/doc/handwritten/for-users/analyzers/JD008.fr.md new file mode 100644 index 00000000..61682c48 --- /dev/null +++ b/doc/handwritten/for-users/analyzers/JD008.fr.md @@ -0,0 +1,60 @@ +# JD008 : ArbitraryValueInTheoryData + +🌍 **Langues :** +🇫🇷 Français (ce fichier) | 🇬🇧 [English](./JD008.en.md) + +| | | +|---|---| +| **Catégorie** | Reproductibilité (`JustDummies.Reproducibility`) | +| **Sévérité** | 🟠 Avertissement | +| **Activée par défaut** | Oui | + +xUnit évalue le fournisseur de données d'une théorie à la **découverte**, avant l'exécution du moindre cas de test. Une valeur tirée là l'est donc une seule fois pour toute la campagne, hors de toute portée de graine — aussi bien `Any.Reproducibly` que `[Reproducible]`. + +Trois défauts tiennent dans cette seule forme, et les trois sont silencieux : + +* tous les cas de la théorie reçoivent la **même** valeur : une théorie qui se lit comme énumérant des cas arbitraires énumère une constante ; +* la valeur n'est rejouable depuis aucune graine rapportée, puisque aucune n'était épinglée au moment du tirage ; +* le tirage a lieu même pour les cas filtrés, et sa position dans la séquence se décale dès qu'on ajoute ou retire un cas. + +Tirez dans le corps du test, ou faites livrer le **générateur** par le fournisseur et matérialisez-le dans le test. + +## Non conforme + +```csharp +public static TheoryData Cases => new() { + Any.String().NonEmpty().Generate(), // JD008 : tiré à la découverte, partagé par tous les cas +}; + +[Theory] +[MemberData(nameof(Cases))] +public void It_is_accepted(string reference) { /* ... */ } +``` + +## Conforme + +```csharp +public static TheoryData> Cases => new() { + Any.String().NonEmpty(), // c'est la recette qui voyage, pas la valeur +}; + +[Theory, Reproducible] +[MemberData(nameof(Cases))] +public void It_is_accepted(IAny reference) { + string value = reference.Generate(); // tiré par cas, dans la portée épinglée +} +``` + +## Ce qu'elle reconnaît comme fournisseur + +Un membre nommé par un `[MemberData]` du même type ; un membre retournant `TheoryData` ou `TheoryData<...>` ; un membre retournant une séquence d'`object[]` ; et un type implémentant cette séquence, ce qui est la forme `[ClassData]`. + +## Ce qui n'est pas signalé + +* Un fournisseur qui livre des générateurs plutôt que des valeurs — la forme conforme ci-dessus. +* Un tirage dans un corps de test ordinaire, ce qui est précisément l'objectif de la règle. +* Un tirage depuis un contexte isolé `Any.WithSeed(...)`, ou un générateur atteint par une variable locale ou un champ plutôt qu'écrit en ligne depuis `Any`. + +--- + +[← Toutes les règles d'analyse](README.fr.md) diff --git a/doc/handwritten/for-users/analyzers/JD009.en.md b/doc/handwritten/for-users/analyzers/JD009.en.md new file mode 100644 index 00000000..dc733ebe --- /dev/null +++ b/doc/handwritten/for-users/analyzers/JD009.en.md @@ -0,0 +1,47 @@ +# JD009: DrawInStaticInitializer + +🌍 **Languages:** +🇬🇧 English (this file) | 🇫🇷 [Français](./JD009.fr.md) + +| | | +|---|---| +| **Category** | Reproducibility (`JustDummies.Reproducibility`) | +| **Severity** | 🟠 Warning | +| **Enabled by default** | Yes | + +A type initializer runs **once, lazily**, when the first test touches the type. A value drawn there is therefore drawn under whatever ambient context that particular test happened to have pinned — and then shared, unchanged, by every other test in the class. + +Three consequences follow, none of them visible: + +* the value never varies between runs, so the arbitrary-by-default property that surfaces a test secretly depending on one particular value is switched off; +* the tests become **order-dependent** — a test can pass because the value was drawn under a sibling's seed, and start failing when the suite is reordered or filtered; +* no reported seed replays it, because the draw belongs to whichever test ran first, not to the one that failed. + +Store the **generator** in the static field and call `Generate()` where the value is needed. The random source is resolved at `Generate()` time, never at construction, so a shared generator is safe and idiomatic — only a shared *value* is not. + +## Noncompliant + +```csharp +private static readonly string Tenant = Any.String().NonEmpty().Generate(); // JD009: one draw for the whole suite +``` + +## Compliant + +```csharp +private static readonly IAny Tenant = Any.String().NonEmpty(); + +// ... per test: +string tenant = Tenant.Generate(); +``` + +## What it does not flag + +* A static field holding the **generator** — the compliant shape, and explicitly safe. +* A draw in an instance field initializer or constructor: that is [JD007](JD007.en.md)'s subject when the class is `[Reproducible]`, and outside its scope otherwise. +* A draw from an isolated `Any.WithSeed(...)` context, or a generator reached through a local or field rather than written inline from `Any`. + +A deliberately process-wide constant — a tenant id no test is coupled to — is a legitimate reading of the noncompliant shape. The hazard analysis still holds, so the rule reports it; suppress it at the site if that is genuinely what you want. + +--- + +[← All analyzer rules](README.md) diff --git a/doc/handwritten/for-users/analyzers/JD009.fr.md b/doc/handwritten/for-users/analyzers/JD009.fr.md new file mode 100644 index 00000000..94a86c62 --- /dev/null +++ b/doc/handwritten/for-users/analyzers/JD009.fr.md @@ -0,0 +1,47 @@ +# JD009 : DrawInStaticInitializer + +🌍 **Langues :** +🇫🇷 Français (ce fichier) | 🇬🇧 [English](./JD009.en.md) + +| | | +|---|---| +| **Catégorie** | Reproductibilité (`JustDummies.Reproducibility`) | +| **Sévérité** | 🟠 Avertissement | +| **Activée par défaut** | Oui | + +Un initialiseur de type s'exécute **une fois, paresseusement**, quand le premier test touche le type. Une valeur tirée là l'est donc sous le contexte ambiant que ce test-là se trouvait avoir épinglé — puis elle est partagée, inchangée, par tous les autres tests de la classe. + +Trois conséquences en découlent, aucune visible : + +* la valeur ne varie plus d'une exécution à l'autre : la propriété « arbitraire par défaut », celle qui révèle un test dépendant secrètement d'une valeur particulière, est désactivée ; +* les tests deviennent **dépendants de l'ordre** — un test peut passer parce que la valeur a été tirée sous la graine d'un voisin, et se mettre à échouer dès qu'on réordonne ou filtre la suite ; +* aucune graine rapportée ne la rejoue, le tirage appartenant au test qui s'est exécuté en premier et non à celui qui a échoué. + +Stockez le **générateur** dans le champ statique et appelez `Generate()` là où la valeur est nécessaire. La source aléatoire est résolue au moment de `Generate()`, jamais à la construction : un générateur partagé est donc sûr et idiomatique — seule une *valeur* partagée ne l'est pas. + +## Non conforme + +```csharp +private static readonly string Tenant = Any.String().NonEmpty().Generate(); // JD009 : un seul tirage pour toute la suite +``` + +## Conforme + +```csharp +private static readonly IAny Tenant = Any.String().NonEmpty(); + +// ... dans chaque test : +string tenant = Tenant.Generate(); +``` + +## Ce qui n'est pas signalé + +* Un champ statique portant le **générateur** — la forme conforme, explicitement sûre. +* Un tirage dans un initialiseur de champ d'instance ou un constructeur : c'est le sujet de [JD007](JD007.fr.md) quand la classe est `[Reproducible]`, et hors de sa portée sinon. +* Un tirage depuis un contexte isolé `Any.WithSeed(...)`, ou un générateur atteint par une variable locale ou un champ plutôt qu'écrit en ligne depuis `Any`. + +Une constante délibérément valable pour tout le processus — un identifiant de locataire auquel aucun test n'est couplé — est une lecture légitime de la forme non conforme. L'analyse du risque tient toujours, donc la règle la signale ; supprimez-la localement si c'est réellement ce que vous voulez. + +--- + +[← Toutes les règles d'analyse](README.fr.md) diff --git a/doc/handwritten/for-users/analyzers/JD010.en.md b/doc/handwritten/for-users/analyzers/JD010.en.md new file mode 100644 index 00000000..fa0f70d5 --- /dev/null +++ b/doc/handwritten/for-users/analyzers/JD010.en.md @@ -0,0 +1,49 @@ +# JD010: ReproducibleOnNonTestMethod + +🌍 **Languages:** +🇬🇧 English (this file) | 🇫🇷 [Français](./JD010.fr.md) + +| | | +|---|---| +| **Category** | Reproducibility (`JustDummies.Reproducibility`) | +| **Severity** | 🟠 Warning | +| **Enabled by default** | Yes | + +xUnit collects `BeforeAfterTestAttribute`s from the **test method**, its declaring **class** and the **assembly** — nowhere else. `[Reproducible]` on a helper method, on an arrange method, or on a method whose `[Fact]` was removed during a refactor, is never read: it pins no seed and reports none. + +What makes this worth a diagnostic is that a *working* `[Reproducible]` is silent by design — a passing test reports nothing. The inert form and the working form are therefore indistinguishable from the outside, right up until a failure that should have named a seed does not. + +Remove it, or move it to the level xUnit actually reads. + +## Noncompliant + +```csharp +public class OrderTests { + [Reproducible] // JD010: never read — this is not a test + private string ArrangeReference() => Any.String().StartingWith("ORD-").Generate(); + + [Fact] + public void It_is_accepted() { /* uses ArrangeReference() */ } +} +``` + +## Compliant + +```csharp +[Reproducible] // the class level covers every test it declares +public class OrderTests { + private string ArrangeReference() => Any.String().StartingWith("ORD-").Generate(); + + [Fact] + public void It_is_accepted() { /* ... */ } +} +``` + +## What it does not flag + +* The attribute on a `[Fact]`, a `[Theory]`, or any third-party attribute implementing xUnit's `IFactAttribute` — the rule keys on that interface, not on a fixed list. +* The attribute on a class or on the assembly, which are exactly the levels xUnit does collect. + +--- + +[← All analyzer rules](README.md) diff --git a/doc/handwritten/for-users/analyzers/JD010.fr.md b/doc/handwritten/for-users/analyzers/JD010.fr.md new file mode 100644 index 00000000..324087d1 --- /dev/null +++ b/doc/handwritten/for-users/analyzers/JD010.fr.md @@ -0,0 +1,49 @@ +# JD010 : ReproducibleOnNonTestMethod + +🌍 **Langues :** +🇫🇷 Français (ce fichier) | 🇬🇧 [English](./JD010.en.md) + +| | | +|---|---| +| **Catégorie** | Reproductibilité (`JustDummies.Reproducibility`) | +| **Sévérité** | 🟠 Avertissement | +| **Activée par défaut** | Oui | + +xUnit collecte les `BeforeAfterTestAttribute` depuis la **méthode de test**, sa **classe** déclarante et l'**assembly** — nulle part ailleurs. `[Reproducible]` sur une méthode utilitaire, sur une méthode d'arrangement, ou sur une méthode dont le `[Fact]` a disparu lors d'un remaniement, n'est jamais lu : il n'épingle aucune graine et n'en rapporte aucune. + +Ce qui justifie un diagnostic, c'est qu'un `[Reproducible]` *qui fonctionne* est silencieux par conception — un test qui passe ne rapporte rien. La forme inerte et la forme active sont donc indiscernables de l'extérieur, jusqu'au jour où un échec qui aurait dû nommer une graine ne le fait pas. + +Retirez-le, ou déplacez-le au niveau qu'xUnit lit réellement. + +## Non conforme + +```csharp +public class OrderTests { + [Reproducible] // JD010 : jamais lu — ce n'est pas un test + private string ArrangeReference() => Any.String().StartingWith("ORD-").Generate(); + + [Fact] + public void It_is_accepted() { /* utilise ArrangeReference() */ } +} +``` + +## Conforme + +```csharp +[Reproducible] // le niveau classe couvre tous les tests qu'elle déclare +public class OrderTests { + private string ArrangeReference() => Any.String().StartingWith("ORD-").Generate(); + + [Fact] + public void It_is_accepted() { /* ... */ } +} +``` + +## Ce qui n'est pas signalé + +* L'attribut sur un `[Fact]`, une `[Theory]`, ou tout attribut tiers implémentant l'`IFactAttribute` d'xUnit — la règle s'appuie sur cette interface, pas sur une liste figée. +* L'attribut sur une classe ou sur l'assembly, qui sont précisément les niveaux qu'xUnit collecte. + +--- + +[← Toutes les règles d'analyse](README.fr.md) diff --git a/doc/handwritten/for-users/analyzers/README.fr.md b/doc/handwritten/for-users/analyzers/README.fr.md index 96d44047..93aaafdb 100644 --- a/doc/handwritten/for-users/analyzers/README.fr.md +++ b/doc/handwritten/for-users/analyzers/README.fr.md @@ -59,6 +59,10 @@ Ces règles sont incluses dans le package **`JustDummies`** (pas FirstClassError | [JD002 DiscardedReproduciblyAsyncResult](JD002.fr.md) | 🔴 Erreur | on | Le `Task` retourné par `Any.ReproduciblyAsync` est jeté (instruction isolée ou `_ =`) ; les échecs du corps sont perdus. Faites `await`. | | [JD003 AwaitableBodyPassedToReproducibly](JD003.fr.md) | 🔴 Erreur | on | Une lambda synchrone dont le corps abandonne une tâche, ou un groupe de méthodes `async void`, atteint `Any.Reproducibly` ; la portée retourne avant l'exécution des assertions, et `CS4014` ne se déclenche pas. | | [JD004 DiscardedSeedingResult](JD004.fr.md) | 🔴 Erreur | on | La poignée retournée par `Any.UseSeed` est jetée, laissant la graine épinglée pour la suite — ou `Any.WithSeed` est appelé pour son effet, alors qu'il n'épingle rien. | +| [JD007 DrawOutsideThePinnedScope](JD007.fr.md) | 🟠 Avertissement | on | Une valeur est tirée pendant la construction d'une classe de test `[Reproducible]`, qu'xUnit exécute avant l'ouverture de la portée de graine ; la graine rapportée ne la rejoue pas. | +| [JD008 ArbitraryValueInTheoryData](JD008.fr.md) | 🟠 Avertissement | on | Le fournisseur de données d'une théorie tire une valeur à la découverte, avant tout épinglage ; tous les cas partagent cette unique valeur. | +| [JD009 DrawInStaticInitializer](JD009.fr.md) | 🟠 Avertissement | on | Un initialiseur statique tire une seule fois pour toute la suite, sous le premier test exécuté, rendant les tests dépendants de l'ordre et rejouables depuis aucune graine. | +| [JD010 ReproducibleOnNonTestMethod](JD010.fr.md) | 🟠 Avertissement | on | `[Reproducible]` sur une méthode qu'xUnit ne traite jamais comme un test ; il n'épingle rien, et ressemble exactement à la forme active. | ## JustDummies — Usage diff --git a/doc/handwritten/for-users/analyzers/README.md b/doc/handwritten/for-users/analyzers/README.md index b12be239..e7f3c08a 100644 --- a/doc/handwritten/for-users/analyzers/README.md +++ b/doc/handwritten/for-users/analyzers/README.md @@ -59,6 +59,10 @@ These rules ship in the **`JustDummies`** package (not FirstClassErrors) and kee | [JD002 DiscardedReproduciblyAsyncResult](JD002.en.md) | 🔴 Error | on | The task returned by Any.ReproduciblyAsync is discarded (a bare statement, or `_ =`); the body's failures are lost. Await it. | | [JD003 AwaitableBodyPassedToReproducibly](JD003.en.md) | 🔴 Error | on | A synchronous lambda whose body drops a task, or an async void method group, reaches Any.Reproducibly; the scope returns before the assertions run, and CS4014 does not fire. | | [JD004 DiscardedSeedingResult](JD004.en.md) | 🔴 Error | on | The handle returned by Any.UseSeed is discarded, leaving the seed pinned for whatever runs next — or Any.WithSeed is called for effect, which pins nothing at all. | +| [JD007 DrawOutsideThePinnedScope](JD007.en.md) | 🟠 Warning | on | A value is drawn during a [Reproducible] test class's construction, which xUnit runs before the seed scope opens; the reported seed does not replay it. | +| [JD008 ArbitraryValueInTheoryData](JD008.en.md) | 🟠 Warning | on | A theory's data provider draws a value at discovery, before any seed is pinned; every case shares the one value. | +| [JD009 DrawInStaticInitializer](JD009.en.md) | 🟠 Warning | on | A static initializer draws once for the whole suite, under whichever test ran first, making the tests order-dependent and replayable from no seed. | +| [JD010 ReproducibleOnNonTestMethod](JD010.en.md) | 🟠 Warning | on | [Reproducible] on a method xUnit never treats as a test; it pins nothing, and looks exactly like the working form. | ## JustDummies — Usage