Skip to content

Commit 7d49526

Browse files
authored
Merge pull request #390 from Reefact/claude/sonar-points-restants-12zlnr
Empty the Sonar backlog
2 parents 0f8cfba + f8f20e8 commit 7d49526

29 files changed

Lines changed: 201 additions & 96 deletions

.editorconfig

Lines changed: 42 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -69,50 +69,24 @@ dotnet_diagnostic.CA1859.severity = none
6969
# --- Sonar rules not yet enforced: the backlog -------------------------------------------------
7070
#
7171
# build/sonar-profile.globalconfig is generated from the SonarCloud quality profile and puts every
72-
# rule it activates at `warning`, so the default is ENFORCE. The 27 rules below are the
73-
# exceptions: each still has violations in this tree, and promoting it now would turn unrelated
74-
# pull requests red. They are demoted to `suggestion` — active, non-blocking — with the number of
75-
# sites measured when this landed.
72+
# rule it activates at `warning`, so the default is ENFORCE. This block held the exceptions: rules
73+
# with violations still in the tree, demoted to `suggestion` — active, non-blocking — so promoting
74+
# one did not turn unrelated pull requests red.
7675
#
77-
# This block IS the backlog, and it shrinks by DELETION: clear a rule's sites, delete its line,
78-
# and the generated file enforces it from the next build with nothing further to write. A rule
79-
# this codebase means to refuse outright does not belong here — it belongs with the declines
80-
# above, at `none`, with its reason (ADR-0060). `suggestion` means "not yet", never "no".
76+
# THE BACKLOG IS EMPTY. All 377 rules the profile activates are enforced as of this commit, measured
77+
# by elevating every entry to `warning` and building the solution: zero sites. The block is kept, with
78+
# nothing in it, because the mechanism outlives the list — the next generated profile may activate a
79+
# rule this tree violates, and this is where it goes.
8180
#
82-
# The other 350 rules the profile activates have zero violations here and are
83-
# enforced as of this commit. Total outstanding: 83 sites. Decision: ADR-0062.
81+
# The block shrinks by DELETION: clear a rule's sites, delete its line, and the generated file
82+
# enforces it from the next build with nothing further to write. A rule this codebase means to refuse
83+
# outright does not belong here — it belongs with the declines above, at `none`, with its reason
84+
# (ADR-0060), or in the test-scoped section below when its whole domain is test-shaped.
85+
# `suggestion` means "not yet", never "no". Decision: ADR-0062.
8486
#
85-
# A rule leaves this block by one of two doors, and both are visible in the tree: its sites are
86-
# cleared, or the few that remain are deliberate and carry a [SuppressMessage] with the reason at
87-
# the site. The second door keeps the rule enforced everywhere else, which parking it never did.
88-
89-
dotnet_diagnostic.S1244.severity = suggestion # 15 Floating point numbers should not be tested for equality
90-
dotnet_diagnostic.S3878.severity = suggestion # 14 Arrays should not be created for params parameters
91-
dotnet_diagnostic.S3218.severity = suggestion # 8 Inner class members should not shadow outer class "static" or type members
92-
dotnet_diagnostic.S107.severity = suggestion # 6 Methods should not have too many parameters
93-
dotnet_diagnostic.S1481.severity = suggestion # 5 Unused local variables should be removed
94-
dotnet_diagnostic.S1854.severity = suggestion # 4 Unused assignments should be removed
95-
dotnet_diagnostic.S4144.severity = suggestion # 3 Methods should not have identical implementations
96-
dotnet_diagnostic.S108.severity = suggestion # 2 Nested blocks of code should not be left empty
97-
dotnet_diagnostic.S125.severity = suggestion # 2 Sections of code should not be commented out
98-
dotnet_diagnostic.S1905.severity = suggestion # 2 Redundant casts should not be used
99-
dotnet_diagnostic.S2326.severity = suggestion # 2 Unused type parameters should be removed
100-
dotnet_diagnostic.S3220.severity = suggestion # 2 Method calls should not resolve ambiguously to overloads with "params"
101-
dotnet_diagnostic.S3358.severity = suggestion # 2 Ternary operators should not be nested
102-
dotnet_diagnostic.S6966.severity = suggestion # 2 Awaitable method should be used
103-
dotnet_diagnostic.S927.severity = suggestion # 2 Parameter names should match base declaration and other partial definitions
104-
dotnet_diagnostic.S1144.severity = suggestion # 1 Unused private types or members should be removed
105-
dotnet_diagnostic.S2219.severity = suggestion # 1 Runtime type checking should be simplified
106-
dotnet_diagnostic.S2342.severity = suggestion # 1 Enumeration types should comply with a naming convention
107-
dotnet_diagnostic.S2692.severity = suggestion # 1 "IndexOf" checks should not be for positive numbers
108-
dotnet_diagnostic.S3376.severity = suggestion # 1 Attribute, EventArgs, and Exception type names should end with the type being extended
109-
dotnet_diagnostic.S3459.severity = suggestion # 1 Unassigned members should be removed
110-
dotnet_diagnostic.S3871.severity = suggestion # 1 Exception types should be "public"
111-
dotnet_diagnostic.S3877.severity = suggestion # 1 Exceptions should not be thrown from unexpected methods
112-
dotnet_diagnostic.S3881.severity = suggestion # 1 "IDisposable" should be implemented correctly
113-
dotnet_diagnostic.S4136.severity = suggestion # 1 Method overloads should be grouped together
114-
dotnet_diagnostic.S6580.severity = suggestion # 1 Use a format provider when parsing date and time
115-
dotnet_diagnostic.S6608.severity = suggestion # 1 Prefer indexing instead of "Enumerable" methods on types implementing "IList"
87+
# A rule leaves by one of two doors, and both are visible in the tree: its sites are cleared, or the
88+
# few that remain are deliberate and carry a [SuppressMessage] with the reason at the site. The second
89+
# door keeps the rule enforced everywhere else, which parking it never did.
11690

