Disallow assertEquals through PHPStan

[hmacr]

Added PHPStan lint check to ensure assertEquals is not used. Instead, stricter assertSame is suggested.

Summary by CodeRabbit

• Chores

  • Upgraded the development static analysis tool to the next major version.
  • Added a new static-analysis rule that flags non-strict equality assertions in tests.

• Tests

  • Tightened many unit tests to use strict identity checks (assertSame) instead of loose equality.
  • Adjusted a few test expectations to align with stricter type comparisons.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

Upgrades PHPStan in composer.json from v1 to v2, registers a new PHPStan service and adds the PHPStan path in phpstan.neon, and adds a PHPStan extension class Utopia\PHPStan\DisallowAssertEqualsExtension that flags PHPUnit\Framework\Assert::assertEquals usages. Updates many PHPUnit tests to replace assertEquals with assertSame and adjusts a few expected test values (notably in WhiteList tests and an added getType() check in IP tests). No production API or control-flow changes beyond tests and static analysis configuration.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Mixed changes: dependency bump + config + one new static-analysis class + many repetitive test edits.
• Test edits are numerous but mostly homogeneous (assertEquals → assertSame), lowering review cost; the extension and config require targeted review.

Areas to watch:

• Correctness and namespace/path casing of the new extension file vs. the phpstan.neon path (src/PHPStan vs src/phpstan).
• Implementation of DisallowAssertEqualsExtension (method/class identification, message, identifier, and correct PHPStan service tag).
• Test edits that change expected values (WhiteList numeric -> string changes, added getType() assertions) — spot-check for intentional semantic changes.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and accurately summarizes the main change: adding a PHPStan check to disallow assertEquals usage.

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9d6b681 and 5b54664.

📒 Files selected for processing (2)

• phpstan.neon (1 hunks)
• src/PHPStan/DisallowAssertEqualsExtension.php (1 hunks)

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (8)

tests/Validator/HostTest.php (1)

31-40: Behavior is correct; consider more idiomatic PHPUnit assertions

The switch from assertEquals to assertSame for these boolean and string checks is behaviorally correct and satisfies the new PHPStan rule.

Optionally, for readability and PHPUnit idioms you could:

• Use assertTrue() / assertFalse() for boolean results instead of assertSame(true/false, …).
• Consider using \Utopia\Validator::TYPE_STRING for the type assertion to stay consistent with other validator tests.

These are style-level only; the current code is functionally fine.

tests/Validator/ArrayListTest.php (1)

39-40: Numeric ArrayList type checks are consistent; minor style nit

Using assertSame(TYPE_MIXED, $arrayList->getType()) is consistent with Numeric::getType() returning TYPE_MIXED. In testCanValidateNumericValuesWithBoundaries, the arguments are reversed (assertSame($arrayList->getType(), TYPE_MIXED)), which is harmless but slightly less conventional.

Consider normalizing the ordering to assertSame(\Utopia\Validator::TYPE_MIXED, $arrayList->getType()) for readability and consistent failure messages across the suite.

Also applies to: 49-50

tests/Validator/RangeTest.php (1)

41-45: Float range tests now assert exact metadata; watch min/max typing

For the float range, enforcing getMin() === 0 and getMax() === 1 is consistent with constructing new Range(0, 1, TYPE_FLOAT). Just be aware this couples tests to the internal choice of storing bounds as ints vs floats.

If you later change Range to normalize float bounds to 0.0/1.0, you’ll need to update these expectations; you may want an explicit test documenting the intended bound types for float ranges.

tests/Validator/MultipleOfTest.php (2)

18-19: AllOf string type and description checks now enforce exact strings

Using assertSame('string', $validator->getType()) and the full description string is consistent with how AllOf + Text(20)/URL should compose their type and description. This will catch any unintentional wording or type changes.

Optionally, you could rely on Validator::TYPE_STRING and Text::getDescription() in the expected string to avoid hard‑coding text in multiple places.

---

55-57: Validator list class checks are correctly tightened

Asserting exact class names for the two validators returned by AnyOf::getValidators() via assertSame is a reasonable, strict check on rule composition. No functional issues here.

You may consider comparing against Text::class and URL::class instead of raw strings to make refactors (e.g., namespace changes) safer.

tests/Validator/BooleanTest.php (1)

25-25: Strict type check for Boolean (strict mode) matches implementation

The strict test now asserts that Boolean::getType() returns exactly TYPE_BOOLEAN, aligning with the implementation. Note the arguments are (actual, expected) rather than the usual (expected, actual), which is functionally fine but slightly unconventional.

