diff --git a/FirstClassErrors.Analyzers/Descriptors.cs b/FirstClassErrors.Analyzers/Descriptors.cs index bc0540f..78fb9c6 100644 --- a/FirstClassErrors.Analyzers/Descriptors.cs +++ b/FirstClassErrors.Analyzers/Descriptors.cs @@ -15,7 +15,7 @@ internal static class Descriptors { category: DiagnosticCategories.ErrorCodes, defaultSeverity: DiagnosticSeverity.Error, isEnabledByDefault: true, - description: "ErrorCode.Create registers each code in a process-wide set and throws when the same code is created twice. Detection is per-compilation and limited to literal codes.", + description: "An ErrorCode is compared by value, so the same literal code created more than once yields equal instances that documentation extraction and lookups collapse into a single identity. Detection is per-compilation and limited to literal codes.", helpLinkUri: HelpLinks.For(DiagnosticIds.DuplicateErrorCode), customTags: new[] { WellKnownDiagnosticTags.CompilationEnd }); diff --git a/FirstClassErrors.Analyzers/DuplicateErrorCodeAnalyzer.cs b/FirstClassErrors.Analyzers/DuplicateErrorCodeAnalyzer.cs index c344b00..6fa516c 100644 --- a/FirstClassErrors.Analyzers/DuplicateErrorCodeAnalyzer.cs +++ b/FirstClassErrors.Analyzers/DuplicateErrorCodeAnalyzer.cs @@ -10,9 +10,10 @@ namespace FirstClassErrors.Analyzers; /// /// FCE001 — reports the same error code created by more than one ErrorCode.Create("X") literal in the -/// compilation. ErrorCode.Create registers each code in a process-wide set and throws when a code is created -/// twice. Detection is per-compilation (cross-assembly duplicates still surface only at runtime) and limited to -/// literal codes — a non-literal code is FCE003's concern. +/// compilation. ErrorCode is compared by value, so a duplicated code yields equal instances that silently +/// collapse two distinct errors into one identity (documentation extraction and dictionary lookups keep a single +/// entry). Detection is per-compilation (cross-assembly duplicates are not seen here — they surface only as a +/// documentation-pipeline warning) and limited to literal codes — a non-literal code is FCE003's concern. /// [DiagnosticAnalyzer(LanguageNames.CSharp)] public sealed class DuplicateErrorCodeAnalyzer : DiagnosticAnalyzer { diff --git a/FirstClassErrors.UnitTests/ErrorCodeTests.cs b/FirstClassErrors.UnitTests/ErrorCodeTests.cs index 1eee6df..3a32745 100644 --- a/FirstClassErrors.UnitTests/ErrorCodeTests.cs +++ b/FirstClassErrors.UnitTests/ErrorCodeTests.cs @@ -1,7 +1,5 @@ #region Usings declarations -using System.Diagnostics.CodeAnalysis; - using JetBrains.Annotations; using NFluent; @@ -11,15 +9,7 @@ namespace FirstClassErrors.UnitTests; [TestSubject(typeof(ErrorCode))] -public class ErrorCodeTests : IDisposable { - - #region Constructors & Destructor - - public ErrorCodeTests() { - ErrorCode.ResetForTests(); - } - - #endregion +public class ErrorCodeTests { [Theory(DisplayName = "Creating an error code with a valid code succeeds.")] [InlineData("ERROR_001")] @@ -45,16 +35,17 @@ public void CreatingErrorCodeWithBlankCodeIsRejected(string? code) { .WithMessage("Error code cannot be null or whitespace. (Parameter 'code')"); } - [Fact(DisplayName = "Creating an error code with a duplicate code is rejected.")] - public void CreatingErrorCodeWithDuplicateCodeIsRejected() { + [Fact(DisplayName = "Creating the same code twice is allowed and produces equal instances.")] + public void CreatingTheSameCodeTwiceIsAllowedAndProducesEqualInstances() { // Setup const string code = "DUPLICATE_ERROR"; - ErrorCode.Create(code); - // Exercise & verify - Check.ThatCode(() => ErrorCode.Create(code)) - .Throws() - .WithMessage("Error code 'DUPLICATE_ERROR' has already been registered."); + // Exercise + ErrorCode first = ErrorCode.Create(code); + ErrorCode second = ErrorCode.Create(code); + + // Verify + Check.That(first).IsEqualTo(second); } [Fact(DisplayName = "Error codes with the same code are equal.")] @@ -62,8 +53,7 @@ public void ErrorCodesWithSameCodeAreEqual() { // Setup const string code = "EQUALS_TEST"; ErrorCode errorCode1 = ErrorCode.Create(code); - ErrorCode.ResetForTests(); - ErrorCode errorCode2 = ErrorCode.Create(code); + ErrorCode errorCode2 = ErrorCode.Create(code); // Exercise & verify Check.That(errorCode1).IsEqualTo(errorCode2); @@ -73,7 +63,6 @@ public void ErrorCodesWithSameCodeAreEqual() { public void ErrorCodesWithDifferentCodesAreNotEqual() { // Setup ErrorCode errorCode1 = ErrorCode.Create("ERROR_1"); - ErrorCode.ResetForTests(); ErrorCode errorCode2 = ErrorCode.Create("ERROR_2"); // Exercise & verify @@ -97,7 +86,6 @@ public void EqualityOperatorReturnsTrueForErrorCodesWithSameCode() { // Setup string code = "OPERATOR_EQUALS"; ErrorCode errorCode1 = ErrorCode.Create(code); - ErrorCode.ResetForTests(); ErrorCode errorCode2 = ErrorCode.Create(code); // Exercise @@ -111,7 +99,6 @@ public void EqualityOperatorReturnsTrueForErrorCodesWithSameCode() { public void InequalityOperatorReturnsTrueForErrorCodesWithDifferentCodes() { // Setup ErrorCode errorCode1 = ErrorCode.Create("ERROR_1"); - ErrorCode.ResetForTests(); ErrorCode errorCode2 = ErrorCode.Create("ERROR_2"); // Exercise @@ -139,8 +126,7 @@ public void ErrorCodesWithSameCodeProduceTheSameHashCode() { // Setup const string code = "HASH_TEST"; ErrorCode errorCode1 = ErrorCode.Create(code); - ErrorCode.ResetForTests(); - ErrorCode errorCode2 = ErrorCode.Create(code); + ErrorCode errorCode2 = ErrorCode.Create(code); // Exercise int hash1 = errorCode1.GetHashCode(); @@ -154,7 +140,6 @@ public void ErrorCodesWithSameCodeProduceTheSameHashCode() { public void ErrorCodesWithDifferentCodesMayProduceDifferentHashCodes() { // Setup ErrorCode errorCode1 = ErrorCode.Create("HASH_1"); - ErrorCode.ResetForTests(); ErrorCode errorCode2 = ErrorCode.Create("HASH_2"); // Exercise @@ -178,29 +163,19 @@ public void ErrorCodeComparedToObjectOfDifferentTypeIsNotEqual() { Check.That(result).IsFalse(); } - [Fact(DisplayName = "Concurrent creation of the same error code results in a single registration.")] - public void ConcurrentCreationOfSameErrorCodeResultsInSingleRegistration() { - const string code = "CONCURRENT"; - - int successes = 0; - int failures = 0; + [Fact(DisplayName = "Concurrent creation of the same code always succeeds and yields equal instances.")] + public void ConcurrentCreationOfSameCodeAlwaysSucceedsAndYieldsEqualInstances() { + // Setup + const string code = "CONCURRENT"; + ErrorCode reference = ErrorCode.Create(code); - Parallel.For(0, 20, _ => { - try { - ErrorCode.Create(code); - Interlocked.Increment(ref successes); - } catch (InvalidOperationException) { - Interlocked.Increment(ref failures); - } - }); + ErrorCode[] created = new ErrorCode[20]; - Check.That(successes).IsEqualTo(1); - Check.That(failures).IsEqualTo(19); - } + // Exercise + Parallel.For(0, 20, index => created[index] = ErrorCode.Create(code)); - [SuppressMessage("Usage", "CA1816", Justification = "IDisposable is used as an xUnit teardown hook. The class has no finalizer and does not own unmanaged resources.")] - public void Dispose() { - ErrorCode.ResetForTests(); + // Verify + Check.That(created.All(errorCode => errorCode == reference)).IsTrue(); } -} \ No newline at end of file +} diff --git a/FirstClassErrors.UnitTests/ErrorContextImmutabilityTests.cs b/FirstClassErrors.UnitTests/ErrorContextImmutabilityTests.cs index a24455d..83cb620 100644 --- a/FirstClassErrors.UnitTests/ErrorContextImmutabilityTests.cs +++ b/FirstClassErrors.UnitTests/ErrorContextImmutabilityTests.cs @@ -18,7 +18,6 @@ public sealed class ErrorContextImmutabilityTests : IDisposable { public ErrorContextImmutabilityTests() { ErrorContextKey.ResetForTests(); - ErrorCode.ResetForTests(); } #endregion @@ -26,7 +25,6 @@ public ErrorContextImmutabilityTests() { [SuppressMessage("Usage", "CA1816", Justification = "xUnit teardown hook.")] public void Dispose() { ErrorContextKey.ResetForTests(); - ErrorCode.ResetForTests(); } [Fact(DisplayName = "The context values are read-only and cannot be mutated.")] diff --git a/FirstClassErrors.UnitTests/ErrorContextKeyTests.cs b/FirstClassErrors.UnitTests/ErrorContextKeyTests.cs index 59c43de..a3d2447 100644 --- a/FirstClassErrors.UnitTests/ErrorContextKeyTests.cs +++ b/FirstClassErrors.UnitTests/ErrorContextKeyTests.cs @@ -81,16 +81,31 @@ public void RegisteringKeyWithBlankNameIsRejected(string? name) { .WithMessage("Value cannot be null or whitespace. (Parameter 'name')"); } - [Fact(DisplayName = "Registering a key with a duplicate name is rejected.")] - public void RegisteringKeyWithDuplicateNameIsRejected() { + [Fact(DisplayName = "Re-declaring a key with the same name and type returns the registered instance.")] + public void RedeclaringKeyWithSameNameAndTypeReturnsTheRegisteredInstance() { // Setup - const string name = "Duplicate"; - ErrorContextKey.Create(name); + const string name = "Duplicate"; + ErrorContextKey first = ErrorContextKey.Create(name); - // Exercise & verify - Check.ThatCode(() => ErrorContextKey.Create(name)) - .Throws() - .WithMessage("An error context key 'Duplicate' has already been registered."); + // Exercise + ErrorContextKey second = ErrorContextKey.Create(name); + + // Verify + Check.That(second).IsSameReferenceAs(first); + Check.That(ErrorContextKey.GetRegisteredKeys().Count).IsEqualTo(1); + } + + [Fact(DisplayName = "Re-declaring a key with a different description keeps the first registered description.")] + public void RedeclaringKeyWithDifferentDescriptionKeepsTheFirstRegisteredDescription() { + // Setup + const string name = "DescribedTwice"; + ErrorContextKey.Create(name, "first"); + + // Exercise + ErrorContextKey second = ErrorContextKey.Create(name, "second"); + + // Verify + Check.That(second.Description).IsEqualTo("first"); } [Fact(DisplayName = "Registering a key with the same name but a different type is rejected.")] @@ -101,7 +116,8 @@ public void RegisteringKeyWithSameNameButDifferentTypeIsRejected() { // Exercise & verify Check.ThatCode(() => ErrorContextKey.Create(name)) - .Throws(); + .Throws() + .WithMessage("An error context key 'SameName' is already registered with value type 'System.String'; it cannot be re-registered with value type 'System.Guid'."); } [Fact(DisplayName = "The registry returns all registered keys.")] @@ -245,22 +261,14 @@ public void ConcurrentRegistrationOfSameKeyNameResultsInSingleRegistration() { // Setup const string name = "Concurrent"; - int successes = 0; - int failures = 0; + ErrorContextKey[] created = new ErrorContextKey[20]; // Exercise - Parallel.For(0, 20, _ => { - try { - ErrorContextKey.Create(name); - Interlocked.Increment(ref successes); - } catch (InvalidOperationException) { - Interlocked.Increment(ref failures); - } - }); + Parallel.For(0, 20, index => created[index] = ErrorContextKey.Create(name)); // Verify - Check.That(successes).IsEqualTo(1); - Check.That(failures).IsEqualTo(19); + Check.That(created.All(key => ReferenceEquals(key, created[0]))).IsTrue(); + Check.That(ErrorContextKey.GetRegisteredKeys().Count).IsEqualTo(1); } [SuppressMessage("Usage", "CA1816", diff --git a/FirstClassErrors.UnitTests/ErrorDocumentationBuilderTests.cs b/FirstClassErrors.UnitTests/ErrorDocumentationBuilderTests.cs index c6e9421..c11f729 100644 --- a/FirstClassErrors.UnitTests/ErrorDocumentationBuilderTests.cs +++ b/FirstClassErrors.UnitTests/ErrorDocumentationBuilderTests.cs @@ -17,7 +17,6 @@ public sealed class ErrorDocumentationBuilderTests : IDisposable { public ErrorDocumentationBuilderTests() { ErrorContextKey.ResetForTests(); - ErrorCode.ResetForTests(); } #endregion @@ -25,7 +24,6 @@ public ErrorDocumentationBuilderTests() { [SuppressMessage("Usage", "CA1816", Justification = "IDisposable is used as an xUnit teardown hook. The class has no finalizer and does not own unmanaged resources.")] public void Dispose() { ErrorContextKey.ResetForTests(); - ErrorCode.ResetForTests(); } [Fact(DisplayName = "An error documentation title cannot be null.")] diff --git a/FirstClassErrors.UnitTests/ErrorTests.cs b/FirstClassErrors.UnitTests/ErrorTests.cs index cd7ddf3..4189114 100644 --- a/FirstClassErrors.UnitTests/ErrorTests.cs +++ b/FirstClassErrors.UnitTests/ErrorTests.cs @@ -18,7 +18,6 @@ public sealed class ErrorTests : IDisposable { public ErrorTests() { ErrorContextKey.ResetForTests(); - ErrorCode.ResetForTests(); } #endregion @@ -26,7 +25,6 @@ public ErrorTests() { [SuppressMessage("Usage", "CA1816", Justification = "IDisposable is used as an xUnit teardown hook. The class has no finalizer and does not own unmanaged resources.")] public void Dispose() { ErrorContextKey.ResetForTests(); - ErrorCode.ResetForTests(); } [Fact(DisplayName = "An error has a unique instance identifier.")] diff --git a/FirstClassErrors/ErrorCode.cs b/FirstClassErrors/ErrorCode.cs index b1e3e3e..37c09d6 100644 --- a/FirstClassErrors/ErrorCode.cs +++ b/FirstClassErrors/ErrorCode.cs @@ -7,9 +7,6 @@ public sealed class ErrorCode : IEquatable { #region Statics members declarations - private static readonly HashSet Registered = new(StringComparer.Ordinal); - private static readonly object Lock = new(); - /// /// Represents an unspecified error condition. This is used as a default value when no specific error code is provided. /// @@ -18,36 +15,24 @@ public sealed class ErrorCode : IEquatable { /// /// Creates a new instance of the class with the specified code. /// - /// The unique string identifier for the error condition. + /// The string identifier for the error condition. /// A new instance representing the specified error condition. + /// + /// An error code is a value: two instances built from the same + /// compare equal, so creating the same code more than once is allowed and never throws. A code is an identity, not a + /// runtime registry entry — a duplicated code silently merges two distinct errors into one, which the FCE001 + /// analyzer flags at build time. + /// /// /// Thrown when the is null, empty, or consists only of /// whitespace. /// - /// Thrown when the has already been registered. public static ErrorCode Create(string code) { if (string.IsNullOrWhiteSpace(code)) { throw new ArgumentException("Error code cannot be null or whitespace.", nameof(code)); } - lock (Lock) { - if (!Registered.Add(code)) { throw new InvalidOperationException($"Error code '{code}' has already been registered."); } - } - return new ErrorCode(code); } - /// - /// Resets the internal state of registered values. - /// - /// - /// This method is intended for use in testing scenarios only. It clears all registered error codes, - /// allowing a clean slate for subsequent tests that rely on registration. - /// - internal static void ResetForTests() { - lock (Lock) { - Registered.Clear(); - } - } - #endregion /// diff --git a/FirstClassErrors/ErrorContextKey.cs b/FirstClassErrors/ErrorContextKey.cs index 057e301..8870d43 100644 --- a/FirstClassErrors/ErrorContextKey.cs +++ b/FirstClassErrors/ErrorContextKey.cs @@ -13,7 +13,8 @@ namespace FirstClassErrors; /// /// An defines the identity of a piece of contextual information associated with an /// error (for example: DealId, UserId, or CorrelationId). Each key is globally unique by its -/// and is registered once at application startup. +/// and is registered once: re-declaring a key with the same name and value type returns the +/// registered instance, while a same-named key with a different value type is rejected. /// /// /// Keys are strongly typed through the generic subclass , which specifies the @@ -65,19 +66,26 @@ public abstract class ErrorContextKey : IEquatable { internal static readonly ErrorContextKey MissingRequiredMessages = Create>("#MISSING_REQUIRED_MESSAGE", "The mandatory messages (by parameter name) that were missing and replaced by a fallback sentinel."); /// - /// Creates a new instance of with the specified name and optional description. + /// Gets or creates the with the specified name and optional description. /// + /// + /// Creation is idempotent: re-declaring a key with the same and the same + /// returns the already-registered instance (whose description wins), so a declaration + /// that is executed again — a reloaded plugin, a re-run test fixture — never fails. A same-named key with a + /// different value type is rejected: keys compare by name inside error contexts, so the two keys would silently + /// collide and make the stored value unreadable through the typed API. + /// /// The type associated with the error context key. /// The unique name of the error context key. Must not be null, empty, or whitespace. /// An optional description providing additional context for the error context key. - /// A new instance of . + /// The registered instance for . /// /// Thrown when is null, empty, or consists only of /// whitespace. /// /// - /// Thrown when an error context key with the specified - /// has already been registered. + /// Thrown when a key with the specified is + /// already registered with a different value type. /// public static ErrorContextKey Create(string name, string? description = null) { Func? descriptionProvider = null; @@ -87,22 +95,24 @@ public static ErrorContextKey Create(string name, string? description = nu } /// - /// Creates a new whose description is resolved lazily, on each read of + /// Gets or creates the whose description is resolved lazily, on each read of /// . /// /// /// Use this to supply a localized description — for example one read from a /// under the current UI culture — so the same registered key /// documents itself in whatever language is in effect when the documentation is extracted. A key is still - /// registered once by its ; only the description text is deferred. + /// registered once by its ; only the description text is deferred. Re-declaring a key + /// with the same name and the same returns the already-registered instance (whose + /// description provider wins); a different value type is rejected. /// /// The type associated with the error context key. /// The unique name of the error context key. Must not be null, empty, or whitespace. /// A function returning the description; invoked each time is read. - /// A new instance of . + /// The registered instance for . /// Thrown when is null, empty, or whitespace. /// Thrown when is null. - /// Thrown when a key with the specified is already registered. + /// Thrown when a key with the specified is already registered with a different value type. public static ErrorContextKey Create(string name, Func descriptionProvider) { if (descriptionProvider is null) { throw new ArgumentNullException(nameof(descriptionProvider)); } @@ -113,7 +123,17 @@ private static ErrorContextKey Register(string name, Func? descri if (string.IsNullOrWhiteSpace(name)) { throw new ArgumentException("Value cannot be null or whitespace.", nameof(name)); } lock (Lock) { - if (Registered.ContainsKey(name)) { throw new InvalidOperationException($"An error context key '{name}' has already been registered."); } + if (Registered.TryGetValue(name, out ErrorContextKey? existing)) { + if (existing.ValueType != typeof(T)) { + throw new InvalidOperationException( + $"An error context key '{name}' is already registered with value type '{existing.ValueType}'; it cannot be re-registered with value type '{typeof(T)}'."); + } + + // Same name, same value type: the declaration is being re-executed (a reloaded plugin, a re-run test + // fixture), not conflicting. The first registered instance is the canonical one — its description + // (or provider) wins — and the cast is safe because ErrorContextKey is the only subclass. + return (ErrorContextKey)existing; + } ErrorContextKey instance = new(name, descriptionProvider); Registered.Add(name, instance); diff --git a/doc/analyzers/FCE001.en.md b/doc/analyzers/FCE001.en.md index 160193c..08523c3 100644 --- a/doc/analyzers/FCE001.en.md +++ b/doc/analyzers/FCE001.en.md @@ -9,7 +9,7 @@ | **Severity** | 🔴 Error | | **Enabled by default** | Yes | -The same literal error code is created by more than one `ErrorCode.Create("...")` in the compilation. `ErrorCode.Create` registers every code in a process-wide set and throws an `InvalidOperationException` the second time a code is registered; this rule turns that runtime failure into a build error and lights up every site that produces the code. +The same literal error code is created by more than one `ErrorCode.Create("...")` in the compilation. An error code is an identity compared by value: two `ErrorCode` instances built from the same string are equal, so a duplicated code silently merges two distinct errors into one — documentation extraction and code lookups keep only a single entry. This rule surfaces that collision as a build error and lights up every site that produces the code. ## Noncompliant @@ -27,7 +27,7 @@ public static readonly ErrorCode B = ErrorCode.Create("MONEY_TRANSFER_AMOUNT_NOT ## Details -Detection is **per compilation** and covers only **literal** codes. Two identical codes in different assemblies, or a code built at run time, still surface only at run time (see [FCE003](FCE003.en.md)). This is a compilation-end diagnostic: it appears at build / full-solution analysis, not necessarily as you type in a single file. +Detection is **per compilation** and covers only **literal** codes. `ErrorCode.Create` does not enforce uniqueness at run time, so two identical codes in different assemblies, or a code built at run time, are not caught here: cross-assembly duplicates are simply equal at run time and are reported as a warning by the documentation pipeline when the catalog is generated, while a runtime-built code is a blind spot for static analysis (see [FCE003](FCE003.en.md)). This is a compilation-end diagnostic: it appears at build / full-solution analysis, not necessarily as you type in a single file. **Related:** [FCE002](FCE002.en.md), [FCE003](FCE003.en.md), [FCE011](FCE011.en.md) diff --git a/doc/analyzers/FCE001.fr.md b/doc/analyzers/FCE001.fr.md index 0bfd096..521a104 100644 --- a/doc/analyzers/FCE001.fr.md +++ b/doc/analyzers/FCE001.fr.md @@ -9,7 +9,7 @@ | **Sévérité** | 🔴 Error | | **Activée par défaut** | Oui | -Le même code d'erreur littéral est créé par plus d'un `ErrorCode.Create("...")` dans la compilation. `ErrorCode.Create` enregistre chaque code dans un ensemble à l'échelle du processus et lève une `InvalidOperationException` au second enregistrement d'un même code ; cette règle transforme cet échec à l'exécution en erreur de build et signale chaque site qui produit le code. +Le même code d'erreur littéral est créé par plus d'un `ErrorCode.Create("...")` dans la compilation. Un code d'erreur est une identité comparée par valeur : deux instances d'`ErrorCode` construites à partir de la même chaîne sont égales, si bien qu'un code dupliqué fusionne silencieusement deux erreurs distinctes en une seule — l'extraction de documentation et les recherches par code ne conservent qu'une seule entrée. Cette règle transforme cette collision en erreur de build et signale chaque site qui produit le code. ## Non conforme @@ -27,7 +27,7 @@ public static readonly ErrorCode B = ErrorCode.Create("MONEY_TRANSFER_AMOUNT_NOT ## Détails -La détection est **par compilation** et ne couvre que les codes **littéraux**. Deux codes identiques dans des assemblies différentes, ou un code construit à l'exécution, n'apparaissent qu'à l'exécution (voir [FCE003](FCE003.fr.md)). C'est un diagnostic de fin de compilation : il apparaît au build / à l'analyse de la solution entière, pas forcément à la frappe dans un seul fichier. +La détection est **par compilation** et ne couvre que les codes **littéraux**. `ErrorCode.Create` n'impose plus l'unicité à l'exécution : deux codes identiques dans des assemblies différentes, ou un code construit à l'exécution, ne sont donc pas détectés ici — des doublons inter-assemblies sont simplement égaux à l'exécution et sont signalés par un avertissement du pipeline de documentation lors de la génération du catalogue, tandis qu'un code construit à l'exécution reste un angle mort de l'analyse statique (voir [FCE003](FCE003.fr.md)). C'est un diagnostic de fin de compilation : il apparaît au build / à l'analyse de la solution entière, pas forcément à la frappe dans un seul fichier. **Voir aussi:** [FCE002](FCE002.fr.md), [FCE003](FCE003.fr.md), [FCE011](FCE011.fr.md)