11791
# Test projects only. `*Tests` matches the thirteen test projects and no shipping one —
11892
# FirstClassErrors.Testing ends in `Testing`, so the rule below does not reach it.
@@ -126,6 +100,33 @@ dotnet_diagnostic.S6608.severity = suggestion # 1 — Prefer indexing instead
126100
# genuinely want it — which is why this is scoped here rather than switched off repository-wide.
127101
dotnet_diagnostic.CA1861.severity = none
128102

103+
# Declined in tests: exact floating-point equality. S1244 assumes an `==` between doubles is an
104+
# accident of arithmetic. In these suites it is the assertion: `Between(value, value)` declares a
105+
# degenerate interval and the property is that the draw IS that value; `bounds.Min == bounds.Max`
106+
# detects that degenerate case to branch on it; `Zero()` pins a value and `== Half.Zero` is the
107+
# contract it promises. A tolerance would not make these checks safer, it would stop them testing
108+
# what they exist to test. Shipping code keeps the rule, where an `==` between computed floats
109+
# really is the bug the rule describes.
110+
dotnet_diagnostic.S1244.severity = none
111+
112+
# Declined in tests: parameter-count ceilings. S107 caps a lambda at seven parameters. The lambdas
113+
# it fires on are the eight-operand `Any.Combine` overload's composer — the arity IS the subject of
114+
# the test, and it is fixed by the API being exercised, not chosen by the test. The rule stays ON
115+
# for shipping code, where a long parameter list is a design smell rather than a fixture.
116+
dotnet_diagnostic.S107.severity = none
117+
118+
# Declined in tests: unused generic type parameters. S2326 is right that a `<T>` nothing reads is
119+
# dead weight — except in a fixture built to be READ BY REFLECTION, where the unused parameter is
120+
# precisely the shape under test (an overload that differs only by arity, a generic-only member the
121+
# documentation reader must find). Removing it would delete the test case.
122+
dotnet_diagnostic.S2326.severity = none
123+
124+
# Declined in tests: empty blocks. S108 asks that `{ }` be filled or removed. In these suites the
125+
# empty body IS the exercise: `using (Any.UseSeed(1, …)) { }` enters and leaves a scope to assert
126+
# what disposal does, and filling the block would add a statement with nothing to say. The rule
127+
# stays ON for shipping code, where an empty block is usually a forgotten branch.
128+
dotnet_diagnostic.S108.severity = none
129+
129130
[*.{csproj,props,targets,nuspec,config,xml}]
130131
indent_size = 2
131132

