danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2025-01-19 03:54:42 +01:00
Code Issues Projects Releases Wiki Activity

Add less severe issue for docblock property type invariance cc @bdsl

Browse Source 
This is less likely to break everything
This commit is contained in:
Matt Brown 2021-02-07 00:52:29 -05:00
parent 04bb2b1182
commit f2d202e2bb
15 changed files with 113 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
config.xsd
View File

@ -313,7 +313,8 @@
<xs:element name="NoInterfaceProperties" type="ClassIssueHandlerType" minOccurs="0" />
<xs:element name="NonStaticSelfCall" type="IssueHandlerType" minOccurs="0" />
<xs:element name="NoValue" type="IssueHandlerType" minOccurs="0" />
<xs:element name="NonInvariantPropertyType" type="MethodIssueHandlerType" minOccurs="0" />
<xs:element name="NonInvariantDocblockPropertyType" type="IssueHandlerType" minOccurs="0" />
<xs:element name="NonInvariantPropertyType" type="IssueHandlerType" minOccurs="0" />
<xs:element name="NullableReturnStatement" type="IssueHandlerType" minOccurs="0" />
<xs:element name="NullArgument" type="ArgumentIssueHandlerType" minOccurs="0" />
<xs:element name="NullArrayAccess" type="IssueHandlerType" minOccurs="0" />

25
docs/running_psalm/issues/NonInvariantDocblockPropertyType.md Normal file
View File

@ -0,0 +1,25 @@
# NonInvariantDocblockPropertyType
Emitted when a public or protected class property has a different docblock type to the matching property in the parent class.
```php
<?php
class A {
/** @var string */
public $foo = 'hello';
}
class B extends A {
/** @var null|string */
public $foo;
}
```
## Why this is bad
For non-typed properties, it can cause a type system violation when code written against the parent class reads or writes a value on an object of the child class.
If the child class widens the type then reading the value may return unexpected value that client code cannot deal with. If the child class narrows the type then code setting the value may set
it to an invalid value.

12
docs/running_psalm/issues/NonInvariantPropertyType.md
View File

