Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@
# 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.
#
# It is also where the EXCEPTIONS to the Sonar rules are written. build/sonar-profile.globalconfig
# is generated from the SonarCloud quality profile and puts every rule it activates at `warning`,
# so the default is enforce and the CI ratchet does the rest. This file overrides it, and only
# ever downwards: `suggestion` for a rule whose existing violations are not cleared yet, `none`
# for one this codebase refuses, with its reason. Membership is generated; every exception is
# written here, and the backlog below is a list that shrinks by deletion. Full procedure:
# doc/handwritten/for-maintainers/workflows/sonar-profile.en.md (decision: ADR-0062).
#
# charset is deliberately left unset: the repository mixes BOM and BOM-less files,
# and pinning it here would rewrite existing files.

Expand Down Expand Up @@ -58,6 +66,56 @@ dotnet_diagnostic.IDE0008.severity = warning
# objects as validating classes rather than structs.
dotnet_diagnostic.CA1859.severity = none

# --- Sonar rules not yet enforced: the backlog -------------------------------------------------
#
# build/sonar-profile.globalconfig is generated from the SonarCloud quality profile and puts every
# rule it activates at `warning`, so the default is ENFORCE. The 33 rules below are the
# exceptions: each still has violations in this tree, and promoting it now would turn unrelated
# pull requests red. They are demoted to `suggestion` — active, non-blocking — with the number of
# sites measured when this landed.
#
# This block IS the backlog, and it shrinks by DELETION: clear a rule's sites, delete its line,
# and the generated file enforces it from the next build with nothing further to write. A rule
# this codebase means to refuse outright does not belong here — it belongs with the declines
# above, at `none`, with its reason (ADR-0060). `suggestion` means "not yet", never "no".
#
# The other 342 rules the profile activates have zero violations here and are
# enforced as of this commit. Total outstanding: 135 sites. Decision: ADR-0062.

dotnet_diagnostic.S3776.severity = suggestion # 19 — Cognitive Complexity of methods should not be too high
dotnet_diagnostic.S1244.severity = suggestion # 15 — Floating point numbers should not be tested for equality
dotnet_diagnostic.S3267.severity = suggestion # 14 — Loops should be simplified with "LINQ" expressions
dotnet_diagnostic.S3878.severity = suggestion # 14 — Arrays should not be created for params parameters
dotnet_diagnostic.S8969.severity = suggestion # 14 — Null-forgiving operators should not be redundant
dotnet_diagnostic.S3218.severity = suggestion # 8 — Inner class members should not shadow outer class "static" or type members
dotnet_diagnostic.S107.severity = suggestion # 6 — Methods should not have too many parameters
dotnet_diagnostic.S1481.severity = suggestion # 5 — Unused local variables should be removed
dotnet_diagnostic.S1854.severity = suggestion # 4 — Unused assignments should be removed
dotnet_diagnostic.S4144.severity = suggestion # 3 — Methods should not have identical implementations
dotnet_diagnostic.S1066.severity = suggestion # 2 — Mergeable "if" statements should be combined
dotnet_diagnostic.S108.severity = suggestion # 2 — Nested blocks of code should not be left empty
dotnet_diagnostic.S1192.severity = suggestion # 2 — String literals should not be duplicated
dotnet_diagnostic.S125.severity = suggestion # 2 — Sections of code should not be commented out
dotnet_diagnostic.S1905.severity = suggestion # 2 — Redundant casts should not be used
dotnet_diagnostic.S2326.severity = suggestion # 2 — Unused type parameters should be removed
dotnet_diagnostic.S3220.severity = suggestion # 2 — Method calls should not resolve ambiguously to overloads with "params"
dotnet_diagnostic.S3358.severity = suggestion # 2 — Ternary operators should not be nested
dotnet_diagnostic.S6966.severity = suggestion # 2 — Awaitable method should be used
dotnet_diagnostic.S927.severity = suggestion # 2 — Parameter names should match base declaration and other partial definitions
dotnet_diagnostic.S1144.severity = suggestion # 1 — Unused private types or members should be removed
dotnet_diagnostic.S1172.severity = suggestion # 1 — Unused method parameters should be removed
dotnet_diagnostic.S2219.severity = suggestion # 1 — Runtime type checking should be simplified
dotnet_diagnostic.S2342.severity = suggestion # 1 — Enumeration types should comply with a naming convention
dotnet_diagnostic.S2692.severity = suggestion # 1 — "IndexOf" checks should not be for positive numbers
dotnet_diagnostic.S3376.severity = suggestion # 1 — Attribute, EventArgs, and Exception type names should end with the type being extended
dotnet_diagnostic.S3459.severity = suggestion # 1 — Unassigned members should be removed
dotnet_diagnostic.S3871.severity = suggestion # 1 — Exception types should be "public"
dotnet_diagnostic.S3877.severity = suggestion # 1 — Exceptions should not be thrown from unexpected methods
dotnet_diagnostic.S3881.severity = suggestion # 1 — "IDisposable" should be implemented correctly
dotnet_diagnostic.S4136.severity = suggestion # 1 — Method overloads should be grouped together
dotnet_diagnostic.S6580.severity = suggestion # 1 — Use a format provider when parsing date and time
dotnet_diagnostic.S6608.severity = suggestion # 1 — Prefer indexing instead of "Enumerable" methods on types implementing "IList"

