diff --git a/.editorconfig b/.editorconfig index aa93d669..8577385f 100644 --- a/.editorconfig +++ b/.editorconfig @@ -1,4 +1,5 @@ -# EditorConfig — whitespace hygiene, plus the one style rule the compiler can enforce. +# EditorConfig — whitespace hygiene, the one style rule the compiler can enforce, and the +# analyzer rules this repository declines. # # FirstClassErrors.sln.DotSettings (ReSharper/Rider) remains the source of truth for # code style, naming and inspection severities. It is read by Rider and by nothing else: @@ -17,6 +18,12 @@ # Note that the DotSettings sets EnableEditorConfigSupport=False, so Rider ignores this # file entirely — each tool reads its own side of the pair. # +# The file also carries the analyzer rules the repository declines, each next to the reason +# it is declined (decision: ADR-0060). Those are not style preferences mirrored from the +# DotSettings but generic Roslyn/analyzer advice that this codebase deliberately does not +# follow; they are switched off here so the compiler stops emitting them, which is also what +# removes them from the SonarQube Cloud report — the scanner reports what the build emits. +# # charset is deliberately left unset: the repository mixes BOM and BOM-less files, # and pinning it here would rewrite existing files. @@ -41,6 +48,28 @@ csharp_style_var_when_type_is_apparent = false csharp_style_var_elsewhere = false dotnet_diagnostic.IDE0008.severity = warning +# Declined: concrete return types for performance. CA1859 asks that non-public members typed +# `IReadOnlyList` / `IEnumerable` be retyped to the concrete collection they happen to +# return, trading an interface dispatch for a direct call. In this codebase the interface IS +# the contract — "read this, do not mutate it" — and the rule only ever fires on non-public +# members, so its whole domain is plumbing where the abstraction was chosen on purpose. +# Honouring it would hand `.Add()` to every caller to buy nanoseconds on error-message and +# test-generator paths. Same trade the repository already refused when it kept its value +# objects as validating classes rather than structs. +dotnet_diagnostic.CA1859.severity = none + +# Test projects only. `*Tests` matches the thirteen test projects and no shipping one — +# FirstClassErrors.Testing ends in `Testing`, so the rule below does not reach it. +[*Tests/**.cs] +# Declined in tests: hoisting constant array arguments. CA1861 asks that a literal array +# passed to a method — the expected values of an assertion, the cases of a generator — be +# lifted into a static readonly field so it is allocated once instead of per call. In a test +# the literal sitting next to its assertion IS the statement being made; hoisting it moves the +# expected values away from the check that reads them, to save an allocation that happens a +# few hundred times in a suite. The rule stays ON for shipping code, where a hot path can +# genuinely want it — which is why this is scoped here rather than switched off repository-wide. +dotnet_diagnostic.CA1861.severity = none + [*.{csproj,props,targets,nuspec,config,xml}] indent_size = 2 diff --git a/.github/workflows/justdummies-mutation.yml b/.github/workflows/justdummies-mutation.yml index f2f84bb7..37970fb4 100644 --- a/.github/workflows/justdummies-mutation.yml +++ b/.github/workflows/justdummies-mutation.yml @@ -25,11 +25,11 @@ concurrency: group: justdummies-mutation-${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true -# Least-privilege FLOOR for the jobs below: this workflow only checks out, builds and runs tests, so -# the token needs nothing beyond read access to the repository contents. A job that needs LESS narrows -# it itself — see the advisory `gate`, which checks nothing out and therefore declares no scope at all. -permissions: - contents: read +# Least privilege is declared PER JOB below rather than here. A workflow-level grant reaches every +# job, including any added later that does not need it, so each job states the narrowest scope it can: +# `changed` and `full` check the repository out and take `contents: read`, the advisory `gate` checks +# nothing out and declares none at all (Sonar githubactions:S8264). A job added here MUST carry its own +# `permissions` block — with none it inherits the repository default instead of a floor set here. env: DOTNET_NOLOGO: 'true' @@ -47,6 +47,9 @@ jobs: name: Mutate the diff (${{ matrix.name }}) if: github.event_name == 'pull_request' runs-on: ubuntu-latest + # Checks the repository out, builds and runs tests; it calls no API and writes nothing back. + permissions: + contents: read # Both remaining legs finish in about ninety seconds, touched or untouched. The cap is a runaway # guard, not a budget: the leg that needed one — the generator's — no longer runs here (ADR-0049). timeout-minutes: 20 @@ -174,13 +177,13 @@ jobs: needs: changed runs-on: ubuntu-latest timeout-minutes: 5 - # Narrower than the workflow-level floor, not wider: this job checks nothing out and calls no API. + # The narrowest of the three: this job checks nothing out and calls no API. # `needs.changed.result` is substituted by GitHub before the shell runs, and `::warning::` is a - # runner workflow command written to stdout, not a REST call. A job-level block REPLACES the - # inherited value rather than merging with it, so `{}` — GitHub's explicit "no scopes" form — leaves - # this job's token with nothing at all (Sonar githubactions:S8264). It must stay the explicit flow - # mapping: a bare `permissions:` is a null, not an empty map. Widen it if a checkout or a `gh` call - # is ever added here, or that step will fail on a 403. + # runner workflow command written to stdout, not a REST call. A job-level block REPLACES whatever + # the job would otherwise inherit rather than merging with it, so `{}` — GitHub's explicit "no + # scopes" form — leaves this job's token with nothing at all (Sonar githubactions:S8264). It must + # stay the explicit flow mapping: a bare `permissions:` is a null, not an empty map. Widen it if a + # checkout or a `gh` call is ever added here, or that step will fail on a 403. permissions: {} steps: - name: Report the mutation legs @@ -202,6 +205,9 @@ jobs: name: Full sweep (${{ matrix.name }}) if: github.event_name != 'pull_request' runs-on: ubuntu-latest + # Same scope as `changed`: checkout, build, test — nothing written back. + permissions: + contents: read # The sweep runs the library's whole test suite once per mutant, and `JustDummies` carries a few # thousand of them — this is the longest job in the repository, and the reason the sweep is # weekly and the gate is diff-scoped. Measured locally on four cores, it runs well past an hour; diff --git a/.github/workflows/mutation.yml b/.github/workflows/mutation.yml index 99163938..c5f01b18 100644 --- a/.github/workflows/mutation.yml +++ b/.github/workflows/mutation.yml @@ -19,11 +19,11 @@ concurrency: group: mutation-${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true -# Least-privilege FLOOR for the jobs below: this workflow only checks out, builds and runs tests, so -# the token needs nothing beyond read access to the repository contents. A job that needs LESS narrows -# it itself — see the advisory `gate`, which checks nothing out and therefore declares no scope at all. -permissions: - contents: read +# Least privilege is declared PER JOB below rather than here. A workflow-level grant reaches every +# job, including any added later that does not need it, so each job states the narrowest scope it can: +# `changed` and `full` check the repository out and take `contents: read`, the advisory `gate` checks +# nothing out and declares none at all (Sonar githubactions:S8264). A job added here MUST carry its own +# `permissions` block — with none it inherits the repository default instead of a floor set here. env: DOTNET_NOLOGO: 'true' @@ -39,6 +39,9 @@ jobs: name: Mutate the diff (${{ matrix.name }}) if: github.event_name == 'pull_request' runs-on: ubuntu-latest + # Checks the repository out, builds and runs tests; it calls no API and writes nothing back. + permissions: + contents: read # A leg whose library the pull request did not touch finishes in ~2 min (analysis, build, initial # test run, mutant generation). The cap covers the worst realistic case instead: a pull request # touching one of the large files — Stryker selects per changed FILE, so `Outcome.cs` alone puts @@ -173,13 +176,13 @@ jobs: needs: changed runs-on: ubuntu-latest timeout-minutes: 5 - # Narrower than the workflow-level floor, not wider: this job checks nothing out and calls no API. + # The narrowest of the three: this job checks nothing out and calls no API. # `needs.changed.result` is substituted by GitHub before the shell runs, and `::warning::` is a - # runner workflow command written to stdout, not a REST call. A job-level block REPLACES the - # inherited value rather than merging with it, so `{}` — GitHub's explicit "no scopes" form — leaves - # this job's token with nothing at all (Sonar githubactions:S8264). It must stay the explicit flow - # mapping: a bare `permissions:` is a null, not an empty map. Widen it if a checkout or a `gh` call - # is ever added here, or that step will fail on a 403. + # runner workflow command written to stdout, not a REST call. A job-level block REPLACES whatever + # the job would otherwise inherit rather than merging with it, so `{}` — GitHub's explicit "no + # scopes" form — leaves this job's token with nothing at all (Sonar githubactions:S8264). It must + # stay the explicit flow mapping: a bare `permissions:` is a null, not an empty map. Widen it if a + # checkout or a `gh` call is ever added here, or that step will fail on a 403. permissions: {} steps: - name: Report the mutation legs @@ -201,6 +204,9 @@ jobs: name: Full sweep (${{ matrix.name }}) if: github.event_name != 'pull_request' runs-on: ubuntu-latest + # Same scope as `changed`: checkout, build, test — nothing written back. + permissions: + contents: read # The sweep runs the project's whole test suite once per mutant, and the six projects together # carry several thousand — the tooling's suites being the slow ones, since they drive Roslyn # compilations and snapshot comparisons. That is the reason the sweep is weekly and the gate is diff --git a/.github/workflows/sonar.yml b/.github/workflows/sonar.yml index 88199a22..f09271f2 100644 --- a/.github/workflows/sonar.yml +++ b/.github/workflows/sonar.yml @@ -72,6 +72,18 @@ jobs: # The Benchmarks project is a measurement harness, never shipped and never unit-tested: excluding it from # COVERAGE (not from analysis) keeps the new-code coverage gate about the library, while its code still # gets the SonarAnalyzer pass. + # + # Two shell rules are declined for every *.sh in the repository (policy: ADR-0060). They are ignored HERE, + # rather than in .editorconfig like the declined Roslyn rules, because they are SonarQube's own analysis + # rather than compiler diagnostics the scanner republishes — nothing the build does can silence them. + # S7682 ("add an explicit return at the end of the function") — every function it flags ends with the + # command whose status IS the function's result: a `cat` heredoc, an `awk` invocation, a `printf`, or + # in one case an `exit`, after which a return is unreachable. `return 0` would mask those failures and + # a bare `return` restates the default, so the rule can only make the scripts worse or longer. + # S7679 ("assign this positional parameter to a local variable") — every script here is `#!/bin/sh`, and + # `local` is not POSIX. tools/trains.sh already shows the price of obeying without it: the one helper + # that needed named parameters carries `_tf_`-prefixed globals instead. Paying that in every two-line + # helper, to name a `$1` sitting one line below the function's own name, buys nothing. - name: Begin analysis env: SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} @@ -84,6 +96,11 @@ jobs: /d:sonar.cs.opencover.reportsPaths="artifacts/coverage/**/coverage.opencover.xml" /d:sonar.exclusions="**/*.verified.*" /d:sonar.coverage.exclusions="FirstClassErrors.RequestBinder.Benchmarks/**" + /d:sonar.issue.ignore.multicriteria="s7682,s7679" + /d:sonar.issue.ignore.multicriteria.s7682.ruleKey="shelldre:S7682" + /d:sonar.issue.ignore.multicriteria.s7682.resourceKey="**/*.sh" + /d:sonar.issue.ignore.multicriteria.s7679.ruleKey="shelldre:S7679" + /d:sonar.issue.ignore.multicriteria.s7679.resourceKey="**/*.sh" - name: Build # Disable the CI warning ratchet (Directory.Build.props) for the analysis build only. The scanner diff --git a/FirstClassErrors.Analyzers.UnitTests/AnalyzerTestHarness.cs b/FirstClassErrors.Analyzers.UnitTests/AnalyzerTestHarness.cs index 3b61d767..ea29823c 100644 --- a/FirstClassErrors.Analyzers.UnitTests/AnalyzerTestHarness.cs +++ b/FirstClassErrors.Analyzers.UnitTests/AnalyzerTestHarness.cs @@ -71,7 +71,7 @@ private static async Task> RunAsync( } private static ImmutableArray BuildBaseReferences() { - List references = new(); + List references = []; // Reference the running runtime's assemblies so snippets resolve System types without pinning a ref pack. string trustedAssemblies = AppContext.GetData("TRUSTED_PLATFORM_ASSEMBLIES") as string ?? string.Empty; @@ -90,7 +90,7 @@ private static ImmutableArray BuildBaseReferences() { } private static ImmutableArray BuildNet472References() { - List references = new(); + List references = []; // The .NET Framework 4.7.2 reference assemblies (including the netstandard facade, so the netstandard2.0 core // resolves). The directory is baked in at build time via the Net472ReferenceAssemblies assembly metadata. diff --git a/FirstClassErrors.Analyzers/DuplicateDocumentedCodeAnalyzer.cs b/FirstClassErrors.Analyzers/DuplicateDocumentedCodeAnalyzer.cs index 54acda40..091b5956 100644 --- a/FirstClassErrors.Analyzers/DuplicateDocumentedCodeAnalyzer.cs +++ b/FirstClassErrors.Analyzers/DuplicateDocumentedCodeAnalyzer.cs @@ -52,7 +52,7 @@ private static void Collect( if (create is null) { return; } if (TryGetCodeField(create.Arguments[0].Value, out ISymbol? codeField)) { - usagesByCodeField.GetOrAdd(codeField!, _ => new ConcurrentBag()) + usagesByCodeField.GetOrAdd(codeField!, _ => []) .Add(method.Locations.FirstOrDefault() ?? Location.None); } } diff --git a/FirstClassErrors.Analyzers/DuplicateErrorCodeAnalyzer.cs b/FirstClassErrors.Analyzers/DuplicateErrorCodeAnalyzer.cs index abd76d84..3adfd15b 100644 --- a/FirstClassErrors.Analyzers/DuplicateErrorCodeAnalyzer.cs +++ b/FirstClassErrors.Analyzers/DuplicateErrorCodeAnalyzer.cs @@ -53,7 +53,7 @@ private static void Collect( // Non-literal codes are FCE003's concern, empty ones FCE002's; only real codes can collide. if (!ErrorCodeFacts.TryGetNonEmptyLiteralCode(argument, out string code)) { return; } - occurrences.GetOrAdd(code, _ => new ConcurrentBag()).Add(argument.Syntax.GetLocation()); + occurrences.GetOrAdd(code, _ => []).Add(argument.Syntax.GetLocation()); } private static void Report( diff --git a/FirstClassErrors.Analyzers/MultipleFactoriesShareDocumentationAnalyzer.cs b/FirstClassErrors.Analyzers/MultipleFactoriesShareDocumentationAnalyzer.cs index 193b8120..13d2857e 100644 --- a/FirstClassErrors.Analyzers/MultipleFactoriesShareDocumentationAnalyzer.cs +++ b/FirstClassErrors.Analyzers/MultipleFactoriesShareDocumentationAnalyzer.cs @@ -44,7 +44,7 @@ private static void Analyze(SymbolAnalysisContext context, KnownSymbols symbols) if (string.IsNullOrEmpty(targetName)) { continue; } if (!referencesByTarget.TryGetValue(targetName!, out List? references)) { - references = new List(); + references = []; referencesByTarget[targetName!] = references; } diff --git a/FirstClassErrors.Cli.UnitTests/TestDoubles.cs b/FirstClassErrors.Cli.UnitTests/TestDoubles.cs index 4846d926..ebcbaa0e 100644 --- a/FirstClassErrors.Cli.UnitTests/TestDoubles.cs +++ b/FirstClassErrors.Cli.UnitTests/TestDoubles.cs @@ -157,10 +157,10 @@ public CatalogSnapshot Extract(CatalogSettings settings, CliConfiguration config /// A logger that records each message per level, so tests can assert what the command reported. internal sealed class RecordingLogger : IGenerationLogger { - public List Infos { get; } = new(); - public List Warnings { get; } = new(); - public List Errors { get; } = new(); - public List Debugs { get; } = new(); + public List Infos { get; } = []; + public List Warnings { get; } = []; + public List Errors { get; } = []; + public List Debugs { get; } = []; public void Info(string message) { Infos.Add(message); } public void Warning(string message) { Warnings.Add(message); } @@ -172,8 +172,8 @@ internal sealed class RecordingLogger : IGenerationLogger { /// An output sink that records what would have been written, instead of touching the console or disk. internal sealed class RecordingOutputSink : IOutputSink { - public List StandardOutput { get; } = new(); - public Dictionary Files { get; } = new(); + public List StandardOutput { get; } = []; + public Dictionary Files { get; } = []; public void WriteStandardOutput(string content) { StandardOutput.Add(content); diff --git a/FirstClassErrors.Cli/CatalogDiffCommand.cs b/FirstClassErrors.Cli/CatalogDiffCommand.cs index 0e22c613..ccba5592 100644 --- a/FirstClassErrors.Cli/CatalogDiffCommand.cs +++ b/FirstClassErrors.Cli/CatalogDiffCommand.cs @@ -40,6 +40,14 @@ internal sealed class CatalogDiffSettings : CatalogSettings { /// internal sealed class CatalogDiffCommand : Command { + #region Constants + + // The default --fail-on policy, and the value the exit code and the error message are both decided against. + // Named because the three readings must agree: parse, decide, report. + private const string FailOnBreaking = "breaking"; + + #endregion + #region Fields private readonly ICatalogSnapshotSource _snapshotSource; @@ -79,8 +87,8 @@ internal int Run(CatalogDiffSettings settings, CancellationToken cancellationTok string configDir = Path.GetDirectoryName(configPath) ?? Directory.GetCurrentDirectory(); // Validate the policy and report options before the (expensive) extraction runs, so a typo fails fast. - string failOn = (settings.FailOn ?? "breaking").Trim().ToLowerInvariant(); - if (failOn is not ("breaking" or "any" or "none")) { + string failOn = (settings.FailOn ?? FailOnBreaking).Trim().ToLowerInvariant(); + if (failOn is not (FailOnBreaking or "any" or "none")) { logger.Error($"Unknown --fail-on value '{settings.FailOn}'. Use breaking, any or none."); return 1; @@ -114,13 +122,13 @@ internal int Run(CatalogDiffSettings settings, CancellationToken cancellationTok }); bool violated = failOn switch { - "breaking" => diff.HasChangesAtOrAbove(CatalogChangeImpact.Breaking), - "any" => !diff.IsEmpty, - _ => false + FailOnBreaking => diff.HasChangesAtOrAbove(CatalogChangeImpact.Breaking), + "any" => !diff.IsEmpty, + _ => false }; if (violated) { - logger.Error(failOn == "breaking" + logger.Error(failOn == FailOnBreaking ? $"The catalog has {diff.BreakingChanges.Count} breaking change(s) against the baseline. Fix them, or accept them deliberately with 'fce catalog update'." : $"The catalog has {diff.Changes.Count} change(s) against the baseline. Accept them with 'fce catalog update'."); diff --git a/FirstClassErrors.GenDoc.UnitTests/SolutionErrorDocumentationGeneratorTests.cs b/FirstClassErrors.GenDoc.UnitTests/SolutionErrorDocumentationGeneratorTests.cs index 380055ca..fb84e0af 100644 --- a/FirstClassErrors.GenDoc.UnitTests/SolutionErrorDocumentationGeneratorTests.cs +++ b/FirstClassErrors.GenDoc.UnitTests/SolutionErrorDocumentationGeneratorTests.cs @@ -131,7 +131,7 @@ public void ASkippedAssemblyIsLoggedWhenContinuing() { private sealed class RecordingGenerationLogger : IGenerationLogger { - public List Warnings { get; } = new(); + public List Warnings { get; } = []; public void Info(string message) { } public void Warning(string message) { Warnings.Add(message); } @@ -208,11 +208,11 @@ public void GenerationIsAbandonedWhenCancellationIsRequested() { public void CrossAssemblyDeduplicationWarnsAboutTheCollision() { // Setup: the same code declared by two different sources (i.e. two different assemblies' workers). RecordingLogger logger = new(); - List documentation = new() { + List documentation = [ new ErrorDocumentation { Code = "SHARED", Title = "From A", Source = "AssemblyA" }, new ErrorDocumentation { Code = "SHARED", Title = "From B", Source = "AssemblyB" }, new ErrorDocumentation { Code = "UNIQUE", Title = "Alone", Source = "AssemblyA" } - }; + ]; // Exercise IReadOnlyList catalog = @@ -232,10 +232,10 @@ public void CrossAssemblyDeduplicationWarnsAboutTheCollision() { public void CrossAssemblyDeduplicationStaysSilentWithoutDuplicates() { // Setup RecordingLogger logger = new(); - List documentation = new() { + List documentation = [ new ErrorDocumentation { Code = "A", Source = "AssemblyA" }, new ErrorDocumentation { Code = "B", Source = "AssemblyB" } - }; + ]; // Exercise IReadOnlyList catalog = @@ -633,7 +633,7 @@ private static string WriteTempProject(string body) { private sealed class RecordingLogger : IGenerationLogger { - public List Warnings { get; } = new(); + public List Warnings { get; } = []; public void Info(string message) { } public void Warning(string message) { Warnings.Add(message); } diff --git a/FirstClassErrors.GenDoc/DocumentationToolchainError.cs b/FirstClassErrors.GenDoc/DocumentationToolchainError.cs index 16aa6bf2..dc7cb2cd 100644 --- a/FirstClassErrors.GenDoc/DocumentationToolchainError.cs +++ b/FirstClassErrors.GenDoc/DocumentationToolchainError.cs @@ -22,6 +22,14 @@ namespace FirstClassErrors.GenDoc; Description = "The toolchain the documentation generator drives: the .NET SDK commands it spawns and the extraction worker process it launches per assembly.")] internal static class DocumentationToolchainError { + #region Constants + + // The assembly path shown in every worker-failure example. One value, so the generated documentation + // illustrates the four errors with the same subject rather than four paths that only look alike. + private const string ExampleAssemblyPath = "/src/app/bin/Release/net8.0/Application.dll"; + + #endregion + #region Statics members declarations /// 'dotnet sln list' failed, so the solution's projects could not be enumerated. @@ -251,7 +259,7 @@ private static ErrorDocumentation WorkerFailedDocumentation() { .AndDiagnostic("A documentation method or example factory of the target threw while the worker executed it.", ErrorOrigin.External, "Read the worker's error output; run the target's documentation methods in a unit test to reproduce.") - .WithExamples(() => WorkerFailed("/src/app/bin/Release/net8.0/Application.dll", 1, "Fatal error while extracting documentation.")); + .WithExamples(() => WorkerFailed(ExampleAssemblyPath, 1, "Fatal error while extracting documentation.")); } private static ErrorDocumentation WorkerOutputMissingDocumentation() { @@ -261,7 +269,7 @@ private static ErrorDocumentation WorkerOutputMissingDocumentation() { .WithDiagnostic("The temporary directory is not writable, or an antivirus or cleanup job removed the file between the worker's exit and its harvesting.", ErrorOrigin.External, "Check the permissions and free space of the temporary directory used by the generation.") - .WithExamples(() => WorkerOutputMissing("/src/app/bin/Release/net8.0/Application.dll")); + .WithExamples(() => WorkerOutputMissing(ExampleAssemblyPath)); } private static ErrorDocumentation WorkerOutputUnreadableDocumentation() { @@ -271,7 +279,7 @@ private static ErrorDocumentation WorkerOutputUnreadableDocumentation() { .WithDiagnostic("The worker and the generator come from different tool versions and no longer agree on the result format.", ErrorOrigin.Internal, "Check that the worker next to the tool belongs to the same installation; reinstall the tool if in doubt.") - .WithExamples(() => WorkerOutputUnreadable("/src/app/bin/Release/net8.0/Application.dll")); + .WithExamples(() => WorkerOutputUnreadable(ExampleAssemblyPath)); } private static ErrorDocumentation WorkerRunFailedDocumentation() { @@ -281,7 +289,7 @@ private static ErrorDocumentation WorkerRunFailedDocumentation() { .WithDiagnostic("A file-system or permission problem around the temporary result file or the worker binary.", ErrorOrigin.InternalOrExternal, "Read the inner exception attached to the failure; check the temporary directory and the tool's installation directory.") - .WithExamples(() => WorkerRunFailed("/src/app/bin/Release/net8.0/Application.dll")); + .WithExamples(() => WorkerRunFailed(ExampleAssemblyPath)); } #endregion diff --git a/FirstClassErrors.GenDoc/SolutionErrorDocumentationGenerator.cs b/FirstClassErrors.GenDoc/SolutionErrorDocumentationGenerator.cs index 2b6eaa76..e7804aa4 100644 --- a/FirstClassErrors.GenDoc/SolutionErrorDocumentationGenerator.cs +++ b/FirstClassErrors.GenDoc/SolutionErrorDocumentationGenerator.cs @@ -94,7 +94,7 @@ public static IEnumerable GetErrorDocumentationFromAssemblie options.Logger.Info($"Starting documentation generation from {assemblyPaths.Count} assembly path(s)."); - List resolved = new(); + List resolved = []; foreach (string assemblyPath in assemblyPaths) { string fullPath = Path.GetFullPath(assemblyPath); if (!File.Exists(fullPath)) { @@ -128,7 +128,7 @@ private static List ReadSolutionProjects(string solutionPath, SolutionGe : DocumentationToolchainError.ProjectEnumerationFailed(solutionPath, result.ExitCode, result.StandardError)); } - List projects = new(); + List projects = []; foreach (string rawLine in result.StandardOutput.Split(['\r', '\n'], StringSplitOptions.RemoveEmptyEntries)) { string line = rawLine.Trim(); @@ -147,9 +147,7 @@ private static List ReadSolutionProjects(string solutionPath, SolutionGe } internal static IReadOnlyList FilterProjects(IReadOnlyList projectPaths, SolutionGenerationOptions options) { - List included = new(); - - included.AddRange(projectPaths.Where(projectPath => ShouldIncludeProject(projectPath, options))); + List included = [.. projectPaths.Where(projectPath => ShouldIncludeProject(projectPath, options))]; // An opt-in declared only in a shared build file (Directory.Build.props, an import) reads as absent in every // .csproj — per project, that is indistinguishable from a genuine absence, so it cannot be diagnosed above. The @@ -491,7 +489,7 @@ private static bool IsTrue(string? value) { private static IEnumerable ExtractFromAssemblies(IReadOnlyList assemblyPaths, SolutionGenerationOptions options) { string workerAssemblyPath = ResolveWorkerAssemblyPath(options); - List results = new(); + List results = []; foreach (string assemblyPath in assemblyPaths) { // Stop launching new workers as soon as cancellation is requested; the running one (if any) is already @@ -516,7 +514,7 @@ private static IEnumerable ExtractFromAssemblies(IReadOnlyLi } internal static IReadOnlyList DeduplicateAcrossAssemblies(IReadOnlyList documentation, IGenerationLogger logger) { - List catalog = new(); + List catalog = []; foreach (IGrouping group in documentation.GroupBy(doc => doc.Code, StringComparer.OrdinalIgnoreCase)) { ErrorDocumentation survivor = group.First(); diff --git a/FirstClassErrors.GenDoc/Versioning/CatalogDiffer.cs b/FirstClassErrors.GenDoc/Versioning/CatalogDiffer.cs index 519e2803..7096db9d 100644 --- a/FirstClassErrors.GenDoc/Versioning/CatalogDiffer.cs +++ b/FirstClassErrors.GenDoc/Versioning/CatalogDiffer.cs @@ -78,7 +78,7 @@ private static Dictionary IndexByCode(CatalogSnaps if (string.IsNullOrWhiteSpace(entry.Code)) { continue; } string code = entry.Code.Trim(); - if (!byCode.ContainsKey(code)) { byCode.Add(code, entry); } + byCode.TryAdd(code, entry); } return byCode; @@ -125,7 +125,7 @@ private static Dictionary IndexByKey(CatalogS if (string.IsNullOrWhiteSpace(key.Key)) { continue; } string name = key.Key.Trim(); - if (!byKey.ContainsKey(name)) { byKey.Add(name, key); } + byKey.TryAdd(name, key); } return byKey; diff --git a/FirstClassErrors.RequestBinder.Benchmarks/BinderBenchmarks.cs b/FirstClassErrors.RequestBinder.Benchmarks/BinderBenchmarks.cs index fb92eb42..db233c05 100644 --- a/FirstClassErrors.RequestBinder.Benchmarks/BinderBenchmarks.cs +++ b/FirstClassErrors.RequestBinder.Benchmarks/BinderBenchmarks.cs @@ -25,6 +25,11 @@ namespace FirstClassErrors.RequestBinder.Benchmarks; [SimpleJob(RunStrategy.Throughput, launchCount: 1, warmupCount: 3, iterationCount: 7)] public class BinderBenchmarks { + // The two fixture values every payload is built from. Named so a benchmark comparing two shapes is + // demonstrably feeding them the same input: a drifting literal would silently compare different work. + private const string SampleEmail = "guest@example.org"; + private const string SampleDate = "2026-08-01"; + private BookingRequest _fullRequest = null!; private FiveScalarsDto _fiveScalars = null!; private FiveScalarsDto _fiveScalarsOneMissing = null!; @@ -45,37 +50,37 @@ public class BinderBenchmarks { [GlobalSetup] public void Setup() { _fullRequest = new BookingRequest { - GuestEmail = "guest@example.org", + GuestEmail = SampleEmail, Reference = "BK-2026-000123", Currency = "EUR", Nights = 3, MaxNights = 10, - Stay = new StayDto { CheckIn = "2026-08-01", CheckOut = "2026-08-04" }, - Tags = new List { "beach", "family", "late-checkout" }, - RoomNumbers = new List { 101, 102, 210 }, - Guests = new List { + Stay = new StayDto { CheckIn = SampleDate, CheckOut = "2026-08-04" }, + Tags = ["beach", "family", "late-checkout"], + RoomNumbers = [101, 102, 210], + Guests = [ new GuestDto { FirstName = "Ada", Email = "ada@example.org" }, new GuestDto { FirstName = "Blaise", Email = null }, - }, + ], }; _fiveScalars = new FiveScalarsDto { - First = "guest@example.org", + First = SampleEmail, Second = "BK-2026-000123", Third = "EUR", Fourth = "beach", - Fifth = "2026-08-01", + Fifth = SampleDate, }; _fiveScalarsOneMissing = new FiveScalarsDto { - First = "guest@example.org", + First = SampleEmail, Second = null, // the missing required argument Third = "EUR", Fourth = "beach", - Fifth = "2026-08-01", + Fifth = SampleDate, }; - _oneScalar = new OneScalarDto { First = "guest@example.org" }; + _oneScalar = new OneScalarDto { First = SampleEmail }; _oneNullableInt = new OneNullableIntDto { Count = 3 }; - _listOfTen = new ListOnlyDto { Items = new List { "a1", "b2", "c3", "d4", "e5", "f6", "g7", "h8", "i9", "j10" } }; - _stay = new StayDto { CheckIn = "2026-08-01", CheckOut = "2026-08-04" }; + _listOfTen = new ListOnlyDto { Items = ["a1", "b2", "c3", "d4", "e5", "f6", "g7", "h8", "i9", "j10"] }; + _stay = new StayDto { CheckIn = SampleDate, CheckOut = "2026-08-04" }; _routeBookingId = "bk_0123456789"; } diff --git a/FirstClassErrors.RequestBinder.Usage/Model/BookingDate.cs b/FirstClassErrors.RequestBinder.Usage/Model/BookingDate.cs index e8de8313..9b0f31c8 100644 --- a/FirstClassErrors.RequestBinder.Usage/Model/BookingDate.cs +++ b/FirstClassErrors.RequestBinder.Usage/Model/BookingDate.cs @@ -39,6 +39,75 @@ public static Outcome Parse(string raw) { #endregion + // A reference-type value object gets `==` from the language, comparing REFERENCES, while Equals compares + // values — the two would disagree silently on two instances of the same date. Declaring the operators is + // what keeps them saying the same thing. The ordering operators follow CompareTo, including its verdict + // that a null sorts before any date. + + /// + /// Determines whether two instances are equal. + /// + /// The first instance to compare. + /// The second instance to compare. + /// + /// true if the specified instances are equal; otherwise, false. + /// + public static bool operator ==(BookingDate? left, BookingDate? right) { + return Equals(left, right); + } + + /// + /// Determines whether two instances are not equal. + /// + /// The first instance to compare. + /// The second instance to compare. + /// + /// true if the specified instances are not equal; otherwise, false. + /// + public static bool operator !=(BookingDate? left, BookingDate? right) { + return !Equals(left, right); + } + + /// Determines whether falls strictly before . + /// The first instance to compare. + /// The second instance to compare. + /// true if is earlier; otherwise, false. + public static bool operator <(BookingDate? left, BookingDate? right) { + return Compare(left, right) < 0; + } + + /// Determines whether falls before or on the same day. + /// The first instance to compare. + /// The second instance to compare. + /// true if is earlier or equal; otherwise, false. + public static bool operator <=(BookingDate? left, BookingDate? right) { + return Compare(left, right) <= 0; + } + + /// Determines whether falls strictly after . + /// The first instance to compare. + /// The second instance to compare. + /// true if is later; otherwise, false. + public static bool operator >(BookingDate? left, BookingDate? right) { + return Compare(left, right) > 0; + } + + /// Determines whether falls after or on the same day. + /// The first instance to compare. + /// The second instance to compare. + /// true if is later or equal; otherwise, false. + public static bool operator >=(BookingDate? left, BookingDate? right) { + return Compare(left, right) >= 0; + } + + // The one place the ordering operators agree on how a null compares, so all four cannot drift apart. + private static int Compare(BookingDate? left, BookingDate? right) { + if (ReferenceEquals(left, right)) { return 0; } + if (left is null) { return -1; } + + return left.CompareTo(right); + } + /// public int CompareTo(BookingDate? other) { if (other is null) { return 1; } diff --git a/FirstClassErrors.RequestBinder.Usage/Model/NightCount.cs b/FirstClassErrors.RequestBinder.Usage/Model/NightCount.cs index a28821d7..78359b4e 100644 --- a/FirstClassErrors.RequestBinder.Usage/Model/NightCount.cs +++ b/FirstClassErrors.RequestBinder.Usage/Model/NightCount.cs @@ -38,6 +38,30 @@ public static Outcome From(int nights) { #endregion + /// + /// Determines whether two instances are equal. + /// + /// The first instance to compare. + /// The second instance to compare. + /// + /// true if the specified instances are equal; otherwise, false. + /// + public static bool operator ==(NightCount left, NightCount right) { + return left.Equals(right); + } + + /// + /// Determines whether two instances are not equal. + /// + /// The first instance to compare. + /// The second instance to compare. + /// + /// true if the specified instances are not equal; otherwise, false. + /// + public static bool operator !=(NightCount left, NightCount right) { + return !left.Equals(right); + } + /// public bool Equals(NightCount other) { return Value == other.Value; diff --git a/FirstClassErrors.RequestBinder.Usage/Model/RoomNumber.cs b/FirstClassErrors.RequestBinder.Usage/Model/RoomNumber.cs index 8be1fc02..6a93a5de 100644 --- a/FirstClassErrors.RequestBinder.Usage/Model/RoomNumber.cs +++ b/FirstClassErrors.RequestBinder.Usage/Model/RoomNumber.cs @@ -42,6 +42,30 @@ public static Outcome From(int number) { #endregion + /// + /// Determines whether two instances are equal. + /// + /// The first instance to compare. + /// The second instance to compare. + /// + /// true if the specified instances are equal; otherwise, false. + /// + public static bool operator ==(RoomNumber left, RoomNumber right) { + return left.Equals(right); + } + + /// + /// Determines whether two instances are not equal. + /// + /// The first instance to compare. + /// The second instance to compare. + /// + /// true if the specified instances are not equal; otherwise, false. + /// + public static bool operator !=(RoomNumber left, RoomNumber right) { + return !left.Equals(right); + } + /// public bool Equals(RoomNumber other) { return Value == other.Value; diff --git a/FirstClassErrors.RequestBinder/ListOfComplexPropertiesConverter.cs b/FirstClassErrors.RequestBinder/ListOfComplexPropertiesConverter.cs index 642feb77..9e27d0fe 100644 --- a/FirstClassErrors.RequestBinder/ListOfComplexPropertiesConverter.cs +++ b/FirstClassErrors.RequestBinder/ListOfComplexPropertiesConverter.cs @@ -73,7 +73,7 @@ public RequiredField> AsOptional(Func empty = new List(); + IReadOnlyList empty = []; return new RequiredField>(_binding, empty); } diff --git a/FirstClassErrors.RequestBinder/ListOfSimplePropertiesConverter.cs b/FirstClassErrors.RequestBinder/ListOfSimplePropertiesConverter.cs index c8f313da..c3362cc6 100644 --- a/FirstClassErrors.RequestBinder/ListOfSimplePropertiesConverter.cs +++ b/FirstClassErrors.RequestBinder/ListOfSimplePropertiesConverter.cs @@ -69,7 +69,7 @@ public RequiredField> AsOptional(Func empty = new List(); + IReadOnlyList empty = []; return new RequiredField>(_binding, empty); } diff --git a/FirstClassErrors.RequestBinder/ListOfSimpleValuePropertiesConverter.cs b/FirstClassErrors.RequestBinder/ListOfSimpleValuePropertiesConverter.cs index 7fdcfb16..74f38265 100644 --- a/FirstClassErrors.RequestBinder/ListOfSimpleValuePropertiesConverter.cs +++ b/FirstClassErrors.RequestBinder/ListOfSimpleValuePropertiesConverter.cs @@ -76,7 +76,7 @@ public RequiredField> AsOptional(Func empty = new List(); + IReadOnlyList empty = []; return new RequiredField>(_binding, empty); } diff --git a/FirstClassErrors.RequestBinder/RequestBinding.cs b/FirstClassErrors.RequestBinder/RequestBinding.cs index c699b8f0..87c463fc 100644 --- a/FirstClassErrors.RequestBinder/RequestBinding.cs +++ b/FirstClassErrors.RequestBinder/RequestBinding.cs @@ -71,7 +71,7 @@ internal RequestBinding(Func envelope, internal void Record(PrimaryPortError error) { // Created on first failure only: an all-valid binding — including every nested and per-element binding — // never allocates its error list. - (_errors ??= new List()).Add(error); + (_errors ??= []).Add(error); } /// @@ -113,7 +113,7 @@ internal RequiredField> ConvertEachElement version check that loudly reports a // converter mutating the list it is binding. - List converted = values is ICollection sized ? new List(sized.Count) : new List(); + List converted = values is ICollection sized ? new List(sized.Count) : []; int index = 0; foreach (TStored element in values) { diff --git a/FirstClassErrors.UnitTests/ErrorContextTests.cs b/FirstClassErrors.UnitTests/ErrorContextTests.cs index 495d7f3c..74797499 100644 --- a/FirstClassErrors.UnitTests/ErrorContextTests.cs +++ b/FirstClassErrors.UnitTests/ErrorContextTests.cs @@ -87,7 +87,7 @@ public void StoredValueCanBeRetrievedWhenTheKeyExistsAndTheTypeMatches() { public void MissingKeyCannotBeRetrievedFromAnErrorContext() { // Setup ErrorContextKey correlationIdKey = ErrorContextKey.Create("CorrelationId"); - ErrorContext context = new(new Dictionary()); + ErrorContext context = new([]); // Exercise bool found = context.TryGet(correlationIdKey, out string? value); diff --git a/FirstClassErrors.UnitTests/ErrorDefensiveTests.cs b/FirstClassErrors.UnitTests/ErrorDefensiveTests.cs index e37763eb..e2abe513 100644 --- a/FirstClassErrors.UnitTests/ErrorDefensiveTests.cs +++ b/FirstClassErrors.UnitTests/ErrorDefensiveTests.cs @@ -197,9 +197,9 @@ public void AConfigureContextDelegateThatThrowsPreservesTheEntriesAddedBeforeThe [Fact(DisplayName = "Inner errors are stored as a defensive copy of the provided collection.")] public void InnerErrorsAreStoredAsADefensiveCopyOfTheProvidedCollection() { // Setup - List innerErrors = new() { + List innerErrors = [ ErrorFactory.Domain(ErrorCode.Unspecified, "first") - }; + ]; DomainError error = DomainError.Create(ErrorCodeFactory.Any(), DiagnosticMessageFactory.Any(), innerErrors).WithPublicMessage(ShortMessageFactory.Any()); // Exercise @@ -212,11 +212,11 @@ public void InnerErrorsAreStoredAsADefensiveCopyOfTheProvidedCollection() { [Fact(DisplayName = "Null entries in the provided inner errors collection are filtered out.")] public void NullEntriesInTheProvidedInnerErrorsCollectionAreFilteredOut() { // Setup - List innerErrors = new() { + List innerErrors = [ ErrorFactory.Domain(ErrorCode.Unspecified, "first"), null!, ErrorFactory.Domain(ErrorCode.Unspecified, "second") - }; + ]; // Exercise DomainError error = DomainError.Create(ErrorCodeFactory.Any(), DiagnosticMessageFactory.Any(), innerErrors).WithPublicMessage(ShortMessageFactory.Any()); @@ -229,7 +229,7 @@ public void NullEntriesInTheProvidedInnerErrorsCollectionAreFilteredOut() { [Fact(DisplayName = "A collection made only of null inner errors yields an empty inner errors list.")] public void ACollectionMadeOnlyOfNullInnerErrorsYieldsAnEmptyInnerErrorsList() { // Setup - List innerErrors = new() { null!, null! }; + List innerErrors = [null!, null!]; // Exercise DomainError error = DomainError.Create(ErrorCodeFactory.Any(), DiagnosticMessageFactory.Any(), innerErrors).WithPublicMessage(ShortMessageFactory.Any()); diff --git a/FirstClassErrors/Error.cs b/FirstClassErrors/Error.cs index e469767c..fe9be432 100644 --- a/FirstClassErrors/Error.cs +++ b/FirstClassErrors/Error.cs @@ -133,12 +133,12 @@ private static ErrorContext BuildContext(Action? configure, private static IReadOnlyList CreateSafeInnerErrors(IEnumerable? innerErrors) { return innerErrors == null - ? new List() + ? [] : innerErrors.Where(innerError => innerError is not null).ToList(); } private static IReadOnlyList CreateSafeInnerErrors(Error? innerError) { - return innerError == null ? new List() : new List { innerError }; + return innerError == null ? [] : [innerError]; } #endregion @@ -179,7 +179,7 @@ protected Error(ErrorCode code, DiagnosticMessage = CoalesceRequiredMessage(diagnosticMessage, MissingDiagnosticMessage); ShortMessage = CoalesceRequiredMessage(shortMessage, MissingShortMessage); DetailedMessage = NormalizeOptionalMessage(detailedMessage); - InnerErrors = new List(); + InnerErrors = []; Context = BuildContext(configureContext, CollectMissingMessageNames(diagnosticMessage, shortMessage)); } diff --git a/FirstClassErrors/ErrorContext.cs b/FirstClassErrors/ErrorContext.cs index 5c576ff4..5d6ca7f4 100644 --- a/FirstClassErrors/ErrorContext.cs +++ b/FirstClassErrors/ErrorContext.cs @@ -18,7 +18,7 @@ public sealed class ErrorContext { #region Static members - private static readonly Dictionary EmptyValues = new(0); + private static readonly Dictionary EmptyValues = []; /// Represents an empty diagnostic context. public static ErrorContext Empty { get; } = new(EmptyValues); diff --git a/FirstClassErrors/ErrorContextBuilder.cs b/FirstClassErrors/ErrorContextBuilder.cs index e0a1e1b9..082c8dcd 100644 --- a/FirstClassErrors/ErrorContextBuilder.cs +++ b/FirstClassErrors/ErrorContextBuilder.cs @@ -11,7 +11,7 @@ public sealed class ErrorContextBuilder { #region Fields - private readonly Dictionary _values = new(); + private readonly Dictionary _values = []; #endregion diff --git a/FirstClassErrors/ErrorDocumentationBuilder.cs b/FirstClassErrors/ErrorDocumentationBuilder.cs index a383afad..5f12a5e6 100644 --- a/FirstClassErrors/ErrorDocumentationBuilder.cs +++ b/FirstClassErrors/ErrorDocumentationBuilder.cs @@ -71,7 +71,7 @@ private static IEnumerable BuildContext( #region Fields declarations private readonly ErrorDocumentation _doc = new(); - private readonly List _diagnostics = new(); + private readonly List _diagnostics = []; #endregion diff --git a/FirstClassErrors/GenDoc/AssemblyErrorDocumentationReader.cs b/FirstClassErrors/GenDoc/AssemblyErrorDocumentationReader.cs index 1f23f785..3604f586 100644 --- a/FirstClassErrors/GenDoc/AssemblyErrorDocumentationReader.cs +++ b/FirstClassErrors/GenDoc/AssemblyErrorDocumentationReader.cs @@ -42,8 +42,8 @@ public static class AssemblyErrorDocumentationReader { public static ErrorDocumentationExtractionResult GetErrorDocumentationFrom(Assembly assembly) { if (assembly is null) { throw new ArgumentNullException(nameof(assembly)); } - List documentation = new(); - List failures = new(); + List documentation = []; + List failures = []; foreach (Type type in GetLoadableTypes(assembly, failures)) { if (!IsExtractableType(type)) { continue; } @@ -51,7 +51,7 @@ public static ErrorDocumentationExtractionResult GetErrorDocumentationFrom(Assem ExtractFromType(type, documentation, failures); } - List deduplicated = new(); + List deduplicated = []; // Order before grouping so that, when several factories share the same Code, the surviving documentation is // chosen deterministically (reflection ordering is not guaranteed). diff --git a/FirstClassErrors/PrimaryPortInnerErrors.cs b/FirstClassErrors/PrimaryPortInnerErrors.cs index 02ec2f18..2a7babab 100644 --- a/FirstClassErrors/PrimaryPortInnerErrors.cs +++ b/FirstClassErrors/PrimaryPortInnerErrors.cs @@ -19,7 +19,7 @@ public sealed class PrimaryPortInnerErrors { #region Fields declarations - private readonly List _errors = new(); + private readonly List _errors = []; #endregion diff --git a/FirstClassErrors/SecondaryPortInnerErrors.cs b/FirstClassErrors/SecondaryPortInnerErrors.cs index 3be0afcf..7b8372ec 100644 --- a/FirstClassErrors/SecondaryPortInnerErrors.cs +++ b/FirstClassErrors/SecondaryPortInnerErrors.cs @@ -18,7 +18,7 @@ public sealed class SecondaryPortInnerErrors { #region Fields declarations - private readonly List _errors = new(); + private readonly List _errors = []; #endregion diff --git a/JustDummies.Analyzers.UnitTests/AnalyzerTestHarness.cs b/JustDummies.Analyzers.UnitTests/AnalyzerTestHarness.cs index 36a3f120..fd5d8e38 100644 --- a/JustDummies.Analyzers.UnitTests/AnalyzerTestHarness.cs +++ b/JustDummies.Analyzers.UnitTests/AnalyzerTestHarness.cs @@ -41,7 +41,7 @@ public static async Task> GetDiagnosticsAsync(Diagnos } private static ImmutableArray BuildReferences() { - List references = new(); + List references = []; // Reference the running runtime's assemblies so snippets resolve System types without pinning a ref pack. string trustedAssemblies = AppContext.GetData("TRUSTED_PLATFORM_ASSEMBLIES") as string ?? string.Empty; diff --git a/JustDummies.Analyzers/AnyChainFacts.cs b/JustDummies.Analyzers/AnyChainFacts.cs index 05ddfc39..08a38452 100644 --- a/JustDummies.Analyzers/AnyChainFacts.cs +++ b/JustDummies.Analyzers/AnyChainFacts.cs @@ -25,7 +25,7 @@ internal static class AnyChainFacts { /// completely. /// public static bool TryGetChain(IInvocationOperation invocation, KnownSymbols symbols, out IReadOnlyList constraints, out IInvocationOperation? factory) { - List collected = new(); + List collected = []; constraints = collected; factory = null; diff --git a/JustDummies.Analyzers/CollectionConstraintsAdmitNoValueAnalyzer.cs b/JustDummies.Analyzers/CollectionConstraintsAdmitNoValueAnalyzer.cs index 94b56883..e5c386b3 100644 --- a/JustDummies.Analyzers/CollectionConstraintsAdmitNoValueAnalyzer.cs +++ b/JustDummies.Analyzers/CollectionConstraintsAdmitNoValueAnalyzer.cs @@ -161,7 +161,7 @@ private static bool TryCountDistinctConstants(IInvocationOperation pool, out int if (argument.ArgumentKind != ArgumentKind.ParamArray) { continue; } if (argument.Value is not IArrayCreationOperation { Initializer: { } initializer }) { return false; } - HashSet distinct = new(); + HashSet distinct = []; foreach (IOperation element in initializer.ElementValues) { Optional constant = GeneratorFacts.Unwrap(element).ConstantValue; if (!constant.HasValue) { return false; } diff --git a/JustDummies.Analyzers/EnumUniverseViolationAnalyzer.cs b/JustDummies.Analyzers/EnumUniverseViolationAnalyzer.cs index e9c9ad02..86878f16 100644 --- a/JustDummies.Analyzers/EnumUniverseViolationAnalyzer.cs +++ b/JustDummies.Analyzers/EnumUniverseViolationAnalyzer.cs @@ -49,16 +49,16 @@ private static void Analyze(OperationAnalysisContext context, KnownSymbols symbo if (factory.TargetMethod.TypeArguments.Length != 1 || factory.TargetMethod.TypeArguments[0] is not INamedTypeSymbol enumType) { return; } if (NegativeTestGuard.IsSoleBodyOfLambdaArgument(invocation.Syntax)) { return; } - HashSet declared = new(enumType.GetMembers() + HashSet declared = [.. enumType.GetMembers() .OfType() .Where(field => field.HasConstantValue) - .Select(field => field.ConstantValue)); + .Select(field => field.ConstantValue)]; if (declared.Count == 0) { return; } bool combinationsAllowed = constraints.Any(constraint => constraint.TargetMethod.Name == "AllowingCombinations"); - HashSet excluded = new(); + HashSet excluded = []; foreach (IInvocationOperation constraint in constraints) { string name = constraint.TargetMethod.Name; diff --git a/JustDummies.Analyzers/StringConstraintsAdmitNoValueAnalyzer.cs b/JustDummies.Analyzers/StringConstraintsAdmitNoValueAnalyzer.cs index 90697b34..fd69838d 100644 --- a/JustDummies.Analyzers/StringConstraintsAdmitNoValueAnalyzer.cs +++ b/JustDummies.Analyzers/StringConstraintsAdmitNoValueAnalyzer.cs @@ -57,7 +57,7 @@ private static void Analyze(OperationAnalysisContext context, KnownSymbols symbo bool requireUpper = false; bool requireLower = false; bool hasValueSet = false; - List<(string Text, IOperation At)> fragments = new(); + List<(string Text, IOperation At)> fragments = []; int? fixedLength = null; int? maximum = null; diff --git a/JustDummies.PropertyTests/CompositionProperties.cs b/JustDummies.PropertyTests/CompositionProperties.cs index bee736df..b53d61c4 100644 --- a/JustDummies.PropertyTests/CompositionProperties.cs +++ b/JustDummies.PropertyTests/CompositionProperties.cs @@ -42,7 +42,7 @@ private static Gen StringPools() { /// length), so the null-element rejection is exercised at every position rather than only at the end. /// private static string[] Poisoned(string[] pool, int index) { - List poisoned = new(pool); + List poisoned = [.. pool]; poisoned.Insert(Math.Min(index, poisoned.Count), null!); return poisoned.ToArray(); @@ -279,8 +279,8 @@ public void OneOfReachesExactlyTheDistinctValuesOfItsPool() { from seed in Generators.Seed() select (pool, seed)).ToArbitrary(), testCase => { - HashSet distinct = new(testCase.pool); - HashSet drawn = new(Expect.Draws(Any.WithSeed(testCase.seed).OneOf(testCase.pool), 96)); + HashSet distinct = [.. testCase.pool]; + HashSet drawn = [.. Expect.Draws(Any.WithSeed(testCase.seed).OneOf(testCase.pool), 96)]; return drawn.SetEquals(distinct); }) diff --git a/JustDummies.PropertyTests/PatternRoundTripProperties.cs b/JustDummies.PropertyTests/PatternRoundTripProperties.cs index aac4b417..f8cfc839 100644 --- a/JustDummies.PropertyTests/PatternRoundTripProperties.cs +++ b/JustDummies.PropertyTests/PatternRoundTripProperties.cs @@ -576,7 +576,7 @@ from seed in Generators.Seed() // then states both halves at once: no branch is dead, and nothing outside the declared set // can come out. AnyPattern generator = Any.WithSeed(testCase.Seed).StringMatching(string.Join("|", testCase.Branches)); - HashSet seen = new(Expect.Draws(generator, BranchSampleCount)); + HashSet seen = [.. Expect.Draws(generator, BranchSampleCount)]; return seen.SetEquals(testCase.Branches); }) diff --git a/JustDummies.PropertyTests/StringShapeProperties.cs b/JustDummies.PropertyTests/StringShapeProperties.cs index a924812e..6d8b179d 100644 --- a/JustDummies.PropertyTests/StringShapeProperties.cs +++ b/JustDummies.PropertyTests/StringShapeProperties.cs @@ -93,7 +93,7 @@ private static bool AllowedByFamily(char character, int family, string pool) { 0 => IsAsciiLetter(character), 1 => IsAsciiDigit(character), 2 => IsAsciiLetter(character) || IsAsciiDigit(character), - _ => pool.IndexOf(character) >= 0 + _ => pool.Contains(character) }; } @@ -213,6 +213,11 @@ public void EndingWithAnchorsTheSuffix() { } [Fact(DisplayName = "Containing embeds the value, whatever the value.")] + [System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "CA2249:Consider using String.Contains instead of String.IndexOf", + Justification = + "string.Contains(string, StringComparison) is not on the netstandard2.0 / net472 floor this suite runs " + + "against (ADR-0022); IndexOf with the same StringComparison.Ordinal carries the identical comparison and " + + "compiles on every leg. The rule is right on net10.0 only.")] public void ContainingEmbedsTheValue() { Prop.ForAll(Affix(DefaultAlphabet, 8).ToArbitrary(), fragment => Expect.EveryDraw(Any.String().Containing(fragment), diff --git a/JustDummies.PropertyTests/UriProperties.cs b/JustDummies.PropertyTests/UriProperties.cs index c5e597aa..8172fe86 100644 --- a/JustDummies.PropertyTests/UriProperties.cs +++ b/JustDummies.PropertyTests/UriProperties.cs @@ -57,7 +57,7 @@ private enum UserInfoChoice { /// The schemes the library is allowed to emit. file is deliberately absent: a file path does not /// round-trip identically across target frameworks, so the unconstrained draw must never reach it. /// - private static readonly HashSet EmittableSchemes = new() { "http", "https", "ws", "wss", "ftp", "mailto" }; + private static readonly HashSet EmittableSchemes = ["http", "https", "ws", "wss", "ftp", "mailto"]; /// /// Arbitrary hosts the library accepts, drawn from the very alphabet UriSpec draws its own hosts from: a @@ -158,7 +158,7 @@ public void UnconstrainedReachesEveryFamily() { seed => { // One family in five per draw, so 120 draws leave a miss far below any rate that could make // this flaky, while a hand-written test can only ever assert it for the one seed it picked. - HashSet seen = new(); + HashSet seen = []; foreach (Uri value in Expect.Draws(Any.WithSeed(seed).Uri(), 120)) { seen.Add(value.IsAbsoluteUri ? value.Scheme : "relative"); } @@ -276,9 +276,9 @@ from query in Gen.Elements(true, false) ? value.Scheme == (testCase.Secure.Value ? "wss" : "ws") : value.Scheme is "ws" or "wss") && rendered.StartsWith(value.Scheme + "://" + testCase.Host, StringComparison.Ordinal) - && (rendered.IndexOf('?') >= 0) == testCase.Query - && rendered.IndexOf('#') < 0 - && rendered.IndexOf('@') < 0; + && rendered.Contains('?') == testCase.Query + && !rendered.Contains('#') + && !rendered.Contains('@'); }); }) .QuickCheckThrowOnFailure(); @@ -357,6 +357,17 @@ from headers in Gen.Elements(true, false) } [Fact(DisplayName = "A relative draw is a relative reference carrying exactly the declared path, query and fragment.")] + [System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1870:Use a cached 'SearchValues' instance", + Justification = + "SearchValues arrived in .NET 8 and this suite also runs on the .NET Framework 4.7.2 support floor " + + "(ADR-0022, build/Net472TestFloor.props), where the type does not exist. The rule is right on net10.0 only; " + + "IndexOfAny over a two-character array carries the same meaning on both legs. Same downlevel wall as " + + "SYSLIB1045 and CA1510 (ADR-0058).")] + [System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1865:Use char overload", + Justification = + "string.StartsWith(char) is not on the .NET Framework 4.7.2 support floor this suite also runs on " + + "(ADR-0022): measured, the net472 leg rejects it with CS1503, cannot convert from 'char' to 'string'. " + + "The explicit StringComparison.Ordinal overload compiles on both legs and states the comparison it uses.")] public void RelativeDrawsAreRelativeReferences() { Gen<(int? Segments, bool Rooted, bool Query, bool Fragment)> cases = from segments in Optional(Generators.Count(6)) @@ -389,8 +400,8 @@ from fragment in Gen.Elements(true, false) && reference.Length > 0 && (!testCase.Rooted || reference.StartsWith("/", StringComparison.Ordinal)) && (testCase.Segments.HasValue ? segments == testCase.Segments.Value : segments <= 2) - && (reference.IndexOf('?') >= 0) == testCase.Query - && (reference.IndexOf('#') >= 0) == testCase.Fragment; + && reference.Contains('?') == testCase.Query + && reference.Contains('#') == testCase.Fragment; }); }) .QuickCheckThrowOnFailure(); diff --git a/JustDummies.UnitTests/AnyCollectionTests.cs b/JustDummies.UnitTests/AnyCollectionTests.cs index db97443e..05ef2acd 100644 --- a/JustDummies.UnitTests/AnyCollectionTests.cs +++ b/JustDummies.UnitTests/AnyCollectionTests.cs @@ -28,7 +28,7 @@ private enum Suit { [Fact(DisplayName = "ListOf: unconstrained draws vary in size, stay within 0..8, and hold elements from the item generator.")] public void ListOfUnconstrained() { - HashSet sizes = new(); + HashSet sizes = []; for (int i = 0; i < SampleCount; i++) { List list = Any.ListOf(Any.Int32().Between(1, 9)).Generate(); sizes.Add(list.Count); @@ -189,7 +189,7 @@ public void ContainingOutsideDomainExtendsCardinality() { // run through the very same CollectionState path, so the correction reaches them too, now exercised directly // through AnyDictionary.ContainingKey (see DictionaryContainingKeyOutsideDomainExtendsCardinality). for (int i = 0; i < SampleCount; i++) { - HashSet list = new(Any.ListOf(Any.Int32().OneOf(1, 2)).Containing(3).WithCount(3).Distinct().Generate()); + HashSet list = [.. Any.ListOf(Any.Int32().OneOf(1, 2)).Containing(3).WithCount(3).Distinct().Generate()]; Check.That(list).Contains(1, 2, 3); HashSet quad = Any.SetOf(Any.Int32().OneOf(1, 2)).Containing(3).Containing(4).WithCount(4).Generate(); diff --git a/JustDummies.UnitTests/AnyDateTimeOffsetOffsetTests.cs b/JustDummies.UnitTests/AnyDateTimeOffsetOffsetTests.cs index 3640a989..252aa3d0 100644 --- a/JustDummies.UnitTests/AnyDateTimeOffsetOffsetTests.cs +++ b/JustDummies.UnitTests/AnyDateTimeOffsetOffsetTests.cs @@ -34,7 +34,7 @@ public void WithOffsetPins() { public void WithOffsetBetweenBounds() { TimeSpan min = TimeSpan.FromHours(-5); TimeSpan max = TimeSpan.FromHours(5); - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { DateTimeOffset value = Any.DateTimeOffset().WithOffsetBetween(min, max).Generate(); Check.That(value.Offset >= min && value.Offset <= max).IsTrue(); @@ -108,7 +108,7 @@ public void AnUnconstrainedOneOfKeepsEveryOffset() { DateTimeOffset utc = new(2020, 1, 1, 0, 0, 0, TimeSpan.Zero); DateTimeOffset plusFive = new(2021, 1, 1, 0, 0, 0, TimeSpan.FromHours(5)); - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { seen.Add(Any.DateTimeOffset().OneOf(utc, plusFive).Generate().Offset); } diff --git a/JustDummies.UnitTests/AnyEnumCombinationTests.cs b/JustDummies.UnitTests/AnyEnumCombinationTests.cs index 9f62675c..ddad9efc 100644 --- a/JustDummies.UnitTests/AnyEnumCombinationTests.cs +++ b/JustDummies.UnitTests/AnyEnumCombinationTests.cs @@ -57,7 +57,7 @@ private enum OrderStatus { [Fact(DisplayName = "A [Flags] enum still draws only declared members until combinations are allowed.")] public void FlagsEnumDrawsDeclaredMembersByDefault() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { seen.Add(Any.Enum().Generate()); } // The contract the opt-in exists to leave untouched: the default never depends on the [Flags] attribute, so a @@ -67,7 +67,7 @@ public void FlagsEnumDrawsDeclaredMembersByDefault() { [Fact(DisplayName = "AllowingCombinations: the universe is every combination, and the declared zero value.")] public void CombinationsCoverTheWholeUniverse() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { seen.Add(Any.Enum().AllowingCombinations().Generate()); } Check.That(seen.Count).IsEqualTo(8); @@ -76,7 +76,7 @@ public void CombinationsCoverTheWholeUniverse() { [Fact(DisplayName = "AllowingCombinations: an enum declaring no zero member never yields the empty combination.")] public void CombinationsOmitZeroWhenItIsNotDeclared() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { seen.Add(Any.Enum().AllowingCombinations().Generate()); } Check.That(seen).IsOnlyMadeOf(Sides.Left, Sides.Right, Sides.Left | Sides.Right); @@ -84,7 +84,7 @@ public void CombinationsOmitZeroWhenItIsNotDeclared() { [Fact(DisplayName = "AllowingCombinations: a declared composite adds nothing — it already is a combination.")] public void DeclaredCompositeDoesNotWidenTheUniverse() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { seen.Add(Any.Enum().AllowingCombinations().Generate()); } Check.That(seen).IsOnlyMadeOf(Access.Read, Access.Write, Access.ReadWrite); @@ -104,7 +104,7 @@ public void CombinationsRequireAFlagsEnum() { public void CombinationsAreIdempotent() { AnyEnum generator = Any.Enum().AllowingCombinations().AllowingCombinations(); - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { seen.Add(generator.Generate()); } // Idempotent, not cumulative: the universe is the same eight values a single application yields. @@ -125,7 +125,7 @@ public void OneOfAcceptsACombinationAfterTheOptIn() { .AllowingCombinations() .OneOf(Permissions.Read | Permissions.Write, Permissions.Exec); - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { seen.Add(generator.Generate()); } Check.That(seen).IsOnlyMadeOf(Permissions.Read | Permissions.Write, Permissions.Exec); @@ -135,7 +135,7 @@ public void OneOfAcceptsACombinationAfterTheOptIn() { public void ExclusionsCompareByEquality() { AnyEnum generator = Any.Enum().AllowingCombinations().Except(Permissions.Read); - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { seen.Add(generator.Generate()); } // Read itself is gone; Read | Write is a different value and stays drawable — Except is not a bit mask. diff --git a/JustDummies.UnitTests/AnyInt32Tests.cs b/JustDummies.UnitTests/AnyInt32Tests.cs index 6af6f426..fc7d45ec 100644 --- a/JustDummies.UnitTests/AnyInt32Tests.cs +++ b/JustDummies.UnitTests/AnyInt32Tests.cs @@ -63,7 +63,7 @@ public void NonZeroIsNeverZero() { [Fact(DisplayName = "Between eventually reaches both inclusive bounds.")] public void BetweenReachesItsBounds() { - HashSet seen = new(Samples(Any.Int32().Between(1, 3))); + HashSet seen = [.. Samples(Any.Int32().Between(1, 3))]; Check.That(seen.Contains(1)).IsTrue(); Check.That(seen.Contains(3)).IsTrue(); diff --git a/JustDummies.UnitTests/AnyLatticeConstraintTests.cs b/JustDummies.UnitTests/AnyLatticeConstraintTests.cs index b978c208..350199d2 100644 --- a/JustDummies.UnitTests/AnyLatticeConstraintTests.cs +++ b/JustDummies.UnitTests/AnyLatticeConstraintTests.cs @@ -28,7 +28,7 @@ public void MultipleOfAlwaysDivisible() { [Fact(DisplayName = "MultipleOf: draws on the grid within the declared range and reaches both ends.")] public void MultipleOfHonoursRangeAndReachesBounds() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { int value = Any.Int32().Between(0, 1000).MultipleOf(100).Generate(); Check.That(value % 100).IsEqualTo(0); @@ -51,7 +51,7 @@ public void MultipleOfHandlesNegativeGrid() { [Fact(DisplayName = "MultipleOf: composes with Except, never yielding an excluded grid point.")] public void MultipleOfComposesWithExcept() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { int value = Any.Int32().Between(0, 30).MultipleOf(10).Except(10, 20).Generate(); Check.That(value % 10).IsEqualTo(0); @@ -98,7 +98,7 @@ public void MultipleOfDeclaredOnce() { [Fact(DisplayName = "MultipleOf: a distinct collection sees the grid cardinality.")] public void MultipleOfFeedsCardinality() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { seen.Add(Any.Int32().Between(0, 20).MultipleOf(10).Generate()); } // Exactly the three grid points are reachable — the cardinality hint a distinct collection relies on. @@ -123,7 +123,7 @@ public void WithScaleStaysOnGrid() { [Fact(DisplayName = "WithScale: reaches both ends of a narrow grid.")] public void WithScaleReachesBounds() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { decimal value = Any.Decimal().Between(0m, 1m).WithScale(1).Generate(); Check.That(value).IsEqualTo(Math.Round(value, 1, MidpointRounding.ToEven)); diff --git a/JustDummies.UnitTests/AnyModernTypeTests.cs b/JustDummies.UnitTests/AnyModernTypeTests.cs index a55418ad..a40a81a7 100644 --- a/JustDummies.UnitTests/AnyModernTypeTests.cs +++ b/JustDummies.UnitTests/AnyModernTypeTests.cs @@ -26,7 +26,7 @@ public void HalfIsUnaffectedByTheOrdinaryWindow() { [Fact(DisplayName = "DateOnly: Between is inclusive and reached; After/Before are exclusive; conflicts surface.")] public void DateOnlyBehaves() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { DateOnly value = Any.DateOnly().Between(AnchorDate, AnchorDate.AddDays(2)).Generate(); seen.Add(value); @@ -65,7 +65,7 @@ public void TimeOnlyBehaves() { [Fact(DisplayName = "Int128: signs, pins, full-width variety, extremes and conflicts.")] public void Int128Behaves() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { seen.Add(Any.Int128().Generate()); Check.That(Any.Int128().Positive().Generate() > 0).IsTrue(); @@ -83,7 +83,7 @@ public void Int128Behaves() { [Fact(DisplayName = "UInt128: bounds, exclusivity and full-width variety.")] public void UInt128Behaves() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { seen.Add(Any.UInt128().Generate()); diff --git a/JustDummies.UnitTests/AnyOneOfTests.cs b/JustDummies.UnitTests/AnyOneOfTests.cs index f2852d99..12070afc 100644 --- a/JustDummies.UnitTests/AnyOneOfTests.cs +++ b/JustDummies.UnitTests/AnyOneOfTests.cs @@ -37,7 +37,7 @@ public void DrawsOnlyTheSuppliedValues() { [Fact(DisplayName = "OneOf eventually reaches every supplied value.")] public void ReachesEverySuppliedValue() { - HashSet seen = new(Samples(Any.OneOf(1, 2, 3))); + HashSet seen = [.. Samples(Any.OneOf(1, 2, 3))]; Check.That(seen).Contains(1, 2, 3); } @@ -51,14 +51,14 @@ public void SingleValueIsPinned() { [Fact(DisplayName = "OneOf varies from draw to draw when the pool holds more than one value.")] public void VariesAcrossDraws() { - HashSet seen = new(Samples(Any.OneOf(1, 2, 3, 4))); + HashSet seen = [.. Samples(Any.OneOf(1, 2, 3, 4))]; Check.That(seen.Count).IsStrictlyGreaterThan(1); } [Fact(DisplayName = "Duplicate values are collapsed under the default comparer: both distinct values are still drawn, nothing else.")] public void DuplicatesAreCollapsed() { - HashSet seen = new(Samples(Any.OneOf(1, 1, 2))); + HashSet seen = [.. Samples(Any.OneOf(1, 1, 2))]; Check.That(seen).IsOnlyMadeOf(1, 2); Check.That(seen).Contains(1, 2); @@ -89,7 +89,7 @@ public void OrNullIsSometimesNull() { Percentage two = Percentage.Create(2); IAny generator = Any.WithSeed(20260721).OneOf(one, two).OrNull(); - List values = new(); + List values = []; for (int i = 0; i < SampleCount; i++) { values.Add(generator.Generate()); } @@ -138,9 +138,9 @@ public void NullElementMessagePointsAtOrNull() { [Fact(DisplayName = "ElementOf draws only from the list it is given.")] public void ElementOfDrawsFromTheList() { - IReadOnlyList pool = new List { 1, 2, 3 }; + IReadOnlyList pool = [1, 2, 3]; - HashSet seen = new(Samples(Any.ElementOf(pool))); + HashSet seen = [.. Samples(Any.ElementOf(pool))]; Check.That(seen).IsOnlyMadeOf(1, 2, 3); Check.That(seen.Count).IsStrictlyGreaterThan(1); @@ -171,7 +171,7 @@ public void ElementOfValidatesItsPool() { Check.ThatCode(() => Any.ElementOf((IEnumerable)null!)).Throws(); Check.ThatCode(() => Any.ElementOf(new List())).Throws(); Check.ThatCode(() => Any.ElementOf(Enumerable.Empty())).Throws(); - Check.ThatCode(() => Any.ElementOf(new List { "a", null! })).Throws(); + Check.ThatCode(() => Any.ElementOf(["a", null!])).Throws(); } [Fact(DisplayName = "DifferentFrom removes a value from the pool — the idiom for drawing another element of a fixture.")] @@ -198,7 +198,7 @@ public void ExceptRemovesEveryValue() { [Fact(DisplayName = "A value that is not in the pool removes nothing.")] public void ExcludingAnAbsentValueRemovesNothing() { - HashSet seen = new(Samples(Any.OneOf(1, 2).DifferentFrom(99))); + HashSet seen = [.. Samples(Any.OneOf(1, 2).DifferentFrom(99))]; Check.That(seen).IsOnlyMadeOf(1, 2); Check.That(seen).Contains(1, 2); @@ -226,9 +226,9 @@ static string Emptied(Func declare) { Check.That(Emptied(() => Any.OneOf(7).DifferentFrom(7))).IsEqualTo(fromOneOf); Check.That(Emptied(() => Any.WithSeed(1).OneOf(7).DifferentFrom(7))).IsEqualTo(fromOneOf); - Check.That(Emptied(() => Any.ElementOf(new List { 7 }).DifferentFrom(7))).IsEqualTo(fromElement); + Check.That(Emptied(() => Any.ElementOf([7]).DifferentFrom(7))).IsEqualTo(fromElement); Check.That(Emptied(() => Any.ElementOf(new List { 7 }.Select(value => value)).DifferentFrom(7))).IsEqualTo(fromElement); - Check.That(Emptied(() => Any.WithSeed(1).ElementOf(new List { 7 }).DifferentFrom(7))).IsEqualTo(fromElement); + Check.That(Emptied(() => Any.WithSeed(1).ElementOf([7]).DifferentFrom(7))).IsEqualTo(fromElement); Check.That(Emptied(() => Any.WithSeed(1).ElementOf(new List { 7 }.Select(value => value)).DifferentFrom(7))).IsEqualTo(fromElement); } @@ -275,7 +275,7 @@ public void ExclusionArgumentsAreValidated() { [Fact(DisplayName = "A seeded context makes OneOf and ElementOf deterministic — the mirrored surface draws from the context's seed.")] public void SeededContextIsDeterministic() { - List pool = new() { 10, 20, 30, 40 }; + List pool = [10, 20, 30, 40]; string oneOfFirst = string.Join("|", Samples(Any.WithSeed(11).OneOf(10, 20, 30, 40)).Take(20)); string oneOfSecond = string.Join("|", Samples(Any.WithSeed(11).OneOf(10, 20, 30, 40)).Take(20)); diff --git a/JustDummies.UnitTests/AnyPatternTests.cs b/JustDummies.UnitTests/AnyPatternTests.cs index 560e189c..e1356a17 100644 --- a/JustDummies.UnitTests/AnyPatternTests.cs +++ b/JustDummies.UnitTests/AnyPatternTests.cs @@ -111,7 +111,7 @@ public void GeneratedValuesMatchTheRealEngine(string pattern) { public void GeneratedValuesVary() { foreach (string pattern in new[] { @"\d{8}", @"[A-Z]{3}", @"(EUR|USD|GBP)", @"[A-Za-z0-9_]+", @"a{2,4}b*c+" }) { AnyPattern generator = Any.WithSeed(4242).StringMatching(pattern); - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { seen.Add(generator.Generate()); } Check.That(seen.Count).IsStrictlyGreaterThan(1); } @@ -129,7 +129,7 @@ public void FixedShape() { [Fact(DisplayName = "Alternation draws each branch and only declared branches.")] public void Alternation() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { string value = Any.StringMatching("(EUR|USD|GBP)").Generate(); Check.That(value == "EUR" || value == "USD" || value == "GBP").IsTrue(); @@ -150,9 +150,9 @@ public void CharacterClasses() { [Fact(DisplayName = "Bounded quantifiers stay within their bounds; unbounded ones draw the minimum plus 0 to 8.")] public void QuantifierBounds() { - HashSet starLengths = new(); - HashSet plusLengths = new(); - HashSet openLengths = new(); + HashSet starLengths = []; + HashSet plusLengths = []; + HashSet openLengths = []; for (int i = 0; i < SampleCount; i++) { int bounded = (Any.StringMatching("a{2,4}").Generate()).Length; diff --git a/JustDummies.UnitTests/AnySetTypeTests.cs b/JustDummies.UnitTests/AnySetTypeTests.cs index f8aaa170..61abe327 100644 --- a/JustDummies.UnitTests/AnySetTypeTests.cs +++ b/JustDummies.UnitTests/AnySetTypeTests.cs @@ -20,7 +20,7 @@ private enum OrderStatus { [Fact(DisplayName = "Boolean: unconstrained draws hit both values; pins pin; contradictory pins conflict.")] public void BooleanBehaves() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { seen.Add(Any.Boolean().Generate()); } Check.That(seen.Count).IsEqualTo(2); @@ -41,7 +41,7 @@ public void BooleanBehaves() { [Fact(DisplayName = "Guid: unconstrained draws are non-empty, varied, and reproducible under a context seed.")] public void GuidBehaves() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { Guid value = Any.Guid().Generate(); seen.Add(value); @@ -111,8 +111,14 @@ public async Task GuidExclusionByteWraparoundTerminates() { } [Fact(DisplayName = "Enum: unconstrained draws yield only declared members and reach all of them.")] + [System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "CA2263:Prefer generic overload when type is known", + Justification = + "Enum.IsDefined(TEnum) arrived in .NET 5 and this suite also runs on the .NET Framework 4.7.2 " + + "support floor (ADR-0022, build/Net472TestFloor.props), where it does not exist. The non-generic overload " + + "is the only spelling that compiles on both legs; the reason is restated at the call site so a reader meets " + + "it there too.")] public void EnumDrawsDeclaredMembers() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { OrderStatus value = Any.Enum().Generate(); seen.Add(value); diff --git a/JustDummies.UnitTests/AnySignedIntegerTests.cs b/JustDummies.UnitTests/AnySignedIntegerTests.cs index f3260860..4ba390cb 100644 --- a/JustDummies.UnitTests/AnySignedIntegerTests.cs +++ b/JustDummies.UnitTests/AnySignedIntegerTests.cs @@ -25,7 +25,7 @@ public void SByteSignConstraints() { [Fact(DisplayName = "SByte: Between is inclusive and reaches both bounds; extremes are generable.")] public void SByteBounds() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { seen.Add(Any.SByte().Between(-1, 1).Generate()); } Check.That(seen.Contains(-1)).IsTrue(); Check.That(seen.Contains(1)).IsTrue(); @@ -54,7 +54,7 @@ public void Int16ExclusiveBounds() { [Fact(DisplayName = "Int64: full-range generation works and crossed bounds conflict naming both sides.")] public void Int64RangeAndConflicts() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { seen.Add(Any.Int64().Generate()); } Check.That(seen.Count).IsStrictlyGreaterThan(1); diff --git a/JustDummies.UnitTests/AnyStringTests.cs b/JustDummies.UnitTests/AnyStringTests.cs index 63e7f304..b21c8e51 100644 --- a/JustDummies.UnitTests/AnyStringTests.cs +++ b/JustDummies.UnitTests/AnyStringTests.cs @@ -60,7 +60,7 @@ public void MinAndMaxLengthAreInclusiveBounds() { [Fact(DisplayName = "WithLengthBetween bounds the length inclusively and reaches its bounds.")] public void WithLengthBetweenIsInclusive() { - HashSet lengths = new(); + HashSet lengths = []; foreach (string value in Samples(Any.String().WithLengthBetween(2, 4))) { lengths.Add(value.Length); Check.That(value.Length).IsGreaterOrEqualThan(2); @@ -374,14 +374,14 @@ public void ExclusionArgumentsAreValidated() { public void WithCharsDrawsFromThePool() { const string pool = "0123456789ABCDEF"; foreach (string value in Samples(Any.String().WithChars(pool).NonEmpty())) { - Check.That(value.All(character => pool.IndexOf(character) >= 0)).IsTrue(); + Check.That(value.All(character => pool.Contains(character))).IsTrue(); } } [Fact(DisplayName = "WithChars reaches every character in the pool.")] public void WithCharsReachesEveryCharacter() { const string pool = "ACGT"; - HashSet seen = new(); + HashSet seen = []; foreach (string value in Samples(Any.String().WithChars(pool).WithLength(8))) { foreach (char character in value) { seen.Add(character); } } @@ -393,7 +393,7 @@ public void WithCharsReachesEveryCharacter() { public void WithCharsReachesNonAscii() { const string pool = "àâäéèêëîïôùûüç"; foreach (string value in Samples(Any.String().WithChars(pool).NonEmpty())) { - Check.That(value.All(character => pool.IndexOf(character) >= 0)).IsTrue(); + Check.That(value.All(character => pool.Contains(character))).IsTrue(); } } diff --git a/JustDummies.UnitTests/AnyStringValueSetTests.cs b/JustDummies.UnitTests/AnyStringValueSetTests.cs index 4e64b11a..700932ef 100644 --- a/JustDummies.UnitTests/AnyStringValueSetTests.cs +++ b/JustDummies.UnitTests/AnyStringValueSetTests.cs @@ -48,7 +48,7 @@ public void DrawsOnlyTheSuppliedValues() { [Fact(DisplayName = "OneOf eventually reaches every supplied value.")] public void ReachesEverySuppliedValue() { - HashSet seen = new(Samples(Any.String().OneOf("EUR", "USD", "GBP"))); + HashSet seen = [.. Samples(Any.String().OneOf("EUR", "USD", "GBP"))]; Check.That(seen).Contains("EUR", "USD", "GBP"); } @@ -62,14 +62,14 @@ public void SingleValueIsPinned() { [Fact(DisplayName = "OneOf varies from draw to draw when the set holds more than one value.")] public void VariesAcrossDraws() { - HashSet seen = new(Samples(Any.String().OneOf("a", "b", "c", "d"))); + HashSet seen = [.. Samples(Any.String().OneOf("a", "b", "c", "d"))]; Check.That(seen.Count).IsStrictlyGreaterThan(1); } [Fact(DisplayName = "Duplicate values are collapsed: both distinct values are still drawn, nothing else.")] public void DuplicatesAreCollapsed() { - HashSet seen = new(Samples(Any.String().OneOf("a", "a", "b"))); + HashSet seen = [.. Samples(Any.String().OneOf("a", "a", "b"))]; Check.That(seen).IsOnlyMadeOf("a", "b"); Check.That(seen).Contains("a", "b"); @@ -103,7 +103,7 @@ public void ComposesThroughAs() { public void OrNullIsSometimesNull() { IAny generator = Any.WithSeed(20260721).String().OneOf("a", "b").OrNull(); - List values = new(); + List values = []; for (int i = 0; i < SampleCount; i++) { values.Add(generator.Generate()); } @@ -337,9 +337,9 @@ public void RejectsInvalidValueLists() { [Fact(DisplayName = "OneOf accepts a sequence, drawing only from its values.")] public void AcceptsASequence() { - IEnumerable vendors = new List { "Apple", "Microsoft", "Google" }; + IEnumerable vendors = ["Apple", "Microsoft", "Google"]; - HashSet seen = new(Samples(Any.String().OneOf(vendors))); + HashSet seen = [.. Samples(Any.String().OneOf(vendors))]; Check.That(seen).IsOnlyMadeOf("Apple", "Microsoft", "Google"); Check.That(seen.Count).IsStrictlyGreaterThan(1); diff --git a/JustDummies.UnitTests/AnyTimeTests.cs b/JustDummies.UnitTests/AnyTimeTests.cs index 423d3aee..97c78f9e 100644 --- a/JustDummies.UnitTests/AnyTimeTests.cs +++ b/JustDummies.UnitTests/AnyTimeTests.cs @@ -29,7 +29,7 @@ public void TimeSpanSigns() { public void TimeSpanBounds() { Check.That(Any.TimeSpan().Zero().Generate()).IsEqualTo(TimeSpan.Zero); - HashSet ticks = new(); + HashSet ticks = []; for (int i = 0; i < SampleCount; i++) { TimeSpan value = Any.TimeSpan().Between(TimeSpan.FromTicks(1), TimeSpan.FromTicks(3)).Generate(); ticks.Add(value.Ticks); diff --git a/JustDummies.UnitTests/AnyUnsignedIntegerTests.cs b/JustDummies.UnitTests/AnyUnsignedIntegerTests.cs index e8f63e52..2ef47ac7 100644 --- a/JustDummies.UnitTests/AnyUnsignedIntegerTests.cs +++ b/JustDummies.UnitTests/AnyUnsignedIntegerTests.cs @@ -12,7 +12,7 @@ public sealed class AnyUnsignedIntegerTests { [Fact(DisplayName = "Byte: Between is inclusive and reaches both bounds; extremes are generable.")] public void ByteBounds() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { byte value = Any.Byte().Between(1, 3).Generate(); seen.Add(value); @@ -51,7 +51,7 @@ public void MidWidthExclusiveBounds() { [Fact(DisplayName = "UInt64: the full-width sampling path yields varied values and honors exclusions.")] public void UInt64FullWidth() { - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < SampleCount; i++) { seen.Add(Any.UInt64().Generate()); } Check.That(seen.Count).IsStrictlyGreaterThan(1); diff --git a/JustDummies.UnitTests/AnyUriTests.cs b/JustDummies.UnitTests/AnyUriTests.cs index cb8e1e60..0db46fdc 100644 --- a/JustDummies.UnitTests/AnyUriTests.cs +++ b/JustDummies.UnitTests/AnyUriTests.cs @@ -57,7 +57,7 @@ public void EveryFamilyGeneratesValidUris() { [Fact(DisplayName = "The unconstrained generator reaches every family.")] public void UnconstrainedReachesEveryFamily() { - HashSet seen = new(); + HashSet seen = []; foreach (Uri value in Sample(Seeded(context => context.Uri()))) { seen.Add(value.IsAbsoluteUri ? value.Scheme : "relative"); } @@ -71,7 +71,7 @@ public void UnconstrainedReachesEveryFamily() { [Fact(DisplayName = "Web reaches both http and https; each generated value is one of them.")] public void WebReachesBothSchemes() { - HashSet seen = new(); + HashSet seen = []; foreach (Uri value in Sample(Seeded(context => context.Uri().Web()))) { seen.Add(value.Scheme); Check.That(value.Scheme is "http" or "https").IsTrue(); @@ -83,7 +83,7 @@ public void WebReachesBothSchemes() { [Fact(DisplayName = "WebSocket reaches both ws and wss.")] public void WebSocketReachesBothSchemes() { - HashSet seen = new(); + HashSet seen = []; foreach (Uri value in Sample(Seeded(context => context.Uri().WebSocket()))) { seen.Add(value.Scheme); Check.That(value.Scheme is "ws" or "wss").IsTrue(); diff --git a/JustDummies.UnitTests/CompositionTests.cs b/JustDummies.UnitTests/CompositionTests.cs index a9c62217..b0366b93 100644 --- a/JustDummies.UnitTests/CompositionTests.cs +++ b/JustDummies.UnitTests/CompositionTests.cs @@ -278,7 +278,7 @@ public void CompositionValidatesArguments() { public void DerivedGeneratorsDrawFreshValues() { IAny generator = Any.Int32().Between(0, 100).As(Percentage.Create); - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < 100; i++) { seen.Add(generator.Generate().Value); } diff --git a/JustDummies.UnitTests/ConcurrentDrawTests.cs b/JustDummies.UnitTests/ConcurrentDrawTests.cs index f7249c20..8652627d 100644 --- a/JustDummies.UnitTests/ConcurrentDrawTests.cs +++ b/JustDummies.UnitTests/ConcurrentDrawTests.cs @@ -42,7 +42,7 @@ public sealed class ConcurrentDrawTests { /// Runs on every thread at once and collects everything it produced. private static List Storm(Func draw) { - ConcurrentBag drawn = new(); + ConcurrentBag drawn = []; Parallel.For(0, Threads, new ParallelOptions { MaxDegreeOfParallelism = Threads }, _ => { for (int index = 0; index < DrawsPerThread; index++) { drawn.Add(draw()); } diff --git a/JustDummies.UnitTests/ConstraintRedeclarationTests.cs b/JustDummies.UnitTests/ConstraintRedeclarationTests.cs index d470a995..3783a0aa 100644 --- a/JustDummies.UnitTests/ConstraintRedeclarationTests.cs +++ b/JustDummies.UnitTests/ConstraintRedeclarationTests.cs @@ -82,7 +82,7 @@ public void IdenticalRedeclarationIsANoOp() { // A constraint declared twice with the same argument is not a contradiction: the domain the second declaration // asks for is exactly the one the first already produced. Refusing it made the fluent reject a specification it // can satisfy — the one thing the eager check exists to avoid. - List refused = new(); + List refused = []; foreach ((string label, Func redeclare) in IdenticalRedeclarations()) { try { redeclare(); @@ -99,7 +99,7 @@ public void IdenticalRedeclarationIsANoOp() { public void ContradictoryRedeclarationStillConflicts() { // The other half, and the reason the fix compares the rendered declaration rather than simply dropping the // guard: tolerating an identical re-declaration must not tolerate a contradictory one. - List accepted = new(); + List accepted = []; foreach ((string label, Func contradict) in ContradictoryRedeclarations()) { try { contradict(); diff --git a/JustDummies.UnitTests/ContinuousExclusionNudgeTests.cs b/JustDummies.UnitTests/ContinuousExclusionNudgeTests.cs index 4683e0b7..fee90a4b 100644 --- a/JustDummies.UnitTests/ContinuousExclusionNudgeTests.cs +++ b/JustDummies.UnitTests/ContinuousExclusionNudgeTests.cs @@ -98,7 +98,7 @@ public void ExhaustedNudgeDoesNotClaimAnEmptyRange() { double max = 1d; for (int step = 0; step < 400; step++) { max = Math.BitIncrement(max); } - List excluded = new(); + List excluded = []; double value = Math.BitIncrement(min); for (int step = 0; step < 399; step++) { excluded.Add(value); diff --git a/JustDummies.UnitTests/CrossEngineReachabilityTests.cs b/JustDummies.UnitTests/CrossEngineReachabilityTests.cs index 7df2da8d..cd2aaf4e 100644 --- a/JustDummies.UnitTests/CrossEngineReachabilityTests.cs +++ b/JustDummies.UnitTests/CrossEngineReachabilityTests.cs @@ -198,7 +198,7 @@ public sealed class CrossEngineReachabilityTests { /// The builder names, one theory row each — a serializable key so every builder is an isolated test. public static TheoryData CaseNames() { - TheoryData data = new(); + TheoryData data = []; foreach (ReachabilityCase one in AllCases) { data.Add(one.Name); } return data; @@ -420,7 +420,7 @@ public override void ContradictoryConstraintsNameBothSides() { private (double Min, double Max, HashSet Seen) Sample(IAny generator, int count) { double min = double.PositiveInfinity; double max = double.NegativeInfinity; - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < count; i++) { T value = generator.Generate(); diff --git a/JustDummies.UnitTests/JustDummies.UnitTests.csproj b/JustDummies.UnitTests/JustDummies.UnitTests.csproj index a947a983..169c294e 100644 --- a/JustDummies.UnitTests/JustDummies.UnitTests.csproj +++ b/JustDummies.UnitTests/JustDummies.UnitTests.csproj @@ -18,6 +18,14 @@ methods, so supplying it would mean shadowing System.ArgumentNullException itself. Suppressed for as long as this project targets below .NET 6; see ADR-0058. --> $(NoWarn);CA1510 + + + $(NoWarn);SYSLIB1045 diff --git a/JustDummies.UnitTests/MaterializationTests.cs b/JustDummies.UnitTests/MaterializationTests.cs index 7e7b171b..cef94063 100644 --- a/JustDummies.UnitTests/MaterializationTests.cs +++ b/JustDummies.UnitTests/MaterializationTests.cs @@ -48,7 +48,7 @@ static int Measure(string text) { public void EachGenerateDrawsAFreshValue() { AnyInt32 generator = Any.Int32().Between(0, int.MaxValue); - HashSet seen = new(); + HashSet seen = []; for (int i = 0; i < 20; i++) { seen.Add(generator.Generate()); } diff --git a/JustDummies.UnitTests/SurfaceParityTests.cs b/JustDummies.UnitTests/SurfaceParityTests.cs index 8df1c76b..d6306dcf 100644 --- a/JustDummies.UnitTests/SurfaceParityTests.cs +++ b/JustDummies.UnitTests/SurfaceParityTests.cs @@ -150,49 +150,53 @@ private static string Signature(MethodInfo method) { "Between", "OneOf", "Except", "DifferentFrom", "WithGranularity", "WithOffset", "WithOffsetBetween" ]; - public static IEnumerable Builders() { + public static TheoryData Builders() { + TheoryData data = new(); + // Signed integers carry MultipleOf; the binary floats do not; Decimal carries WithScale; TimeSpan (a signed // magnitude) carries WithGranularity — the lattice constraint is what forks the former shared signed family. - yield return [typeof(AnyInt32), SignedIntegerAlgebra]; - yield return [typeof(AnySByte), SignedIntegerAlgebra]; - yield return [typeof(AnyInt16), SignedIntegerAlgebra]; - yield return [typeof(AnyInt64), SignedIntegerAlgebra]; - yield return [typeof(AnyDouble), FloatingPointAlgebra]; - yield return [typeof(AnySingle), FloatingPointAlgebra]; - yield return [typeof(AnyDecimal), DecimalAlgebra]; - yield return [typeof(AnyTimeSpan), TimeSpanAlgebra]; - - yield return [typeof(AnyByte), UnsignedIntegerAlgebra]; - yield return [typeof(AnyUInt16), UnsignedIntegerAlgebra]; - yield return [typeof(AnyUInt32), UnsignedIntegerAlgebra]; - yield return [typeof(AnyUInt64), UnsignedIntegerAlgebra]; - - yield return [typeof(AnyDateTime), InstantWithGranularityAlgebra]; - yield return [typeof(AnyDateTimeOffset), InstantWithGranularityAndOffsetAlgebra]; + data.Add(typeof(AnyInt32), SignedIntegerAlgebra); + data.Add(typeof(AnySByte), SignedIntegerAlgebra); + data.Add(typeof(AnyInt16), SignedIntegerAlgebra); + data.Add(typeof(AnyInt64), SignedIntegerAlgebra); + data.Add(typeof(AnyDouble), FloatingPointAlgebra); + data.Add(typeof(AnySingle), FloatingPointAlgebra); + data.Add(typeof(AnyDecimal), DecimalAlgebra); + data.Add(typeof(AnyTimeSpan), TimeSpanAlgebra); + + data.Add(typeof(AnyByte), UnsignedIntegerAlgebra); + data.Add(typeof(AnyUInt16), UnsignedIntegerAlgebra); + data.Add(typeof(AnyUInt32), UnsignedIntegerAlgebra); + data.Add(typeof(AnyUInt64), UnsignedIntegerAlgebra); + + data.Add(typeof(AnyDateTime), InstantWithGranularityAlgebra); + data.Add(typeof(AnyDateTimeOffset), InstantWithGranularityAndOffsetAlgebra); // The remaining scalar builders each carry their own deliberate set. - yield return [typeof(AnyBoolean), new[] { "True", "False", "DifferentFrom" }]; - yield return [typeof(AnyGuid), new[] { "NonEmpty", "Empty", "OneOf", "Except", "DifferentFrom" }]; + data.Add(typeof(AnyBoolean), new[] { "True", "False", "DifferentFrom" }); + data.Add(typeof(AnyGuid), new[] { "NonEmpty", "Empty", "OneOf", "Except", "DifferentFrom" }); // AnyEnum adds AllowingCombinations, the opt-in widening the draw from the declared members to their // combinations — meaningful only for a [Flags] enum, hence a constraint rather than a second factory. - yield return [typeof(AnyEnum), new[] { "AllowingCombinations", "OneOf", "Except", "DifferentFrom" }]; - yield return [typeof(AnyChar), new[] { "Alpha", "AlphaNumeric", "Numeric", "UpperCase", "LowerCase", "OneOf", "Except", "DifferentFrom" }]; + data.Add(typeof(AnyEnum), new[] { "AllowingCombinations", "OneOf", "Except", "DifferentFrom" }); + data.Add(typeof(AnyChar), new[] { "Alpha", "AlphaNumeric", "Numeric", "UpperCase", "LowerCase", "OneOf", "Except", "DifferentFrom" }); // AnyString carries the exclusion pair Except/DifferentFrom (met by a bounded redraw, since strings are not // ordinal-mapped) and, like every other family, a composable OneOf that returns the builder itself. - yield return [typeof(AnyString), new[] { + data.Add(typeof(AnyString), new[] { "NonEmpty", "WithLength", "WithMinLength", "WithMaxLength", "WithLengthBetween", "StartingWith", "EndingWith", "Containing", "Alpha", "AlphaNumeric", "Numeric", "WithChars", "UpperCase", "LowerCase", "OneOf", "Except", "DifferentFrom" - }]; + }); #if NET8_0_OR_GREATER - yield return [typeof(AnyInt128), SignedIntegerAlgebra]; - yield return [typeof(AnyHalf), FloatingPointAlgebra]; - yield return [typeof(AnyUInt128), UnsignedIntegerAlgebra]; - yield return [typeof(AnyDateOnly), InstantAlgebra]; - yield return [typeof(AnyTimeOnly), InstantWithGranularityAlgebra]; + data.Add(typeof(AnyInt128), SignedIntegerAlgebra); + data.Add(typeof(AnyHalf), FloatingPointAlgebra); + data.Add(typeof(AnyUInt128), UnsignedIntegerAlgebra); + data.Add(typeof(AnyDateOnly), InstantAlgebra); + data.Add(typeof(AnyTimeOnly), InstantWithGranularityAlgebra); #endif + + return data; } [Theory(DisplayName = "Each builder exposes exactly its family's constraint method set.")] diff --git a/JustDummies.UnitTests/XmlDocCrefConventionTests.cs b/JustDummies.UnitTests/XmlDocCrefConventionTests.cs index 263d5e56..d532a241 100644 --- a/JustDummies.UnitTests/XmlDocCrefConventionTests.cs +++ b/JustDummies.UnitTests/XmlDocCrefConventionTests.cs @@ -76,7 +76,7 @@ public void CrefsSpellPredefinedTypesWithTheirCSharpKeyword() { Check.WithCustomMessage($"Only {files.Count} source file(s) found under the JustDummies projects; the scan lost its target.") .That(files.Count).IsStrictlyGreaterThan(100); - List offenders = new(); + List offenders = []; int scanned = 0; foreach (string file in files) { @@ -111,7 +111,7 @@ public void CrefsInsideTheFactoryHostsNameTheTypeAndNotTheFactory() { Check.WithCustomMessage($"Only {hosts.Count} file(s) declare Any or AnyContext; the scan lost its target.") .That(hosts.Count).IsStrictlyGreaterThan(4); - List offenders = new(); + List offenders = []; foreach (string host in hosts) { foreach (Match cref in CrefAttribute.Matches(File.ReadAllText(host))) { @@ -183,6 +183,11 @@ private static string TypePositions(string cref) { return positions.ToString(); } + [System.Diagnostics.CodeAnalysis.SuppressMessage("Performance", "CA1870:Use a cached 'SearchValues' instance", + Justification = + "SearchValues arrived in .NET 8 and this suite also runs on the .NET Framework 4.7.2 support floor " + + "(ADR-0022), where the type does not exist. IndexOfAny over two characters, run once per cref in a " + + "convention test, is not the cost this rule exists to remove.")] private static string MemberPath(string cref) { int end = cref.IndexOfAny(new[] { '{', '(' }); diff --git a/JustDummies/AnyChar.cs b/JustDummies/AnyChar.cs index 3eb7065a..dfd12e47 100644 --- a/JustDummies/AnyChar.cs +++ b/JustDummies/AnyChar.cs @@ -177,8 +177,7 @@ private AnyChar WithCasing(LetterCasing casing, string applying) { } private AnyChar WithExcluded(char[] values, string applying) { - List excluded = new(_excluded); - excluded.AddRange(values); + List excluded = [.. _excluded, .. values]; return Validated(new AnyChar(_source, _charset, _charsetConstraint, _casing, _casingConstraint, _allowed, _allowedConstraint, excluded), applying); } diff --git a/JustDummies/AnyDateTime.cs b/JustDummies/AnyDateTime.cs index 2281a120..3a2cc528 100644 --- a/JustDummies/AnyDateTime.cs +++ b/JustDummies/AnyDateTime.cs @@ -147,7 +147,7 @@ public AnyDateTime OneOf(params DateTime[] values) { // Remember the supplied values by instant, so generation returns them as given: the ordinal space // only carries the ticks, and rebuilding from it would silently normalize the Kind to Utc. - Dictionary originals = new(); + Dictionary originals = []; foreach (DateTime value in values) { if (!originals.ContainsKey(Ord(value))) { originals.Add(Ord(value), value); } } diff --git a/JustDummies/AnyDateTimeOffset.cs b/JustDummies/AnyDateTimeOffset.cs index 5a6f32c8..799da160 100644 --- a/JustDummies/AnyDateTimeOffset.cs +++ b/JustDummies/AnyDateTimeOffset.cs @@ -219,7 +219,7 @@ public AnyDateTimeOffset OneOf(params DateTimeOffset[] values) { // Remember the supplied values by instant, so generation returns them as given: the ordinal space // only carries the instant, and rebuilding from it would silently normalize the offset to UTC. - Dictionary originals = new(); + Dictionary originals = []; foreach (DateTimeOffset value in admitted) { if (!originals.ContainsKey(Ord(value))) { originals.Add(Ord(value), value); } } diff --git a/JustDummies/AnyEnum.cs b/JustDummies/AnyEnum.cs index 6fcb8078..b4c82d24 100644 --- a/JustDummies/AnyEnum.cs +++ b/JustDummies/AnyEnum.cs @@ -63,7 +63,7 @@ private static TEnum[] Combinations { if (_combinations is not null) { return _combinations; } ulong[] generators = Declared.Select(ToUInt64).Where(bits => bits != 0UL).ToArray(); - HashSet reachable = new(); + HashSet reachable = []; foreach (ulong generator in generators) { // Union of what was reachable, what becomes reachable by adding this generator to it, and the // generator alone — the OR-closure, built without enumerating the 2^k subsets that collapse. @@ -263,8 +263,7 @@ private string DescribeOutsideUniverse() { } private AnyEnum WithExcluded(TEnum[] values, string applying) { - List excluded = new(_excluded); - excluded.AddRange(values); + List excluded = [.. _excluded, .. values]; return Validated(new AnyEnum(_source, _universe, _combinable, _allowed, _allowedConstraint, excluded), applying); } diff --git a/JustDummies/AnyGuid.cs b/JustDummies/AnyGuid.cs index 74edf8b3..66e416e3 100644 --- a/JustDummies/AnyGuid.cs +++ b/JustDummies/AnyGuid.cs @@ -61,7 +61,7 @@ private AnyGuid(RandomSource source, Guid? pinned, string? pinnedConstraint, _excluded = excluded; // Materialized once here — "constrain once, draw many": Generate never refilters the allow-list, and // the exclusion walk tests membership against a set rather than rescanning the list on every step. - _excludedSet = new HashSet(excluded); + _excludedSet = [.. excluded]; _effectiveAllowed = allowed?.Where(value => !_excludedSet.Contains(value)).ToList(); } @@ -161,8 +161,7 @@ public Guid Generate() { } private AnyGuid WithExcluded(Guid[] values, string applying) { - List excluded = new(_excluded); - excluded.AddRange(values); + List excluded = [.. _excluded, .. values]; return Validated(new AnyGuid(_source, _pinned, _pinnedConstraint, _allowed, _allowedConstraint, excluded), applying); } diff --git a/JustDummies/AnyPattern.cs b/JustDummies/AnyPattern.cs index 5587cefa..0eb29659 100644 --- a/JustDummies/AnyPattern.cs +++ b/JustDummies/AnyPattern.cs @@ -187,8 +187,13 @@ public string Generate() { } } + [System.Diagnostics.CodeAnalysis.SuppressMessage("Major Code Smell", "S3267:Loops should be simplified using the \"Where\" LINQ method", + Justification = + "The loop body MUTATES the very list its condition reads: each accepted value is appended to `excluded`, " + + "so a later duplicate in `values` is rejected against the values already taken. A Where clause would read " + + "as a filter over a fixed collection, which is precisely what this is not.")] private AnyPattern Excluding(IReadOnlyList values) { - List excluded = new(_excluded); + List excluded = [.. _excluded]; foreach (string value in values) { if (!excluded.Contains(value, StringComparer.Ordinal)) { excluded.Add(value); } } diff --git a/JustDummies/AnyString.cs b/JustDummies/AnyString.cs index 0372b247..ebd5ada4 100644 --- a/JustDummies/AnyString.cs +++ b/JustDummies/AnyString.cs @@ -94,6 +94,10 @@ internal AnyString(RandomSource source, StringSpec spec) { // generator could produce it is a question, not a boundary violation: the answer is simply no, since a value set // rejects a null element at declaration. The specification's own guard stays the internal boundary (ADR-0045); // this membership answer must not turn a pinned null into an exception the pool generator never raises. + [System.Diagnostics.CodeAnalysis.SuppressMessage("Major Code Smell", "S125:Sections of code should not be commented out", + Justification = + "False positive on prose. The lines above are the explanation of this member's null handling; the rule " + + "reads the trailing \"(ADR-0045);\" of an English sentence as a statement. Nothing here is commented-out code.")] bool ICardinalityHint.Contains(string value) => value is not null && _spec.Contains(value); /// Requires at least one character. diff --git a/JustDummies/CollectionState.cs b/JustDummies/CollectionState.cs index e4d0cd61..d2809e55 100644 --- a/JustDummies/CollectionState.cs +++ b/JustDummies/CollectionState.cs @@ -40,7 +40,7 @@ private static string Elements(int count) { } private static IReadOnlyList Append(IReadOnlyList list, TItem value) { - List copy = new(list) { value }; + List copy = [.. list, value]; return copy; } diff --git a/JustDummies/ContinuousIntervalSpec.cs b/JustDummies/ContinuousIntervalSpec.cs index 26d49a0a..10d7a0f6 100644 --- a/JustDummies/ContinuousIntervalSpec.cs +++ b/JustDummies/ContinuousIntervalSpec.cs @@ -176,7 +176,7 @@ internal ContinuousIntervalSpec WithExcluded(double[] values, string applying) { // The applied constraint tags its own values, so a later exhaustion message can name the exclusion // that actually emptied the domain rather than a bound that merely happens to border it. - List<(string Constraint, double[] Ordinals)> exclusions = new(_exclusions) { (applying, values) }; + List<(string Constraint, double[] Ordinals)> exclusions = [.. _exclusions, (applying, values)]; return Validated(new ContinuousIntervalSpec(_typeName, _render, _quantize, _nextUp, _min, _minConstraint, _max, _maxConstraint, _allowed, _allowedConstraint, exclusions), applying); } @@ -370,7 +370,7 @@ private string DescribeExhaustion(string applying) { /// surviving domain never bit, so naming it would mislead; first-declared order is preserved. /// private IReadOnlyList ExcludingConstraintsInEffect() { - List names = new(); + List names = []; foreach ((string constraint, double[] values) in _exclusions) { if (names.Contains(constraint)) { continue; } if (values.Any(WouldAllowIgnoringExclusions)) { names.Add(constraint); } @@ -399,7 +399,7 @@ private static string Forbids(IReadOnlyList names, string applying) { /// Names the bounds that pinned the domain to its single value, for the "forbids X, the only value ... leaves" form. private string PinningClause() { - List bounds = new(); + List bounds = []; if (_minConstraint is not null) { bounds.Add(_minConstraint); } if (_maxConstraint is not null && _maxConstraint != _minConstraint) { bounds.Add(_maxConstraint); } diff --git a/JustDummies/DecimalIntervalSpec.cs b/JustDummies/DecimalIntervalSpec.cs index e4a575a9..d181ed57 100644 --- a/JustDummies/DecimalIntervalSpec.cs +++ b/JustDummies/DecimalIntervalSpec.cs @@ -165,7 +165,7 @@ internal DecimalIntervalSpec WithExcluded(decimal[] values, string applying) { // The applied constraint tags its own values, so a later exhaustion message can name the exclusion // that actually emptied the domain rather than a bound that merely happens to border it. - List<(string Constraint, decimal[] Ordinals)> exclusions = new(_exclusions) { (applying, values) }; + List<(string Constraint, decimal[] Ordinals)> exclusions = [.. _exclusions, (applying, values)]; return Validated(new DecimalIntervalSpec(_typeName, _render, _min, _minConstraint, _max, _maxConstraint, _allowed, _allowedConstraint, exclusions, _scale, _scaleConstraint), applying); } @@ -434,7 +434,7 @@ private string DescribeExhaustion(string applying) { /// outside the surviving domain never bit, so naming it would mislead; first-declared order is preserved. /// private IReadOnlyList ExcludingConstraintsInEffect() { - List names = new(); + List names = []; foreach ((string constraint, decimal[] values) in _exclusions) { if (names.Contains(constraint)) { continue; } if (values.Any(WouldAllowIgnoringExclusions)) { names.Add(constraint); } @@ -464,7 +464,7 @@ private static string Forbids(IReadOnlyList names, string applying) { /// Names the bounds that pinned the domain to its single value, for the "forbids X, the only value ... leaves" form. private string PinningClause() { - List bounds = new(); + List bounds = []; if (_minConstraint is not null) { bounds.Add(_minConstraint); } if (_maxConstraint is not null && _maxConstraint != _minConstraint) { bounds.Add(_maxConstraint); } diff --git a/JustDummies/OrdinalIntervalSpec.cs b/JustDummies/OrdinalIntervalSpec.cs index 5888542d..09b2fbdb 100644 --- a/JustDummies/OrdinalIntervalSpec.cs +++ b/JustDummies/OrdinalIntervalSpec.cs @@ -152,7 +152,7 @@ private OrdinalIntervalSpec(string typeName, Func render, ulong d } if (allowed is not null) { - HashSet forbidden = new(excluded); + HashSet forbidden = [.. excluded]; _effectiveAllowed = allowed.Where(value => value >= min && value <= max && !forbidden.Contains(value) && (step <= 1UL || IsOnLattice(value, anchor, step))).ToList(); } } @@ -236,7 +236,7 @@ internal OrdinalIntervalSpec WithExcluded(ulong[] ordinals, string applying) { // The applied constraint tags its own ordinals, so a later exhaustion message can name the exclusion // that actually emptied the domain rather than a bound that merely happens to border it. - List<(string Constraint, ulong[] Ordinals)> exclusions = new(_exclusions) { (applying, ordinals) }; + List<(string Constraint, ulong[] Ordinals)> exclusions = [.. _exclusions, (applying, ordinals)]; return Validated(new OrdinalIntervalSpec(_typeName, _render, _domainMin, _domainMax, _min, _minConstraint, _max, _maxConstraint, _allowed, _allowedConstraint, exclusions, _step, _anchor, _stepConstraint), applying); } @@ -428,7 +428,7 @@ private string DescribeExhaustion(string applying) { /// the surviving domain never bit, so naming it would mislead; first-declared order is preserved. /// private IReadOnlyList ExcludingConstraintsInEffect() { - List names = new(); + List names = []; foreach ((string constraint, ulong[] ordinals) in _exclusions) { if (names.Contains(constraint)) { continue; } if (ordinals.Any(WouldAllowIgnoringExclusions)) { names.Add(constraint); } @@ -458,7 +458,7 @@ private static string Forbids(IReadOnlyList names, string applying) { /// Names the bounds that pinned the domain to its single value, for the "forbids X, the only value ... leaves" form. private string PinningClause() { - List bounds = new(); + List bounds = []; if (_minConstraint is not null) { bounds.Add(_minConstraint); } if (_maxConstraint is not null && _maxConstraint != _minConstraint) { bounds.Add(_maxConstraint); } diff --git a/JustDummies/RegexParser.cs b/JustDummies/RegexParser.cs index 8f881339..b7c06aa6 100644 --- a/JustDummies/RegexParser.cs +++ b/JustDummies/RegexParser.cs @@ -61,7 +61,7 @@ private static void AddClassShorthand(HashSet set, char shorthand) { } private static HashSet ExpandCase(HashSet set) { - HashSet expanded = new(); + HashSet expanded = []; foreach (char character in set) { foreach (char variant in RegexAlphabet.WithBothCases(character)) { expanded.Add(variant); } } @@ -120,14 +120,14 @@ private bool Eat(char character) { } private RegexNode ParseAlternation() { - List branches = new() { ParseSequence() }; + List branches = [ParseSequence()]; while (Eat('|')) { branches.Add(ParseSequence()); } return branches.Count == 1 ? branches[0] : new RegexAlternation(branches.ToArray()); } private RegexNode ParseSequence() { - List parts = new(); + List parts = []; while (!AtEnd && Peek() != '|' && Peek() != ')') { // Anchors are no-ops for a whole-string generator, but only where they are guaranteed to match: // '^' at the start and '$' at the end of the pattern or of a top-level alternation branch. A run of @@ -352,7 +352,7 @@ private void ValidateGroupName(string name, int position) { // A '-' marks a balancing group; it manipulates the capture stack (the backreference family), so it is // non-regular and refused here even when its target is undefined (see SkipGroupName for why the divergence // from the real engine's malformed-pattern verdict on that case is accepted). - if (name.Contains("-")) { throw Unsupported("a balancing group '(?…)'", position); } + if (name.Contains('-')) { throw Unsupported("a balancing group '(?…)'", position); } // A name opening with a digit is an explicit capture number: the real engine accepts it only as a positive // integer with no leading zero, so '0', '01' and '1a' are refused while '1' and '10' pass. @@ -445,7 +445,7 @@ private RegexNode ParseClass() { int position = _index; _index++; // consume '[' bool negated = Eat('^'); - HashSet set = new(); + HashSet set = []; bool first = true; while (true) { diff --git a/JustDummies/StringSpec.cs b/JustDummies/StringSpec.cs index d8782883..84ffd1a0 100644 --- a/JustDummies/StringSpec.cs +++ b/JustDummies/StringSpec.cs @@ -221,7 +221,7 @@ internal StringSpec WithSuffix(string suffix, string applying) { internal StringSpec WithFragment(string fragment, string applying) { if (fragment is null) { throw new ArgumentNullException(nameof(fragment)); } if (applying is null) { throw new ArgumentNullException(nameof(applying)); } - List fragments = new(_fragments) { fragment }; + List fragments = [.. _fragments, fragment]; StringSpec candidate = new(_exactLength, _exactConstraint, _minLength, _minConstraint, _maxLength, _maxConstraint, _prefix, _prefixConstraint, _suffix, _suffixConstraint, fragments, @@ -301,7 +301,7 @@ internal StringSpec WithExcluded(IReadOnlyList values, string applying) // The applied constraint tags its own values, so a conflict message can name the exclusion that actually // emptied an allow-list rather than a shape constraint that merely borders it. - List<(string Constraint, string[] Values)> exclusions = new(_exclusions) { (applying, values.ToArray()) }; + List<(string Constraint, string[] Values)> exclusions = [.. _exclusions, (applying, values.ToArray())]; StringSpec candidate = new(_exactLength, _exactConstraint, _minLength, _minConstraint, _maxLength, _maxConstraint, _prefix, _prefixConstraint, _suffix, _suffixConstraint, _fragments, @@ -545,7 +545,7 @@ private string DescribeEmptyPool(string applying, StringSpec previous) { /// could loosen without changing the verdict. /// private IReadOnlyList ConstraintsRejectingAll(IReadOnlyList values) { - List culprits = new(); + List culprits = []; foreach ((string constraint, Func admits) in DeclaredConstraints()) { if (!values.Any(admits)) { culprits.Add(constraint); } } @@ -588,6 +588,11 @@ private Func AdmittedBy(string constraint) { }); } + [System.Diagnostics.CodeAnalysis.SuppressMessage("Usage", "CA2249:Consider using String.Contains instead of String.IndexOf", + Justification = + "string.Contains(string, StringComparison) does not exist on netstandard2.0, which this library targets " + + "(ADR-0022). IndexOf with StringComparison.Ordinal is the same comparison and the only spelling that " + + "compiles on the shipped asset. Same downlevel wall as CA1510 (ADR-0058).")] private IEnumerable<(string Constraint, Func Admits)> Declarations() { if (_exactLength is int exact) { yield return (_exactConstraint!, value => value.Length == exact); } if (_minLength > 0) { yield return (_minConstraint!, value => value.Length >= _minLength); } @@ -621,7 +626,7 @@ private bool Admits(string value) { } private (string Description, bool Several) DescribeFragments() { - List parts = new(); + List parts = []; if (_prefix is not null) { parts.Add($"the prefix \"{_prefix}\""); } foreach (string fragment in _fragments) { parts.Add($"the contained value \"{fragment}\""); } if (_suffix is not null) { parts.Add($"the suffix \"{_suffix}\""); } diff --git a/JustDummies/WideIntervalSpec.cs b/JustDummies/WideIntervalSpec.cs index 35400360..01bac100 100644 --- a/JustDummies/WideIntervalSpec.cs +++ b/JustDummies/WideIntervalSpec.cs @@ -116,7 +116,7 @@ private WideIntervalSpec(string typeName, Func render, UInt128 } if (allowed is not null) { - HashSet forbidden = new(excluded); + HashSet forbidden = [.. excluded]; _effectiveAllowed = allowed.Where(value => value >= min && value <= max && !forbidden.Contains(value) && (step <= UInt128.One || IsOnLattice(value, anchor, step))).ToList(); } } @@ -186,7 +186,7 @@ internal WideIntervalSpec WithExcluded(UInt128[] ordinals, string applying) { // The applied constraint tags its own ordinals, so a later exhaustion message can name the exclusion // that actually emptied the domain rather than a bound that merely happens to border it. - List<(string Constraint, UInt128[] Ordinals)> exclusions = new(_exclusions) { (applying, ordinals) }; + List<(string Constraint, UInt128[] Ordinals)> exclusions = [.. _exclusions, (applying, ordinals)]; return Validated(new WideIntervalSpec(_typeName, _render, _domainMin, _domainMax, _min, _minConstraint, _max, _maxConstraint, _allowed, _allowedConstraint, exclusions, _step, _anchor, _stepConstraint), applying); } @@ -374,7 +374,7 @@ private string DescribeExhaustion(string applying) { /// the surviving domain never bit, so naming it would mislead; first-declared order is preserved. /// private IReadOnlyList ExcludingConstraintsInEffect() { - List names = new(); + List names = []; foreach ((string constraint, UInt128[] ordinals) in _exclusions) { if (names.Contains(constraint)) { continue; } if (ordinals.Any(WouldAllowIgnoringExclusions)) { names.Add(constraint); } @@ -404,7 +404,7 @@ private static string Forbids(IReadOnlyList names, string applying) { /// Names the bounds that pinned the domain to its single value, for the "forbids X, the only value ... leaves" form. private string PinningClause() { - List bounds = new(); + List bounds = []; if (_minConstraint is not null) { bounds.Add(_minConstraint); } if (_maxConstraint is not null && _maxConstraint != _minConstraint) { bounds.Add(_maxConstraint); } diff --git a/doc/handwritten/for-maintainers/adr/0060-let-stated-intent-outrank-generic-analyzer-advice.fr.md b/doc/handwritten/for-maintainers/adr/0060-let-stated-intent-outrank-generic-analyzer-advice.fr.md new file mode 100644 index 00000000..f8dd542f --- /dev/null +++ b/doc/handwritten/for-maintainers/adr/0060-let-stated-intent-outrank-generic-analyzer-advice.fr.md @@ -0,0 +1,224 @@ +# ADR-0060 | Faire primer l'intention énoncée sur le conseil générique d'un analyseur + +🌍 🇫🇷 Français (ce fichier) · 🇬🇧 [English](0060-let-stated-intent-outrank-generic-analyzer-advice.md) + +**Statut :** Proposé +**Proposé :** 2026-07-29 +**Décideurs :** Reefact + +## Contexte + +Le rapport SonarQube Cloud du projet porte 255 constats ouverts. Quatre de ses +règles signalent du code qui n'est pas défectueux mais délibéré, et représentent +ensemble 65 de ces constats. Elles se répartissent en deux familles, selon +l'endroit où le constat est produit. + +Deux arrivent sous l'espace de noms `external_roslyn`, c'est-à-dire qu'elles ne +relèvent pas de l'analyse propre à SonarQube : ce sont des diagnostics émis par +le compilateur .NET et par les analyseurs de la BCL pendant la compilation, que +le scanner observe via MSBuild et republie. Une règle réglée sur `none` n'est +jamais émise ; le rapport la perd donc à la source. Les deux autres relèvent de +l'analyse shell propre à SonarQube, qu'aucun réglage de compilation n'atteint — +rien de ce que fait le compilateur ne les produit ni ne les supprime. + +Les quatre règles, et ce que fait aujourd'hui le code qu'elles signalent : + +* **`CA1859` — 22 constats.** Demande que les membres non publics typés + `IReadOnlyList` ou `IEnumerable` soient retypés vers la collection + concrète qu'on les observe retourner, afin que les appelants fassent un appel + direct plutôt qu'une répartition par interface. La règle ne se déclenche que + sur des membres non publics. Dans le code signalé, l'interface exprime un + contrat : une aide construisant un message d'erreur retourne + `IReadOnlyList` pour que ses appelants ne puissent pas muter le + résultat, et les aides de test prennent `IAny` précisément parce que c'est + l'abstraction publique qui est sous test. +* **`CA1861` — 22 constats, tous dans un projet de test.** Demande qu'un tableau + constant passé en argument soit hissé dans un champ `static readonly`, pour + n'être alloué qu'une fois au lieu d'une fois par appel. Les arguments signalés + sont les valeurs attendues d'assertions et les listes de cas de générateurs de + propriétés, écrites en ligne à côté de la vérification qui les lit. +* **`S7682` — 12 constats**, dans l'outillage shell du dépôt et les *hooks* + Claude. Demande un `return` explicite à la fin d'une fonction shell. Chaque + fonction signalée se termine par la commande dont le code de sortie est le + résultat voulu de la fonction — un `cat` avec document en ligne, un appel à + `awk`, un `printf` — et l'une d'elles se termine par `exit`, après quoi un + `return` est inatteignable. +* **`S7679` — 9 constats**, dans les mêmes scripts. Demande qu'un paramètre + positionnel soit affecté à une variable locale. Tous les scripts du dépôt + déclarent `#!/bin/sh`, et `local` ne fait pas partie de POSIX ; + `tools/trains.sh` montre déjà ce que coûte l'obéissance sans lui, puisque la + seule aide qui y avait besoin de paramètres nommés porte des variables + globales préfixées `_tf_`. Les autres fonctions signalées sont des aides d'une + ou deux lignes dont le `$1` se trouve une ligne sous le nom de la fonction. + +Le code visé par ces règles se trouve sur des chemins de construction d'erreurs, +dans l'outillage de documentation, dans les suites de tests et dans les scripts +du dépôt. Rien de tout cela n'est un chemin chaud mesuré, et aucune exigence de +performance n'est consignée à son encontre. + +Le dépôt porte déjà les deux précédents entre lesquels cette décision s'inscrit. +L'ADR-0055 a établi qu'une règle de style que le compilateur sait exprimer est +redite dans `.editorconfig` et appliquée à la compilation, le fichier DotSettings +restant la référence pour tout ce que Roslyn ne sait pas exprimer. L'ADR-0058 a +décliné `CA1510` et choisi une suppression par projet plutôt qu'à l'échelle du +dépôt, au motif exprès que les projets capables d'honorer une règle doivent la +conserver. Par ailleurs, les règles de codage du dépôt tranchent déjà un +arbitrage performance-contre-invariant en faveur de l'invariant : les objets +valeur et les résultats restent des classes validantes plutôt que des structures, +parce que la correction prime l'allocation sur les chemins d'erreur. + +## Décision + +Le conseil générique d'un analyseur est décliné — par écrit, à côté de sa raison, +et à la portée la plus étroite qui couvre le constat — partout où le code +signalé exprime délibérément une intention, la lisibilité et les contrats énoncés +primant la micro-performance tant qu'aucun besoin mesuré n'est consigné. + +## Justification + +* **Les règles sont génériques ; le code est spécifique.** Chacune des trois est + juste là où elle a été écrite, et fausse ici pour une raison que l'analyseur ne + peut pas voir. `CA1859` ne sait pas distinguer une abstraction fortuite d'un + contrat : elle lit `IReadOnlyList` comme un oubli alors que c'est tout + le propos, et l'honorer offrirait `.Add()` à chaque appelant contre quelques + nanosecondes sur un chemin parcouru une fois par conflit de validation. + `CA1861` ne sait pas distinguer une boucle chaude d'une assertion : elle + éloignerait les valeurs attendues d'un test de la vérification qui les lit, + pour économiser une allocation survenant quelques centaines de fois dans une + suite. Les désactiver n'est pas esquiver le conseil, c'est y répondre. +* **Décliner dans la configuration vaut mieux que décliner dans le rapport.** + Partout où un constat naît de la compilation, une sévérité `none` l'empêche + d'être produit ; là où ce n'est pas le cas — les deux règles shell — c'est la + configuration du scanner qui porte le refus. Dans les deux cas la décision + atterrit dans un fichier qui vit dans le dépôt et porte sa raison en ligne, là + où un « ne sera pas corrigé » sur le serveur SonarQube mettrait le + raisonnement à un endroit que le code ne montre jamais. +* **L'endroit où le refus est écrit suit l'endroit où le constat est produit.** + Les règles Roslyn sont déclinées dans `.editorconfig`, que lit le compilateur : + la compilation cesse de les émettre et chaque contributeur rencontre la raison + là même où la règle se serait déclenchée. Les règles shell ne sont pas + joignables ainsi et sont déclinées dans l'invocation du scanner. Les séparer + n'est pas une incohérence, mais le seul agencement où chaque refus siège là où + vit sa règle. +* **La portée suit la raison, non la commodité.** La justification de `CA1861` + porte sur les tests, et tous ses constats sont dans des projets de test : elle + est donc déclinée pour les projets de test et laissée active pour le code + livré, où un chemin chaud peut réellement la vouloir. `CA1859` et les deux + règles shell sont déclinées sur tout le code qu'elles atteignent, parce que + leurs justifications valent partout où elles se déclenchent. Le principe de l'ADR-0058 — un projet capable + d'honorer une règle la conserve — est ainsi préservé, tout en reconnaissant + qu'ici la raison de décliner est uniforme plutôt qu'un accident de plateforme. +* **Le volet performance est un arbitrage que ce dépôt a déjà rendu.** Les deux + règles de performance réclament la même monnaie : de la lisibilité dépensée + pour une vitesse que personne n'a demandée. La règle des objets valeur en + classes a tranché le même arbitrage dans le même sens. Le décider une fois, de + façon générale, évite de le rejouer à chaque constat. +* **On décline le conseil que le code contredit, pas celui qui coûte cher.** Le + même rapport portait un cinquième candidat, `IDE0028`, avec 147 constats — de + loin le plus gros groupe et le moins cher à faire disparaître. Il est appliqué + au contraire, parce que ses constats signalaient une dérive réelle (le code + écrivait les initialiseurs de collection des deux façons, 85 sites contre 147) + et non un choix délibéré. Le volume n'est pas un argument pour décliner une + règle, et cette ADR ne veut pas être lue comme s'il l'était. + +## Alternatives envisagées + +### Appliquer les quatre règles + +Élimine 65 constats en s'y conformant, et ne laisse aucune suppression à +expliquer. + +Rejetée parce qu'elle inverse le propos de l'exercice. Chacune des quatre +dégraderait le code qu'elle touche : élargir un contrat de lecture seule en +contrat mutable, séparer les données d'un test de l'assertion qui les lit, +ajouter un `return` qui soit masque un échec soit redit le comportement par +défaut, et introduire un `local` non POSIX dans des scripts qui déclarent +`#!/bin/sh`. + +### Supprimer site par site avec `[SuppressMessage]` et une justification + +La portée la plus fine possible, chaque suppression portant sa raison à la ligne +exacte qui l'a levée. + +Rejetée sur le volume, sur le message et sur la portée. Soixante-cinq attributs +ajouteraient plus de lignes que les corrections qu'ils remplacent, répéter un +argument une fois par site l'énonce de nombreuses fois sans jamais l'énoncer une +seule, et les règles shell ne disposent d'aucun mécanisme de ce genre. La raison +est ici une politique, non une exception locale, et une politique tient en un +seul endroit. + +### Marquer les constats « ne sera pas corrigé » dans SonarQube Cloud + +Ne coûte rien dans le dépôt et vide le rapport immédiatement. + +Rejetée parce qu'elle place la décision hors du code. La compilation continuerait +d'émettre les diagnostics, chaque nouvelle occurrence devrait être écartée à la +main, et un contributeur lisant les sources ne trouverait aucune trace du +raisonnement — exactement l'échec que l'ADR-0056 a consigné lorsqu'une règle ne +vivait que là où les lecteurs du code ne pouvaient pas la voir. + +### Décliner `CA1861` à l'échelle du dépôt également + +Plus simple, et symétrique des deux autres. + +Rejetée parce que la justification ne porte pas si loin. L'argument est qu'un +littéral à côté de son assertion est plus clair qu'un champ hissé ; dans du code +livré à l'intérieur d'une boucle, c'est l'argument de la règle qui l'emporte. +La décliner là où elle n'est pas justifiée échangerait une décision précise +contre une décision ordonnée, et retirerait le rappel au seul endroit où il +pourrait compter. + +## Conséquences + +### Positives + +* 65 constats sur 255 disparaissent, et toute occurrence future disparaît avec + eux au lieu de s'accumuler. +* Le raisonnement vit à côté de son effet — dans `.editorconfig` pour les règles + que la compilation produit, dans l'invocation du scanner pour les deux qu'elle + ne produit pas — lisible par quiconque, humain ou agent, édite le dépôt. +* Les deux volets de la politique sont énoncés une fois et peuvent être cités, + si bien que le même argument n'est pas rejoué à chaque nouveau constat. +* `CA1861` reste active là où elle pourrait réellement payer : la décision + conserve donc sa propre porte de sortie. + +### Négatives + +* Plus aucun analyseur n'orientera un chemin livré réellement chaud vers un type + de retour concret, puisque `CA1859` est éteinte partout. Ce jugement repose + désormais entièrement sur l'auteur et le relecteur. +* Quatre règles déclinées, c'est une liste qui peut croître, et elle vit dans + deux fichiers. Chaque ajout exige la même justification, et rien d'autre que la + relecture ne l'impose. + +### Risques + +* S'en tenir au décompte surestime le changement : aucun code n'a été amélioré. + La valeur tient ici à une politique consignée et à un rapport qui ne montre + plus que ce sur quoi il vaut la peine d'agir. +* Un contributeur pourrait lire les sections de règles déclinées comme une + licence d'éteindre tout analyseur gênant. Elles sont bornées à quatre + identifiants de règle nommés, chacun portant sa raison, précisément pour + rendre cette lecture difficile à tenir. +* Si une exigence de performance venait à être consignée sur un chemin couvert + par ces règles, la décision devrait y être réexaminée plutôt que supposée + toujours valide. + +## Actions de suivi + +* Réexaminer la décision sur `CA1859` pour tout chemin de code qui acquerrait une + exigence de performance mesurée. + +## Références + +* ADR-0055 — la redite dans `.editorconfig` des règles de style exprimables par + le compilateur, et leur application à la compilation. +* ADR-0056 — énoncer les règles de codage là où un agent peut s'en saisir, et + pourquoi une décision consignée hors de portée du code ne tient pas. +* ADR-0058 — le refus de `CA1510`, et le principe de portée que cette ADR suit. +* `.editorconfig` — où vivent les trois règles Roslyn déclinées, chacune avec sa + raison. +* `.github/workflows/sonar.yml` — où vivent les deux règles shell déclinées, pour + la même raison et au seul endroit qui puisse la porter. +* `CONTRIBUTING.md`, `CLAUDE.md` — les règles de codage, dont l'arbitrage + « objets valeur en classes » cité dans la Justification. diff --git a/doc/handwritten/for-maintainers/adr/0060-let-stated-intent-outrank-generic-analyzer-advice.md b/doc/handwritten/for-maintainers/adr/0060-let-stated-intent-outrank-generic-analyzer-advice.md new file mode 100644 index 00000000..d78a0f0e --- /dev/null +++ b/doc/handwritten/for-maintainers/adr/0060-let-stated-intent-outrank-generic-analyzer-advice.md @@ -0,0 +1,212 @@ +# ADR-0060 | Let stated intent outrank generic analyzer advice + +🌍 🇬🇧 English (this file) · 🇫🇷 [Français](0060-let-stated-intent-outrank-generic-analyzer-advice.fr.md) + +**Status:** Proposed +**Proposed:** 2026-07-29 +**Decision Makers:** Reefact + +## Context + +The SonarQube Cloud report for this project carries 255 open findings. Four of +its rules flag code that is not defective but deliberate, and together they +account for 65 of those findings. They fall into two families, distinguished by +where the finding is produced. + +Two arrive under the `external_roslyn` namespace, meaning they are not +SonarQube's own analysis: they are diagnostics the .NET compiler and the BCL +analyzers emit during the build, which the scanner observes through MSBuild and +republishes. A rule configured to `none` is never emitted, so the report loses +it at the source. The other two are SonarQube's own shell analysis, which no +build setting can reach — nothing the compiler does produces or suppresses them. + +The four rules, and what the flagged code does today: + +* **`CA1859` — 22 findings.** Asks that non-public members typed + `IReadOnlyList` or `IEnumerable` be retyped to the concrete collection + they are observed to return, so callers make a direct call instead of an + interface dispatch. The rule fires only on non-public members. In the flagged + code the interface expresses a contract — a helper building an error message + returns `IReadOnlyList` so its callers cannot mutate the result, and + the test helpers take `IAny` precisely because the public abstraction is + what is under test. +* **`CA1861` — 22 findings, every one of them in a test project.** Asks that a + constant array passed as an argument be hoisted into a static readonly field, + so it is allocated once rather than per call. The flagged arguments are the + expected values of assertions and the case lists of property generators, + written inline next to the check that reads them. +* **`S7682` — 12 findings**, in the repository's shell tooling and Claude hooks. + Asks for an explicit `return` at the end of a shell function. Every function + it flags ends with the command whose exit status is the function's intended + result — a `cat` heredoc, an `awk` invocation, a `printf` — and one of them + ends with `exit`, after which a `return` is unreachable. +* **`S7679` — 9 findings**, in the same scripts. Asks that a positional + parameter be assigned to a local variable. Every script in the repository + declares `#!/bin/sh`, and `local` is not part of POSIX; `tools/trains.sh` + already shows what obeying costs without it, since the one helper there + needing named parameters carries `_tf_`-prefixed globals instead. The + remaining flagged functions are one- and two-line helpers whose `$1` sits a + line below the function's own name. + +The code these rules flag is on error-construction paths, documentation +tooling, test suites and repository scripts. None of it is a measured hot path, +and no performance requirement is recorded against any of it. + +The repository already holds the two precedents this decision sits between. +ADR-0055 established that a style rule the compiler can express is restated in +`.editorconfig` and enforced at build time, with the DotSettings authoritative +for everything Roslyn cannot express. ADR-0058 declined `CA1510` and chose a +per-project suppression over a repository-wide one, on the express ground that +projects able to honour a rule should keep it. Separately, the repository's +coding rules already resolve one performance-versus-invariant trade in favour +of the invariant: value objects and results stay validating classes rather than +becoming structs, because correctness outranks allocation on error paths. + +## Decision + +Generic analyzer advice is declined — in writing, next to the reason, and at +the narrowest scope that covers the finding — wherever the code it flags is a +deliberate expression of intent, with readability and stated contracts +outranking micro-performance unless a measured need is recorded. + +## Rationale + +* **The rules are generic; the code is specific.** Each of the three is sound + where it was written for, and wrong here for a reason the analyzer cannot + see. `CA1859` cannot tell an incidental abstraction from a contract, so it + reads `IReadOnlyList` as an oversight when it is the whole point: + honouring it would hand `.Add()` to every caller in exchange for nanoseconds + on a path that runs once per validation conflict. `CA1861` cannot tell a hot + loop from an assertion, so it would move the expected values of a test away + from the check that reads them to save an allocation occurring a few hundred + times in a suite. Suppressing them is not evading the advice; it is answering + it. +* **Declining in the configuration beats declining in the report.** Wherever a + finding originates in the build, a severity of `none` stops it being produced + at all; where it does not — the two shell rules — the scanner's own + configuration carries the refusal. Either way the decision lands in a file + that lives in the repository and carries its reason inline, where marking the + findings "won't fix" on the SonarQube server would put the reasoning + somewhere the code never shows it. +* **Where the refusal is written follows where the finding is produced.** The + Roslyn rules are declined in `.editorconfig`, which the compiler reads, so + the build stops emitting them and every contributor meets the reason at the + same place the rule would have fired. The shell rules cannot be reached that + way and are declined in the scanner invocation instead. Splitting them is not + an inconsistency but the only arrangement in which each refusal sits where + its rule lives. +* **Scope follows the reason, not convenience.** `CA1861`'s justification is + about tests, and every one of its findings is in a test project, so it is + declined for test projects and left live for shipping code, where a hot path + can genuinely want it. `CA1859` and the two shell rules are declined across + the code they reach, because their justifications hold everywhere they fire. This keeps ADR-0058's + principle — a project that can honour a rule keeps it — while recognising + that here the reason to decline is uniform rather than a platform accident. +* **The performance limb is a trade this repository has already made.** Both + performance rules ask for the same currency: legibility spent on speed that + nothing has asked for. The value-objects-as-classes rule settled the same + trade the same way. Deciding it once, generally, stops it being re-argued at + each finding. +* **Declining is for advice the code contradicts, not for advice that costs.** + The same report carried a fifth candidate, `IDE0028`, with 147 findings — + by far the largest group and the cheapest to make disappear. It is being + applied instead, because its findings marked a genuine drift (the codebase + spelled collection initializers both ways, 85 sites against 147) rather than + a deliberate choice. Volume is not an argument for declining a rule, and this + ADR does not want to be read as one. + +## Alternatives Considered + +### Apply all four rules + +Clears 65 findings by complying, and leaves no suppression to explain. + +Rejected because it inverts the point of the exercise. Every one of the four +would degrade the code it touches: widening a read-only contract into a mutable +one, separating test data from the assertion that reads it, adding a `return` +that either masks a failure or restates the default, and introducing a +non-POSIX `local` into scripts that declare `#!/bin/sh`. + +### Suppress per site with `[SuppressMessage]` and a justification + +The finest possible scope, and each suppression carries its reason at the exact +line that raised it. + +Rejected on volume, on message, and on reach. Sixty-five attributes would add +more lines than the fixes they replace, repeating one argument once per site +states it many times without ever stating it once, and the shell rules have no +such mechanism available at all. The reason here is a policy, not a local +exception, and a policy belongs in one place. + +### Mark the findings "won't fix" in SonarQube Cloud + +Costs nothing in the repository and clears the report immediately. + +Rejected because it puts the decision outside the code. The build would keep +emitting the diagnostics, every new occurrence would have to be dismissed by +hand, and a contributor reading the source would find no trace of the reasoning +— exactly the failure ADR-0056 recorded when a rule lived only where the code's +readers could not see it. + +### Decline `CA1861` repository-wide as well + +Simpler and symmetrical with the other two. + +Rejected because the justification does not reach that far. The argument is +that a literal beside its assertion is clearer than a hoisted field; in +shipping code inside a loop, the rule's own argument wins instead. Declining it +where it is not justified would trade a precise decision for a tidy one, and +would remove the nudge in the only place it could matter. + +## Consequences + +### Positive + +* 65 of 255 findings clear, and every future occurrence clears with them + rather than accumulating. +* The reasoning lives beside the effect — in `.editorconfig` for the rules the + build produces, in the scanner invocation for the two it does not — readable + by anyone, human or agent, editing the repository. +* The two limbs of the policy are stated once and can be cited, so the same + argument is not re-run at each new analyzer finding. +* `CA1861` remains live where it could genuinely pay, so the decision keeps its + own escape hatch. + +### Negative + +* No analyzer will nudge a genuinely hot shipping path toward a concrete return + type any more, since `CA1859` is off everywhere. That judgement now rests + entirely with the author and the reviewer. +* Four declined rules is a list that can grow, and it lives in two files. Each + addition needs the same justification, and nothing but review enforces that. + +### Risks + +* Reading the count alone overstates the change: no code improved. The value + here is a recorded policy and a report that shows only what is worth acting + on. +* A contributor may read the declined-rules sections as licence to switch off + any inconvenient analyzer. They are scoped to four named rule ids, each + carrying its reason, precisely so that reading is hard to sustain. +* If a performance requirement is ever recorded against a path these rules + cover, the decision has to be revisited there rather than assumed still + valid. + +## Follow-up Actions + +* Re-examine the `CA1859` decision for any code path that acquires a measured + performance requirement. + +## References + +* ADR-0055 — restating the compiler-expressible style rules in `.editorconfig` + and enforcing them at build time. +* ADR-0056 — stating the coding rules where an agent can act on them, the + reason a decision recorded out of the code's reach does not hold. +* ADR-0058 — declining `CA1510`, and the scoping principle this ADR follows. +* `.editorconfig` — where the three declined Roslyn rules live, each with its + reason. +* `.github/workflows/sonar.yml` — where the two declined shell rules live, for + the same reason and in the only place that can carry it. +* `CONTRIBUTING.md`, `CLAUDE.md` — the coding rules, including the + value-objects-as-classes trade cited in the Rationale. diff --git a/doc/handwritten/for-maintainers/adr/README.md b/doc/handwritten/for-maintainers/adr/README.md index 56b8c80b..e311b71d 100644 --- a/doc/handwritten/for-maintainers/adr/README.md +++ b/doc/handwritten/for-maintainers/adr/README.md @@ -261,3 +261,4 @@ Optional supporting material: | [ADR-0057](0057-keep-one-dated-line-per-state-an-adr-reached.md) | Keep one dated line per state an ADR reached, and never overwrite one | Accepted | | [ADR-0058](0058-suppress-ca1510-while-the-netstandard-floor-stands.md) | Suppress CA1510 while the pre-.NET-6 floor stands | Accepted | | [ADR-0059](0059-guard-the-recipe-versus-value-boundary-with-analyzers.md) | Guard the recipe-versus-value boundary with analyzers where the type system cannot reach it | Proposed | +| [ADR-0060](0060-let-stated-intent-outrank-generic-analyzer-advice.md) | Let stated intent outrank generic analyzer advice, and record the refusal beside the rule | Proposed | diff --git a/tools/dependabot-autofix/apply-fix.sh b/tools/dependabot-autofix/apply-fix.sh index 19cc34ab..bf10cd5f 100755 --- a/tools/dependabot-autofix/apply-fix.sh +++ b/tools/dependabot-autofix/apply-fix.sh @@ -25,6 +25,9 @@ out="${GITHUB_OUTPUT:-/dev/stdout}" report() { printf '%s\n' "$@" >> "$out"; } skip() { echo "apply-fix: skipping — $1"; report "applied=false" "reason=$1"; exit 0; } +# The counterpart to skip: every outcome the workflow reads goes out through one of the two, +# so `applied` is written in exactly two places rather than once per branch of the case. +applied() { report "applied=true" "impact=$1" "summary=$2"; } action="$(jq -r '.action // "none"' "$verdict")" [ "$action" = "none" ] && skip "no actionable fix" @@ -45,7 +48,7 @@ case "$action" in title="$(jq -r '.pr_title // ""' "$verdict")" [ -z "$title" ] && skip "retitle_pr without a pr_title" gh pr edit "$pr" --repo "$repo" --title "$title" || skip "gh pr edit failed" - report "applied=true" "impact=trivial" "summary=retitled the pull request" + applied trivial "retitled the pull request" ;; rewrite_commit_message) @@ -53,7 +56,7 @@ case "$action" in [ -s .da-msg.txt ] || skip "rewrite_commit_message without a commit_message" git commit --amend -F .da-msg.txt || skip "git commit --amend failed" git push --force-with-lease origin "HEAD:${branch}" || skip "force-push failed" - report "applied=true" "impact=trivial" "summary=rewrote the commit header" + applied trivial "rewrote the commit header" ;; rebase) @@ -63,7 +66,7 @@ case "$action" in skip "rebase onto main hit conflicts (needs a human)" fi git push --force-with-lease origin "HEAD:${branch}" || skip "force-push failed" - report "applied=true" "impact=trivial" "summary=rebased onto main" + applied trivial "rebased onto main" ;; apply_patch) @@ -77,7 +80,7 @@ case "$action" in git commit -F .da-msg.txt || skip "git commit failed" # A new commit on top of Dependabot's: a fast-forward, no force needed. git push origin "HEAD:${branch}" || skip "push failed" - report "applied=true" "impact=code" "summary=applied a code patch" + applied code "applied a code patch" ;; *)