Skip to content

Commit

Permalink
ApiInstanceofRule - report "maybe" on @api-covered classes
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Jun 17, 2022
1 parent ff4d02d commit 996bc69
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 1 deletion.
26 changes: 25 additions & 1 deletion src/Rules/Api/ApiInstanceofRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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(),
];
}

}
14 changes: 14 additions & 0 deletions tests/PHPStan/Rules/Api/ApiInstanceofRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
],
]);
}

Expand Down
20 changes: 20 additions & 0 deletions tests/PHPStan/Rules/Api/data/instanceof-out-of-phpstan.php
Original file line number Diff line number Diff line change
Expand Up @@ -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

}
}

}

0 comments on commit 996bc69

Please sign in to comment.