@ -6,21 +6,15 @@ Emitted when a public or protected class property has a different type to the ma
<?php
class A {
/** @var string */
public $foo = 'hello';
public string $foo = 'hello';
}
class B extends A {
/** @var null|string */
public $foo;
public ?string $foo;
}
```
## Why this is bad
For typed properties, this can cause a compile error. For non-typed
properties, it can cause a type system violation when code written against the parent class reads or writes a value on an object of the child class.
If the child class widens the type then reading the value may return unexpected value that client code cannot deal with. If the child class narrows the type then code setting the value may set
it to an invalid value.
For typed properties, this can cause a compile error.

50
src/Psalm/Internal/Analyzer/ClassAnalyzer.php
View File

@ -31,6 +31,7 @@ use Psalm\Issue\MissingImmutableAnnotation;
use Psalm\Issue\MissingPropertyType;
use Psalm\Issue\MissingTemplateParam;
use Psalm\Issue\MutableDependency;
use Psalm\Issue\NonInvariantDocblockPropertyType;
use Psalm\Issue\NonInvariantPropertyType;
use Psalm\Issue\OverriddenPropertyAccess;
use Psalm\Issue\PropertyNotSetInConstructor;
@ -663,20 +664,43 @@ class ClassAnalyzer extends ClassLikeAnalyzer
}
}
if ($property_storage->type && $guide_property_storage->type && $property_storage->location &&
!$property_storage->type->equals($guide_property_storage->type)) {
if (IssueBuffer::accepts(
new NonInvariantPropertyType(
'Property ' . $fq_class_name . '::$' . $property_name
. ' has type ' . $property_storage->type->getId()
. ", not invariant with " . $guide_class_name . '::$' . $property_name . ' of type ' .
$guide_property_storage->type->getId(),
$property_storage->location
),
$property_storage->suppressed_issues
)) {
// fall through
if ($property_storage->type
&& $guide_property_storage->type
&& $property_storage->location
&& !$property_storage->type->equals($guide_property_storage->type)
) {
if ($property_storage->type->from_docblock
&& $guide_property_storage->type->from_docblock
) {
if (IssueBuffer::accepts(
new NonInvariantDocblockPropertyType(
'Property ' . $fq_class_name . '::$' . $property_name
. ' has type ' . $property_storage->type->getId()
. ", not invariant with " . $guide_class_name . '::$'
. $property_name . ' of type '
. $guide_property_storage->type->getId(),
$property_storage->location
),
$property_storage->suppressed_issues
)) {
// fall through
}
} else {
if (IssueBuffer::accepts(
new NonInvariantPropertyType(
'Property ' . $fq_class_name . '::$' . $property_name
. ' has type ' . $property_storage->type->getId()
. ", not invariant with " . $guide_class_name . '::$'
. $property_name . ' of type '
. $guide_property_storage->type->getId(),
$property_storage->location
),
$property_storage->suppressed_issues
)) {
// fall through
}
}
}
}
}

2
src/Psalm/Internal/Analyzer/ClassLikeAnalyzer.php
View File

@ -72,7 +72,7 @@ abstract class ClassLikeAnalyzer extends SourceAnalyzer
protected $class;
/**
* @psalm-suppress NonInvariantPropertyType
* @psalm-suppress NonInvariantDocblockPropertyType
* @var StatementsSource
*/
protected $source;

2
src/Psalm/Internal/Analyzer/ClosureAnalyzer.php
View File

@ -24,7 +24,7 @@ use function preg_match;
class ClosureAnalyzer extends FunctionLikeAnalyzer
{
/**
* @psalm-suppress NonInvariantPropertyType
* @psalm-suppress NonInvariantDocblockPropertyType
* @var PhpParser\Node\Expr\Closure|PhpParser\Node\Expr\ArrowFunction
*/
protected $function;

2
src/Psalm/Internal/Analyzer/FunctionAnalyzer.php
View File

@ -13,7 +13,7 @@ class FunctionAnalyzer extends FunctionLikeAnalyzer
{
/**
* @var PhpParser\Node\Stmt\Function_
* @psalm-suppress NonInvariantPropertyType
* @psalm-suppress NonInvariantDocblockPropertyType
*/
protected $function;

2
src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
View File

@ -71,7 +71,7 @@ abstract class FunctionLikeAnalyzer extends SourceAnalyzer
/**
* @var StatementsSource
* @psalm-suppress NonInvariantPropertyType
* @psalm-suppress NonInvariantDocblockPropertyType
*/
protected $source;

4
src/Psalm/Internal/Analyzer/MethodAnalyzer.php
View File

@ -22,7 +22,7 @@ use function in_array;
class MethodAnalyzer extends FunctionLikeAnalyzer
{
/**
* @psalm-suppress NonInvariantPropertyType
* @psalm-suppress NonInvariantDocblockPropertyType
* @var PhpParser\Node\Stmt\ClassMethod
*/
protected $function;
@ -166,7 +166,7 @@ class MethodAnalyzer extends FunctionLikeAnalyzer
return null;
}
public static function isMethodVisible(
\Psalm\Internal\MethodIdentifier $method_id,
Context $context,

2
src/Psalm/Internal/Analyzer/NamespaceAnalyzer.php
View File

@ -20,7 +20,7 @@ class NamespaceAnalyzer extends SourceAnalyzer
/**
* @var FileAnalyzer
* @psalm-suppress NonInvariantPropertyType
* @psalm-suppress NonInvariantDocblockPropertyType
*/
protected $source;

2
src/Psalm/Internal/Provider/PropertyTypeProvider.php
View File

@ -97,7 +97,7 @@ class PropertyTypeProvider
): ?Type\Union {
if ($source) {
$source->addSuppressedIssues(['NonInvariantPropertyType']);
$source->addSuppressedIssues(['NonInvariantDocblockPropertyType']);
}
foreach (self::$handlers[strtolower($fq_classlike_name)] ?? [] as $property_handler) {

11
src/Psalm/Issue/NonInvariantDocblockPropertyType.php Normal file
View File

@ -0,0 +1,11 @@
<?php
declare(strict_types=1);
namespace Psalm\Issue;
final class NonInvariantDocblockPropertyType extends CodeIssue
{
public const ERROR_LEVEL = 3;
public const SHORTCODE = 267;
}

2
src/Psalm/Type/Atomic/TArray.php
View File

@ -14,7 +14,7 @@ class TArray extends \Psalm\Type\Atomic
/**
* @var array{\Psalm\Type\Union, \Psalm\Type\Union}
* @psalm-suppress NonInvariantPropertyType
* @psalm-suppress NonInvariantDocblockPropertyType
*/
public $type_params;

2
tests/Config/PluginTest.php
View File

@ -40,7 +40,7 @@ class PluginTest extends \Psalm\Tests\TestCase
/**
* @var ?\Psalm\Internal\Analyzer\ProjectAnalyzer
* @psalm-suppress NonInvariantPropertyType
* @psalm-suppress NonInvariantDocblockPropertyType
*/
protected $project_analyzer;

54
tests/PropertyTypeInvariance.php
View File

@ -50,38 +50,34 @@ class PropertyTypeInvariance extends TestCase
public function providerInvalidCodeParse(): iterable
{
return [
'variantDocblockProperties' => [
'<?php
class ParentClass
{
/** @var null|string */
protected $mightExist;
}
class ChildClass extends ParentClass
{
/** @var string */
protected $mightExist = "";
}',
'error_message' => 'NonInvariantDocblockPropertyType',
],
'variantProperties' => [
'
<?php
'<?php
class ParentClass
{
protected ?string $mightExist = null;
}
class ParentClass
{
/** @var null|string */
protected $mightExist;
protected ?string $mightExistNative = null;
/** @var string */
protected $doesExist = "";
protected string $doesExistNative = "";
}
class ChildClass extends ParentClass
{
/** @var string */
protected $mightExist = "";
protected string $mightExistNative = "";
/** @var null|string */
protected $doesExist = "";
protected ?string $doesExistNative = "";
}
',
class ChildClass extends ParentClass
{
protected string $mightExist = "";
}',
'error_message' => 'NonInvariantPropertyType',
]
],
];
}
}