diff --git a/JustDummies.UnitTests/ValueIdentityTests.cs b/JustDummies.UnitTests/ValueIdentityTests.cs new file mode 100644 index 00000000..c2bcfe7d --- /dev/null +++ b/JustDummies.UnitTests/ValueIdentityTests.cs @@ -0,0 +1,172 @@ +#region Usings declarations + +using JetBrains.Annotations; + +using NFluent; + +#endregion + +namespace JustDummies.UnitTests; + +/// +/// The value identity of the small values the failure-reporting path is built from. Each is immutable and +/// documented as a value, so each answers "is this the same one?" by what it holds rather than by which instance +/// it is — the answer a reference type gives by default, and gives silently. +/// +/// +/// Example-suite material (ADR-0040): each case pins one named pair, and there is no argument to quantify over. +/// has its own equality cases in ; this fixture +/// covers the two values built beside it. +/// +[TestSubject(typeof(ConstraintClaim))] +public sealed class ValueIdentityTests { + + #region Statics members declarations + + private static ConstraintCall Length(string bound) { + return ConstraintCall.Of("WithLength", bound); + } + + #endregion + + // The halves are asserted directly rather than only through ToString and equality: a getter carries no logic, so + // a mutation score says nothing about it, and Constraint is what the blame choice reads. + [Fact(DisplayName = "A claim on a constraint exposes the constraint, its rendering and its clause.")] + public void ClaimOnAConstraintExposesItsHalves() { + ConstraintCall length = Length("3"); + ConstraintClaim claim = ConstraintClaim.Of(length, "already fixes the length at 3"); + + Check.That(claim.Constraint).IsEqualTo(length); + Check.That(claim.Subject).IsEqualTo("WithLength(3)"); + Check.That(claim.Claims).IsEqualTo("already fixes the length at 3"); + Check.That(claim.ToString()).IsEqualTo("WithLength(3) already fixes the length at 3"); + } + + [Fact(DisplayName = "A claim on a phrase exposes the phrase and carries no constraint.")] + public void ClaimOnAPhraseCarriesNoConstraint() { + ConstraintClaim claim = ConstraintClaim.OfPhrase("the contained value \"ABC\"", "contains 'x', which it does not allow"); + + Check.That(claim.Constraint).IsNull(); + Check.That(claim.Subject).IsEqualTo("the contained value \"ABC\""); + Check.That(claim.Claims).IsEqualTo("contains 'x', which it does not allow"); + Check.That(claim.ToString()).IsEqualTo("the contained value \"ABC\" contains 'x', which it does not allow"); + } + + [Fact(DisplayName = "A replay exposes the seed it was given and the guidance naming it.")] + public void ReplayExposesItsSeedAndGuidance() { + FixedRandomSource source = new(7); + + Replay full = Replay.Of(source, 42); + Replay partial = Replay.PartialOf(source); + + Check.That(full.Seed).IsEqualTo(42); + Check.That(full.Guidance).Contains("42"); + // The seed is supplied, not read back from the source, so the two need not agree. + Check.That(partial.Seed).IsEqualTo(7); + Check.That(partial.Guidance).Contains("not reproducible from this seed alone"); + } + + // Rendered for a reader rather than for a message: nothing appends a replay whole — AnyGenerationException takes + // its guidance and its seed separately — so this exists for the debugger, which the type points at it. + [Fact(DisplayName = "A replay renders its seed and guidance rather than its type name.")] + public void ReplayRendersItself() { + FixedRandomSource source = new(7); + + string rendered = Replay.Of(source, 42).ToString(); + + Check.That(rendered).StartsWith("seed 42:"); + Check.That(rendered).Contains("Any.WithSeed(42)"); + Check.That(rendered).Not.Contains(nameof(Replay)); + } + + [Fact(DisplayName = "Two claims blaming the same constraint for the same thing are equal.")] + public void ClaimsWithTheSameConstraintAndClaimAreEqual() { + ConstraintClaim first = ConstraintClaim.Of(Length("3"), "already fixes the length at 3"); + ConstraintClaim second = ConstraintClaim.Of(Length("3"), "already fixes the length at 3"); + + Check.That(first.Equals(second)).IsTrue(); + Check.That(first == second).IsTrue(); + Check.That(first != second).IsFalse(); + Check.That(first.GetHashCode()).IsEqualTo(second.GetHashCode()); + } + + [Fact(DisplayName = "A claim differs when its constraint differs, and when its claim does.")] + public void ClaimsDifferOnEitherHalf() { + ConstraintClaim reference = ConstraintClaim.Of(Length("3"), "already fixes the length at 3"); + + Check.That(reference == ConstraintClaim.Of(Length("5"), "already fixes the length at 3")).IsFalse(); + Check.That(reference == ConstraintClaim.Of(Length("3"), "already caps the length at 3")).IsFalse(); + } + + // The blame choice turns on whether a claim's subject IS the constraint being applied, so a phrase that merely + // reads like one must not pass for it — which is what keeps the two apart here. + [Fact(DisplayName = "A phrase never equals a claim on the constraint it reads like.")] + public void APhraseIsNotTheConstraintItReadsLike() { + ConstraintClaim onAConstraint = ConstraintClaim.Of(Length("3"), "already fixes the length at 3"); + ConstraintClaim onAPhrase = ConstraintClaim.OfPhrase("WithLength(3)", "already fixes the length at 3"); + + Check.That(onAConstraint.ToString()).IsEqualTo(onAPhrase.ToString()); + Check.That(onAConstraint == onAPhrase).IsFalse(); + } + + [Fact(DisplayName = "A claim equals neither null nor a value of another type.")] + public void ClaimEqualsNeitherNullNorAnotherType() { + ConstraintClaim claim = ConstraintClaim.Of(Length("3"), "already fixes the length at 3"); + ConstraintClaim? nothing = null; + object text = "WithLength(3) already fixes the length at 3"; + + Check.That(claim.Equals(nothing)).IsFalse(); + Check.That(claim.Equals(text)).IsFalse(); + Check.That(claim == nothing).IsFalse(); + Check.That(claim != nothing).IsTrue(); + Check.That(nothing == null).IsTrue(); + } + + [Fact(DisplayName = "Two replays of the same run under the same seed are equal.")] + public void ReplaysOfTheSameRunAreEqual() { + FixedRandomSource source = new(7); + + Replay first = Replay.Of(source, 42); + Replay second = Replay.Of(source, 42); + + Check.That(first.Equals(second)).IsTrue(); + Check.That(first == second).IsTrue(); + Check.That(first.GetHashCode()).IsEqualTo(second.GetHashCode()); + } + + [Fact(DisplayName = "A replay differs when its seed differs.")] + public void ReplaysDifferOnTheirSeed() { + FixedRandomSource source = new(7); + + Check.That(Replay.Of(source, 42) == Replay.Of(source, 43)).IsFalse(); + Check.That(Replay.Of(source, 42) != Replay.Of(source, 43)).IsTrue(); + } + + // The seed alone does not settle it: the same seed replays a run in full or only in part depending on whether a + // foreign generator contributed values this source never drew. + [Fact(DisplayName = "A partial replay differs from a full one carrying the same seed.")] + public void APartialReplayIsNotAFullOne() { + FixedRandomSource source = new(7); + + Replay full = Replay.Of(source); + Replay partial = Replay.PartialOf(source); + + Check.That(full.Seed).IsEqualTo(partial.Seed); + Check.That(full == partial).IsFalse(); + } + + [Fact(DisplayName = "A replay equals neither null nor a value of another type.")] + public void ReplayEqualsNeitherNullNorAnotherType() { + FixedRandomSource source = new(7); + Replay replay = Replay.Of(source, 42); + Replay? nothing = null; + object text = "42"; + + Check.That(replay.Equals(nothing)).IsFalse(); + Check.That(replay.Equals(text)).IsFalse(); + Check.That(replay == nothing).IsFalse(); + Check.That(replay != nothing).IsTrue(); + Check.That(nothing == null).IsTrue(); + } + +} diff --git a/JustDummies.UnitTests/ValueObjectConventionTests.cs b/JustDummies.UnitTests/ValueObjectConventionTests.cs new file mode 100644 index 00000000..cbde8225 --- /dev/null +++ b/JustDummies.UnitTests/ValueObjectConventionTests.cs @@ -0,0 +1,125 @@ +#region Usings declarations + +using System.Diagnostics; +using System.Reflection; + +using NFluent; + +#endregion + +namespace JustDummies.UnitTests; + +/// +/// The value-object convention, enforced by reflection over the whole library: a type marked +/// [ValueObject] is sealed, immutable, renders itself, and carries the full identity set — +/// , both Equals overloads, GetHashCode, ToString behind a +/// , and ==/!=. +/// +/// +/// +/// This exists because the gap it closes is silent. A reference type compares by identity when nobody writes +/// another answer, and nothing complains: not the compiler, not a test, not a reviewer reading a type that +/// calls itself a value in its own remarks. Two of this library's values shipped that way, and only the third +/// had its equality — because code happened to compare it with ==, which forced the question. Nothing +/// forced it for the others. +/// +/// +/// The marker is what makes the rule enforceable without guessing: immutability alone would sweep in the +/// generators and the specifications, which are immutable recipes rather than values. Declaring a value is +/// therefore the decision, and this test is what holds it to it. +/// +/// +/// Structure is all reflection can settle, and it is the part that goes missing: whether two equal instances +/// really hash alike belongs to each type's own tests. What is checked here cannot be satisfied by accident. +/// +/// +public sealed class ValueObjectConventionTests { + + private static readonly Assembly LibraryAssembly = typeof(Any).Assembly; + + [Fact(DisplayName = "Every type declared a value object carries a full value identity.")] + public void EveryDeclaredValueObjectCarriesAValueIdentity() { + List values = LibraryAssembly.GetTypes() + .Where(type => type.GetCustomAttribute() is not null) + .OrderBy(type => type.Name, StringComparer.Ordinal) + .ToList(); + + // Guards the scan itself: a renamed attribute or a moved assembly would leave the enumeration empty and every + // assertion below would pass vacuously. Emptiness is the failure mode; the exact count is not pinned, so + // retiring a value never trips this instead of saying what really changed. + Check.WithCustomMessage("No type is marked [ValueObject]; the scan lost its target.") + .That(values).Not.IsEmpty(); + + List violations = []; + foreach (Type value in values) { + violations.AddRange(MissingFrom(value).Select(missing => $"{value.Name}: {missing}")); + } + + Check.WithCustomMessage( + $"Value-object convention — {violations.Count} missing member(s) or property(ies):{Environment.NewLine}" + + string.Join(Environment.NewLine, violations)) + .That(violations) + .IsEmpty(); + } + + #region Per-type verification + + private static IEnumerable MissingFrom(Type value) { + // A struct yields a zero-initialized instance through its parameterless constructor, bypassing every + // validating factory — which is why a value enforcing an invariant is a class in this repository. + if (value.IsValueType) { yield return "is a struct; a value enforcing an invariant is a class here"; } + + // An unsealed value cannot keep equality symmetric: a subclass compares unequal to its base under one + // direction of the comparison and equal under the other. + if (!value.IsValueType && !value.IsSealed) { yield return "is not sealed"; } + + if (!typeof(IEquatable<>).MakeGenericType(value).IsAssignableFrom(value)) { + yield return $"does not implement IEquatable<{value.Name}>"; + } + + if (!DeclaresMethod(value, nameof(Equals), typeof(object))) { yield return "does not override Equals(object)"; } + if (!DeclaresMethod(value, nameof(GetHashCode))) { yield return "does not override GetHashCode()"; } + + // A value that does not render itself shows a debugger its type name, which is the one thing the reader + // already knows. The attribute is what puts that rendering in front of them without expanding the instance. + if (!DeclaresMethod(value, nameof(ToString))) { yield return "does not override ToString()"; } + + DebuggerDisplayAttribute? display = value.GetCustomAttribute(); + if (display is null) { + yield return "does not carry [DebuggerDisplay]"; + } else if (display.Value?.Contains(nameof(ToString)) != true) { + yield return $"carries [DebuggerDisplay(\"{display.Value}\")] rather than forwarding to ToString()"; + } + + // The operator pair is the silent half of the contract: without it `a == b` compiles and compares references, + // where a missing Equals would at least be visible to anyone reading the type. + if (!DeclaresOperator(value, "op_Equality")) { yield return "does not define operator =="; } + if (!DeclaresOperator(value, "op_Inequality")) { yield return "does not define operator !="; } + + foreach (string mutable in MutableStateOf(value)) { yield return mutable; } + } + + private static IEnumerable MutableStateOf(Type value) { + foreach (FieldInfo field in value.GetFields(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)) { + if (!field.IsInitOnly) { yield return $"field '{field.Name}' is not readonly"; } + } + + foreach (PropertyInfo property in value.GetProperties(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)) { + if (property.SetMethod is not null) { yield return $"property '{property.Name}' has a setter"; } + } + } + + private static bool DeclaresMethod(Type value, string name, params Type[] parameters) { + MethodInfo? declared = value.GetMethod(name, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance, null, parameters, null); + + return declared is not null && declared.DeclaringType == value; + } + + private static bool DeclaresOperator(Type value, string name) { + return value.GetMethods(BindingFlags.Public | BindingFlags.Static) + .Any(method => method.Name == name && method.GetParameters().Length == 2); + } + + #endregion + +} diff --git a/JustDummies/ConstraintCall.cs b/JustDummies/ConstraintCall.cs index 4440126a..18ab1835 100644 --- a/JustDummies/ConstraintCall.cs +++ b/JustDummies/ConstraintCall.cs @@ -1,3 +1,9 @@ +#region Usings declarations + +using System.Diagnostics; + +#endregion + namespace JustDummies; /// @@ -40,6 +46,8 @@ namespace JustDummies; /// restate it, and could not be reached from C# without defeating the annotation it duplicates. /// /// +[DebuggerDisplay("{ToString()}")] +[ValueObject] internal sealed class ConstraintCall : IEquatable { #region Statics members declarations diff --git a/JustDummies/ConstraintClaim.cs b/JustDummies/ConstraintClaim.cs index 1b2ac760..9dd43fe0 100644 --- a/JustDummies/ConstraintClaim.cs +++ b/JustDummies/ConstraintClaim.cs @@ -1,3 +1,9 @@ +#region Usings declarations + +using System.Diagnostics; + +#endregion + namespace JustDummies; /// @@ -17,14 +23,24 @@ namespace JustDummies; /// applied — the comparison the blame choice turns on. /// /// +/// Two claims are equal when they blame the same subject for the same thing. Being a value with no identity +/// beyond what it holds, it says so rather than leaving the reference comparison a reader would get by +/// default — the same reason carries its own (ADR-0065). Nothing compares two +/// claims today; a value that answers the question wrongly the first time it is asked is worse than one that +/// answers it, so the answer is written now rather than when a caller needs it. +/// +/// /// It carries no argument guard, and says so with : instances are /// built at a throw site, as an argument to an exception factory, so a guard here would throw while a failure /// is being reported and lose it (ADR-0064). The contract is the compiler's — the members are non-nullable -/// where a value is required, so a caller that cannot prove one is CS8604 at build time. +/// where a value is required, so a caller that cannot prove one is CS8604 at build time. Comparing and +/// hashing stay on that footing: neither composes anything, so neither can fail while a failure is reported. /// /// [BuiltOnTheFailurePath] -internal sealed class ConstraintClaim { +[DebuggerDisplay("{ToString()}")] +[ValueObject] +internal sealed class ConstraintClaim : IEquatable { #region Statics members declarations @@ -46,6 +62,22 @@ internal static ConstraintClaim OfPhrase(string subject, string claims) { #endregion + /// Determines whether two claims blame the same subject for the same thing. + /// The first claim to compare. + /// The second claim to compare. + /// true when both hold the same subject and claim, or both are null; otherwise false. + public static bool operator ==(ConstraintClaim? left, ConstraintClaim? right) { + return Equals(left, right); + } + + /// Determines whether two claims differ in their subject or in what they claim. + /// The first claim to compare. + /// The second claim to compare. + /// true when they differ, or exactly one is null; otherwise false. + public static bool operator !=(ConstraintClaim? left, ConstraintClaim? right) { + return !Equals(left, right); + } + private ConstraintClaim(string subject, ConstraintCall? constraint, string claims) { Subject = subject; Constraint = constraint; @@ -66,4 +98,32 @@ public override string ToString() { return $"{Subject} {Claims}"; } + /// + /// + /// The constraint is compared alongside the text, not merely implied by it: a claim whose subject is a + /// constraint and a phrase that happens to read the same are not the same value, because only the first can + /// be recognised as the constraint being applied. + /// + public bool Equals(ConstraintClaim? other) { + return other is not null + && string.Equals(Subject, other.Subject, StringComparison.Ordinal) + && string.Equals(Claims, other.Claims, StringComparison.Ordinal) + && Constraint == other.Constraint; + } + + /// + public override bool Equals(object? obj) { + return obj is ConstraintClaim other && Equals(other); + } + + /// + public override int GetHashCode() { + unchecked { + int hash = StringComparer.Ordinal.GetHashCode(Subject); + hash = (hash * 397) ^ StringComparer.Ordinal.GetHashCode(Claims); + + return (hash * 397) ^ (Constraint?.GetHashCode() ?? 0); + } + } + } diff --git a/JustDummies/Replay.cs b/JustDummies/Replay.cs index 95aab3e4..b72d3003 100644 --- a/JustDummies/Replay.cs +++ b/JustDummies/Replay.cs @@ -1,3 +1,9 @@ +#region Usings declarations + +using System.Diagnostics; + +#endregion + namespace JustDummies; /// @@ -9,9 +15,16 @@ namespace JustDummies; /// A class rather than a struct, like every value object here: a struct carries a parameterless constructor that /// would yield a seedless, guidance-less instance bypassing the factories below. /// +/// Two replays are equal when they replay the same run the same way — the seed alone does not settle it, +/// since the same seed replays a run in full or only in part depending on what drew. Being a value with no +/// identity beyond what it holds, it says so rather than leaving the reference comparison a reader would get +/// by default (ADR-0065). +/// +/// /// Built while a failure is being reported, so it guards nothing (ADR-0064): a guard on this path would throw /// while a failure is being reported and lose the original. Its parameters are non-nullable instead, which -/// makes the contract the compiler's. +/// makes the contract the compiler's. Comparing and hashing keep that footing: neither composes anything, so +/// neither can fail while a failure is reported. /// /// /// The seed is supplied rather than read back from the source, because the two are not always the same @@ -21,7 +34,9 @@ namespace JustDummies; /// /// [BuiltOnTheFailurePath] -internal sealed class Replay { +[DebuggerDisplay("{ToString()}")] +[ValueObject] +internal sealed class Replay : IEquatable { #region Statics members declarations @@ -52,6 +67,22 @@ internal static Replay PartialOf(RandomSource source) { #endregion + /// Determines whether two replays replay the same run the same way. + /// The first replay to compare. + /// The second replay to compare. + /// true when both carry the same seed and guidance, or both are null; otherwise false. + public static bool operator ==(Replay? left, Replay? right) { + return Equals(left, right); + } + + /// Determines whether two replays differ in their seed or in what they promise to replay. + /// The first replay to compare. + /// The second replay to compare. + /// true when they differ, or exactly one is null; otherwise false. + public static bool operator !=(Replay? left, Replay? right) { + return !Equals(left, right); + } + private Replay(int seed, string guidance) { Seed = seed; Guidance = guidance; @@ -63,4 +94,29 @@ private Replay(int seed, string guidance) { /// The sentence naming the seed and scoping what it replays, appended to the failure message. internal string Guidance { get; } + /// + /// The seed and what it replays, as a reader needs them — the form + /// shows, since a value that renders as its own type name tells a debugger nothing. + /// + public override string ToString() { + return $"seed {Seed}: {Guidance}"; + } + + /// + public bool Equals(Replay? other) { + return other is not null && Seed == other.Seed && string.Equals(Guidance, other.Guidance, StringComparison.Ordinal); + } + + /// + public override bool Equals(object? obj) { + return obj is Replay other && Equals(other); + } + + /// + public override int GetHashCode() { + unchecked { + return (Seed * 397) ^ StringComparer.Ordinal.GetHashCode(Guidance); + } + } + } diff --git a/JustDummies/ValueObjectAttribute.cs b/JustDummies/ValueObjectAttribute.cs new file mode 100644 index 00000000..2d418ceb --- /dev/null +++ b/JustDummies/ValueObjectAttribute.cs @@ -0,0 +1,27 @@ +namespace JustDummies; + +/// +/// Marks a type whose instances are values: two of them holding the same thing are the same one, and nothing +/// about which instance you hold matters. +/// +/// +/// A reference type answers "is this the same one?" by identity unless somebody writes another answer, and it +/// answers silently — no compiler warning, no failing test, just a comparison that quietly means something else. +/// The marker turns that into a contract ValueObjectConventionTests enforces by reflection: a marked type +/// is sealed, immutable, and carries the full set — , both +/// Equals overloads, GetHashCode, and the ==/!= pair, whose absence is the silent +/// case since it degrades to reference comparison rather than failing to compile. +/// +/// It is a declaration, not a detection. Immutability alone does not make a value: the generators and the +/// specifications are immutable too, yet two identically constrained generators are two recipes, not one +/// value, and comparing them would claim a meaning they do not have. Only a type that says it is a value is +/// held to the contract. +/// +/// +/// Marking a struct is rejected by the same convention rather than by usage rules alone: a struct exposes a +/// parameterless constructor that yields an instance bypassing every validating factory, which is why a value +/// enforcing an invariant is a class throughout this repository. +/// +/// +[AttributeUsage(AttributeTargets.Class | AttributeTargets.Struct)] +internal sealed class ValueObjectAttribute : Attribute { } diff --git a/doc/handwritten/for-maintainers/adr/0066-declare-a-value-object-and-enforce-its-identity.fr.md b/doc/handwritten/for-maintainers/adr/0066-declare-a-value-object-and-enforce-its-identity.fr.md new file mode 100644 index 00000000..8faf7ade --- /dev/null +++ b/doc/handwritten/for-maintainers/adr/0066-declare-a-value-object-and-enforce-its-identity.fr.md @@ -0,0 +1,150 @@ +# ADR-0066 | Déclarer un objet-valeur par un attribut, et faire respecter son identité par convention + +🌍 🇫🇷 Français (ce fichier) · 🇬🇧 [English](0066-declare-a-value-object-and-enforce-its-identity.md) + +**Statut :** Proposé +**Proposé :** 2026-07-30 +**Décideurs :** Reefact + +## Contexte + +La bibliothèque porte trois valeurs faites pour être comparées ou transportées par leur contenu plutôt que par +l'instance qu'on tient : une contrainte déclarée (ADR-0065), la paire d'un sujet blâmé et de ce qu'il affirme, et ce +dont un tirage échoué a besoin pour être rejoué. Deux des trois se décrivent dans leurs propres remarques comme des +valeurs comme toutes les autres de ce dépôt, et sont immuables, à constructeur privé atteint par des factories. + +Une seule des trois portait une identité de valeur. Les deux autres répondaient « est-ce le même ? » par référence, +et en silence : un type référence compare par identité tant que personne n'écrit une autre réponse, ce qui ne lève +aucun avertissement du compilateur, ne fait échouer aucun test, et ne se lit pas du tout pour un relecteur. Celle qui +avait son identité l'avait parce que du code la comparait avec `==`, ce qui forçait la question ; rien ne la forçait +pour les deux autres, et le trou est parti en production. + +L'opérateur `==` est la moitié qui se dégrade le plus discrètement. Un type auquel manque `Equals` en manque au moins +visiblement pour qui lit le type ; un type auquel manquent les opérateurs compile encore à chaque `a == b`, et y +compare des références. + +L'immuabilité ne désigne pas une valeur ici. Les générateurs et les spécifications sont immuables aussi — ils sont +reconstruits plutôt que mutés à chaque contrainte — mais deux générateurs identiquement contraints sont deux +recettes, pas une valeur ; les comparer par contenu répondrait à une question qui n'a pas de sens pour eux. + +Le dépôt traite déjà une règle de cette forme par un marqueur plus une convention par réflexion : la convention de +garde null d'ADR-0045 découvre les membres au lieu de les nommer, et ADR-0064 déclare son exemption par +`[BuiltOnTheFailurePath]` plutôt que de l'inférer. ADR-0056 consigne ce qu'il advient d'une règle dans ce dépôt quand +rien ne peut agir dessus : une règle de type explicite a dérivé à 203 violations tant qu'elle vivait là où seul un +lecteur pouvait l'appliquer. + +## Décision + +Un type dont les instances sont des valeurs se déclare par `[ValueObject]`, et une convention par réflexion tient +chaque type marqué à une identité de valeur complète et à se rendre lui-même pour un lecteur. + +## Justification + +Le trou que cela ferme est invisible par construction, ce qui fait de la convention le bon instrument plutôt que +l'attention ou la relecture. Rien, chez une valeur privée de son égalité, ne paraît fautif : le type est immuable, +ses factories sont nommées, ses remarques disent que c'est une valeur. Seule la question posée révèle la réponse, et +deux valeurs sur trois sont parties sans que personne ne la pose. + +Le marqueur gagne sa place parce que la règle ne peut pas être déduite. Détecter les valeurs par l'immuabilité +embarquerait les générateurs et les spécifications et leur exigerait une égalité qui les décrirait mal. Les déduire +d'un motif de nommage serait pire : cela ferait reposer l'application sur une convention pas moins fragile que celle +qu'on applique. Déclarer est une décision qu'un humain prend une fois par type, et une décision est exactement ce +qu'un attribut consigne — le raisonnement même qu'ADR-0064 a appliqué à sa propre exemption plutôt que de l'inférer +de la forme d'un type. + +Faire respecter la paire d'opérateurs est ce qui rentabilise le mieux le coût. C'est le seul membre de l'ensemble +dont l'absence change le comportement sans changer le fait que le code compile : donc celui qu'un relecteur est le +moins capable d'attraper, et une convention le plus. + +La convention vérifie la structure, et s'y arrête délibérément. Savoir si deux instances égales hachent pareil, et +si les champs choisis pour l'égalité sont les bons, sont des questions sur le sens d'un type précis auxquelles +aucune réflexion sur sa forme ne peut répondre ; elles appartiennent aux tests de ce type. Ce que la réflexion peut +trancher — scellé, immuable, et l'ensemble des membres présent — est précisément la moitié qui disparaît quand +personne ne regarde, et elle ne peut pas être satisfaite par accident. + +Le scellement est exigé plutôt qu'encouragé parce qu'une valeur non scellée ne peut pas garder son égalité +symétrique : une sous-classe portant un champ de plus est égale à sa base dans un sens et inégale dans l'autre, ce +qui rompt le contrat dont dépend tout type de collection. Rejeter une structure marquée redit, là où c'est +applicable, la règle permanente selon laquelle une valeur gardant un invariant est une classe : une structure expose +un constructeur sans paramètre produisant une instance ayant contourné toute factory. + +## Alternatives considérées + +### Exiger l'identité de tout type immuable, sans marqueur + +Considérée parce qu'elle ne demande rien à déclarer et ne peut pas être oubliée sur un type neuf. + +Rejetée parce qu'elle n'est pas vraie de tout type immuable ici. Les générateurs et les spécifications sont +immuables et ne sont pas des valeurs : la règle leur imposerait donc une égalité dénuée de sens, ou exigerait une +liste d'exclusion — qui est un marqueur inversé, et qui grossit en silence à mesure que la bibliothèque grandit. + +### Déduire les valeurs d'une convention de nommage ou d'espace de noms + +Considérée parce qu'elle ne demanderait ni attribut ni liste. + +Rejetée parce qu'elle ferait reposer l'application sur une convention exactement aussi peu appliquée que celle +qu'elle remplace. Un type renommé hors du motif quitterait la convention en silence, ce qui est précisément +l'échec que cette décision existe pour empêcher. + +### S'appuyer sur un analyseur plutôt qu'un test + +Considérée parce que le dépôt livre des analyseurs de première main (ADR-0044) et en emploie un là où le système de +types ne peut pas exprimer une règle (ADR-0059). + +Rejetée parce que la règle porte sur les types propres de la bibliothèque, non sur la façon dont un consommateur +écrit son code. Un analyseur est le bon instrument quand le diagnostic doit atteindre le build d'un consommateur ; +ici le public est ce dépôt, et sa propre suite applique déjà des conventions de cette forme par réflexion. + +### Utiliser des `record` pour ces valeurs + +Considérée parce qu'un `record` génère tout l'ensemble d'identité, donc le trou ne pourrait pas survenir. + +Rejetée parce que l'égalité générée porte sur tous les membres, ce qui n'est pas toujours la bonne réponse : l'une +de ces valeurs compare une contrainte **en plus** du texte qui la rend, précisément pour qu'une phrase se lisant +comme une contrainte ne soit pas prise pour elle. Un `record` ferait de plus du constructeur primaire un point +d'entrée public, là où ces types font délibérément passer la construction par des factories nommées. + +## Conséquences + +### Positives + +* Une valeur qui oublie son identité fait échouer un test au lieu de partir en production. +* La paire d'opérateurs — le membre dont l'absence est silencieuse — est appliquée comme le reste. +* Ce qu'est un type est déclaré là où le type est, et un lecteur l'apprend du type lui-même. +* Les types marqués sont énumérables : l'ensemble des valeurs de la bibliothèque est désormais une question qui a + une réponse. + +### Négatives + +* Une valeur nouvelle doit être marquée pour être couverte ; oublier le marqueur la laisse non vérifiée, et seul un + relecteur l'attrape. +* La convention contraint ses types marqués au-delà de l'égalité — scellé, immuable, classe — donc une valeur future + légitime qui devrait être autrement devrait argumenter plutôt que simplement différer. + +### Risques + +* La vérification structurelle peut se lire comme suffisante. Un type peut porter tout l'ensemble et comparer + quand même les mauvais champs ; la convention ne dit rien là-dessus, et sa propre documentation le dit plutôt que + de laisser le lecteur supposer l'inverse. +* Le marqueur peut être posé sur ce qui n'est pas une valeur, ce qui exigerait une égalité la décrivant mal. + Atténué par la seule relecture — l'attribut est une affirmation, et une affirmation fausse est une décision + fausse, pas une règle cassée. + +## Actions de suivi + +* Examiner si les valeurs de `FirstClassErrors` — qui portent déjà leurs identités — doivent se déclarer de la même + façon ; les deux assemblages ne peuvent pas partager l'attribut, JustDummies étant autonome par ADR-0011. + +## Références + +* [ADR-0011](0011-host-dummies-as-a-standalone-package.fr.md) — JustDummies ne dépend de rien dans ce dépôt. +* [ADR-0044](0044-ship-justdummies-analyzers.fr.md) — analyseurs de première main. +* [ADR-0045](0045-guard-public-and-internal-arguments-against-null.fr.md) — une convention qui découvre les membres + au lieu de les nommer. +* [ADR-0056](0056-state-the-coding-rules-where-an-agent-can-act-on-them.fr.md) — une règle sur laquelle rien ne peut + agir dérive. +* [ADR-0059](0059-guard-the-recipe-versus-value-boundary-with-analyzers.fr.md) — quand l'analyseur est l'instrument. +* [ADR-0064](0064-exempt-the-whole-failure-reporting-path-from-the-null-guard-convention.fr.md) — un marqueur + consignant une décision plutôt que de l'inférer. +* [ADR-0065](0065-carry-a-declared-constraint-as-a-value-object.fr.md) — la valeur dont l'égalité a forcé la + question. diff --git a/doc/handwritten/for-maintainers/adr/0066-declare-a-value-object-and-enforce-its-identity.md b/doc/handwritten/for-maintainers/adr/0066-declare-a-value-object-and-enforce-its-identity.md new file mode 100644 index 00000000..c406e541 --- /dev/null +++ b/doc/handwritten/for-maintainers/adr/0066-declare-a-value-object-and-enforce-its-identity.md @@ -0,0 +1,152 @@ +# ADR-0066 | Declare a value object with an attribute, and enforce its identity by convention + +🌍 🇬🇧 English (this file) · 🇫🇷 [Français](0066-declare-a-value-object-and-enforce-its-identity.fr.md) + +**Status:** Proposed +**Proposed:** 2026-07-30 +**Decision Makers:** Reefact + +## Context + +The library holds three values built to be compared or carried by their content rather than by which instance one +holds: a declared constraint (ADR-0065), the pair of a blamed subject and what it claims, and what a failed draw +needs in order to be replayed. Two of the three describe themselves in their own remarks as values like every other +in this repository, and are immutable with private constructors reached through factories. + +Only one of the three carried a value identity. The other two answered "is this the same one?" by reference, and did +so silently: a reference type compares by identity when nobody writes another answer, which raises no compiler +warning, fails no test, and reads as nothing at all to a reviewer. The one that had its identity had it because code +happened to compare it with `==`, which forced the question; nothing forced it for the other two, and the gap +shipped. + +The `==` operator is the half of this that degrades quietest. A type missing `Equals` is at least visibly missing it +to anyone reading the type; a type missing the operators still compiles at every `a == b` and compares references +there. + +Immutability does not identify a value here. The generators and the specifications are immutable too — they are +rebuilt rather than mutated on every constraint — but two identically constrained generators are two recipes, not +one value; comparing them by content would answer a question that has no meaning for them. + +The repository already meets a rule of this shape with a marker plus a reflection convention: ADR-0045's null-guard +convention discovers members rather than naming them, and ADR-0064 declares its exemption with +`[BuiltOnTheFailurePath]` rather than inferring it. ADR-0056 records what becomes of a rule in this repository when +nothing can act on it: an explicit-type rule drifted to 203 violations while it lived where only a reader could +enforce it. + +## Decision + +A type whose instances are values declares itself with `[ValueObject]`, and a reflection convention holds every +marked type to a full value identity and to rendering itself for a reader. + +## Rationale + +The gap this closes is invisible by construction, which is what makes a convention the right instrument rather than +attention or review. Nothing about a value missing its equality looks wrong: the type is immutable, its factories +are named, its remarks say it is a value. Only asking the question reveals the answer, and two of three values +shipped without anyone asking. + +The marker earns its place because the rule cannot be derived. Detecting values by immutability would sweep in the +generators and the specifications and demand of them an equality that would misstate what they are. Deriving them +from a naming pattern would be worse: it would depend on a convention no less fragile than the one being enforced. +Declaring is a decision a human makes once per type, and a decision is exactly what an attribute records — the same +reasoning ADR-0064 applied to its own exemption rather than inferring it from a type's shape. + +Enforcing the operator pair is the part that most repays the cost. It is the only member of the set whose absence +changes behaviour without changing whether the code compiles, so it is the one a reviewer is least able to catch and +a convention is most able to. + +The convention checks structure, and stops there deliberately. Whether two equal instances hash alike, and whether +the fields chosen for equality are the right ones, are questions about a specific type's meaning that no reflection +over its shape can answer; they belong to that type's own tests. What reflection can settle — sealed, immutable, and +the full member set present — is precisely the half that goes missing when nobody is looking, and it cannot be +satisfied by accident. + +Rendering is part of the contract for the same reason the rest is: a value that does not override `ToString` shows a +debugger the one thing its reader already knows — its type name — and nothing about that looks wrong either. The +repository had already settled the form, `[DebuggerDisplay]` forwarding to `ToString`, on the values in +`FirstClassErrors`; it was followed there by attention alone, and the values added since did not follow it. That is +the same drift this decision exists to stop, so the convention carries it rather than a reader. + +Sealedness is required rather than encouraged because an unsealed value cannot keep its equality symmetric: a +subclass carrying an extra field compares equal to its base in one direction and unequal in the other, which breaks +the contract every collection type relies on. Rejecting a marked struct restates, where it can be enforced, the +standing rule that a value guarding an invariant is a class: a struct exposes a parameterless constructor yielding +an instance that bypassed every factory. + +## Alternatives Considered + +### Require the identity of every immutable type, with no marker + +Considered because it needs nothing declared and cannot be forgotten on a new type. + +Rejected because it is not true of every immutable type here. The generators and the specifications are immutable +and are not values, so the rule would either force a meaningless equality on them or need an exclusion list — which +is a marker, inverted, and one that grows silently as the library does. + +### Infer values from a naming or namespace convention + +Considered because it would need no attribute and no list. + +Rejected because it would rest the enforcement on a convention exactly as unenforced as the one it replaces. A type +renamed out of the pattern would leave the convention silently, which is the failure this decision exists to +prevent. + +### Rely on an analyzer instead of a test + +Considered because the repository ships first-party analyzers (ADR-0044) and reaches for one where the type system +cannot express a rule (ADR-0059). + +Rejected because the rule is about the library's own types rather than about how a consumer writes code. An analyzer +is the right instrument when the diagnostic must reach a consumer's build; here the audience is this repository, and +its own suite already enforces conventions of this shape by reflection. + +### Use records for the values + +Considered because a record generates the whole identity set, so the gap could not occur. + +Rejected because the generated equality is over all members, which is not always the right answer — one of these +values compares a constraint alongside the text that renders it, precisely so that a phrase reading like a +constraint is not mistaken for it. A record would also make the primary constructor a public entry point, where +these types deliberately route construction through named factories. + +## Consequences + +### Positive + +* A value that forgets its identity fails a test instead of shipping. +* The operator pair — the member whose absence is silent — is enforced like the rest. +* What a type is, is declared where the type is, and a reader learns it from the type itself. +* Marked types are enumerable, so the set of values in the library is now a question with an answer. + +### Negative + +* A new value must be marked to be covered; forgetting the marker leaves it unchecked, and only a reviewer catches + that. +* The convention constrains its marked types beyond equality — sealed, immutable, class — so a legitimate future + value that needed to be otherwise would have to argue the point rather than simply differ. + +### Risks + +* Structural checking can read as sufficient. A type can carry the whole member set and still compare on the wrong + fields; the convention says nothing about that, and its own documentation says so rather than leaving the reader + to assume otherwise. +* The marker can be applied to something that is not a value, which would demand an equality that misstates it. + Mitigated only by review — the attribute is a claim, and a wrong claim is a wrong decision, not a broken rule. + +## Follow-up Actions + +* Consider whether the values in `FirstClassErrors` — which carry their identities already — should declare + themselves the same way; the two assemblies cannot share the attribute, since JustDummies is standalone by + ADR-0011. + +## References + +* [ADR-0011](0011-host-dummies-as-a-standalone-package.md) — JustDummies depends on nothing in this repository. +* [ADR-0044](0044-ship-justdummies-analyzers.md) — first-party analyzers. +* [ADR-0045](0045-guard-public-and-internal-arguments-against-null.md) — a convention that discovers members rather + than naming them. +* [ADR-0056](0056-state-the-coding-rules-where-an-agent-can-act-on-them.md) — a rule nothing can act on drifts. +* [ADR-0059](0059-guard-the-recipe-versus-value-boundary-with-analyzers.md) — when an analyzer is the instrument. +* [ADR-0064](0064-exempt-the-whole-failure-reporting-path-from-the-null-guard-convention.md) — a marker declaring a + decision rather than inferring it. +* [ADR-0065](0065-carry-a-declared-constraint-as-a-value-object.md) — the value whose equality forced the question. diff --git a/doc/handwritten/for-maintainers/adr/README.md b/doc/handwritten/for-maintainers/adr/README.md index 712d68e1..94c6f412 100644 --- a/doc/handwritten/for-maintainers/adr/README.md +++ b/doc/handwritten/for-maintainers/adr/README.md @@ -267,3 +267,4 @@ Optional supporting material: | [ADR-0063](0063-throw-the-library-s-own-exceptions-through-named-factories.md) | Throw the library's own exceptions through named factories, and only those — the `System` types keep their guard clauses | Accepted | | [ADR-0064](0064-exempt-the-whole-failure-reporting-path-from-the-null-guard-convention.md) | Exempt the whole failure-reporting path from the null-guard convention, declared with `[BuiltOnTheFailurePath]`; supersedes ADR-0045 | Accepted | | [ADR-0065](0065-carry-a-declared-constraint-as-a-value-object.md) | Carry a declared constraint as a value object, not as its rendered text | Proposed | +| [ADR-0066](0066-declare-a-value-object-and-enforce-its-identity.md) | Declare a value object with an attribute, and enforce its identity by convention | Proposed |