# 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]
Expand Down
79 changes: 79 additions & 0 deletions .github/workflows/sonar-profile.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
name: sonar-profile

# Watches the SonarCloud C# quality profile and reports when the repository has drifted from it.
#
# Why this exists. The rules the report is scored against live on the server, and nothing in the
# repository used to know what they were: the SonarAnalyzer NuGet package ships its own, narrower
# default set — measured, it leaves S3776 and S1192 disabled although the profile activates them —
# so the build and the report were talking about different rules. build/sonar-profile.globalconfig
# is that list, written down, generated by tools/sonar-profile/sync-profile.sh. A list written down
# once starts rotting the moment somebody edits the profile, and nobody would notice. This job is
# what notices.
#
# It REPORTS, it does not repair. The run fails with the diff, and a human regenerates the file.
# That is deliberate: the alternative is a scheduled job holding write access to the very file that
# governs which rules block a merge, which is the shape a workflow-security audit flagged twice on
# this repository already. Promoting it to open a pull request is a small change if the trade is
# ever judged worth it; see the workflow reference.
#
# A hand-edit of the generated file is caught by exactly the same mechanism, because a hand-edit is
# drift. There is no separate guard for it and none is needed.

on:
schedule:
# Weekly, not nightly. The profile is SonarSource's built-in "Sonar way" — measured,
# isBuiltIn is true and userUpdatedAt is null, so nobody here has ever edited it and nobody
# can. Drift therefore arrives with an analyzer release, a handful of times a year, and a
# nightly would ask a question whose answer changes on a vendor's cadence. Monday 05:47 UTC,
# after the mutation sweeps (03:23, 03:47) and Scorecard (05:23), before CodeQL (06:17).
- cron: '47 5 * * 1'
workflow_dispatch:

