danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2024-11-26 20:34:47 +01:00
Code Issues Projects Releases Wiki Activity

Detect fatal issues where property access is overridden

Browse Source 
Fixes #547
This commit is contained in:
Matthew Brown 2018-03-04 12:24:50 -05:00
parent 9d48585b0e
commit a0ce8791d3
10 changed files with 114 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
config.xsd
View File

@ -182,6 +182,7 @@
<xs:element name="NullPropertyFetch" type="IssueHandlerType" minOccurs="0" />
<xs:element name="NullReference" type="IssueHandlerType" minOccurs="0" />
<xs:element name="OverriddenMethodAccess" type="IssueHandlerType" minOccurs="0" />
<xs:element name="OverriddenPropertyAccess" type="IssueHandlerType" minOccurs="0" />
<xs:element name="ParadoxicalCondition" type="IssueHandlerType" minOccurs="0" />
<xs:element name="ParentNotFound" type="IssueHandlerType" minOccurs="0" />
<xs:element name="PossiblyFalseArgument" type="IssueHandlerType" minOccurs="0" />

15
docs/issues.md
View File

@ -1082,6 +1082,21 @@ class B extends A {
}
```
### OverriddenPropertyAccess
Emitted when a property is less accessible than the same-named property in its parent class
```php
class A {
/** @var string|null */
public $foo;
}
class B extends A {
/** @var string|null */
protected $foo;
}
```
### ParadoxicalCondition
Emitted when a paradox is encountered in your programs logic that could not be caught by `RedundantCondition`

48
src/Psalm/Checker/ClassChecker.php
View File

@ -12,6 +12,7 @@ use Psalm\Issue\DeprecatedInterface;
use Psalm\Issue\InaccessibleMethod;
use Psalm\Issue\MissingConstructor;
use Psalm\Issue\MissingPropertyType;
use Psalm\Issue\OverriddenPropertyAccess;
use Psalm\Issue\PropertyNotSetInConstructor;
use Psalm\Issue\ReservedWord;
use Psalm\Issue\UndefinedTrait;
@ -306,14 +307,39 @@ class ClassChecker extends ClassLikeChecker
$property_class_name = $codebase->properties->getDeclaringClassForProperty($appearing_property_id);
$property_class_storage = $classlike_storage_provider->get((string)$property_class_name);
$property = $property_class_storage->properties[$property_name];
$property_storage = $property_class_storage->properties[$property_name];
if ($property->type) {
$property_type = clone $property->type;
if (isset($storage->overridden_property_ids[$property_name])) {
foreach ($storage->overridden_property_ids[$property_name] as $overridden_property_id) {
list($guide_class_name) = explode('::$', $overridden_property_id);
$guide_class_storage = $classlike_storage_provider->get($guide_class_name);
$guide_property_storage = $guide_class_storage->properties[$property_name];
if ($property_storage->visibility > $guide_property_storage->visibility
&& $property_storage->location
) {
if (IssueBuffer::accepts(
new OverriddenPropertyAccess(
'Property ' . $guide_class_storage->name . '::$' . $property_name
. ' has different access level than '
. $storage->name . '::$' . $property_name,
$property_storage->location
)
)) {
return false;
}
return null;
}
}
}
if ($property_storage->type) {
$property_type = clone $property_storage->type;
if (!$property_type->isMixed() &&
!$property->has_default &&
!$property->type->isNullable()
!$property_storage->has_default &&
!$property_storage->type->isNullable()
) {
$property_type->initialized = false;
}
@ -326,17 +352,23 @@ class ClassChecker extends ClassLikeChecker
$property_type = Type::getMixed();
}
if ($property->type_location && !$property_type->isMixed()) {
if ($property_storage->type_location && !$property_type->isMixed()) {
$fleshed_out_type = ExpressionChecker::fleshOutType(
$project_checker,
$property_type,
$this->fq_class_name,
$this->fq_class_name
);
$fleshed_out_type->check($this, $property->type_location, $this->getSuppressedIssues(), [], false);
$fleshed_out_type->check(
$this,
$property_storage->type_location,
$this->getSuppressedIssues(),
[],
false
);
}
if ($property->is_static) {
if ($property_storage->is_static) {
$property_id = $this->fq_class_name . '::$' . $property_name;
$class_context->vars_in_scope[$property_id] = $property_type;

1
src/Psalm/Codebase.php
View File

@ -170,7 +170,6 @@ class Codebase
$classlike_storage_provider,
$file_storage_provider,
$this->classlikes,
$this->methods,
$debug_output
);
}

17
src/Psalm/Codebase/Methods.php
View File

@ -388,23 +388,6 @@ class Methods
}
}
/**
* @param string $method_id
* @param string $overridden_method_id
*
* @return void
*/
public function setOverriddenMethodId(
$method_id,
$overridden_method_id
) {
list($fq_class_name, $method_name) = explode('::', $method_id);
$class_storage = $this->classlike_storage_provider->get($fq_class_name);
$class_storage->overridden_method_ids[$method_name][] = $overridden_method_id;
}
/**
* @param string $method_id
*

23
src/Psalm/Codebase/Populator.php
View File

@ -39,11 +39,6 @@ class Populator
*/
private $classlikes;
/**
* @var Methods
*/
private $methods;
/**
* @var Config
*/
@ -57,13 +52,11 @@ class Populator
ClassLikeStorageProvider $classlike_storage_provider,
FileStorageProvider $file_storage_provider,
ClassLikes $classlikes,
Methods $methods,
$debug_output
) {
$this->classlike_storage_provider = $classlike_storage_provider;
$this->file_storage_provider = $file_storage_provider;
$this->classlikes = $classlikes;
$this->methods = $methods;
$this->debug_output = $debug_output;
$this->config = $config;
}
@ -362,10 +355,7 @@ class Populator
foreach ($interface_method_implementers as $method_name => $interface_method_ids) {
if (count($interface_method_ids) === 1) {
$this->methods->setOverriddenMethodId(
$storage->name . '::' . $method_name,
$interface_method_ids[0]
);
$storage->overridden_method_ids[$method_name][] = $interface_method_ids[0];
} else {
$storage->interface_method_ids[$method_name] = $interface_method_ids;
}
@ -492,12 +482,7 @@ class Populator
// register where they're declared
foreach ($parent_storage->inheritable_method_ids as $method_name => $declaring_method_id) {
if (!$parent_storage->is_trait) {
$implemented_method_id = $fq_class_name . '::' . $method_name;
$this->methods->setOverriddenMethodId(
$implemented_method_id,
$declaring_method_id
);
$storage->overridden_method_ids[$method_name][] = $declaring_method_id;
}
if ($parent_storage->is_trait
@ -579,6 +564,10 @@ class Populator
continue;
}
if (!$parent_storage->is_trait) {
$storage->overridden_property_ids[$property_name][] = $inheritable_property_id;
}
$storage->inheritable_property_ids[$property_name] = $inheritable_property_id;
}
}

5
src/Psalm/Codebase/Reflection.php
View File

@ -462,10 +462,7 @@ class Reflection
$storage->declaring_method_ids[$method_name] = $declaring_method_id;
$storage->inheritable_method_ids[$method_name] = $declaring_method_id;
$this->codebase->methods->setOverriddenMethodId(
$implemented_method_id,
$declaring_method_id
);
$storage->overridden_method_ids[$method_name][] = $declaring_method_id;
}
}

6
src/Psalm/Issue/OverriddenPropertyAccess.php Normal file
View File

@ -0,0 +1,6 @@
<?php
namespace Psalm\Issue;
class OverriddenPropertyAccess extends CodeIssue
{
}

5
src/Psalm/Storage/ClassLikeStorage.php
View File

@ -185,6 +185,11 @@ class ClassLikeStorage
*/
public $inheritable_property_ids = [];
/**
* @var array<string, array<string>>
*/
public $overridden_property_ids = [];
/**
* @var array<string, string>|null
*/

41
tests/ClassTest.php
View File

@ -151,7 +151,7 @@ class ClassTest extends TestCase
echo A::HELLO;',
'error_message' => 'UndefinedConstant',
],
'overridePublicAccessLevelToPublic' => [
'overridePublicAccessLevelToPrivate' => [
'<?php
class A {
public function fooFoo(): void {}
@ -184,6 +184,45 @@ class ClassTest extends TestCase
}',
'error_message' => 'OverriddenMethodAccess',
],
'overridePublicPropertyAccessLevelToPrivate' => [
'<?php
class A {
/** @var string|null */
public $foo;
}
class B extends A {
/** @var string|null */
private $foo;
}',
'error_message' => 'OverriddenPropertyAccess',
],
'overridePublicPropertyAccessLevelToProtected' => [
'<?php
class A {
/** @var string|null */
public $foo;
}
class B extends A {
/** @var string|null */
protected $foo;
}',
'error_message' => 'OverriddenPropertyAccess',
],
'overrideProtectedPropertyAccessLevelToPrivate' => [
'<?php
class A {
/** @var string|null */
protected $foo;
}
class B extends A {
/** @var string|null */
private $foo;
}',
'error_message' => 'OverriddenPropertyAccess',
],
'classRedefinition' => [
'<?php
class Foo {}