Skip to content

Drop unwanted rules, adapt to PHPStan 1.11 #199

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 17 additions & 111 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ parameters:
shipmonkRules:
allowComparingOnlyComparableTypes:
enabled: true
allowNamedArgumentOnlyInAttributes:
enabled: true
backedEnumGenerics:
enabled: true
classSuffixNaming:
Expand All @@ -46,37 +44,11 @@ parameters:
forbidArithmeticOperationOnNonNumber:
enabled: true
allowNumericString: false
forbidAssignmentNotMatchingVarDoc:
enabled: true
allowNarrowing: false
forbidCast:
enabled: true
blacklist: ['(array)', '(object)', '(unset)']
forbidCheckedExceptionInCallable:
enabled: true
immediatelyCalledCallables:
array_reduce: 1
array_intersect_ukey: 2
array_uintersect: 2
array_uintersect_assoc: 2
array_intersect_uassoc: 2
array_uintersect_uassoc: [2, 3]
array_diff_ukey: 2
array_udiff: 2
array_udiff_assoc: 2
array_diff_uassoc: 2
array_udiff_uassoc: [2, 3]
array_filter: 1
array_map: 0
array_walk_recursive: 1
array_walk: 1
call_user_func: 0
call_user_func_array: 0
forward_static_call: 0
forward_static_call_array: 0
uasort: 1
uksort: 1
usort: 1
allowedCheckedExceptionCallables: []
forbidCheckedExceptionInYieldingMethod:
enabled: true
Expand Down Expand Up @@ -113,7 +85,7 @@ parameters:
enabled: true
forbidReturnValueInYieldingMethod:
enabled: true
reportRegardlessOfReturnType: false
reportRegardlessOfReturnType: true
forbidVariableTypeOverwriting:
enabled: true
forbidUnsetClassField:
Expand Down Expand Up @@ -162,27 +134,6 @@ new DateTime() > '2040-01-02'; // comparing different types is denied
200 > '1e2'; // comparing different types is denied
```

### allowNamedArgumentOnlyInAttributes
- Allows usage of named arguments only in native attributes
- Before native attributes, we used [DisallowNamedArguments](https://github.com/slevomat/coding-standard/blob/master/doc/functions.md#slevomatcodingstandardfunctionsdisallownamedarguments) sniff. But we used Doctrine annotations, which almost "require" named arguments when converted to native attributes.
```php
class User {
#[Column(type: Types::STRING, nullable: false)] // allowed
private string $email;

public function __construct(string $email) {
$this->setEmail(email: $email); // forbidden
}
}
```
- This one is highly opinionated and will probably be disabled/dropped next major version as it does not provide any extra strictness, you can disable it by:
```neon
parameters:
shipmonkRules:
allowNamedArgumentOnlyInAttributes:
enabled: false
```

### backedEnumGenerics *
- Ensures that every BackedEnum child defines generic type
- This rule makes sense only when BackedEnum was hacked to be generic by stub as described in [this article](https://rnd.shipmonk.com/hacking-generics-into-backedenum-in-php-8-1/)
Expand Down Expand Up @@ -352,58 +303,6 @@ function add(string $a, string $b) {
}
```

### forbidAssignmentNotMatchingVarDoc
- Verifies if defined type in `@var` phpdoc accepts the assigned type during assignment
- No other places except assignment are checked

```php
/** @var string $foo */
$foo = $this->methodReturningInt(); // invalid var phpdoc
```

