Skip to content

Commit

Permalink
enforceEnumMatch: update docs with latest PHPStan behaviour (#158)
Browse files Browse the repository at this point in the history
  • Loading branch information
janedbal committed Oct 3, 2023
1 parent 414ba47 commit 44cb256
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 2 deletions.
7 changes: 5 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ enum MyEnum: string { // missing @implements tag

### enforceEnumMatchRule
- Enforces usage of `match ($enum)` instead of exhaustive conditions like `if ($enum === Enum::One) elseif ($enum === Enum::Two)`
- This rule aims to "fix" a bit problematic behaviour of PHPStan (introduced at 1.10). It understands enum cases very well and forces you to adjust following code:
- This rule aims to "fix" a bit problematic behaviour of PHPStan (introduced at 1.10.0 and fixed in [1.10.34](https://github.com/phpstan/phpstan-src/commit/fc7c0283176e5dc3867ade26ac835ee7f52599a9)). It understands enum cases very well and forces you to adjust following code:
```php
enum MyEnum {
case Foo;
Expand All @@ -213,7 +213,7 @@ enum MyEnum {

if ($enum === MyEnum::Foo) {
// ...
} elseif ($enum === MyEnum::Bar) { // always true reported by phpstan
} elseif ($enum === MyEnum::Bar) { // always true reported by phpstan (for versions 1.10.0 - 1.10.34)
// ...
} else {
throw new LogicException('Unknown case'); // phpstan knows it cannot happen
Expand Down Expand Up @@ -241,6 +241,9 @@ Very good approach within similar cases is to use `match` construct so that (ide
PHPStan even adds tip about `match` in those cases since `1.10.11`.
For those reasons, this rule detects any always-true/false enum comparisons and forces you to rewrite it to `match ($enum)`.

Since PHPStan [1.10.34](https://github.com/phpstan/phpstan-src/commit/fc7c0283176e5dc3867ade26ac835ee7f52599a9), the behaviour is much better as it does not report error on the last elseif in case that it is followed by else with thrown exception.
Such case raises exception in your tests if you add new enum case, but it is [still silent in PHPStan](https://phpstan.org/r/a4fdc0ab-5d1e-4f38-80ab-8da2e71a6205). This leaves space for error being deployed to production.
So we still believe this rule makes sense even in latest PHPStan.

### enforceIteratorToArrayPreserveKeys
- Enforces presence of second parameter in [iterator_to_array](https://www.php.net/manual/en/function.iterator-to-array.php) call (`$preserve_keys`) as the default value `true` is generally dangerous (risk of data loss / failure)
Expand Down
13 changes: 13 additions & 0 deletions tests/Rule/data/EnforceEnumMatchRule/code.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,19 @@ public function basicExhaustive(): int
}
}

public function basicExhaustiveWithSafetyException(): int
{
if ($this === self::Three) {
return -1;
} elseif ($this === self::Two) {
return 0;
} elseif ($this === self::One) { // error: This condition contains always-true enum comparison of EnforceEnumMatchRule\SomeEnum::One. Use match expression instead, PHPStan will report unhandled enum cases
return 1;
} else {
throw new \LogicException('cannot happen');
}
}

public function notExhaustiveWithNegatedConditionInLastElseif(): int
{
if ($this === self::Three) {
Expand Down

0 comments on commit 44cb256

Please sign in to comment.