For consistency and clearer failure messages, you might swap the order to assertSame(\Utopia\Validator::TYPE_BOOLEAN, $boolean->getType()) as used in the loose test below.

tests/Validator/WhiteListTest.php (1)

24-25: Consider keeping PHPUnit’s conventional assertSame($expected, $actual) order

The new assertSame checks look fine semantically, but you’re passing $whiteList->getList() first and the literal array second. To keep consistency and clearer failure messages, you might prefer:

$this->assertSame(['string1', 'string2', 3, 4], $whiteList->getList());
$this->assertSame(['string1', 'string2', '3', '4'], $whiteList->getList());
$this->assertSame(\Utopia\Validator::TYPE_STRING, $whiteList->getType());

This is purely a style/readability tweak; behavior is unchanged.

Also applies to: 42-42, 56-56

src/phpstan/DisallowAssertEqualsExtension.php (1)

10-29: Extension implementation looks correct; just address Pint + PHPMD noise

The restriction logic itself is solid and matches PHPStan’s RestrictedMethodUsageExtension contract: it only flags PHPUnit\Framework\Assert::assertEquals, and the error identifier/message are well‑formed.

Two small follow‑ups:

1. Pint method_argument_space warning

The linter is complaining about the method signature formatting. One simple way to satisfy PSR‑12 and avoid multi‑line argument spacing issues is to collapse the signature to a single line and drop the trailing comma:

