From be9b7474bea06858c3829ca54b45b612d5210453 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Jul 2026 07:45:30 +0000 Subject: [PATCH 1/3] Treat warnings as errors on CI builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The codebase currently builds with zero warnings. Lock that state in as a ratchet: on CI, every warning becomes an error, so a new warning can never merge and the count can never silently creep up. * Directory.Build.props: TreatWarningsAsErrors=true conditioned on GITHUB_ACTIONS — the same CI signal already used for ContinuousIntegrationBuild — so every CI build (ci.yml both OS legs, sonar.yml, analyzers.yml) is covered with zero workflow edits, while local builds stay friendly to iteration: a half-finished refactoring may warn without blocking the inner loop. * NuGet security-audit advisories (NU1901-NU1905) are kept as warnings (WarningsNotAsErrors): a CVE published overnight against a dependency would otherwise turn every PR red without any code change. The advisory still shows in logs; Dependabot and CodeQL are the channels that act on it. * tools/floor-check/FloorCheck.csproj opts out (TreatWarningsAsErrors=false): it builds under the OLD floor SDK, whose toolchain may emit warnings the .NET 10 legs never see, and its failure signal must stay narrow — analyzer loadability (the targeted CS8032/AD0001 escalation is unchanged). Verified locally: the solution builds green with GITHUB_ACTIONS=true; a planted #warning fails the CI-simulated build (error CS1030) and merely warns locally; the same planted warning in Usage sources leaves the floor-check build green (opt-out effective); the full test suite passes under the CI-simulated build (559 tests, 4 projects). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AbXi2ZEaUFeFWodNxdY6Zk --- Directory.Build.props | 15 +++++++++++++++ tools/floor-check/FloorCheck.csproj | 5 +++++ 2 files changed, 20 insertions(+) diff --git a/Directory.Build.props b/Directory.Build.props index 087ff2c..62a38a0 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -22,6 +22,21 @@ true + + + true + + $(WarningsNotAsErrors);NU1901;NU1902;NU1903;NU1904;NU1905 + + $(WarningsAsErrors);CS8032;AD0001 + + false From c06c4f23b80c89fa92157d5faae9dd75102a7064 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Jul 2026 08:03:08 +0000 Subject: [PATCH 2/3] Ratchet MSBuild/SDK task warnings too, not only compiler warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review (P2): TreatWarningsAsErrors is compiler-scoped — it promotes CS* diagnostics only. Warnings raised by MSBuild/SDK tasks and pack targets (MSB*, NETSDK*, ...) were left able to merge despite the zero-warning ratchet. Add MSBuildTreatWarningsAsErrors=true (CI only) alongside it so both classes are promoted, and mirror the opt-out in tools/floor-check with both switches off. The NU security-audit exclusion stays in WarningsNotAsErrors and is NOT moved to MSBuildWarningsNotAsErrors — verified with a real vulnerable package (System.Net.Http 4.3.0 -> NU1903): NuGet honours TreatWarningsAsErrors for its restore warnings, WarningsNotAsErrors suppresses the escalation, and MSBuildWarningsNotAsErrors does NOT. So the earlier exclusion was necessary, not a no-op, and remains the correct lever. Verified locally on the .NET 10 SDK: * a planted MSBuild task warning (Warning task, code MSB9999) now fails the CI-simulated build (error MSB9999) and merely warns in a plain local build; * the full solution still builds green under GITHUB_ACTIONS=true (0 warnings); * the floor-check build stays green under GITHUB_ACTIONS=true with the same planted MSBuild warning present (both opt-out switches effective). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AbXi2ZEaUFeFWodNxdY6Zk --- Directory.Build.props | 12 ++++++++++-- tools/floor-check/FloorCheck.csproj | 10 ++++++---- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index 62a38a0..2efe55e 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -30,10 +30,18 @@ tools/floor-check/FloorCheck.csproj opts back out — its failure signal is deliberately narrow. --> + true + true + against a dependency would otherwise turn every PR red without any code change (NuGet honours + TreatWarningsAsErrors for its own restore warnings, so they ARE promoted by default). The + advisory still shows in logs; Dependabot and CodeQL are the channels that act on it. This + exclusion MUST live in WarningsNotAsErrors, not MSBuildWarningsNotAsErrors: NuGet reads the + compiler-scoped list — the MSBuild-scoped one does not suppress NU* audit errors (verified). --> $(WarningsNotAsErrors);NU1901;NU1902;NU1903;NU1904;NU1905 diff --git a/tools/floor-check/FloorCheck.csproj b/tools/floor-check/FloorCheck.csproj index 635ed67..f0138a5 100644 --- a/tools/floor-check/FloorCheck.csproj +++ b/tools/floor-check/FloorCheck.csproj @@ -22,11 +22,13 @@ $(WarningsAsErrors);CS8032;AD0001 - + false + false From c0d0272b62d605713bb1645d1b6a9cf25889d1fc Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Jul 2026 08:19:34 +0000 Subject: [PATCH 3/3] Disable the CI warning ratchet for the Sonar analysis build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex review (P2): the SonarScanner build (between `begin` and `end`) must complete so the scanner can collect and upload SonarAnalyzer diagnostics. The scanner neutralises TreatWarningsAsErrors, but not MSBuildTreatWarningsAsErrors — and that switch promotes compiler/analyzer warnings too, not only MSBuild task warnings. So a Sonar rule warning surfaced during the scan build would be turned into an error and fail the build before `end` reports results. Verified: with MSBuildTreatWarningsAsErrors=true and TreatWarningsAsErrors=false (the exact state the scanner leaves), a plain compiler warning (CS0219) is promoted to an error. The scan passed today only because the current sources emit no analyzer warning during the build — luck, not safety. Pass -p:TreatWarningsAsErrors=false -p:MSBuildTreatWarningsAsErrors=false on the sonar.yml build so the analysis leg is exempt. The ratchet stays enforced by ci.yml (both OS legs), which is the actual gate; the Sonar leg is for analysis, not warning-gating, so nothing is lost. Verified the override neutralises the Directory.Build.props ratchet under GITHUB_ACTIONS=true (CS0219 back to a warning, build green). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AbXi2ZEaUFeFWodNxdY6Zk --- .github/workflows/sonar.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/sonar.yml b/.github/workflows/sonar.yml index c785094..5432770 100644 --- a/.github/workflows/sonar.yml +++ b/.github/workflows/sonar.yml @@ -76,7 +76,13 @@ jobs: /d:sonar.cs.opencover.reportsPaths="artifacts/coverage/**/coverage.opencover.xml" - name: Build - run: dotnet build FirstClassErrors.sln -c Release + # Disable the CI warning ratchet (Directory.Build.props) for the analysis build only. The scanner + # needs this compilation to COMPLETE so it can collect the SonarAnalyzer diagnostics and upload them + # in `end`. The scanner already neutralises TreatWarningsAsErrors, but not MSBuildTreatWarningsAsErrors + # — which also promotes compiler/analyzer warnings — so a Sonar rule warning would fail this build + # before results are reported. The ratchet stays enforced by ci.yml (both OS legs), which is the gate; + # this analysis leg is not. + run: dotnet build FirstClassErrors.sln -c Release -p:TreatWarningsAsErrors=false -p:MSBuildTreatWarningsAsErrors=false - name: Test with coverage # Reuse coverage.runsettings so the OpenCover report matches the one the ci workflow produces.