fix(result): Result<T> hash collision
A success result wrapping a value whose GetHashCode is 0 (e.g. Result<int> with 0) collides with a failure result, both hashing to 0.
This commit is contained in:
parent
06a40184ee
commit
fbbe94bfa8
3 changed files with 75 additions and 3 deletions
|
|
@ -48,6 +48,7 @@ To have a consistent experience across all packages, some public interfaces have
|
|||
### Fixed
|
||||
|
||||
- **request.result:** Correct the namespace in the `Prelude` doc comment (`Geekeey.Extensions.Result` → `Geekeey.Request.Result`)
|
||||
- **request.result:** Fold `IsSuccess` into `Result<T>.GetHashCode` to avoid collisions between a failure and a success value whose hash is `0`
|
||||
|
||||
### Removed
|
||||
|
||||
|
|
|
|||
|
|
@ -166,11 +166,28 @@ internal sealed class ResultEqualityTests
|
|||
}
|
||||
|
||||
[Test]
|
||||
public async Task I_can_get_hashcode_and_get_zero_for_failure()
|
||||
public async Task I_can_get_hashcode_and_get_non_zero_for_failure()
|
||||
{
|
||||
var result = Prelude.Failure<int>("error");
|
||||
|
||||
await Assert.That(result.GetHashCode()).IsZero();
|
||||
await Assert.That(result.GetHashCode()).IsNotEqualTo(0);
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task I_can_get_hashcode_and_not_collide_non_generic_success_and_failure()
|
||||
{
|
||||
await Assert.That(Prelude.Success().GetHashCode())
|
||||
.IsNotEqualTo(Prelude.Failure("error").GetHashCode());
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task I_can_get_hashcode_and_not_collide_success_value_zero_with_failure()
|
||||
{
|
||||
var success = Prelude.Success(0);
|
||||
var failure = Prelude.Failure<int>("error");
|
||||
|
||||
await Assert.That(success.Equals(failure)).IsFalse();
|
||||
await Assert.That(success.GetHashCode()).IsNotEqualTo(failure.GetHashCode());
|
||||
}
|
||||
|
||||
[Test]
|
||||
|
|
@ -199,4 +216,53 @@ internal sealed class ResultEqualityTests
|
|||
|
||||
await Assert.That(a.Equals(b)).IsFalse();
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task I_can_use_equality_operator_and_get_false_for_success_and_failure()
|
||||
{
|
||||
var success = Prelude.Success(2);
|
||||
var failure = Prelude.Failure<int>("error");
|
||||
|
||||
await Assert.That(success == failure).IsFalse();
|
||||
await Assert.That(success != failure).IsTrue();
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task I_can_use_equality_operator_and_get_true_for_successes_with_equal_value()
|
||||
{
|
||||
await Assert.That(Prelude.Success(2) == Prelude.Success(2)).IsTrue();
|
||||
await Assert.That(Prelude.Success(2) != Prelude.Success(3)).IsTrue();
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task I_can_use_equality_operator_and_compare_result_to_value()
|
||||
{
|
||||
await Assert.That(Prelude.Success(2) == 2).IsTrue();
|
||||
await Assert.That(Prelude.Success(2) != 3).IsTrue();
|
||||
await Assert.That(Prelude.Failure<int>("x") == 2).IsFalse();
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task I_can_equal_object_and_get_false_for_null()
|
||||
{
|
||||
object result = Prelude.Success(2);
|
||||
|
||||
await Assert.That(result.Equals(null)).IsFalse();
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task I_can_equal_object_and_get_false_for_wrong_type()
|
||||
{
|
||||
object result = Prelude.Success(2);
|
||||
|
||||
await Assert.That(result.Equals("not a result")).IsFalse();
|
||||
}
|
||||
|
||||
[Test]
|
||||
public async Task I_can_equal_object_and_get_true_for_boxed_equal_result()
|
||||
{
|
||||
object result = Prelude.Success(2);
|
||||
|
||||
await Assert.That(result.Equals(Prelude.Success(2))).IsTrue();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -181,8 +181,13 @@ public partial class Result<T> : IEquatable<Result<T>>, IEquatable<T>
|
|||
return comparer.GetHashCode(result.Value);
|
||||
}
|
||||
|
||||
if (result is { IsSuccess: true, Value: null })
|
||||
{
|
||||
return 0;
|
||||
}
|
||||
|
||||
return result.Error?.GetHashCode() ?? 0;
|
||||
}
|
||||
}
|
||||
|
||||
public partial class Result<T> : IEqualityOperators<Result<T>, Result<T>, bool>, IEqualityOperators<Result<T>, T, bool>
|
||||
|
|
|
|||
Loading…
Reference in a new issue