From 33799441a78eeb0134c45bbc9f7fe67cac67d925 Mon Sep 17 00:00:00 2001 From: Gustavo Freze Date: Sat, 23 May 2026 17:50:56 -0300 Subject: [PATCH] fix: Honor private and protected properties in equals and hashCode. --- .claude/rules/php-library-code-style.md | 82 +++-- .claude/rules/php-library-testing.md | 5 +- composer.json | 4 +- phpstan.neon.dist | 12 + src/Internal/ArrayEquality.php | 2 +- src/Internal/ArrayHash.php | 7 +- src/Internal/ObjectProperties.php | 26 ++ src/Internal/ValueObjectEquality.php | 9 +- src/Internal/ValueObjectHash.php | 5 +- tests/Models/MixedVisibilityProfile.php | 17 + tests/Models/PrivateMoney.php | 17 + tests/Models/Rating.php | 20 ++ tests/Unit/ValueObjectBehaviorEqualsTest.php | 296 +++++++++++------- .../Unit/ValueObjectBehaviorHashCodeTest.php | 260 +++++++++------ 14 files changed, 495 insertions(+), 267 deletions(-) create mode 100644 src/Internal/ObjectProperties.php create mode 100644 tests/Models/MixedVisibilityProfile.php create mode 100644 tests/Models/PrivateMoney.php create mode 100644 tests/Models/Rating.php diff --git a/.claude/rules/php-library-code-style.md b/.claude/rules/php-library-code-style.md index 8485df7..997b294 100644 --- a/.claude/rules/php-library-code-style.md +++ b/.claude/rules/php-library-code-style.md @@ -44,27 +44,45 @@ Verify every item before producing any PHP code. If any item fails, revise befor 5. Classes follow the rules in "Inheritance and constructors". `final readonly` is the default, with documented exceptions for extension points and for parents that are not `readonly`. 6. Members are ordered constants first, then constructor, then static methods, then instance - methods. Within each group, order by body size ascending (number of lines between `{` and `}`). - Constants and enum cases, which have no body, are ordered by name length ascending. This - ordering may be overridden only when the alternative carries explicit documentation value: - grouping by domain class with section markers (HTTP status codes by 1xx/2xx/3xx/etc), - mirroring the order of an implemented interface, or similar evident structure. The override - must be obvious at first reading. + methods. Within each group, order by **member name length ascending** (count the name only, + without parentheses, arguments, or return type). Constants, enum cases, and methods share + the same name-length-ascending rule, applied within their respective groups. This mirrors + the rule that governs constructor parameters and named arguments (rule 7). When two names + have equal length, order them alphabetically. This ordering may be overridden only when the + alternative carries explicit documentation value: grouping by domain class with section + markers (HTTP status codes by 1xx/2xx/3xx/etc), mirroring the order of an implemented + interface, or similar evident structure. The override must be obvious at first reading. **At call sites** (chained method calls in production code, tests, or documentation - examples), consecutive method invocations on the same receiver are ordered by the **visible - width** of each call expression ascending. The body is not visible at the call site, so the - visible width is the practical proxy for body size. Boolean toggles such as `->secure()` and - `->httpOnly()` come before parameterized `with*` builders for the same reason. When two - calls have equal width, order them alphabetically by method name. + examples), consecutive method invocations on the same receiver are ordered by **method name + length ascending**, the same rule that governs member declarations. Boolean toggles such as + `->secure()` and `->httpOnly()` come before parameterized `with*` builders because their + names are shorter, not because the expression is narrower. When two method names have equal + length, order them alphabetically. **Terminal methods that change the receiver type** stay at the end of the chain regardless - of width. A `build()` that returns the built value, a `commit()` that finalizes a unit of - work, a `send()` that flushes a request, are terminal: the chain ends with them. The + of name length. A `build()` that returns the built value, a `commit()` that finalizes a unit + of work, a `send()` that flushes a request, are terminal: the chain ends with them. The ordering rule applies only to consecutive calls on the same receiver type; calls that transition to a different type are not reorderable. The same applies in reverse to the factory or accessor that starts the chain (`Cookie::create(...)`, `$repository`) — it stays at its position. + + **PHPUnit test classes** follow a dedicated sub-grouping inside the instance-methods group + that overrides the name-length-ascending rule: + + 1. **Lifecycle hooks** first, in PHPUnit execution order: + `setUpBeforeClass` → `setUp` → `tearDown` → `tearDownAfterClass`. Only those actually + defined appear; never introduce an empty hook to satisfy the rule. + 2. **Test methods** (prefix `test`) next, ordered by name length ascending (alphabetical + tiebreak). + 3. **Data providers** last, ordered by name length ascending (alphabetical tiebreak). + + A method is a data provider if and only if its name appears as the string argument of a + `#[DataProvider('')]` attribute or a `@dataProvider ` docblock annotation on a + test method in the same class. The naming convention (`*DataProvider`) is informational + only; the reference is the authoritative signal. A method named `*DataProvider` that no + test references is dead code under rule 17, not a data provider. 7. Constructor parameters are ordered by parameter name length ascending (count the name only, without `$` or type), except when parameters have an implicit semantic order (for example, `$start/$end`, `$from/$to`, `$startAt/$endAt`), which takes precedence. Parameters with default @@ -225,10 +243,7 @@ are `canceled` (not `cancelled`), `organization` (not `organisation`), `initiali ### When required -- Every method of an interface, **including interfaces declared inside `src/Internal/`**. - Interfaces define contracts. The contract is documentation by definition, regardless of - namespace. The `Internal/` boundary applies to implementations, not to the contracts that - internal collaborators expose to each other. +- Every method of an interface. - Every public method of a concrete class outside `src/Internal/`. Public classes are at the public API boundary by definition. Consumers call every public method directly, and the PHPDoc is the contract for each call. Trivial getters and `with*` methods are not exempt. @@ -244,10 +259,7 @@ are `canceled` (not `cancelled`), `organization` (not `organisation`), `initiali interface. The interface carries the docblock. - Anything inside `src/Internal/`. Internal types are implementation detail and must not carry PHPDoc. The namespace itself is the boundary. See `php-library-architecture.md` for the - architectural meaning of `Internal/`. **Exception**: interfaces and their methods. An - interface declared inside `src/Internal/` still defines a contract, and the contract is - documented per `### When required` regardless of namespace. The prohibition covers concrete - classes, traits, enums, and anonymous classes inside `Internal/`, never interfaces. + architectural meaning of `Internal/`. - Anywhere inside `tests/`. Test methods name the scenario via the `testXxxWhenYyyGivenThenZzz` naming convention, and the `@Given`/`@When`/`@Then`/`@And` annotation blocks defined in `php-library-testing.md` describe the steps. PHPDoc documentation (summary plus @@ -270,10 +282,7 @@ The PHPDoc prohibitions above take priority over the typed-array case. When PHPS - On a **constructor parameter** → suppress via `ignoreErrors` in `phpstan.neon.dist`. Do not add PHPDoc. -- On anything inside **`src/Internal/`** (concrete classes, traits, enums) → suppress via - `ignoreErrors`. Do not add PHPDoc. Interfaces inside `src/Internal/` are the exception: - they carry PHPDoc per `### When required`, and the PHPStan errors they raise are resolved - through the PHPDoc, never through `ignoreErrors`. +- On anything inside **`src/Internal/`** → suppress via `ignoreErrors`. Do not add PHPDoc. - On anything inside **`tests/`** → suppress via `ignoreErrors`. Do not add PHPDoc. - On a **public method of a public (non-Internal) class** → add full PHPDoc with summary, `@param` descriptions, and the typed-array information. The bare-tag form remains @@ -338,8 +347,7 @@ public function __construct(public array $entries) } ``` -**Prohibited.** PHPDoc on a **concrete class** inside `src/Internal/` (the prohibition does -not extend to interfaces; see "Correct" below for an Internal/ interface): +**Prohibited.** PHPDoc on anything inside `src/Internal/`: ```php namespace TinyBlocks\Http\Internal\Client; @@ -353,26 +361,6 @@ final readonly class Url } ``` -**Correct.** Interface declared **inside `src/Internal/`** still carries PHPDoc on every -method. The Internal/ prohibition covers concrete classes; interfaces are exempt because they -are the contract: - -```php -namespace TinyBlocks\Http\Internal\Client; - -interface RequestResolver -{ - /** - * Resolves the given URL against the configured base URL. - * - * @param string $url The path or absolute URL to resolve. - * @return string The absolute URL to dispatch. - * @throws MalformedPath If the URL violates RFC 3986. - */ - public function resolve(string $url): string; -} -``` - **Correct.** Generic array type with summary and `@param` description: ```php diff --git a/.claude/rules/php-library-testing.md b/.claude/rules/php-library-testing.md index 86a0c10..81fdfb8 100644 --- a/.claude/rules/php-library-testing.md +++ b/.claude/rules/php-library-testing.md @@ -36,7 +36,8 @@ Verify every item before producing any test code. If any item fails, revise befo 4. No intermediate variables used only once. Chain method calls when the intermediate state is not referenced elsewhere (e.g., `Money::of(...)->add(...)` instead of `$money = Money::of(...)` followed by `$money->add(...)`). -5. No private or helper methods in test classes. The only non-test methods allowed are data +5. No private or helper methods in test classes. The only non-test methods allowed are PHPUnit + lifecycle hooks (`setUp`, `setUpBeforeClass`, `tearDown`, `tearDownAfterClass`) and data providers. Setup logic complex enough to extract belongs in a dedicated fixture class. 6. Test only the public API. Never assert on private state or `Internal/` classes directly. 7. Test the behavior that **raises** an exception, never the exception itself. Exception classes @@ -69,6 +70,8 @@ Verify every item before producing any test code. If any item fails, revise befo 15. Never use `@codeCoverageIgnore`, attributes, or configuration that exclude code from coverage. Never suppress mutants via `infection.json.dist` or any other mechanism. See "Coverage and mutation discipline". +16. Member ordering in test classes follows `php-library-code-style.md` rule 6 (PHPUnit + test-class sub-grouping). ## Structure: Given/When/Then (BDD) diff --git a/composer.json b/composer.json index b203f1c..0176d26 100644 --- a/composer.json +++ b/composer.json @@ -27,8 +27,8 @@ "php": "^8.5" }, "require-dev": { - "ergebnis/composer-normalize": "^2.51", - "infection/infection": "^0.32", + "ergebnis/composer-normalize": "^2.52", + "infection/infection": "^0.33", "phpstan/phpstan": "^2.1", "phpunit/phpunit": "^13.1", "squizlabs/php_codesniffer": "^4.0" diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 5ecd3cc..4b37955 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -15,4 +15,16 @@ parameters: # Internal array-hash collaborator accepts arrays with arbitrary value types by design. - identifier: missingType.iterableValue path: src/Internal/ArrayHash.php + + # Internal property extractor returns an untyped array by design; PHPDoc is prohibited in Internal/. + - identifier: missingType.iterableValue + path: src/Internal/ObjectProperties.php + + # Private properties are read via reflection; PHPStan cannot trace reflection-based access. + - identifier: property.onlyWritten + path: tests/Models/PrivateMoney.php + + # Private property is read via reflection; PHPStan cannot trace reflection-based access. + - identifier: property.onlyWritten + path: tests/Models/MixedVisibilityProfile.php reportUnmatchedIgnoredErrors: true diff --git a/src/Internal/ArrayEquality.php b/src/Internal/ArrayEquality.php index 8330272..468f3f1 100644 --- a/src/Internal/ArrayEquality.php +++ b/src/Internal/ArrayEquality.php @@ -14,7 +14,7 @@ public static function areEqual(array $left, array $right): bool return array_all( $left, - fn($element, $key) => StructuralEquality::areEqual( + fn(mixed $element, int|string $key): bool => StructuralEquality::areEqual( left: $element, right: $right[$key] ) diff --git a/src/Internal/ArrayHash.php b/src/Internal/ArrayHash.php index 085587a..af592ca 100644 --- a/src/Internal/ArrayHash.php +++ b/src/Internal/ArrayHash.php @@ -11,9 +11,12 @@ public static function hash(array $subject): string $serialized = '['; foreach ($subject as $key => $element) { - $serialized = sprintf('%s%s=%s;', $serialized, $key, StructuralHash::hash(subject: $element)); + $template = '%s%s=%s;'; + $serialized = sprintf($template, $serialized, $key, StructuralHash::hash(subject: $element)); } - return sprintf('%s]', $serialized); + $template = '%s]'; + + return sprintf($template, $serialized); } } diff --git a/src/Internal/ObjectProperties.php b/src/Internal/ObjectProperties.php new file mode 100644 index 0000000..76853f0 --- /dev/null +++ b/src/Internal/ObjectProperties.php @@ -0,0 +1,26 @@ +getProperties() as $property) { + if ($property->isStatic()) { + continue; + } + + $properties[$property->getName()] = $property->getValue(object: $subject); + } + + return $properties; + } +} diff --git a/src/Internal/ValueObjectEquality.php b/src/Internal/ValueObjectEquality.php index e9e1506..cb6dbed 100644 --- a/src/Internal/ValueObjectEquality.php +++ b/src/Internal/ValueObjectEquality.php @@ -14,11 +14,14 @@ public static function areEqual(ValueObject $left, ValueObject $right): bool return false; } - $rightProperties = get_object_vars($right); + $rightProperties = ObjectProperties::extract(subject: $right); return array_all( - get_object_vars($left), - fn($element, $name) => StructuralEquality::areEqual(left: $element, right: $rightProperties[$name]) + ObjectProperties::extract(subject: $left), + fn(mixed $element, string $name): bool => StructuralEquality::areEqual( + left: $element, + right: $rightProperties[$name] + ) ); } } diff --git a/src/Internal/ValueObjectHash.php b/src/Internal/ValueObjectHash.php index c4bdb2b..b3f3a26 100644 --- a/src/Internal/ValueObjectHash.php +++ b/src/Internal/ValueObjectHash.php @@ -12,8 +12,9 @@ public static function hash(ValueObject $subject): string { $serialized = $subject::class; - foreach (get_object_vars($subject) as $name => $element) { - $serialized = sprintf('%s|%s=%s', $serialized, $name, StructuralHash::hash(subject: $element)); + foreach (ObjectProperties::extract(subject: $subject) as $name => $element) { + $template = '%s|%s=%s'; + $serialized = sprintf($template, $serialized, $name, StructuralHash::hash(subject: $element)); } return hash('xxh128', $serialized); diff --git a/tests/Models/MixedVisibilityProfile.php b/tests/Models/MixedVisibilityProfile.php new file mode 100644 index 0000000..1ab4b63 --- /dev/null +++ b/tests/Models/MixedVisibilityProfile.php @@ -0,0 +1,17 @@ +equals(other: $right); + + /** @Then the result is false */ + self::assertFalse($areEqual); + } + + public function testEqualsWhenPrivatePropertyDiffersThenReturnsFalse(): void + { + /** @Given a PrivateMoney instance for one hundred BRL */ + $left = new PrivateMoney(amount: 100, currency: Currency::BRL); + + /** @And another PrivateMoney instance with a different amount */ + $right = new PrivateMoney(amount: 200, currency: Currency::BRL); + + /** @When comparing both instances */ + $areEqual = $left->equals(other: $right); + + /** @Then the result is false */ + self::assertFalse($areEqual); + } + public function testEqualsWhenSameClassAndAllScalarsMatchThenReturnsTrue(): void { /** @Given a Money instance priced in BRL */ @@ -44,13 +77,46 @@ public function testEqualsWhenSameClassAndAllScalarsMatchThenReturnsTrue(): void self::assertTrue($areEqual); } - public function testEqualsWhenScalarPropertyDiffersThenReturnsFalse(): void + public function testEqualsWhenBothNullablePropertiesAreNullThenReturnsTrue(): void { - /** @Given a Money instance for one hundred BRL */ - $left = new Money(amount: 100, currency: Currency::BRL); + /** @Given a Profile without a nickname */ + $left = new Profile(name: 'Ada', nickname: null); - /** @And another Money instance with a different amount */ - $right = new Money(amount: 200, currency: Currency::BRL); + /** @And another Profile without a nickname */ + $right = new Profile(name: 'Ada', nickname: null); + + /** @When comparing both instances */ + $areEqual = $left->equals(other: $right); + + /** @Then the result is true */ + self::assertTrue($areEqual); + } + + public function testEqualsWhenExternalObjectInstanceIsSharedThenReturnsTrue(): void + { + /** @Given a shared DateTimeImmutable instance */ + $moment = new DateTimeImmutable('2026-01-01T00:00:00+00:00'); + + /** @And a Period spanning that moment */ + $left = new Period(startAt: $moment, endAt: $moment); + + /** @And another Period sharing the same DateTimeImmutable instance */ + $right = new Period(startAt: $moment, endAt: $moment); + + /** @When comparing both periods */ + $areEqual = $left->equals(other: $right); + + /** @Then the result is true */ + self::assertTrue($areEqual); + } + + public function testEqualsWhenOnlyOneNullablePropertyIsNullThenReturnsFalse(): void + { + /** @Given a Profile without a nickname */ + $left = new Profile(name: 'Ada', nickname: null); + + /** @And another Profile carrying a nickname */ + $right = new Profile(name: 'Ada', nickname: 'Lovelace'); /** @When comparing both instances */ $areEqual = $left->equals(other: $right); @@ -74,13 +140,13 @@ public function testEqualsWhenDifferentClassesShareSameShapeThenReturnsFalse(): self::assertFalse($areEqual); } - public function testEqualsWhenBothNullablePropertiesAreNullThenReturnsTrue(): void + public function testEqualsWhenPrivatePropertiesShareSameStateThenReturnsTrue(): void { - /** @Given a Profile without a nickname */ - $left = new Profile(name: 'Ada', nickname: null); + /** @Given a PrivateMoney instance priced in BRL */ + $left = new PrivateMoney(amount: 100, currency: Currency::BRL); - /** @And another Profile without a nickname */ - $right = new Profile(name: 'Ada', nickname: null); + /** @And another PrivateMoney instance with identical state */ + $right = new PrivateMoney(amount: 100, currency: Currency::BRL); /** @When comparing both instances */ $areEqual = $left->equals(other: $right); @@ -89,75 +155,117 @@ public function testEqualsWhenBothNullablePropertiesAreNullThenReturnsTrue(): vo self::assertTrue($areEqual); } - public function testEqualsWhenOnlyOneNullablePropertyIsNullThenReturnsFalse(): void + public function testEqualsWhenAssociativeArraysShareSameEntriesThenReturnsTrue(): void { - /** @Given a Profile without a nickname */ - $left = new Profile(name: 'Ada', nickname: null); + /** @Given an Order indexed by SKU */ + $left = new Order(items: ['sku-a' => 1, 'sku-b' => 2], number: 1); - /** @And another Profile carrying a nickname */ - $right = new Profile(name: 'Ada', nickname: 'Lovelace'); + /** @And another Order with the same SKU keys and values */ + $right = new Order(items: ['sku-a' => 1, 'sku-b' => 2], number: 1); + + /** @When comparing both orders */ + $areEqual = $left->equals(other: $right); + + /** @Then the result is true */ + self::assertTrue($areEqual); + } + + public function testEqualsWhenBackedEnumPropertiesShareSameCaseThenReturnsTrue(): void + { + /** @Given a Money instance priced in USD */ + $left = new Money(amount: 50, currency: Currency::USD); + + /** @And another Money instance priced in USD */ + $right = new Money(amount: 50, currency: Currency::USD); /** @When comparing both instances */ $areEqual = $left->equals(other: $right); - /** @Then the result is false */ - self::assertFalse($areEqual); + /** @Then the result is true */ + self::assertTrue($areEqual); } - public function testEqualsWhenNestedValueObjectsAreDistinctInstancesWithSameStateThenReturnsTrue(): void + public function testEqualsWhenArraysAreNestedRecursivelyThenComparesElementWise(): void { - /** @Given an Invoice with a Money total */ - $left = new Invoice(total: new Money(amount: 500, currency: Currency::USD), number: 1); + /** @Given an Order with nested arrays of Money instances */ + $left = new Order( + items: [ + [new Money(amount: 1, currency: Currency::BRL), new Money(amount: 2, currency: Currency::BRL)], + [new Money(amount: 3, currency: Currency::USD)] + ], + number: 1 + ); - /** @And another Invoice with a distinct Money instance carrying the same state */ - $right = new Invoice(total: new Money(amount: 500, currency: Currency::USD), number: 1); + /** @And another Order with separately constructed nested arrays matching the same state */ + $right = new Order( + items: [ + [new Money(amount: 1, currency: Currency::BRL), new Money(amount: 2, currency: Currency::BRL)], + [new Money(amount: 3, currency: Currency::USD)] + ], + number: 1 + ); - /** @When comparing both invoices */ + /** @When comparing both orders */ $areEqual = $left->equals(other: $right); /** @Then the result is true */ self::assertTrue($areEqual); } - public function testEqualsWhenNestedValueObjectsCarryDifferentStateThenReturnsFalse(): void + public function testEqualsWhenAssociativeArrayHasExtraKeyOnOneSideThenReturnsFalse(): void { - /** @Given an Invoice totaling five hundred USD */ - $left = new Invoice(total: new Money(amount: 500, currency: Currency::USD), number: 1); + /** @Given an Order with two SKU entries */ + $left = new Order(items: ['sku-a' => 1, 'sku-b' => 2], number: 1); - /** @And another Invoice totaling seven hundred and fifty USD */ - $right = new Invoice(total: new Money(amount: 750, currency: Currency::USD), number: 1); + /** @And another Order with an additional SKU entry */ + $right = new Order(items: ['sku-a' => 1, 'sku-b' => 2, 'sku-c' => 3], number: 1); - /** @When comparing both invoices */ + /** @When comparing both orders */ $areEqual = $left->equals(other: $right); /** @Then the result is false */ self::assertFalse($areEqual); } - public function testEqualsWhenScalarArraysShareSameElementsInSameOrderThenReturnsTrue(): void + public function testEqualsWhenArrayElementsMixValueObjectsAndArraysThenReturnsFalse(): void { - /** @Given an Order with a list of scalar items */ - $left = new Order(items: [10, 20, 30], number: 1); + /** @Given an Order whose first item is a Money value object */ + $left = new Order(items: [new Money(amount: 1, currency: Currency::BRL)], number: 1); - /** @And another Order with the same scalars in the same order */ - $right = new Order(items: [10, 20, 30], number: 1); + /** @And another Order whose first item is an array carrying the same shape */ + $right = new Order(items: [['amount' => 1, 'currency' => Currency::BRL]], number: 1); /** @When comparing both orders */ $areEqual = $left->equals(other: $right); - /** @Then the result is true */ - self::assertTrue($areEqual); + /** @Then the result is false */ + self::assertFalse($areEqual); } - public function testEqualsWhenScalarArraysShareElementsInDifferentOrderThenReturnsFalse(): void + public function testEqualsWhenMixedVisibilityPrivatePropertyDiffersThenReturnsFalse(): void { - /** @Given an Order with scalar items in ascending order */ - $left = new Order(items: [10, 20, 30], number: 1); + /** @Given a MixedVisibilityProfile with a specific nickname */ + $left = new MixedVisibilityProfile(name: 'Ada', nickname: 'Lovelace'); - /** @And another Order with the same scalars in descending order */ - $right = new Order(items: [30, 20, 10], number: 1); + /** @And another MixedVisibilityProfile with the same name but a different nickname */ + $right = new MixedVisibilityProfile(name: 'Ada', nickname: 'Byron'); - /** @When comparing both orders */ + /** @When comparing both instances */ + $areEqual = $left->equals(other: $right); + + /** @Then the result is false */ + self::assertFalse($areEqual); + } + + public function testEqualsWhenNestedValueObjectsCarryDifferentStateThenReturnsFalse(): void + { + /** @Given an Invoice totaling five hundred USD */ + $left = new Invoice(total: new Money(amount: 500, currency: Currency::USD), number: 1); + + /** @And another Invoice totaling seven hundred and fifty USD */ + $right = new Invoice(total: new Money(amount: 750, currency: Currency::USD), number: 1); + + /** @When comparing both invoices */ $areEqual = $left->equals(other: $right); /** @Then the result is false */ @@ -191,13 +299,28 @@ public function testEqualsWhenArraysOfNestedValueObjectsHaveSameStateThenReturns self::assertTrue($areEqual); } - public function testEqualsWhenAssociativeArraysShareSameEntriesThenReturnsTrue(): void + public function testEqualsWhenMixedVisibilityPropertiesShareSameStateThenReturnsTrue(): void { - /** @Given an Order indexed by SKU */ - $left = new Order(items: ['sku-a' => 1, 'sku-b' => 2], number: 1); + /** @Given a MixedVisibilityProfile with a name and nickname */ + $left = new MixedVisibilityProfile(name: 'Ada', nickname: 'Lovelace'); - /** @And another Order with the same SKU keys and values */ - $right = new Order(items: ['sku-a' => 1, 'sku-b' => 2], number: 1); + /** @And another MixedVisibilityProfile with identical state */ + $right = new MixedVisibilityProfile(name: 'Ada', nickname: 'Lovelace'); + + /** @When comparing both instances */ + $areEqual = $left->equals(other: $right); + + /** @Then the result is true */ + self::assertTrue($areEqual); + } + + public function testEqualsWhenScalarArraysShareSameElementsInSameOrderThenReturnsTrue(): void + { + /** @Given an Order with a list of scalar items */ + $left = new Order(items: [10, 20, 30], number: 1); + + /** @And another Order with the same scalars in the same order */ + $right = new Order(items: [10, 20, 30], number: 1); /** @When comparing both orders */ $areEqual = $left->equals(other: $right); @@ -206,13 +329,13 @@ public function testEqualsWhenAssociativeArraysShareSameEntriesThenReturnsTrue() self::assertTrue($areEqual); } - public function testEqualsWhenAssociativeArrayHasExtraKeyOnOneSideThenReturnsFalse(): void + public function testEqualsWhenScalarArraysShareElementsInDifferentOrderThenReturnsFalse(): void { - /** @Given an Order with two SKU entries */ - $left = new Order(items: ['sku-a' => 1, 'sku-b' => 2], number: 1); + /** @Given an Order with scalar items in ascending order */ + $left = new Order(items: [10, 20, 30], number: 1); - /** @And another Order with an additional SKU entry */ - $right = new Order(items: ['sku-a' => 1, 'sku-b' => 2, 'sku-c' => 3], number: 1); + /** @And another Order with the same scalars in descending order */ + $right = new Order(items: [30, 20, 10], number: 1); /** @When comparing both orders */ $areEqual = $left->equals(other: $right); @@ -221,33 +344,30 @@ public function testEqualsWhenAssociativeArrayHasExtraKeyOnOneSideThenReturnsFal self::assertFalse($areEqual); } - public function testEqualsWhenBackedEnumPropertiesShareSameCaseThenReturnsTrue(): void + public function testEqualsWhenStaticPropertyExistsThenInstanceStateAloneGovernsEquality(): void { - /** @Given a Money instance priced in USD */ - $left = new Money(amount: 50, currency: Currency::USD); + /** @Given a Rating instance with a specific score */ + $left = new Rating(score: 5); - /** @And another Money instance priced in USD */ - $right = new Money(amount: 50, currency: Currency::USD); + /** @And another Rating instance with the same score */ + $right = new Rating(score: 5); /** @When comparing both instances */ $areEqual = $left->equals(other: $right); - /** @Then the result is true */ + /** @Then the result is true because static properties are excluded from equality */ self::assertTrue($areEqual); } - public function testEqualsWhenExternalObjectInstanceIsSharedThenReturnsTrue(): void + public function testEqualsWhenNestedValueObjectsAreDistinctInstancesWithSameStateThenReturnsTrue(): void { - /** @Given a shared DateTimeImmutable instance */ - $moment = new DateTimeImmutable('2026-01-01T00:00:00+00:00'); - - /** @And a Period spanning that moment */ - $left = new Period(startAt: $moment, endAt: $moment); + /** @Given an Invoice with a Money total */ + $left = new Invoice(total: new Money(amount: 500, currency: Currency::USD), number: 1); - /** @And another Period sharing the same DateTimeImmutable instance */ - $right = new Period(startAt: $moment, endAt: $moment); + /** @And another Invoice with a distinct Money instance carrying the same state */ + $right = new Invoice(total: new Money(amount: 500, currency: Currency::USD), number: 1); - /** @When comparing both periods */ + /** @When comparing both invoices */ $areEqual = $left->equals(other: $right); /** @Then the result is true */ @@ -274,46 +394,4 @@ public function testEqualsWhenExternalObjectsAreDistinctInstancesEvenWithSameSta /** @Then the result is false */ self::assertFalse($areEqual); } - - public function testEqualsWhenArrayElementsMixValueObjectsAndArraysThenReturnsFalse(): void - { - /** @Given an Order whose first item is a Money value object */ - $left = new Order(items: [new Money(amount: 1, currency: Currency::BRL)], number: 1); - - /** @And another Order whose first item is an array carrying the same shape */ - $right = new Order(items: [['amount' => 1, 'currency' => Currency::BRL]], number: 1); - - /** @When comparing both orders */ - $areEqual = $left->equals(other: $right); - - /** @Then the result is false */ - self::assertFalse($areEqual); - } - - public function testEqualsWhenArraysAreNestedRecursivelyThenComparesElementWise(): void - { - /** @Given an Order with nested arrays of Money instances */ - $left = new Order( - items: [ - [new Money(amount: 1, currency: Currency::BRL), new Money(amount: 2, currency: Currency::BRL)], - [new Money(amount: 3, currency: Currency::USD)] - ], - number: 1 - ); - - /** @And another Order with separately constructed nested arrays matching the same state */ - $right = new Order( - items: [ - [new Money(amount: 1, currency: Currency::BRL), new Money(amount: 2, currency: Currency::BRL)], - [new Money(amount: 3, currency: Currency::USD)] - ], - number: 1 - ); - - /** @When comparing both orders */ - $areEqual = $left->equals(other: $right); - - /** @Then the result is true */ - self::assertTrue($areEqual); - } } diff --git a/tests/Unit/ValueObjectBehaviorHashCodeTest.php b/tests/Unit/ValueObjectBehaviorHashCodeTest.php index 6cc18a3..a102cc6 100644 --- a/tests/Unit/ValueObjectBehaviorHashCodeTest.php +++ b/tests/Unit/ValueObjectBehaviorHashCodeTest.php @@ -8,78 +8,83 @@ use TinyBlocks\Vo\Models\Coordinate; use TinyBlocks\Vo\Models\Currency; use TinyBlocks\Vo\Models\Invoice; +use TinyBlocks\Vo\Models\MixedVisibilityProfile; use TinyBlocks\Vo\Models\Money; use TinyBlocks\Vo\Models\Order; use TinyBlocks\Vo\Models\Point; +use TinyBlocks\Vo\Models\PrivateMoney; use TinyBlocks\Vo\Models\Profile; +use TinyBlocks\Vo\Models\Rating; final class ValueObjectBehaviorHashCodeTest extends TestCase { - public function testHashCodeWhenInvokedTwiceOnSameInstanceThenHashesAreDeterministic(): void + public function testHashCodeWhenArrayOrderDiffersThenHashesDiffer(): void { - /** @Given a Money instance */ - $money = new Money(amount: 250, currency: Currency::EUR); + /** @Given an Order with items in ascending order */ + $left = new Order(items: [1, 2, 3], number: 1); - /** @When invoked twice */ - $first = $money->hashCode(); - $second = $money->hashCode(); + /** @And another Order with the same items in descending order */ + $right = new Order(items: [3, 2, 1], number: 1); - /** @Then both invocations return the same hash */ - self::assertSame($first, $second); + /** @When checking whether both hashes differ */ + $haveDifferentHashes = $left->hashCode() !== $right->hashCode(); + + /** @Then the result is true */ + self::assertTrue($haveDifferentHashes); } - public function testHashCodeWhenInstancesAreEqualByEqualsThenHashesMatch(): void + public function testHashCodeWhenBackedEnumCaseDiffersThenHashesDiffer(): void { /** @Given a Money instance priced in BRL */ - $left = new Money(amount: 100, currency: Currency::BRL); + $left = new Money(amount: 50, currency: Currency::BRL); - /** @And another Money instance equal by equals */ - $right = new Money(amount: 100, currency: Currency::BRL); + /** @And another Money instance with the same amount priced in USD */ + $right = new Money(amount: 50, currency: Currency::USD); - /** @When checking whether both hashes match */ - $haveSameHash = $left->hashCode() === $right->hashCode(); + /** @When checking whether both hashes differ */ + $haveDifferentHashes = $left->hashCode() !== $right->hashCode(); /** @Then the result is true */ - self::assertTrue($haveSameHash); + self::assertTrue($haveDifferentHashes); } - public function testHashCodeWhenDifferentClassesShareSameShapeThenHashesDiffer(): void + public function testHashCodeWhenScalarPropertyDiffersThenHashesDiffer(): void { - /** @Given a Coordinate instance */ - $coordinate = new Coordinate(latitude: 10.0, longitude: 20.0); + /** @Given a Money instance for one hundred BRL */ + $left = new Money(amount: 100, currency: Currency::BRL); - /** @And a Point instance with identical scalar state */ - $point = new Point(latitude: 10.0, longitude: 20.0); + /** @And another Money instance with a different amount */ + $right = new Money(amount: 200, currency: Currency::BRL); /** @When checking whether both hashes differ */ - $haveDifferentHashes = $coordinate->hashCode() !== $point->hashCode(); + $haveDifferentHashes = $left->hashCode() !== $right->hashCode(); /** @Then the result is true */ self::assertTrue($haveDifferentHashes); } - public function testHashCodeWhenBackedEnumPropertiesShareSameCaseInDistinctInstancesThenHashesMatch(): void + public function testHashCodeWhenPrivatePropertyDiffersThenHashesDiffer(): void { - /** @Given a Money instance priced in USD */ - $left = new Money(amount: 75, currency: Currency::USD); + /** @Given a PrivateMoney instance for one hundred BRL */ + $left = new PrivateMoney(amount: 100, currency: Currency::BRL); - /** @And another distinct Money instance priced in USD */ - $right = new Money(amount: 75, currency: Currency::USD); + /** @And another PrivateMoney instance with a different amount */ + $right = new PrivateMoney(amount: 200, currency: Currency::BRL); - /** @When checking whether both hashes match */ - $haveSameHash = $left->hashCode() === $right->hashCode(); + /** @When checking whether both hashes differ */ + $haveDifferentHashes = $left->hashCode() !== $right->hashCode(); /** @Then the result is true */ - self::assertTrue($haveSameHash); + self::assertTrue($haveDifferentHashes); } - public function testHashCodeWhenNestedValueObjectsAreDistinctInstancesWithSameStateThenHashesMatch(): void + public function testHashCodeWhenInstancesAreEqualByEqualsThenHashesMatch(): void { - /** @Given an Invoice with a Money total */ - $left = new Invoice(total: new Money(amount: 99, currency: Currency::USD), number: 1); + /** @Given a Money instance priced in BRL */ + $left = new Money(amount: 100, currency: Currency::BRL); - /** @And another Invoice with a separately constructed Money carrying the same state */ - $right = new Invoice(total: new Money(amount: 99, currency: Currency::USD), number: 1); + /** @And another Money instance equal by equals */ + $right = new Money(amount: 100, currency: Currency::BRL); /** @When checking whether both hashes match */ $haveSameHash = $left->hashCode() === $right->hashCode(); @@ -88,40 +93,28 @@ public function testHashCodeWhenNestedValueObjectsAreDistinctInstancesWithSameSt self::assertTrue($haveSameHash); } - public function testHashCodeWhenArrayOfNestedValueObjectsCarriesSameStateThenHashesMatch(): void + public function testHashCodeWhenScalarArrayElementDiffersThenHashesDiffer(): void { - /** @Given an Order containing distinct Money instances */ - $left = new Order( - items: [ - new Money(amount: 10, currency: Currency::BRL), - new Money(amount: 20, currency: Currency::USD) - ], - number: 1 - ); + /** @Given an Order with a single scalar item */ + $left = new Order(items: [10], number: 1); - /** @And another Order with separately constructed Money instances matching the same state */ - $right = new Order( - items: [ - new Money(amount: 10, currency: Currency::BRL), - new Money(amount: 20, currency: Currency::USD) - ], - number: 1 - ); + /** @And another Order with a different scalar item */ + $right = new Order(items: [20], number: 1); - /** @When checking whether both hashes match */ - $haveSameHash = $left->hashCode() === $right->hashCode(); + /** @When checking whether both hashes differ */ + $haveDifferentHashes = $left->hashCode() !== $right->hashCode(); /** @Then the result is true */ - self::assertTrue($haveSameHash); + self::assertTrue($haveDifferentHashes); } - public function testHashCodeWhenScalarPropertyDiffersThenHashesDiffer(): void + public function testHashCodeWhenNestedValueObjectStateDiffersThenHashesDiffer(): void { - /** @Given a Money instance for one hundred BRL */ - $left = new Money(amount: 100, currency: Currency::BRL); + /** @Given an Invoice for five hundred USD */ + $left = new Invoice(total: new Money(amount: 500, currency: Currency::USD), number: 1); - /** @And another Money instance with a different amount */ - $right = new Money(amount: 200, currency: Currency::BRL); + /** @And another Invoice carrying a different Money total */ + $right = new Invoice(total: new Money(amount: 750, currency: Currency::USD), number: 1); /** @When checking whether both hashes differ */ $haveDifferentHashes = $left->hashCode() !== $right->hashCode(); @@ -130,58 +123,68 @@ public function testHashCodeWhenScalarPropertyDiffersThenHashesDiffer(): void self::assertTrue($haveDifferentHashes); } - public function testHashCodeWhenBackedEnumCaseDiffersThenHashesDiffer(): void + public function testHashCodeWhenDifferentClassesShareSameShapeThenHashesDiffer(): void { - /** @Given a Money instance priced in BRL */ - $left = new Money(amount: 50, currency: Currency::BRL); + /** @Given a Coordinate instance */ + $coordinate = new Coordinate(latitude: 10.0, longitude: 20.0); - /** @And another Money instance with the same amount priced in USD */ - $right = new Money(amount: 50, currency: Currency::USD); + /** @And a Point instance with identical scalar state */ + $point = new Point(latitude: 10.0, longitude: 20.0); /** @When checking whether both hashes differ */ - $haveDifferentHashes = $left->hashCode() !== $right->hashCode(); + $haveDifferentHashes = $coordinate->hashCode() !== $point->hashCode(); /** @Then the result is true */ self::assertTrue($haveDifferentHashes); } - public function testHashCodeWhenScalarArrayElementDiffersThenHashesDiffer(): void + public function testHashCodeWhenPrivatePropertiesShareSameStateThenHashesMatch(): void { - /** @Given an Order with a single scalar item */ - $left = new Order(items: [10], number: 1); + /** @Given a PrivateMoney instance priced in BRL */ + $left = new PrivateMoney(amount: 100, currency: Currency::BRL); - /** @And another Order with a different scalar item */ - $right = new Order(items: [20], number: 1); + /** @And another PrivateMoney instance with identical state */ + $right = new PrivateMoney(amount: 100, currency: Currency::BRL); - /** @When checking whether both hashes differ */ - $haveDifferentHashes = $left->hashCode() !== $right->hashCode(); + /** @When computing the hash of both instances */ + $haveSameHash = $left->hashCode() === $right->hashCode(); - /** @Then the result is true */ - self::assertTrue($haveDifferentHashes); + /** @Then both hashes match */ + self::assertTrue($haveSameHash); } - public function testHashCodeWhenArrayOrderDiffersThenHashesDiffer(): void + public function testHashCodeWhenStaticPropertyExistsThenOnlyInstanceStateIsHashed(): void { - /** @Given an Order with items in ascending order */ - $left = new Order(items: [1, 2, 3], number: 1); + /** @Given a Rating instance whose class also declares a static property */ + $rating = new Rating(score: 5); - /** @And another Order with the same items in descending order */ - $right = new Order(items: [3, 2, 1], number: 1); + /** @When computing the structural hash */ + $hashCode = $rating->hashCode(); - /** @When checking whether both hashes differ */ - $haveDifferentHashes = $left->hashCode() !== $right->hashCode(); + /** @Then the hash reflects only the instance state, excluding the static property */ + self::assertSame(hash('xxh128', "TinyBlocks\\Vo\\Models\\Rating|score=5"), $hashCode); + } - /** @Then the result is true */ - self::assertTrue($haveDifferentHashes); + public function testHashCodeWhenInvokedTwiceOnSameInstanceThenHashesAreDeterministic(): void + { + /** @Given a Money instance */ + $money = new Money(amount: 250, currency: Currency::EUR); + + /** @When invoked twice */ + $first = $money->hashCode(); + $second = $money->hashCode(); + + /** @Then both invocations return the same hash */ + self::assertSame($first, $second); } - public function testHashCodeWhenNestedValueObjectStateDiffersThenHashesDiffer(): void + public function testHashCodeWhenNullablePropertyDiffersFromPopulatedThenHashesDiffer(): void { - /** @Given an Invoice for five hundred USD */ - $left = new Invoice(total: new Money(amount: 500, currency: Currency::USD), number: 1); + /** @Given a Profile without a nickname */ + $left = new Profile(name: 'Ada', nickname: null); - /** @And another Invoice carrying a different Money total */ - $right = new Invoice(total: new Money(amount: 750, currency: Currency::USD), number: 1); + /** @And another Profile carrying a nickname */ + $right = new Profile(name: 'Ada', nickname: 'Lovelace'); /** @When checking whether both hashes differ */ $haveDifferentHashes = $left->hashCode() !== $right->hashCode(); @@ -205,13 +208,52 @@ public function testHashCodeWhenNullablePropertyIsNullInBothInstancesThenHashesM self::assertTrue($haveSameHash); } - public function testHashCodeWhenNullablePropertyDiffersFromPopulatedThenHashesDiffer(): void + public function testHashCodeWhenNullablePropertyIsNullThenSerializesNullAsLiteralNull(): void { - /** @Given a Profile without a nickname */ - $left = new Profile(name: 'Ada', nickname: null); + /** @Given a Profile whose nickname is null */ + $profile = new Profile(name: 'Ada', nickname: null); - /** @And another Profile carrying a nickname */ - $right = new Profile(name: 'Ada', nickname: 'Lovelace'); + /** @When computing the structural hash */ + $hashCode = $profile->hashCode(); + + /** @Then the hash matches the xxh128 of the documented serialization with NULL for the null property */ + self::assertSame(hash('xxh128', "TinyBlocks\\Vo\\Models\\Profile|name='Ada'|nickname=NULL"), $hashCode); + } + + public function testHashCodeWhenArrayOfNestedValueObjectsCarriesSameStateThenHashesMatch(): void + { + /** @Given an Order containing distinct Money instances */ + $left = new Order( + items: [ + new Money(amount: 10, currency: Currency::BRL), + new Money(amount: 20, currency: Currency::USD) + ], + number: 1 + ); + + /** @And another Order with separately constructed Money instances matching the same state */ + $right = new Order( + items: [ + new Money(amount: 10, currency: Currency::BRL), + new Money(amount: 20, currency: Currency::USD) + ], + number: 1 + ); + + /** @When checking whether both hashes match */ + $haveSameHash = $left->hashCode() === $right->hashCode(); + + /** @Then the result is true */ + self::assertTrue($haveSameHash); + } + + public function testHashCodeWhenMixedVisibilityPropertiesDifferOnPrivateThenHashesDiffer(): void + { + /** @Given a MixedVisibilityProfile with a specific nickname */ + $left = new MixedVisibilityProfile(name: 'Ada', nickname: 'Lovelace'); + + /** @And another MixedVisibilityProfile with the same name but a different nickname */ + $right = new MixedVisibilityProfile(name: 'Ada', nickname: 'Byron'); /** @When checking whether both hashes differ */ $haveDifferentHashes = $left->hashCode() !== $right->hashCode(); @@ -220,15 +262,33 @@ public function testHashCodeWhenNullablePropertyDiffersFromPopulatedThenHashesDi self::assertTrue($haveDifferentHashes); } - public function testHashCodeWhenNullablePropertyIsNullThenSerializesNullAsLiteralNull(): void + public function testHashCodeWhenNestedValueObjectsAreDistinctInstancesWithSameStateThenHashesMatch(): void { - /** @Given a Profile whose nickname is null */ - $profile = new Profile(name: 'Ada', nickname: null); + /** @Given an Invoice with a Money total */ + $left = new Invoice(total: new Money(amount: 99, currency: Currency::USD), number: 1); - /** @When computing the structural hash */ - $hashCode = $profile->hashCode(); + /** @And another Invoice with a separately constructed Money carrying the same state */ + $right = new Invoice(total: new Money(amount: 99, currency: Currency::USD), number: 1); - /** @Then the hash matches the xxh128 of the documented serialization with NULL for the null property */ - self::assertSame(hash('xxh128', "TinyBlocks\\Vo\\Models\\Profile|name='Ada'|nickname=NULL"), $hashCode); + /** @When checking whether both hashes match */ + $haveSameHash = $left->hashCode() === $right->hashCode(); + + /** @Then the result is true */ + self::assertTrue($haveSameHash); + } + + public function testHashCodeWhenBackedEnumPropertiesShareSameCaseInDistinctInstancesThenHashesMatch(): void + { + /** @Given a Money instance priced in USD */ + $left = new Money(amount: 75, currency: Currency::USD); + + /** @And another distinct Money instance priced in USD */ + $right = new Money(amount: 75, currency: Currency::USD); + + /** @When checking whether both hashes match */ + $haveSameHash = $left->hashCode() === $right->hashCode(); + + /** @Then the result is true */ + self::assertTrue($haveSameHash); } }