diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f6ac8c..46783a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to Celerity are documented here. This project follows [Keep ## [Unreleased] +### Fixed + +- `StringFnV1AHasher.Hash(null)` now throws `ArgumentNullException` (parameter name `"key"`) instead of `NullReferenceException`. Public APIs should signal a null argument as an explicit contract violation, not as an unchecked dereference. The Celerity dictionaries store the out-of-band `null` / `default(TKey)` key entry without ever calling the hasher, so the surface area of this change is limited to direct `StringFnV1AHasher` usage and to consumers that plug the hasher into custom `IHashProvider` callers that do not handle the null-key slot themselves. The XML doc comment on `Hash` now declares the exception, and `StringFnV1AHasherTests.Hash_NullString_*` is updated to assert `ArgumentNullException` rather than pinning the previous wart. Closes #71. + ## [1.2.0] - 2026-05-10 ### Added diff --git a/src/Celerity.Tests/Hashing/StringFnV1AHasherTests.cs b/src/Celerity.Tests/Hashing/StringFnV1AHasherTests.cs index 9f2b7ca..d168122 100644 --- a/src/Celerity.Tests/Hashing/StringFnV1AHasherTests.cs +++ b/src/Celerity.Tests/Hashing/StringFnV1AHasherTests.cs @@ -66,13 +66,18 @@ public void Hash_DifferentCaseStrings_ProduceDifferentValues() } /// - /// Demonstrates what happens if you pass null into the current implementation. - /// The current code will throw NullReferenceException (or potentially an NRE). - /// If you'd prefer an explicit ArgumentNullException, modify the struct. + /// Hashing a null reference must throw + /// (with the parameter name "key"), not the NullReferenceException the + /// implicit dereference would otherwise produce. Celerity dictionaries + /// route the out-of-band null/default-key entry around the hasher, so the + /// check only ever fires for direct hasher usage — but when it fires, the + /// thrown exception must reflect a contract violation, not an unchecked + /// dereference. Closes issue #71. /// [Fact] - public void Hash_NullString_ThrowsNullReferenceException() + public void Hash_NullString_ThrowsArgumentNullException() { - Assert.Throws(() => _hasher.Hash(null)); + ArgumentNullException ex = Assert.Throws(() => _hasher.Hash(null!)); + Assert.Equal("key", ex.ParamName); } } diff --git a/src/Celerity/Hashing/StringFnV1AHasher.cs b/src/Celerity/Hashing/StringFnV1AHasher.cs index 27b7bc6..d7e9208 100644 --- a/src/Celerity/Hashing/StringFnV1AHasher.cs +++ b/src/Celerity/Hashing/StringFnV1AHasher.cs @@ -13,10 +13,22 @@ namespace Celerity.Hashing; /// public struct StringFnV1AHasher : IHashProvider { - /// + /// + /// Computes the FNV-1a 32-bit hash of the specified string. + /// + /// The string to hash. Must not be null. + /// The signed 32-bit FNV-1a hash of . + /// + /// is null. Celerity dictionaries store the + /// out-of-band null-key entry without calling the hasher, so this + /// check only surfaces when the hasher is used directly or plugged into a + /// consumer that does not handle null keys out-of-band. + /// [MethodImpl(MethodImplOptions.AggressiveInlining)] public int Hash(string key) { + ArgumentNullException.ThrowIfNull(key); + // The FNV-1a 32-bit parameters const uint fnvPrime = 16777619; const uint offsetBasis = 2166136261;