# Cancel superseded runs (a dispatch on top of a scheduled one).
concurrency:
group: sonar-profile-${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true

jobs:
drift:
name: Quality profile drift
runs-on: ubuntu-latest
# One checkout and two API calls; the cap only guards a hung request.
timeout-minutes: 10
# Declared per job so a job added later inherits nothing it did not ask for (Sonar
# githubactions:S8264). This job reads the checkout and calls a public API — nothing else.
permissions:
contents: read
steps:
- name: Checkout
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7

- name: Compare the committed rule list against the quality profile
# The project is public, so the API answers unauthenticated and this job needs no secret.
# SONAR_TOKEN is passed anyway, from the same secret sonar.yml uses: the day the project
# stops being public, this keeps working instead of failing on a 403. A missing secret is
# an empty string, which the script treats as "unauthenticated".
env:
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
run: tools/sonar-profile/sync-profile.sh --check

- name: Say what to do about it
# Only on failure, and only as a pointer — the diff itself is already in the log above.
if: failure()
run: |
# shellcheck disable=SC2016 # the backticks are Markdown for the step summary, not command substitution
{
echo '### SonarCloud quality profile drift'
echo
echo 'The C# quality profile no longer matches `build/sonar-profile.globalconfig`.'
echo 'The diff is in the previous step.'
echo
echo 'To resolve:'
echo
echo '1. Run `tools/sonar-profile/sync-profile.sh` and commit the regenerated file.'
echo '2. A rule the profile ADDED lands at `suggestion` — visible, not blocking.'
echo ' Nothing else is required to merge.'
echo '3. Promote it in `.editorconfig` to `warning` only once its existing violations'
echo ' are cleared, or refuse it with `none` and a reason. That decision is a'
echo ' separate pull request, and it is the only thing that makes a rule block.'
} >> "$GITHUB_STEP_SUMMARY"
34 changes: 34 additions & 0 deletions Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,38 @@
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
</PropertyGroup>

<!--
The Sonar C# rules, brought forward from the report to the build. Until now they existed only
inside the scanner-hooked compilation in .github/workflows/sonar.yml, so a contributor — human
or agent — met them after the merge, in a report, and never while writing the code. That is the
same failure ADR-0055 and ADR-0056 recorded about the DotSettings, and it is how 147 IDE0028
accumulated unseen.

The analyzer is a build-time asset only: PrivateAssets="all" keeps it out of every published
package's dependency graph. The ItemGroup is conditioned on central package management because
tools/floor-check/FloorCheck.csproj deliberately opts out of it (see its own file) and a
version-less PackageReference would fail to restore there.

build/sonar-profile.globalconfig is GENERATED by tools/sonar-profile/sync-profile.sh from the
project's SonarCloud quality profile, and lists every rule that profile activates. The package
alone is NOT the profile — measured, its default set leaves S3776 and
S1192 disabled although the profile activates them — so reading the profile is the only way the
build and the report can be talking about the same rules.

The default is ENFORCE: the generated file puts every rule at `warning`, which the ratchet above
turns into an error in CI. `suggestion` was measured as the default and rejected — at that
severity a Sonar diagnostic prints nothing in `dotnet build` at any verbosity, so the list would
have been invisible to the reader it exists for.

The EXCEPTIONS are in .editorconfig, which wins over a global AnalyzerConfig: `suggestion` for a
rule whose existing violations are not cleared yet (with its count), `none` for one this
codebase refuses (with its reason). 342 of the 375 rules had zero violations and are enforced as
of this commit; the 33 that did not are parked there and named. Membership is generated; every
exception is written down. Decision: ADR-0062.
-->
<ItemGroup Condition="'$(ManagePackageVersionsCentrally)' == 'true'">
<PackageReference Include="SonarAnalyzer.CSharp" PrivateAssets="all" />
<GlobalAnalyzerConfigFiles Include="$(MSBuildThisFileDirectory)build/sonar-profile.globalconfig" />
</ItemGroup>

</Project>
1 change: 1 addition & 0 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
<PackageVersion Include="Microsoft.Sbom.Targets" Version="4.1.5" />
<PackageVersion Include="NFluent" Version="3.1.0" />
<PackageVersion Include="NSubstitute" Version="6.0.0" />
<PackageVersion Include="SonarAnalyzer.CSharp" Version="10.31.0.145097" />
<PackageVersion Include="Spectre.Console.Cli" Version="0.55.0" />
<PackageVersion Include="Verify.XunitV3" Version="31.27.0" />
<PackageVersion Include="xunit.runner.visualstudio" Version="3.1.5" />
Expand Down
8 changes: 8 additions & 0 deletions build/Net472TestFloor.IsExternalInit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@
// ReSharper disable once CheckNamespace -- the compiler resolves this type by its exact fully-qualified name.
namespace System.Runtime.CompilerServices {

// Empty BY CONTRACT. The compiler looks this type up by name and never reads a member, so there is
// nothing to write and it cannot become an interface: it must be a class the compiler can bind to.
// Sonar's S2094 only fires on the net472 inner build, which is the only one that compiles this file.
[System.Diagnostics.CodeAnalysis.SuppressMessage("Minor Code Smell", "S2094:Classes should not be empty",
Justification =
"A compiler-recognised marker type. Its emptiness is the specification: the compiler binds " +
"System.Runtime.CompilerServices.IsExternalInit by name to enable `init` accessors and records, " +
"and reads nothing from it. It cannot be an interface, and giving it a member would be noise.")]
internal static class IsExternalInit { }

}
Loading
Loading