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
83 changes: 42 additions & 41 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -69,50 +69,24 @@ 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 27 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.
# rule it activates at `warning`, so the default is ENFORCE. This block held the exceptions: rules
# with violations still in the tree, demoted to `suggestion` — active, non-blocking — so promoting
# one did not turn unrelated pull requests red.
#
# 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 BACKLOG IS EMPTY. All 377 rules the profile activates are enforced as of this commit, measured
# by elevating every entry to `warning` and building the solution: zero sites. The block is kept, with
# nothing in it, because the mechanism outlives the list — the next generated profile may activate a
# rule this tree violates, and this is where it goes.
#
# The other 350 rules the profile activates have zero violations here and are
# enforced as of this commit. Total outstanding: 83 sites. Decision: ADR-0062.
# The block 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), or in the test-scoped section below when its whole domain is test-shaped.
# `suggestion` means "not yet", never "no". Decision: ADR-0062.
#
# A rule leaves this block by one of two doors, and both are visible in the tree: its sites are
# cleared, or the few that remain are deliberate and carry a [SuppressMessage] with the reason at
# the site. The second door keeps the rule enforced everywhere else, which parking it never did.

dotnet_diagnostic.S1244.severity = suggestion # 15 — Floating point numbers should not be tested for equality
dotnet_diagnostic.S3878.severity = suggestion # 14 — Arrays should not be created for params parameters
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.S108.severity = suggestion # 2 — Nested blocks of code should not be left empty
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.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"
# A rule leaves by one of two doors, and both are visible in the tree: its sites are cleared, or the
# few that remain are deliberate and carry a [SuppressMessage] with the reason at the site. The second
# door keeps the rule enforced everywhere else, which parking it never did.

# 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.
Expand All @@ -126,6 +100,33 @@ dotnet_diagnostic.S6608.severity = suggestion # 1 — Prefer indexing instead
# genuinely want it — which is why this is scoped here rather than switched off repository-wide.
dotnet_diagnostic.CA1861.severity = none

# Declined in tests: exact floating-point equality. S1244 assumes an `==` between doubles is an
# accident of arithmetic. In these suites it is the assertion: `Between(value, value)` declares a
# degenerate interval and the property is that the draw IS that value; `bounds.Min == bounds.Max`
# detects that degenerate case to branch on it; `Zero()` pins a value and `== Half.Zero` is the
# contract it promises. A tolerance would not make these checks safer, it would stop them testing
# what they exist to test. Shipping code keeps the rule, where an `==` between computed floats
# really is the bug the rule describes.
dotnet_diagnostic.S1244.severity = none

# Declined in tests: parameter-count ceilings. S107 caps a lambda at seven parameters. The lambdas
# it fires on are the eight-operand `Any.Combine` overload's composer — the arity IS the subject of
# the test, and it is fixed by the API being exercised, not chosen by the test. The rule stays ON
# for shipping code, where a long parameter list is a design smell rather than a fixture.
dotnet_diagnostic.S107.severity = none

# Declined in tests: unused generic type parameters. S2326 is right that a `<T>` nothing reads is
# dead weight — except in a fixture built to be READ BY REFLECTION, where the unused parameter is
# precisely the shape under test (an overload that differs only by arity, a generic-only member the
# documentation reader must find). Removing it would delete the test case.
dotnet_diagnostic.S2326.severity = none

# Declined in tests: empty blocks. S108 asks that `{ }` be filled or removed. In these suites the
# empty body IS the exercise: `using (Any.UseSeed(1, …)) { }` enters and leaves a scope to assert
# what disposal does, and filling the block would add a statement with nothing to say. The rule
# stays ON for shipping code, where an empty block is usually a forgotten branch.
dotnet_diagnostic.S108.severity = none

