Skip to content

Commit

Permalink
bugfix: properly inherit assertions from parents or implemented inter…
Browse files Browse the repository at this point in the history
…faces

Signed-off-by: Maximilian Bösing <[email protected]>
  • Loading branch information
boesing committed Aug 27, 2023
1 parent 77436b1 commit 5e520e6
Show file tree
Hide file tree
Showing 3 changed files with 188 additions and 2 deletions.
2 changes: 2 additions & 0 deletions src/Psalm/Codebase.php
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,8 @@ public function classOrInterfaceExists(

/**
* Check whether a class/interface exists
*
* @psalm-assert-if-true class-string|interface-string|enum-string $fq_class_name
*/
public function classOrInterfaceOrEnumExists(
string $fq_class_name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
use Psalm\IssueBuffer;
use Psalm\Node\Expr\VirtualFuncCall;
use Psalm\Plugin\EventHandler\Event\AfterMethodCallAnalysisEvent;
use Psalm\Storage\Assertion;
use Psalm\Storage\ClassLikeStorage;
use Psalm\Storage\Possibilities;
use Psalm\Type;
use Psalm\Type\Atomic;
Expand All @@ -44,10 +46,13 @@

use function array_filter;
use function array_map;
use function array_merge;
use function array_values;
use function count;
use function explode;
use function in_array;
use function is_string;
use function reset;
use function strpos;
use function strtolower;

Expand Down Expand Up @@ -415,11 +420,45 @@ public static function analyze(
}
}

if ($method_storage->assertions) {
$assertions = $method_storage->assertions;
if ($method_storage->inheritdoc) {
$inherited_classes_and_interfaces = array_values(array_filter(array_merge(
$class_storage->parent_classes,
$class_storage->class_implements,
), fn(string $classOrInterface) => $codebase->classOrInterfaceOrEnumExists($classOrInterface)));

foreach ($inherited_classes_and_interfaces as $potential_assertion_providing_class) {
$potential_assertion_providing_classlike_storage = $codebase->classlike_storage_provider->get(
$potential_assertion_providing_class,
);
if (!isset($potential_assertion_providing_classlike_storage->methods[$method_name_lc])) {
continue;
}

$potential_assertion_providing_method_storage = $potential_assertion_providing_classlike_storage
->methods[$method_name_lc];

/**
* Since the inheritance does not provide its own assertions, we have to detect those
* from inherited classes
*/
$assertions += array_map(
static fn(Possibilities $possibilities) => self::modifyAssertionsForInheritance(
$possibilities,
$codebase,
$class_storage,
$inherited_classes_and_interfaces,
),
$potential_assertion_providing_method_storage->assertions,
);
}
}

if ($assertions) {
self::applyAssertionsToContext(
$stmt_name,
ExpressionIdentifier::getExtendedVarId($stmt->var, null, $statements_analyzer),
$method_storage->assertions,
$assertions,
$args,
$template_result,
$context,
Expand Down Expand Up @@ -690,4 +729,65 @@ private static function getMagicGetterOrSetterProperty(

return null;
}

/**
* In case the called class is either implementing or extending a class/interface which does also has the
* template we are searching for, we assume that the called method has the same assertions.
*
* @param list<class-string> $potential_assertion_providing_classes
*/
private static function modifyAssertionsForInheritance(
Possibilities $possibilities,
Codebase $codebase,
ClassLikeStorage $called_class,
array $potential_assertion_providing_classes
): Possibilities {
$replacement = new Possibilities($possibilities->var_id, []);
$extended_params = $called_class->template_extended_params;
foreach ($possibilities->rule as $assertion) {
if (!$assertion instanceof Assertion\IsType
|| !$assertion->type instanceof TTemplateParam) {
$replacement->rule[] = $assertion;
continue;
}

/** Called class does not extend the template parameter */
$extended_templates = $called_class->template_extended_params;
if (!isset($extended_templates[$assertion->type->defining_class][$assertion->type->param_name])) {
$replacement->rule[] = $assertion;
continue;
}

foreach ($potential_assertion_providing_classes as $potential_assertion_providing_class) {
if (!isset($extended_params[$potential_assertion_providing_class][$assertion->type->param_name])) {
continue;
}

if (!$codebase->classlike_storage_provider->has($potential_assertion_providing_class)) {
continue;
}

$potential_assertion_providing_classlike_storage = $codebase->classlike_storage_provider->get(
$potential_assertion_providing_class,
);
if (!isset(
$potential_assertion_providing_classlike_storage->template_types[$assertion->type->param_name],
)) {
continue;
}

$replacement->rule[] = new Assertion\IsType(new TTemplateParam(
$assertion->type->param_name,
reset($potential_assertion_providing_classlike_storage->template_types[$assertion->type->param_name]),
$potential_assertion_providing_class,
));

continue 2;
}

$replacement->rule[] = $assertion;
}

return $replacement;
}
}
84 changes: 84 additions & 0 deletions tests/AssertAnnotationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2883,6 +2883,90 @@ public static function doAssert($value): void
'$iterable===' => 'non-empty-list<string>',
],
],
'assertFromInheritedDocBlock' => [
'code' => '<?php
namespace Namespace1 {
/** @template InstanceType */
interface PluginManagerInterface
{
/** @return InstanceType */
public function get(): mixed;
/** @psalm-assert InstanceType $value */
public function validate(mixed $value): void;
}
/**
* @template InstanceType
* @template-implements PluginManagerInterface<InstanceType>
*/
abstract class AbstractPluginManager implements PluginManagerInterface
{
/** @param InstanceType $value */
public function __construct(private readonly mixed $value)
{}
/** {@inheritDoc} */
public function get(): mixed
{
return $this->value;
}
}
/**
* @template InstanceType of object
* @template-extends AbstractPluginManager<InstanceType>
*/
abstract class AbstractSingleInstancePluginManager extends AbstractPluginManager
{
/**
* An object type that the created instance must be instanced of
*
* @var class-string<InstanceType>
*/
protected string $instanceOf;
/** {@inheritDoc} */
public function get(): object
{
return parent::get();
}
/** {@inheritDoc} */
public function validate(mixed $value): void
{
}
}
}
namespace Namespace2 {
use Namespace1\AbstractSingleInstancePluginManager;
use stdClass;
/** @template-extends AbstractSingleInstancePluginManager<stdClass> */
final class Qoo extends AbstractSingleInstancePluginManager
{
/** @var class-string<stdClass> */
protected string $instanceOf = stdClass::class;
}
}
namespace {
$baz = new \Namespace2\Qoo(new stdClass);
/** @var mixed $object */
$object = null;
$baz->validate($object);
}
',
'assertions' => [
'$object===' => 'stdClass',
],
'ignored_issues' => [],
'php_version' => '8.1',
],
];
}

Expand Down

0 comments on commit 5e520e6

Please sign in to comment.