From 996bc69fa977aa64f601bd82b8a0ae39be0cbeef Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 17 Jun 2022 16:47:24 +0200 Subject: [PATCH] ApiInstanceofRule - report "maybe" on `@api`-covered classes --- src/Rules/Api/ApiInstanceofRule.php | 26 ++++++++++++++++++- .../Rules/Api/ApiInstanceofRuleTest.php | 14 ++++++++++ .../Api/data/instanceof-out-of-phpstan.php | 20 ++++++++++++++ 3 files changed, 59 insertions(+), 1 deletion(-) diff --git a/src/Rules/Api/ApiInstanceofRule.php b/src/Rules/Api/ApiInstanceofRule.php index 1b15163d12..12e22e7ff2 100644 --- a/src/Rules/Api/ApiInstanceofRule.php +++ b/src/Rules/Api/ApiInstanceofRule.php @@ -4,9 +4,12 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; +use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; +use PHPStan\Rules\RuleError; use PHPStan\Rules\RuleErrorBuilder; +use PHPStan\Type\Constant\ConstantBooleanType; use function count; use function sprintf; @@ -61,11 +64,32 @@ public function processNode(Node $node, Scope $scope): array foreach ($docBlock->getPhpDocNodes() as $phpDocNode) { $apiTags = $phpDocNode->getTagsByName('@api'); if (count($apiTags) > 0) { - return []; + return $this->processCoveredClass($node, $scope, $classReflection); } } return [$ruleError]; } + /** + * @return RuleError[] + */ + private function processCoveredClass(Node\Expr\Instanceof_ $node, Scope $scope, ClassReflection $classReflection): array + { + $instanceofType = $scope->getType($node); + if ($instanceofType instanceof ConstantBooleanType) { + return []; + } + + return [ + RuleErrorBuilder::message(sprintf( + 'Although %s is covered by backward compatibility promise, this instanceof assumption might break because it\'s not guaranteed to always stay the same.', + $classReflection->getDisplayName(), + ))->tip(sprintf( + "In case of questions how to solve this correctly, open a discussion:\n %s\n\n See also:\n https://phpstan.org/developing-extensions/backward-compatibility-promise", + 'https://github.com/phpstan/phpstan/discussions', + ))->build(), + ]; + } + } diff --git a/tests/PHPStan/Rules/Api/ApiInstanceofRuleTest.php b/tests/PHPStan/Rules/Api/ApiInstanceofRuleTest.php index 32df695f33..9c124f6157 100644 --- a/tests/PHPStan/Rules/Api/ApiInstanceofRuleTest.php +++ b/tests/PHPStan/Rules/Api/ApiInstanceofRuleTest.php @@ -28,13 +28,27 @@ public function testRuleOutOfPhpStan(): void "If you think it should be covered by backward compatibility promise, open a discussion:\n %s\n\n See also:\n https://phpstan.org/developing-extensions/backward-compatibility-promise", 'https://github.com/phpstan/phpstan/discussions', ); + $instanceofTip = sprintf( + "In case of questions how to solve this correctly, open a discussion:\n %s\n\n See also:\n https://phpstan.org/developing-extensions/backward-compatibility-promise", + 'https://github.com/phpstan/phpstan/discussions', + ); $this->analyse([__DIR__ . '/data/instanceof-out-of-phpstan.php'], [ + [ + 'Although PHPStan\Reflection\ClassReflection is covered by backward compatibility promise, this instanceof assumption might break because it\'s not guaranteed to always stay the same.', + 13, + $instanceofTip, + ], [ 'Asking about instanceof PHPStan\Reflection\BetterReflection\SourceLocator\AutoloadSourceLocator is not covered by backward compatibility promise. The class might change in a minor PHPStan version.', 17, $tip, ], + [ + 'Although PHPStan\Reflection\ClassReflection is covered by backward compatibility promise, this instanceof assumption might break because it\'s not guaranteed to always stay the same.', + 37, + $instanceofTip, + ], ]); } diff --git a/tests/PHPStan/Rules/Api/data/instanceof-out-of-phpstan.php b/tests/PHPStan/Rules/Api/data/instanceof-out-of-phpstan.php index d286fddb62..2009c665b9 100644 --- a/tests/PHPStan/Rules/Api/data/instanceof-out-of-phpstan.php +++ b/tests/PHPStan/Rules/Api/data/instanceof-out-of-phpstan.php @@ -20,3 +20,23 @@ public function doFoo(object $o): void } } + +class Bar +{ + + public function doBar(ClassReflection $classReflection, object $o): void + { + if ($classReflection instanceof ClassReflection) { // yes - do not report + + } + + if ($classReflection instanceof ClassReflection) { // no - do not report + + } + + if ($o instanceof ClassReflection) { // maybe - report + + } + } + +}