[*.{csproj,props,targets,nuspec,config,xml}]
indent_size = 2

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ public static class Sample {
[InlineData("System.Version", "new System.Version(raw)")] // ctor -> TryParse
[InlineData("System.Uri", "new System.Uri(raw, System.UriKind.Absolute)")] // ctor -> TryCreate
[InlineData("System.Net.Mail.MailAddress", "new System.Net.Mail.MailAddress(raw)")] // ctor -> TryCreate
[System.Diagnostics.CodeAnalysis.SuppressMessage("Major Code Smell", "S4144:Methods should not have identical implementations",
Justification =
"The bodies are identical because the DATA is what differs: this theory drives the constructor cases " +
"(ctor -> TryParse, ctor -> TryCreate) and its sibling the method cases. Merging them would lose the two " +
"claims their names make; extracting the shared body would leave two one-line theories pointing at it.")]
public async Task Reports_a_constructor_that_has_a_matching_counterpart(string resultType, string call) {
string source = $$"""
using FirstClassErrors;
Expand Down
10 changes: 5 additions & 5 deletions FirstClassErrors.Cli.UnitTests/CatalogCommandsEndToEndTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public void FailOnAnyFailsOnCompatibleChange() {
string baseline = dir.File("errors-baseline.json");
BaselineStore.Save(baseline, Snapshot("A"));

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

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

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

(int exit, string _, RecordingLogger __) = RunUpdate(new RecordingSnapshotSource(Snapshot("A")), new CatalogUpdateSettings {
(int exit, string _, RecordingLogger _) = RunUpdate(new RecordingSnapshotSource(Snapshot("A")), new CatalogUpdateSettings {
ConfigPath = configPath
});

Expand Down Expand Up @@ -336,7 +336,7 @@ public void NewerSchemaBaselineIsRefused() {
public void CancellationExitsOneThirty() {
using TempDir dir = new();

(int exit, string _, RecordingLogger __) = RunUpdate(new CancellingSnapshotSource(), new CatalogUpdateSettings {
(int exit, string _, RecordingLogger _) = RunUpdate(new CancellingSnapshotSource(), new CatalogUpdateSettings {
ConfigPath = CliTestHelpers.NonExistentConfigPath(),
BaselinePath = dir.File("errors-baseline.json")
});
Expand Down
5 changes: 5 additions & 0 deletions FirstClassErrors.GenDoc.UnitTests/CatalogSnapshotTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,11 @@ public void NullArgumentsAreRejected() {
}

[Fact(DisplayName = "A snapshot declaring a newer schema is rejected as a distinct CatalogSchemaTooNewException.")]
[System.Diagnostics.CodeAnalysis.SuppressMessage("Major Code Smell", "S2219:Runtime type checking should be simplified",
Justification =
"Not a disguised null check: caught is already the narrower snapshot exception, and this line asserts the " +
"compatibility guarantee the comment above states — that the distinct, catchable type still derives from " +
"InvalidOperationException, so existing handlers keep working.")]
public void ASnapshotDeclaringANewerSchemaIsRejected() {
// Exercise
CatalogSchemaTooNewException? caught = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ private static XElement AnalyzerProjectReference() {

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

return string.Equals(fileName, "FirstClassErrors.Analyzers.csproj", StringComparison.OrdinalIgnoreCase);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,15 @@ private sealed class DerivedDto : BaseDto {

}

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

public int Count { get; init; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@ public void OptionalReferencePresentButInvalidRecords() {
}

[Fact(DisplayName = "An optional value property yields a real null when absent — never default(T): an absent count is null, not 0.")]
[System.Diagnostics.CodeAnalysis.SuppressMessage("Major Code Smell", "S125:Sections of code should not be commented out",
Justification =
"Prose, not code. The line explains why the outcome is projected to a bool — Nullable<T> is not `notnull`, " +
"so New's TCommand cannot be int? — and the rule reads the type names and the semicolon as a statement.")]
public void OptionalValueYieldsNullWhenAbsent() {
RequestBinder absent = Bind.Request(BookingEnvelopeError.CommandInvalid);
PropertySource<BookingRequest> absentBody = absent.PropertiesOf(Request(nights: null));
Expand Down
20 changes: 19 additions & 1 deletion FirstClassErrors.RequestBinder.UnitTests/TestModel.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
#region Usings declarations

using System.Globalization;

#endregion

namespace FirstClassErrors.RequestBinder.UnitTests;

#region Request DTOs
Expand Down Expand Up @@ -57,7 +63,7 @@ private BookingDate(DateOnly value) {
public DateOnly Value { get; }

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

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

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

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

public static readonly ErrorCode CommandInvalid = ErrorCode.Create("TEST_BOOKING_COMMAND_INVALID");
Expand Down
5 changes: 5 additions & 0 deletions FirstClassErrors.UnitTests/ErrorContextKeyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ namespace FirstClassErrors.UnitTests;

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

#region Constructors & Destructor
Expand Down
Loading
Loading