|
1 | 1 | # ShipMonk PHPStan rules |
2 | 2 | About **30 super-strict rules** we found useful in ShipMonk. |
3 | | -We tend to have PHPStan set up as strict as possible (bleedingEdge, strict-rules, checkUninitializedProperties, ...), but that still was not strict enough for us. |
| 3 | +We tend to have PHPStan set up as strict as possible ([bleedingEdge](https://phpstan.org/blog/what-is-bleeding-edge), [strict-rules](https://github.com/phpstan/phpstan-strict-rules), [checkUninitializedProperties](https://phpstan.org/config-reference#checkuninitializedproperties), ...), but that still was not strict enough for us. |
4 | 4 | This set of rules should fill the missing gaps we found. |
5 | 5 |
|
6 | 6 | If you find some rules opinionated, you can easily disable them. |
@@ -44,6 +44,28 @@ parameters: |
44 | 44 | forbidCast: |
45 | 45 | enabled: true |
46 | 46 | blacklist: ['(array)', '(object)', '(unset)'] |
| 47 | + forbidCheckedExceptionInCallable: |
| 48 | + enabled: true |
| 49 | + immediatelyCalledCallables: |
| 50 | + array_reduce: 1 |
| 51 | + array_intersect_ukey: 2 |
| 52 | + array_uintersect: 2 |
| 53 | + array_uintersect_assoc: 2 |
| 54 | + array_intersect_uassoc: 2 |
| 55 | + array_uintersect_uassoc: [2, 3] |
| 56 | + array_diff_ukey: 2 |
| 57 | + array_udiff: 2 |
| 58 | + array_udiff_assoc: 2 |
| 59 | + array_diff_uassoc: 2 |
| 60 | + array_udiff_uassoc: [2, 3] |
| 61 | + array_filter: 1 |
| 62 | + array_map: 0 |
| 63 | + array_walk_recursive: 1 |
| 64 | + array_walk: 1 |
| 65 | + uasort: 1 |
| 66 | + uksort: 1 |
| 67 | + usort: 1 |
| 68 | + allowedCheckedExceptionCallables: [] |
47 | 69 | forbidCheckedExceptionInYieldingMethod: |
48 | 70 | enabled: true |
49 | 71 | forbidCustomFunctions: |
@@ -335,9 +357,69 @@ parameters: |
335 | 357 | blacklist!: ['(array)', '(object)', '(unset)'] |
336 | 358 | ``` |
337 | 359 |
|
| 360 | +### forbidCheckedExceptionInCallable |
| 361 | +- Denies throwing [checked exception](https://phpstan.org/blog/bring-your-exceptions-under-control) in callables (Closures and First class callables) as those cannot be tracked as checked by PHPStan analysis, because it is unknown when the callable is about to be called |
| 362 | +- It allows configuration of functions/methods, where the callable is called immediately, those cases are allowed and are also added to [dynamic throw type extension](https://phpstan.org/developing-extensions/dynamic-throw-type-extensions) which causes those exceptions to be tracked properly in your codebase (!) |
| 363 | + - By default, native functions like `array_map` are present. So it is recommended not to overwrite the defaults here (by `!` char). |
| 364 | +- It allows configuration of functions/methods, where the callable is handling all thrown exceptions and it is safe to throw anything from there; this basically makes such calls ignored by this rule |
| 365 | +- It ignores [implicitly thrown Throwable](https://phpstan.org/blog/bring-your-exceptions-under-control#what-does-absent-%40throws-above-a-function-mean%3F) |
| 366 | + |
| 367 | +```neon |
| 368 | +parameters: |
| 369 | + shipmonkRules: |
| 370 | + forbidCheckedExceptionInCallable: |
| 371 | + immediatellyCalledCallables: |
| 372 | + 'Doctrine\ORM\EntityManager::transactional': 0 # 0 is argument index where the closure appears, you can use list if needed |
| 373 | + 'Symfony\Contracts\Cache\CacheInterface::get': 1 |
| 374 | + 'Acme\my_custom_function': 0 |
| 375 | + allowedCheckedExceptionCallables: |
| 376 | + 'Symfony\Component\Console\Question::setValidator': 0 # symfony automatically converts all thrown exceptions to error output, so it is safe to throw anything here |
| 377 | +``` |
| 378 | + |
| 379 | +- We recommend using following config for checked exceptions: |
| 380 | + - Also, [bleedingEdge](https://phpstan.org/blog/what-is-bleeding-edge) enables proper analysis of dead types in multi-catch, so we recommend enabling even that |
| 381 | + |
| 382 | +```neon |
| 383 | +parameters: |
| 384 | + exceptions: |
| 385 | + check: |
| 386 | + missingCheckedExceptionInThrows: true # enforce checked exceptions to be stated in @throws |
| 387 | + tooWideThrowType: true # report invalid @throws (exceptions that are not actually thrown in annotated method) |
| 388 | + implicitThrows: false # no @throws means nothing is thrown (otherwise Throwable is thrown) |
| 389 | + checkedExceptionClasses: |
| 390 | + - YourApp\TopLevelRuntimeException # track only your exceptions (children of some, typically RuntimeException) |
| 391 | +``` |
| 392 | + |
| 393 | + |
| 394 | +```php |
| 395 | +class UserEditFacade |
| 396 | +{ |
| 397 | + /** |
| 398 | + * @throws UserNotFoundException |
| 399 | + * ^ This throws would normally be reported as never thrown in native phpstan, but we know the closure is immediately called |
| 400 | + */ |
| 401 | + public function updateUserEmail(UserId $userId, Email $email): void |
| 402 | + { |
| 403 | + $this->entityManager->transactional(function () use ($userId, $email) { |
| 404 | + $user = $this->userRepository->get($userId); // throws checked UserNotFoundException |
| 405 | + $user->updateEmail($email); |
| 406 | + }) |
| 407 | + } |
| 408 | + |
| 409 | + public function getUpdateEmailCallback(UserId $userId, Email $email): callable |
| 410 | + { |
| 411 | + return function () use ($userId, $email) { |
| 412 | + $user = $this->userRepository->get($userId); // this usage is denied, it throws checked exception, but you don't know when, thus it cannot be tracked by phpstan |
| 413 | + $user->updateEmail($email); |
| 414 | + }; |
| 415 | + } |
| 416 | +} |
| 417 | +``` |
| 418 | + |
338 | 419 | ### forbidCheckedExceptionInYieldingMethod |
339 | 420 | - Denies throwing [checked exception](https://phpstan.org/blog/bring-your-exceptions-under-control) within yielding methods as those exceptions are not throw upon method call, but when generator gets iterated. |
340 | 421 | - This behaviour cannot be easily reflected within PHPStan exception analysis and may cause [false negatives](https://phpstan.org/r/d07ac0f0-a49d-4f82-b1dd-1939058bbeed). |
| 422 | +- Make sure you have enabled checked exceptions, otherwise, this rule does nothing |
341 | 423 |
|
342 | 424 | ```php |
343 | 425 | class Provider { |
|
0 commit comments