mirror of
https://github.com/danog/psalm.git
synced 2024-11-30 04:39:00 +01:00
Fix #903 - make sure parent::foo is executed in context of method’s class
and not immediate parent
This commit is contained in:
Matthew Brown
parent
ef1c1995a3
commit
c97329da06
@ -383,6 +383,7 @@ class ClassChecker extends ClassLikeChecker
|
||||
}
|
||||
|
||||
$constructor_checker = null;
|
||||
$constructor_appearing_fqcln = $fq_class_name;
|
||||
$member_stmts = [];
|
||||
|
||||
foreach ($class->stmts as $stmt) {
|
||||
@ -447,27 +448,6 @@ class ClassChecker extends ClassLikeChecker
|
||||
continue;
|
||||
}
|
||||
|
||||
$constructor_class_storage = null;
|
||||
|
||||
if (isset($storage->methods['__construct'])) {
|
||||
$constructor_class_storage = $storage;
|
||||
} elseif (isset($property_class_storage->methods['__construct'])
|
||||
&& $property_class_storage !== $storage
|
||||
) {
|
||||
$constructor_class_storage = $property_class_storage;
|
||||
} elseif (!empty($property_class_storage->overridden_method_ids['__construct'])) {
|
||||
list($construct_fqcln) =
|
||||
explode('::', $property_class_storage->overridden_method_ids['__construct'][0]);
|
||||
$constructor_class_storage = $classlike_storage_provider->get($construct_fqcln);
|
||||
}
|
||||
|
||||
if ($constructor_class_storage
|
||||
&& $constructor_class_storage->all_properties_set_in_constructor
|
||||
&& $constructor_class_storage->methods['__construct']->visibility !== self::VISIBILITY_PRIVATE
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$uninitialized_variables[] = '$this->' . $property_name;
|
||||
$uninitialized_properties[$property_name] = $property;
|
||||
}
|
||||
@ -478,9 +458,10 @@ class ClassChecker extends ClassLikeChecker
|
||||
&& isset($storage->declaring_method_ids['__construct'])
|
||||
&& $class->extends
|
||||
) {
|
||||
list($construct_fqcln) = explode('::', $storage->declaring_method_ids['__construct']);
|
||||
list($constructor_declaring_fqcln) = explode('::', $storage->declaring_method_ids['__construct']);
|
||||
list($constructor_appearing_fqcln) = explode('::', $storage->appearing_method_ids['__construct']);
|
||||
|
||||
$constructor_class_storage = $classlike_storage_provider->get($construct_fqcln);
|
||||
$constructor_class_storage = $classlike_storage_provider->get($constructor_declaring_fqcln);
|
||||
|
||||
// ignore oldstyle constructors and classes without any declared properties
|
||||
if ($constructor_class_storage->user_defined
|
||||
@ -550,35 +531,47 @@ class ClassChecker extends ClassLikeChecker
|
||||
if ($constructor_checker) {
|
||||
$method_context = clone $class_context;
|
||||
$method_context->collect_initializations = true;
|
||||
$method_context->self = $fq_class_name;
|
||||
$method_context->vars_in_scope['$this'] = Type::parseString($fq_class_name);
|
||||
$method_context->vars_possibly_in_scope['$this'] = true;
|
||||
|
||||
$constructor_checker->analyze($method_context, $global_context, true);
|
||||
|
||||
$all_properties_set_in_constructor = true;
|
||||
|
||||
foreach ($uninitialized_properties as $property_name => $property) {
|
||||
foreach ($uninitialized_properties as $property_name => $property_storage) {
|
||||
if (!isset($method_context->vars_in_scope['$this->' . $property_name])) {
|
||||
throw new \UnexpectedValueException('$this->' . $property_name . ' should be in scope');
|
||||
}
|
||||
|
||||
$end_type = $method_context->vars_in_scope['$this->' . $property_name];
|
||||
|
||||
if (!$end_type->initialized) {
|
||||
$all_properties_set_in_constructor = false;
|
||||
$property_id = $constructor_appearing_fqcln . '::$' . $property_name;
|
||||
|
||||
$constructor_class_property_storage = $property_storage;
|
||||
|
||||
if ($fq_class_name !== $constructor_appearing_fqcln) {
|
||||
$a_class_storage = $classlike_storage_provider->get($constructor_appearing_fqcln);
|
||||
|
||||
if (!isset($a_class_storage->declaring_property_ids[$property_name])) {
|
||||
$constructor_class_property_storage = null;
|
||||
} else {
|
||||
$declaring_property_class = $a_class_storage->declaring_property_ids[$property_name];
|
||||
$constructor_class_property_storage = $classlike_storage_provider
|
||||
->get($declaring_property_class)
|
||||
->properties[$property_name];
|
||||
}
|
||||
}
|
||||
|
||||
if (!$end_type->initialized && $property->location) {
|
||||
$property_id = $this->fq_class_name . '::$' . $property_name;
|
||||
|
||||
if ($property_storage->location
|
||||
&& (!$end_type->initialized || $property_storage !== $constructor_class_property_storage)
|
||||
) {
|
||||
if (!$config->reportIssueInFile(
|
||||
'PropertyNotSetInConstructor',
|
||||
$property->location->file_path
|
||||
$property_storage->location->file_path
|
||||
) && $class->extends
|
||||
) {
|
||||
$error_location = new CodeLocation($this, $class->extends);
|
||||
} else {
|
||||
$error_location = $property->location;
|
||||
$error_location = $property_storage->location;
|
||||
}
|
||||
|
||||
if (IssueBuffer::accepts(
|
||||
@ -593,8 +586,6 @@ class ClassChecker extends ClassLikeChecker
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
$storage->all_properties_set_in_constructor = $all_properties_set_in_constructor;
|
||||
} elseif (!$storage->abstract) {
|
||||
$first_uninitialized_property = array_shift($uninitialized_properties);
|
||||
|
||||
|
||||
@ -15,6 +15,7 @@ use Psalm\Issue\DeprecatedClass;
|
||||
use Psalm\Issue\InvalidStringClass;
|
||||
use Psalm\Issue\ParentNotFound;
|
||||
use Psalm\Issue\UndefinedClass;
|
||||
use Psalm\Issue\UndefinedMethod;
|
||||
use Psalm\IssueBuffer;
|
||||
use Psalm\Type;
|
||||
use Psalm\Type\Atomic\TNamedObject;
|
||||
@ -79,17 +80,40 @@ class StaticCallChecker extends \Psalm\Checker\Statements\Expression\CallChecker
|
||||
|
||||
$fq_class_name = $class_storage->name;
|
||||
|
||||
if ($stmt->name instanceof PhpParser\Node\Identifier && $class_storage->user_defined) {
|
||||
if ($stmt->name instanceof PhpParser\Node\Identifier
|
||||
&& $class_storage->user_defined
|
||||
&& ($context->collect_mutations || $context->collect_initializations)
|
||||
) {
|
||||
$method_id = $fq_class_name . '::' . strtolower($stmt->name->name);
|
||||
|
||||
$appearing_method_id = $codebase->getAppearingMethodId($method_id);
|
||||
|
||||
if (!$appearing_method_id) {
|
||||
if (IssueBuffer::accepts(
|
||||
new UndefinedMethod(
|
||||
'Method ' . $method_id . ' does not exist',
|
||||
new CodeLocation($statements_checker->getSource(), $stmt),
|
||||
$method_id
|
||||
),
|
||||
$statements_checker->getSuppressedIssues()
|
||||
)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
list($appearing_method_class_name) = explode('::', $appearing_method_id);
|
||||
|
||||
$old_context_include_location = $context->include_location;
|
||||
$old_self = $context->self;
|
||||
$context->include_location = new CodeLocation($statements_checker->getSource(), $stmt);
|
||||
$context->self = $fq_class_name;
|
||||
$context->self = $appearing_method_class_name;
|
||||
|
||||
if ($context->collect_mutations) {
|
||||
$file_checker->getMethodMutations($method_id, $context);
|
||||
} elseif ($context->collect_initializations) {
|
||||
} else {
|
||||
// collecting initalisations
|
||||
$local_vars_in_scope = [];
|
||||
$local_vars_possibly_in_scope = [];
|
||||
|
||||
|
||||
@ -652,7 +652,7 @@ class Populator
|
||||
}
|
||||
|
||||
// register where they're declared
|
||||
foreach ($parent_storage->declaring_property_ids as $property_name => $declaring_property_id) {
|
||||
foreach ($parent_storage->declaring_property_ids as $property_name => $declaring_property_class) {
|
||||
if (isset($storage->declaring_property_ids[$property_name])) {
|
||||
continue;
|
||||
}
|
||||
@ -664,7 +664,7 @@ class Populator
|
||||
continue;
|
||||
}
|
||||
|
||||
$storage->declaring_property_ids[$property_name] = $declaring_property_id;
|
||||
$storage->declaring_property_ids[$property_name] = $declaring_property_class;
|
||||
}
|
||||
|
||||
// register where they're declared
|
||||
|
||||
@ -47,11 +47,10 @@ class Properties
|
||||
|
||||
if (isset($class_storage->declaring_property_ids[$property_name])) {
|
||||
if ($this->collect_references && $code_location) {
|
||||
$declaring_property_id = $class_storage->declaring_property_ids[$property_name];
|
||||
list($declaring_property_class, $declaring_property_name) = explode('::$', $declaring_property_id);
|
||||
$declaring_property_class = $class_storage->declaring_property_ids[$property_name];
|
||||
|
||||
$declaring_class_storage = $this->classlike_storage_provider->get($declaring_property_class);
|
||||
$declaring_property_storage = $declaring_class_storage->properties[$declaring_property_name];
|
||||
$declaring_property_storage = $declaring_class_storage->properties[$property_name];
|
||||
|
||||
if ($declaring_property_storage->referencing_locations === null) {
|
||||
$declaring_property_storage->referencing_locations = [];
|
||||
@ -80,9 +79,7 @@ class Properties
|
||||
$class_storage = $this->classlike_storage_provider->get($fq_class_name);
|
||||
|
||||
if (isset($class_storage->declaring_property_ids[$property_name])) {
|
||||
$declaring_property_id = $class_storage->declaring_property_ids[$property_name];
|
||||
|
||||
return explode('::$', $declaring_property_id)[0];
|
||||
return $class_storage->declaring_property_ids[$property_name];
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -115,7 +115,7 @@ class Reflection
|
||||
|
||||
$property_id = (string)$class_property->class . '::$' . $property_name;
|
||||
|
||||
$storage->declaring_property_ids[$property_name] = $property_id;
|
||||
$storage->declaring_property_ids[$property_name] = (string)$class_property->class;
|
||||
$storage->appearing_property_ids[$property_name] = $property_id;
|
||||
|
||||
if (!$class_property->isPrivate()) {
|
||||
@ -131,7 +131,7 @@ class Reflection
|
||||
|
||||
$property_id = $class_name . '::$' . $property_name;
|
||||
|
||||
$storage->declaring_property_ids[$property_name] = $property_id;
|
||||
$storage->declaring_property_ids[$property_name] = $class_name;
|
||||
$storage->appearing_property_ids[$property_name] = $property_id;
|
||||
$storage->inheritable_property_ids[$property_name] = $property_id;
|
||||
}
|
||||
@ -430,7 +430,7 @@ class Reflection
|
||||
}
|
||||
|
||||
// register where they're declared
|
||||
foreach ($parent_storage->declaring_property_ids as $property_name => $declaring_property_id) {
|
||||
foreach ($parent_storage->declaring_property_ids as $property_name => $declaring_property_class) {
|
||||
if (!$parent_storage->is_trait
|
||||
&& isset($parent_storage->properties[$property_name])
|
||||
&& $parent_storage->properties[$property_name]->visibility === ClassLikeChecker::VISIBILITY_PRIVATE
|
||||
@ -438,7 +438,7 @@ class Reflection
|
||||
continue;
|
||||
}
|
||||
|
||||
$storage->declaring_property_ids[$property_name] = $declaring_property_id;
|
||||
$storage->declaring_property_ids[$property_name] = $declaring_property_class;
|
||||
}
|
||||
|
||||
// register where they're declared
|
||||
|
||||
@ -65,11 +65,6 @@ class ClassLikeStorage
|
||||
*/
|
||||
public $stubbed = false;
|
||||
|
||||
/**
|
||||
* @var bool
|
||||
*/
|
||||
public $all_properties_set_in_constructor = false;
|
||||
|
||||
/**
|
||||
* @var bool
|
||||
*/
|
||||
|
||||
@ -691,7 +691,7 @@ class DependencyFinderVisitor extends PhpParser\NodeVisitorAbstract implements P
|
||||
|
||||
$property_id = $fq_classlike_name . '::$' . $property_name;
|
||||
|
||||
$storage->declaring_property_ids[$property_name] = $property_id;
|
||||
$storage->declaring_property_ids[$property_name] = $fq_classlike_name;
|
||||
$storage->appearing_property_ids[$property_name] = $property_id;
|
||||
}
|
||||
}
|
||||
@ -1737,7 +1737,7 @@ class DependencyFinderVisitor extends PhpParser\NodeVisitorAbstract implements P
|
||||
|
||||
$property_id = $fq_classlike_name . '::$' . $property->name->name;
|
||||
|
||||
$storage->declaring_property_ids[$property->name->name] = $property_id;
|
||||
$storage->declaring_property_ids[$property->name->name] = $fq_classlike_name;
|
||||
$storage->appearing_property_ids[$property->name->name] = $property_id;
|
||||
|
||||
if ($property_is_initialized) {
|
||||
|
||||
@ -1484,6 +1484,82 @@ class PropertyTypeTest extends TestCase
|
||||
}',
|
||||
'error_message' => 'PropertyNotSetInConstructor',
|
||||
],
|
||||
'privatePropertySameNameNotSetInConstructor' => [
|
||||
'<?php
|
||||
class A {
|
||||
/** @var string */
|
||||
private $b;
|
||||
|
||||
public function __construct() {
|
||||
$this->b = "foo";
|
||||
}
|
||||
}
|
||||
|
||||
class B extends A {
|
||||
/** @var string */
|
||||
private $b;
|
||||
}',
|
||||
'error_message' => 'PropertyNotSetInConstructor',
|
||||
],
|
||||
'privateMethodCalledInParentConstructor' => [
|
||||
'<?php
|
||||
class C extends B {}
|
||||
|
||||
abstract class B extends A {
|
||||
/** @var string */
|
||||
private $b;
|
||||
|
||||
/** @var string */
|
||||
protected $c;
|
||||
}
|
||||
|
||||
class A {
|
||||
public function __construct() {
|
||||
$this->publicMethod();
|
||||
}
|
||||
|
||||
publiic function publicMethod() : void {
|
||||
$this->privateMethod();
|
||||
}
|
||||
|
||||
private function privateMethod() : void {}
|
||||
}',
|
||||
'error_message' => 'MissingConstructor',
|
||||
],
|
||||
'privatePropertySetInParentConstructorReversedOrder' => [
|
||||
'<?php
|
||||
class B extends A {
|
||||
/** @var string */
|
||||
private $b;
|
||||
}
|
||||
|
||||
class A {
|
||||
public function __construct() {
|
||||
if ($this instanceof B) {
|
||||
$this->b = "foo";
|
||||
}
|
||||
}
|
||||
}',
|
||||
'error_message' => 'PropertyNotSetInConstructor',
|
||||
],
|
||||
'privatePropertySetInParentConstructor' => [
|
||||
'<?php
|
||||
class A {
|
||||
public function __construct() {
|
||||
if ($this instanceof B) {
|
||||
$this->b = "foo";
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
class B extends A {
|
||||
/** @var string */
|
||||
private $b;
|
||||
}
|
||||
|
||||
',
|
||||
'error_message' => 'InaccessibleProperty',
|
||||
],
|
||||
'undefinedPropertyClass' => [
|
||||
'<?php
|
||||
class A {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user