Answer the remaining SonarQube findings, rule by rule

.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<T>` / `IEnumerable<T>` 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
 

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

.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

.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

FirstClassErrors.Analyzers.UnitTests/AnalyzerTestHarness.cs

@@ -71,7 +71,7 @@ private static async Task<ImmutableArray<Diagnostic>> RunAsync(
     }
 
     private static ImmutableArray<MetadataReference> BuildBaseReferences() {
-        List<MetadataReference> references = new();
+        List<MetadataReference> 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<MetadataReference> BuildBaseReferences() {
     }
 
     private static ImmutableArray<MetadataReference> BuildNet472References() {
-        List<MetadataReference> references = new();
+        List<MetadataReference> 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.

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<Location>())
+            usagesByCodeField.GetOrAdd(codeField!, _ => [])
                              .Add(method.Locations.FirstOrDefault() ?? Location.None);
         }
     }

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<Location>()).Add(argument.Syntax.GetLocation());
+        occurrences.GetOrAdd(code, _ => []).Add(argument.Syntax.GetLocation());
     }
 
     private static void Report(

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<AttributeData>? references)) {
-                references                     = new List<AttributeData>();
+                references                     = [];
                 referencesByTarget[targetName!] = references;
             }
 

FirstClassErrors.Cli.UnitTests/TestDoubles.cs

@@ -157,10 +157,10 @@ public CatalogSnapshot Extract(CatalogSettings settings, CliConfiguration config
 /// <summary>A logger that records each message per level, so tests can assert what the command reported.</summary>
 internal sealed class RecordingLogger : IGenerationLogger {
 
-    public List<string> Infos    { get; } = new();
-    public List<string> Warnings { get; } = new();
-    public List<string> Errors   { get; } = new();
-    public List<string> Debugs   { get; } = new();
+    public List<string> Infos    { get; } = [];
+    public List<string> Warnings { get; } = [];
+    public List<string> Errors   { get; } = [];
+    public List<string> 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 {
 /// <summary>An output sink that records what would have been written, instead of touching the console or disk.</summary>
 internal sealed class RecordingOutputSink : IOutputSink {
 
-    public List<string>               StandardOutput { get; } = new();
-    public Dictionary<string, string> Files          { get; } = new();
+    public List<string>               StandardOutput { get; } = [];
+    public Dictionary<string, string> Files          { get; } = [];
 
     public void WriteStandardOutput(string content) {
         StandardOutput.Add(content);

FirstClassErrors.Cli/CatalogDiffCommand.cs

@@ -40,6 +40,14 @@ internal sealed class CatalogDiffSettings : CatalogSettings {
 /// </remarks>
 internal sealed class CatalogDiffCommand : Command<CatalogDiffSettings> {
 
+    #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'.");