- For reasons of imperfect implementation of [type infering in phpstan-doctrine](https://github.com/phpstan/phpstan-doctrine#query-type-inference), there is an option to check only array-shapes and forget all other types by using `check-shape-only`
- This is helpful for cases where field nullability is eliminated by WHERE field IS NOT NULL which is not propagated to the inferred types
```php
/** @var array<array{id: int}> $result check-shape-only */
$result = $queryBuilder->select('t.id')
->from(Table::class, 't')
->andWhere('t.id IS NOT NULL')
->getResult();
```

- It is possible to explicitly allow narrowing of types by `@var` phpdoc by using `allow-narrowing`
```php
/** @var SomeClass $result allow-narrowing */
$result = $service->getSomeClassOrNull();
```
- Or you can enable it widely by using:
```neon
parameters:
shipmonkRules:
forbidAssignmentNotMatchingVarDoc:
allowNarrowing: true
```

#### Differences with native check:

- Since `phpstan/phpstan:1.10.0` with bleedingEdge, there is a [very similar check within PHPStan itself](https://phpstan.org/blog/phpstan-1-10-comes-with-lie-detector#validate-inline-phpdoc-%40var-tag-type).
- The main difference is that it allows only subtype (narrowing), not supertype (widening) in `@var` phpdoc.
- This rule allows only widening, narrowing is allowed only when marked by `allow-narrowing` or configured by `allowNarrowing: true`.
- Basically, **there are 3 ways for you to check inline `@var` phpdoc**:
- allow only narrowing
- this rule disabled, native check enabled
- allow narrowing and widening
- this rule enabled with `allowNarrowing: true`, native check disabled
- allow only widening
- this rule enabled, native check disabled

- You can disable native check while keeping bleedingEdge by:
```neon
parameters:
featureToggles:
varTagType: false
```

### forbidCast
- Deny casting you configure
- Possible values to use:
Expand All @@ -428,8 +327,7 @@ parameters:

### forbidCheckedExceptionInCallable
- Denies throwing [checked exception](https://phpstan.org/blog/bring-your-exceptions-under-control) in callables (Closures, Arrow functions 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
- 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 (!)
- By default, native functions like `array_map` are present. So it is recommended not to overwrite the defaults here (by `!` char).
- It is allowed to throw checked exceptions in immediately called callables (e.g. params marked by `@param-immediately-invoked-callable`, see [docs](https://phpstan.org/writing-php-code/phpdocs-basics#callables))
- 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
- It ignores [implicitly thrown Throwable](https://phpstan.org/blog/bring-your-exceptions-under-control#what-does-absent-%40throws-above-a-function-mean%3F)
- Learn more in 🇨🇿 [talk about checked exceptions in general](https://www.youtube.com/watch?v=UQsP1U0sVZM)
Expand All @@ -438,10 +336,6 @@ parameters:
parameters:
shipmonkRules:
forbidCheckedExceptionInCallable:
immediatelyCalledCallables:
'Doctrine\ORM\EntityManager::transactional': 0 # 0 is argument index where the closure appears, you can use list if needed
'Symfony\Contracts\Cache\CacheInterface::get': 1
'Acme\my_custom_function': 0
allowedCheckedExceptionCallables:
'Symfony\Component\Console\Question::setValidator': 0 # symfony automatically converts all thrown exceptions to error output, so it is safe to throw anything here
```
Expand All @@ -462,16 +356,26 @@ parameters:


```php
class TransactionManager {
/**
* @param-immediately-invoked-callable $callback
*/
public function transactional(callable $callback): void {
// ...
$callback();
// ...
}
}

class UserEditFacade
{
/**
* @throws UserNotFoundException
* ^ This throws would normally be reported as never thrown in native phpstan, but we know the closure is immediately called
*/
public function updateUserEmail(UserId $userId, Email $email): void
{
$this->entityManager->transactional(function () use ($userId, $email) {
$user = $this->userRepository->get($userId); // throws checked UserNotFoundException
$this->transactionManager->transactional(function () use ($userId, $email) {
$user = $this->userRepository->get($userId); // can throw checked UserNotFoundException
$user->updateEmail($email);
})
}
Expand Down Expand Up @@ -852,6 +756,8 @@ parameters:
checkMissingCallableSignature: true # https://phpstan.org/config-reference#vague-typehints
checkTooWideReturnTypesInProtectedAndPublicMethods: true # https://phpstan.org/config-reference#checktoowidereturntypesinprotectedandpublicmethods
reportAnyTypeWideningInVarTag: true # https://phpstan.org/config-reference#reportanytypewideninginvartag
reportPossiblyNonexistentConstantArrayOffset: true # https://phpstan.org/config-reference#reportpossiblynonexistentconstantarrayoffset
reportPossiblyNonexistentGeneralArrayOffset: true # https://phpstan.org/config-reference#reportpossiblynonexistentgeneralarrayoffset
```

## Contributing
Expand Down
33 changes: 33 additions & 0 deletions bin/verify-inline-ignore.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php declare(strict_types = 1);

$error = false;
$paths = [
__DIR__ . '/../src',
__DIR__ . '/../tests',
];

foreach ($paths as $path) {
$iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($path));

foreach ($iterator as $entry) {
/** @var DirectoryIterator $entry */
if (!$entry->isFile() || $entry->getExtension() !== 'php') {
continue;
}

$realpath = realpath($entry->getPathname());
$contents = file_get_contents($realpath);
if (strpos($contents, '@phpstan-ignore-') !== false) {
$error = true;
echo $realpath . ' uses @phpstan-ignore-line, use \'@phpstan-ignore identifier (reason)\' instead' . PHP_EOL;
}
}
}

if ($error) {
exit(1);
} else {
echo 'Ok, no non-identifier based inline ignore found' . PHP_EOL;
exit(0);
}

3 changes: 1 addition & 2 deletions composer-dependency-analyser.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,4 @@
require_once('phar://phpstan.phar/preload.php'); // prepends PHPStan's PharAutolaoder to composer's autoloader

return (new Configuration())
->addPathToExclude(__DIR__ . '/tests/Rule/data')
->addPathToExclude(__DIR__ . '/tests/Extension/data');
->addPathToExclude(__DIR__ . '/tests/Rule/data');
7 changes: 4 additions & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
],
"require": {
"php": "^7.4 || ^8.0",
"phpstan/phpstan": "^1.10.51"
"phpstan/phpstan": "1.11.x-dev"
},
"require-dev": {
"editorconfig-checker/editorconfig-checker": "^10.6.0",
Expand All @@ -34,8 +34,7 @@
"ShipMonk\\PHPStan\\": "tests/"
},
"classmap": [
"tests/Rule/data",
"tests/Extension/data"
"tests/Rule/data"
]
},
"config": {
Expand All @@ -61,13 +60,15 @@
"@check:tests",
"@check:dependencies",
"@check:collisions",
"@check:ignores",
"@check:readme"
],
"check:collisions": "detect-collisions src tests",
"check:composer": "composer normalize --dry-run --no-check-lock --no-update-lock",
"check:cs": "phpcs",
"check:dependencies": "composer-dependency-analyser",
"check:ec": "ec src tests",
"check:ignores": "php bin/verify-inline-ignore.php",
"check:readme": "php bin/verify-readme-default-config.php",
"check:tests": "phpunit -vvv tests",
"check:types": "phpstan analyse -vvv --ansi",
Expand Down
23 changes: 11 additions & 12 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 10 additions & 6 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ parameters:
checkBenevolentUnionTypes: true
checkImplicitMixed: true
checkTooWideReturnTypesInProtectedAndPublicMethods: true
reportAnyTypeWideningInVarTag: true
reportPossiblyNonexistentConstantArrayOffset: true
reportPossiblyNonexistentGeneralArrayOffset: true
exceptions:
check:
missingCheckedExceptionInThrows: true
Expand All @@ -37,8 +40,8 @@ parameters:
PHPStan\Rules\Rule: Rule
PhpParser\NodeVisitor: Visitor
ShipMonk\PHPStan\RuleTestCase: RuleTest
forbidAssignmentNotMatchingVarDoc:
enabled: false # native check is better now; this rule will be dropped / reworked in 3.0
enforceClosureParamNativeTypehint:
enabled: false # we support even PHP 7.4, some typehints cannot be used

ignoreErrors:
-
Expand All @@ -50,7 +53,8 @@ parameters:
message: "#but it's missing from the PHPDoc @throws tag\\.$#" # allow uncatched exceptions in tests
path: tests/*

-
message: '#^Call to function method_exists\(\) with PHPStan\\Analyser\\Scope and ''getKeepVoidType'' will always evaluate to true\.$#'
path: src/Rule/ForbidUnusedMatchResultRule.php
reportUnmatched: false # fails only for PHPStan > 1.10.49
# ignore BC promises
- identifier: phpstanApi.class
- identifier: phpstanApi.method
- identifier: phpstanApi.interface
- identifier: phpstanApi.instanceofAssumption
Loading