FirstClassErrors.Analyzers.UnitTests/Fce021PreferNonThrowingAlternativeToTryTests.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ public static class Sample {
4141
[InlineData("System.Version", "new System.Version(raw)")] // ctor -> TryParse
4242
[InlineData("System.Uri", "new System.Uri(raw, System.UriKind.Absolute)")] // ctor -> TryCreate
4343
[InlineData("System.Net.Mail.MailAddress", "new System.Net.Mail.MailAddress(raw)")] // ctor -> TryCreate
44+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Major Code Smell", "S4144:Methods should not have identical implementations",
45+
Justification =
46+
"The bodies are identical because the DATA is what differs: this theory drives the constructor cases " +
47+
"(ctor -> TryParse, ctor -> TryCreate) and its sibling the method cases. Merging them would lose the two " +
48+
"claims their names make; extracting the shared body would leave two one-line theories pointing at it.")]
4449
public async Task Reports_a_constructor_that_has_a_matching_counterpart(string resultType, string call) {
4550
string source = $$"""
4651
using FirstClassErrors;

FirstClassErrors.Cli.UnitTests/CatalogCommandsEndToEndTests.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public void FailOnAnyFailsOnCompatibleChange() {
7272
string baseline = dir.File("errors-baseline.json");
7373
BaselineStore.Save(baseline, Snapshot("A"));
7474

75-
(int exit, string _, RecordingLogger __) = RunDiff(new RecordingSnapshotSource(Snapshot("A", "B")), new CatalogDiffSettings {
75+
(int exit, string _, RecordingLogger _) = RunDiff(new RecordingSnapshotSource(Snapshot("A", "B")), new CatalogDiffSettings {
7676
ConfigPath = CliTestHelpers.NonExistentConfigPath(),
7777
BaselinePath = baseline,
7878
FailOn = "any"
@@ -102,7 +102,7 @@ public void FailOnNoneNeverFails() {
102102
string baseline = dir.File("errors-baseline.json");
103103
BaselineStore.Save(baseline, Snapshot("A", "B"));
104104

105-
(int exit, string _, RecordingLogger __) = RunDiff(new RecordingSnapshotSource(Snapshot("A")), new CatalogDiffSettings {
105+
(int exit, string _, RecordingLogger _) = RunDiff(new RecordingSnapshotSource(Snapshot("A")), new CatalogDiffSettings {
106106
ConfigPath = CliTestHelpers.NonExistentConfigPath(),
107107
BaselinePath = baseline,
108108
FailOn = "none"
@@ -188,7 +188,7 @@ public void CancellationExitsOneThirty() {
188188
string baseline = dir.File("errors-baseline.json");
189189
BaselineStore.Save(baseline, Snapshot("A"));
190190

191-
(int exit, string _, RecordingLogger __) = RunDiff(new CancellingSnapshotSource(), new CatalogDiffSettings {
191+
(int exit, string _, RecordingLogger _) = RunDiff(new CancellingSnapshotSource(), new CatalogDiffSettings {
192192
ConfigPath = CliTestHelpers.NonExistentConfigPath(),
193193
BaselinePath = baseline
194194
});
@@ -305,7 +305,7 @@ public void ConfiguredBaselineResolvesRelativeToConfig() {
305305
string configPath = dir.File("fce.json");
306306
File.WriteAllText(configPath, """{ "baseline": "errors-baseline.json" }""");
307307

308-
(int exit, string _, RecordingLogger __) = RunUpdate(new RecordingSnapshotSource(Snapshot("A")), new CatalogUpdateSettings {
308+
(int exit, string _, RecordingLogger _) = RunUpdate(new RecordingSnapshotSource(Snapshot("A")), new CatalogUpdateSettings {
309309
ConfigPath = configPath
310310
});
311311

@@ -336,7 +336,7 @@ public void NewerSchemaBaselineIsRefused() {
336336
public void CancellationExitsOneThirty() {
337337
using TempDir dir = new();
338338

339-
(int exit, string _, RecordingLogger __) = RunUpdate(new CancellingSnapshotSource(), new CatalogUpdateSettings {
339+
(int exit, string _, RecordingLogger _) = RunUpdate(new CancellingSnapshotSource(), new CatalogUpdateSettings {
340340
ConfigPath = CliTestHelpers.NonExistentConfigPath(),
341341
BaselinePath = dir.File("errors-baseline.json")
342342
});

FirstClassErrors.GenDoc.UnitTests/CatalogSnapshotTests.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,11 @@ public void NullArgumentsAreRejected() {
203203
}
204204

205205
[Fact(DisplayName = "A snapshot declaring a newer schema is rejected as a distinct CatalogSchemaTooNewException.")]
206+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Major Code Smell", "S2219:Runtime type checking should be simplified",
207+
Justification =
208+
"Not a disguised null check: caught is already the narrower snapshot exception, and this line asserts the " +
209+
"compatibility guarantee the comment above states — that the distinct, catchable type still derives from " +
210+
"InvalidOperationException, so existing handlers keep working.")]
206211
public void ASnapshotDeclaringANewerSchemaIsRejected() {
207212
// Exercise
208213
CatalogSchemaTooNewException? caught = null;

FirstClassErrors.RequestBinder.UnitTests/AnalyzerDogfoodingTests.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ private static XElement AnalyzerProjectReference() {
5151

5252
private static bool ReferencesTheAnalyzers(XElement projectReference) {
5353
string include = (string?)projectReference.Attribute("Include") ?? string.Empty;
54-
string fileName = include.Replace('\\', '/').Split('/').Last();
54+
string[] segments = include.Replace('\\', '/').Split('/');
55+
string fileName = segments[segments.Length - 1];
5556

5657
return string.Equals(fileName, "FirstClassErrors.Analyzers.csproj", StringComparison.OrdinalIgnoreCase);
5758
}

FirstClassErrors.RequestBinder.UnitTests/PropertyGetterCacheTests.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,15 @@ private sealed class DerivedDto : BaseDto {
150150

151151
}
152152

153+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Major Code Smell", "S3459:Unassigned members should be removed",
154+
Justification =
155+
"Deliberately misdeclared, as the name says. This DTO exists so the binder can refuse a non-nullable value-type " +
156+
"property; the refusal happens when the property is SELECTED, so nothing ever assigns or reads it. Assigning it " +
157+
"would remove the very defect under test.")]
158+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Major Code Smell", "S1144:Unused private types or members should be removed",
159+
Justification =
160+
"Same reason as the S3459 suppression above: the property is a fixture for a refusal that fires before any value " +
161+
"is set, so its init accessor is unreachable by design.")]
153162
private sealed record MisdeclaredDto {
154163

155164
public int Count { get; init; }

FirstClassErrors.RequestBinder.UnitTests/SimplePropertyBindingTests.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,10 @@ public void OptionalReferencePresentButInvalidRecords() {
140140
}
141141

142142
[Fact(DisplayName = "An optional value property yields a real null when absent — never default(T): an absent count is null, not 0.")]
143+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Major Code Smell", "S125:Sections of code should not be commented out",
144+
Justification =
145+
"Prose, not code. The line explains why the outcome is projected to a bool — Nullable<T> is not `notnull`, " +
146+
"so New's TCommand cannot be int? — and the rule reads the type names and the semicolon as a statement.")]
143147
public void OptionalValueYieldsNullWhenAbsent() {
144148
RequestBinder absent = Bind.Request(BookingEnvelopeError.CommandInvalid);
145149
PropertySource<BookingRequest> absentBody = absent.PropertiesOf(Request(nights: null));

FirstClassErrors.RequestBinder.UnitTests/TestModel.cs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
#region Usings declarations
2+
3+
using System.Globalization;
4+
5+
#endregion
6+
17
namespace FirstClassErrors.RequestBinder.UnitTests;
28

39
#region Request DTOs
@@ -57,7 +63,7 @@ private BookingDate(DateOnly value) {
5763
public DateOnly Value { get; }
5864

5965
public static Outcome<BookingDate> Parse(string raw) {
60-
return DateOnly.TryParse(raw, out DateOnly parsed)
66+
return DateOnly.TryParse(raw, CultureInfo.InvariantCulture, out DateOnly parsed)
6167
? Outcome<BookingDate>.Success(new BookingDate(parsed))
6268
: Outcome<BookingDate>.Failure(BookingDomainError.DateInvalid(raw));
6369
}
@@ -138,6 +144,12 @@ internal static DomainError NotAPositiveNumber(string raw) {
138144
.WithPublicMessage("The number must be strictly positive.");
139145
}
140146

147+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Minor Code Smell", "S3218:Inner class members should not shadow outer class \"static\" or type members",
148+
Justification =
149+
"The mirror IS the point: each code is named after the factory that raises it, so the call site reads " +
150+
"Code.DateInvalid inside DateInvalid(). Renaming to DateInvalidCode to satisfy the rule would break the " +
151+
"one-to-one correspondence that makes this file scannable, and buy nothing — the nested class is private " +
152+
"and its members are only ever reached through it.")]
141153
private static class Code {
142154

143155
public static readonly ErrorCode EmailInvalid = ErrorCode.Create("TEST_EMAIL_INVALID");
@@ -168,6 +180,12 @@ internal static PrimaryPortError GuestInvalid(PrimaryPortInnerErrors violations)
168180
.WithPublicMessage("A guest's information is invalid.");
169181
}
170182

183+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Minor Code Smell", "S3218:Inner class members should not shadow outer class \"static\" or type members",
184+
Justification =
185+
"The mirror IS the point: each code is named after the factory that raises it, so the call site reads " +
186+
"Code.DateInvalid inside DateInvalid(). Renaming to DateInvalidCode to satisfy the rule would break the " +
187+
"one-to-one correspondence that makes this file scannable, and buy nothing — the nested class is private " +
188+
"and its members are only ever reached through it.")]
171189
private static class Code {
172190

173191
public static readonly ErrorCode CommandInvalid = ErrorCode.Create("TEST_BOOKING_COMMAND_INVALID");

FirstClassErrors.UnitTests/ErrorContextKeyTests.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ namespace FirstClassErrors.UnitTests;
1212

1313
[Collection("SmartEnumSideEffects")]
1414
[TestSubject(typeof(ErrorContextKey))]
15+
[System.Diagnostics.CodeAnalysis.SuppressMessage("Major Code Smell", "S3881:\"IDisposable\" should be implemented correctly",
16+
Justification =
17+
"xUnit's teardown hook, not a resource owner. The class holds nothing unmanaged and is instantiated once per " +
18+
"test by the framework, which calls Dispose itself; the full pattern (virtual Dispose(bool), a finalizer, " +
19+
"GC.SuppressFinalize) would add ceremony around a single ResetForTests() call.")]
1520
public class ErrorContextKeyTests : IDisposable {
1621

1722
#region Constructors & Destructor

0 commit comments

Comments
 (0)