-  public function isRestrictedMethodUsage(
-    ExtendedMethodReflection $methodReflection,
-    Scope $scope,
-  ): ?RestrictedUsage {
+    public function isRestrictedMethodUsage(ExtendedMethodReflection $methodReflection, Scope $scope): ?RestrictedUsage
+    {

(Adjust indentation to match the project’s usual 4‑space style if needed.)

2. PHPMD UnusedFormalParameter for $scope

$scope is required by the interface but not used in this implementation. If PHPMD warnings are noisy here, you can explicitly suppress them:

/**
 * @SuppressWarnings(PHPMD.UnusedFormalParameter)
 */
public function isRestrictedMethodUsage(ExtendedMethodReflection $methodReflection, Scope $scope): ?RestrictedUsage
{
    // ...
}

This keeps the method signature correct for PHPStan while silencing the false‑positive from PHPMD.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5c57d5b and 71833b5.

⛔ Files ignored due to path filters (1)

• composer.lock is excluded by !**/*.lock

📒 Files selected for processing (21)

• composer.json (1 hunks)
• phpstan.neon (1 hunks)
• src/phpstan/DisallowAssertEqualsExtension.php (1 hunks)
• tests/Validator/ArrayListTest.php (4 hunks)
• tests/Validator/AssocTest.php (1 hunks)
• tests/Validator/BooleanTest.php (2 hunks)
• tests/Validator/DomainTest.php (5 hunks)
• tests/Validator/FloatValidatorTest.php (2 hunks)
• tests/Validator/HexColorTest.php (1 hunks)
• tests/Validator/HostTest.php (1 hunks)
• tests/Validator/HostnameTest.php (1 hunks)
• tests/Validator/IPTest.php (1 hunks)
• tests/Validator/IntegerTest.php (2 hunks)
• tests/Validator/JSONTest.php (1 hunks)
• tests/Validator/MultipleOfTest.php (2 hunks)
• tests/Validator/NumericTest.php (1 hunks)
• tests/Validator/RangeTest.php (2 hunks)
• tests/Validator/TextTest.php (1 hunks)
• tests/Validator/URLTest.php (1 hunks)
• tests/Validator/WhiteListTest.php (3 hunks)
• tests/Validator/WildcardTest.php (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (14)

tests/Validator/TextTest.php (2)

src/Validator.php (2)

• Validator (5-57)
• getType (56-56)

src/Validator/Text.php (1)

• getType (101-104)

tests/Validator/JSONTest.php (2)

src/Validator.php (2)

• Validator (5-57)
• getType (56-56)

src/Validator/JSON.php (1)

• getType (36-39)

tests/Validator/ArrayListTest.php (4)

src/Validator.php (4)

• getDescription (28-28)
• isValid (47-47)
• Validator (5-57)
• getType (56-56)

src/Validator/Integer.php (4)

• getDescription (37-40)
• Integer (12-88)
• isValid (74-87)
• getType (61-64)

src/Validator/Numeric.php (3)

• getDescription (21-24)
• isValid (58-65)
• getType (45-48)

src/Validator/ArrayList.php (4)

• getDescription (45-58)
• ArrayList (12-120)
• isValid (102-119)
• getType (79-82)

tests/Validator/FloatValidatorTest.php (2)

src/Validator.php (2)

• Validator (5-57)
• getType (56-56)

src/Validator/FloatValidator.php (1)

• getType (61-64)

tests/Validator/IntegerTest.php (2)

src/Validator.php (2)

• Validator (5-57)
• getType (56-56)

src/Validator/Integer.php (1)

• getType (61-64)

tests/Validator/MultipleOfTest.php (1)

src/Validator/AnyOf.php (3)

• getType (83-86)
• getDescription (42-51)
• getValidators (30-33)

tests/Validator/RangeTest.php (2)

src/Validator/Range.php (5)

• getMin (46-49)
• getMax (56-59)
• isArray (90-93)
• getFormat (66-69)
• getType (102-105)

src/Validator.php (3)

• isArray (37-37)
• Validator (5-57)
• getType (56-56)

tests/Validator/NumericTest.php (2)

src/Validator.php (2)

• Validator (5-57)
• getType (56-56)

src/Validator/Numeric.php (1)

• getType (45-48)

tests/Validator/WhiteListTest.php (1)

src/Validator/WhiteList.php (2)

• getList (56-59)
• getType (92-95)

tests/Validator/DomainTest.php (2)

src/Validator/Domain.php (2)

• isValid (65-119)
• Domain (14-144)

src/Validator.php (1)

• isValid (47-47)

tests/Validator/WildcardTest.php (2)

src/Validator.php (2)

• Validator (5-57)
• getType (56-56)

src/Validator/Wildcard.php (1)

• getType (58-61)

tests/Validator/HostnameTest.php (2)

src/Validator.php (2)

• Validator (5-57)
• getType (56-56)

src/Validator/Hostname.php (1)

• getType (53-56)

tests/Validator/BooleanTest.php (2)

src/Validator.php (2)

• getType (56-56)
• Validator (5-57)

src/Validator/Boolean.php (1)

• getType (61-64)

tests/Validator/HexColorTest.php (1)

src/Validator/HexColor.php (1)

• getType (36-39)

🪛 GitHub Actions: Linter

src/phpstan/DisallowAssertEqualsExtension.php

[error] 1-1: PSR-12 style issue detected: method_argument_space (lint/style issue) reported by Pint during 'composer lint'.

🪛 PHPMD (2.15.0)

src/phpstan/DisallowAssertEqualsExtension.php

14-14: Avoid unused parameters such as '$scope'. (undefined)

(UnusedFormalParameter)

🔇 Additional comments (26)

tests/Validator/HostnameTest.php (1)

13-13: Stricter type assertion for Hostname validator looks good

Using assertSame against \Utopia\Validator::TYPE_STRING matches the declared getType() contract and enforces both type and value strictly.

tests/Validator/AssocTest.php (1)

28-28: Assoc type check now uses strict comparison appropriately

Switching to assertSame(\Utopia\Validator::TYPE_ARRAY, $this->assoc->getType()) is correct and consistent with the validator type contract.

tests/Validator/TextTest.php (1)

20-20: Text validator type assertion tightened correctly

The move to assertSame(\Utopia\Validator::TYPE_STRING, $validator->getType()) is correct and in line with the new assertEquals restriction.

tests/Validator/HexColorTest.php (1)

24-24: HexColor type assertion correctly switched to assertSame

assertSame(\Utopia\Validator::TYPE_STRING, $hexColor->getType()) is the right strict check and matches the validator’s getType() implementation.

tests/Validator/FloatValidatorTest.php (1)

21-21: Consistent strict type checks for FloatValidator in both modes

Using assertSame(\Utopia\Validator::TYPE_FLOAT, $validator->getType()) in both strict and loose validation tests is correct and keeps type expectations consistent across modes.

Also applies to: 37-37

tests/Validator/JSONTest.php (1)

26-26: JSON validator type assertion correctly tightened

assertSame(\Utopia\Validator::TYPE_OBJECT, $json->getType()) enforces strict equality and aligns with the new no-assertEquals rule.

composer.json (1)

27-30: PHPStan 2.x upgrade is fully compatible with existing setup; verification unnecessary

The upgrade to phpstan/phpstan "2.*" is confirmed compatible. The custom DisallowAssertEqualsExtension uses only PHPStan 2.x APIs (RestrictedUsage, RestrictedMethodUsageExtension), and the phpstan.neon configuration is correctly structured with proper service registration and tags. No deprecated patterns from PHPStan 1.x were detected.

tests/Validator/IPTest.php (4)

27-34: Stricter boolean checks and added type assertion look good

Using assertSame(true/false, …) here correctly enforces boolean return types from IP::isValid, and the new getType() check against 'string' is consistent with the validator contract. No issues spotted.

Please run the IP test suite under your target PHP/PHPUnit versions to confirm there are no type surprises from IP::isValid() or IP::getType().

---

42-48: ALL‑mode IP validation assertions are safely tightened

The ALL‑mode tests now strictly assert boolean results for both IPv4 and IPv6 candidates. This matches the intended behavior of IP(IP::ALL) and should only surface real type mismatches.

Confirm CI/phpunit runs for IPTest::testIsValidIPALL to ensure no regressions from stricter comparison.

---

56-62: IPv4‑only expectations remain unchanged, now with strict bools

The expectations for IP(IP::V4) are unchanged logically and assertSame enforces that isValid returns actual booleans. Looks correct.

Please verify the IPv4 tests still pass on all supported PHP versions, in case filter_var or internal logic ever returns non‑bools.

---

70-76: IPv6‑only tests are now more type‑strict without altering behavior

The V6‑only test cases still match the intended acceptance/rejection set; using assertSame simply tightens the type check on isValid. No further changes needed.

Re‑run the IPv6 tests to confirm there are no latent type differences (e.g., truthy/non‑truthy values) exposed by assertSame.

tests/Validator/ArrayListTest.php (2)

13-17: Description string assertions correctly tightened

The two description checks now use assertSame against the full, composite description coming from ArrayList + Integer, which matches the documented behavior of ArrayList::getDescription() and Integer::getDescription(). This is an appropriate place for strict string comparison.

If descriptions are expected to be stable API, keep these exact‑string tests; otherwise consider centralizing expected descriptions to reduce brittleness.

---

29-29: Type assertion for text ArrayList matches underlying validator

Asserting TYPE_STRING via assertSame aligns with ArrayList::getType() delegating to the inner Text validator. Looks correct and should surface any unintended type change.

Confirm there are no alternative code paths where ArrayList::getType() might return non‑string or a different constant for text validators.

tests/Validator/RangeTest.php (1)

22-26: Integer range metadata checks are safely made strict

The strict checks for getMin(), getMax(), getFormat(), and getType() on an integer Range instance line up with the Range implementation (min/max as numbers, format/type as TYPE_INTEGER). No behavior change beyond catching type drift.

If you ever modify Range’s constructor or storage types, re‑run these tests to ensure min/max continue to be stored as ints in the integer case.

tests/Validator/URLTest.php (2)

35-45: URL validator behavior is unchanged; assertions are stricter

All expectations around description and validity of various URL shapes remain the same; replacing them with assertSame just enforces boolean type and exact description text. This aligns with the described behavior of the URL validator (no protocol whitelist, but requires some scheme).

Please confirm these still pass under your current URL implementation—especially the more exotic percent‑encoded and non‑standard protocol cases.

---

50-53: Allowed‑schemes tests correctly use strict description and result checks

The description string including (http, https) and the strict booleans for allowed vs disallowed schemes look correct for a URL validator configured with scheme allow‑list. No additional changes needed.

If the description format for scheme lists ever changes (ordering, separators), remember to update these tests accordingly.

tests/Validator/NumericTest.php (1)

22-22: TYPE_MIXED type assertion tightened as intended

Switching to assertSame(TYPE_MIXED, $numeric->getType()) matches Numeric::getType() and ensures no accidental widening/narrowing of the reported type. This is exactly what you want from the new PHPStan rule.

Run the numeric tests across supported PHP versions to verify Numeric::getType() consistently returns the TYPE_MIXED constant.

tests/Validator/DomainTest.php (5)

31-58: Core domain validity matrix now uses strict booleans; expectations remain sensible

The primary testIsValid matrix still matches the documented behavior of Domain::isValid() with default (hostname‑strict) settings: typical domains and localhost accepted; invalid characters, spacing, leading/trailing dots/dashes, empty strings, non‑strings, and arrays/numbers rejected. assertSame(true/false, …) simply hardens type checking.

Given the breadth of cases, ensure CI runs this suite on all supported PHP versions to guard against behavior shifts in FILTER_VALIDATE_DOMAIN or related PHP internals.

---

69-87: Permissive (hostnames=false) behavior is clearly captured with strict asserts

The permissive validator now has strict expectations for internationalized domains (punycode), numeric labels, localhost/intranet, underscores, long labels, extra subdomains, unsafe dash placements, and spaces—matching the described semantics when hostnames=false (no FILTER_FLAG_HOSTNAME). Tightening to assertSame is appropriate here.

These tests closely mirror FILTER_VALIDATE_DOMAIN quirks; re‑run them when upgrading PHP to detect any change in that filter’s permissive behavior.

---

98-101: Invalid‑even‑permissive and edge‑case permissive domains now strictly asserted

The cases that must remain invalid even with hostnames=false (double dots, leading/trailing dots, trailing dash, too‑long labels/domains) and the “surprisingly valid” examples (spaces, @, #, protocol, port, path) are all now using strict boolean assertions. This tightly documents current behavior.

Because these tests codify some non‑intuitive behaviors of FILTER_VALIDATE_DOMAIN, they’re good canaries—verify they still pass when PHP or extensions are upgraded.

Also applies to: 105-106, 111-112, 115-120

---

130-147: Restriction‑based validation scenarios remain unchanged, now stricter

The restriction tests (levels, prefix deny‑list, and shard rules) still express plausible business rules; switching to assertSame just enforces bool returns. The combinations for appwrite.network and fra.appwrite.run look self‑consistent given the restriction configuration.

If restriction behavior changes (e.g., new restrictions added), extend this matrix rather than loosening assertions to keep coverage strong.

---

155-176: Hostnames parameter behavior is well‑covered with strict booleans

testHostnamesParameter nicely contrasts strict (hostnames=true) vs permissive (hostnames=false) behavior for underscores, spaces, and dash placements, plus shared valid domains. The new assertSame calls are appropriate and should expose any accidental regression in how the flag is wired through.

Keep this as a primary regression test when changing Domain’s use of FILTER_FLAG_HOSTNAME or introducing additional normalization.

tests/Validator/BooleanTest.php (1)

44-44: Loose Boolean validator also correctly asserts TYPE_BOOLEAN strictly

Here the argument order is conventional (expected constant first), and assertSame ensures the type constant isn’t accidentally changed. This pairs well with the strict‑mode test.

Please confirm both strict and loose boolean tests pass across your supported PHP versions after these assertion changes.

tests/Validator/IntegerTest.php (1)

20-20: Switch to assertSame looks correct

Using assertSame(\Utopia\Validator::TYPE_INTEGER, $validator->getType()) is consistent with the validator’s string return type and the new project convention.

Also applies to: 34-34

tests/Validator/WildcardTest.php (1)

19-19: assertSame usage here is appropriate

The strict type assertion for \Utopia\Validator::TYPE_STRING versus $validator->getType() matches the validator’s contract and the new test convention.

phpstan.neon (1)

6-10: PHPStan extension wiring looks correct

Registering Utopia\PHPStan\DisallowAssertEqualsExtension under the phpstan.restrictedMethodUsageExtension tag is consistent with PHPStan 2’s configuration for restricted usage extensions. Just ensure the Utopia\PHPStan namespace is autoloadable (e.g., via Composer) so PHPStan can instantiate the service.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/phpstan/DisallowAssertEqualsExtension.php (1)

14-14: Optional: Consider suppressing the PHPMD warning for unused parameter.

The $scope parameter is required by the RestrictedMethodUsageExtension interface but not needed for this specific check. You can optionally add a PHPDoc annotation to suppress the PHPMD warning:

+    /**
+     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
+     */
     public function isRestrictedMethodUsage(

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 71833b5 and 9d6b681.

📒 Files selected for processing (1)

• src/phpstan/DisallowAssertEqualsExtension.php (1 hunks)

🧰 Additional context used

🪛 PHPMD (2.15.0)

src/phpstan/DisallowAssertEqualsExtension.php

14-14: Avoid unused parameters such as '$scope'. (undefined)

(UnusedFormalParameter)

🔇 Additional comments (2)

src/phpstan/DisallowAssertEqualsExtension.php (2)

10-29: LGTM! Well-implemented PHPStan extension.

The implementation correctly identifies assertEquals calls on PHPUnit\Framework\Assert and provides a clear error message directing developers to use the stricter assertSame() instead.

---

10-29: The code correctly implements the PHPStan 2.x API for RestrictedMethodUsageExtension.

The implementation is verified as correct:

• The isRestrictedMethodUsage() method signature matches PHPStan 2.x specification
• RestrictedUsage::create() is called with the correct parameters: errorMessage and identifier (both strings)
• All imports from PHPStan\Rules\RestrictedUsage are appropriate for PHPStan 2.x
• Named parameters are valid given the project requires PHP 8.0+