danog/psalm
1
0
Fork 0
mirror of https://github.com/danog/psalm.git synced 2025-01-22 05:41:20 +01:00
Code Issues Projects Releases Wiki Activity

Merge pull request #7252 from AndrolGenhald/feature/allow-assertions-on-mutable-object-properties

Browse Source
This commit is contained in:
Bruce Weirdan 2022-01-02 07:29:02 +02:00 committed by GitHub
commit 697db76dc1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 356 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
docs/annotating_code/supported_annotations.md
View File

@ -240,6 +240,8 @@ $b->s = "boo"; // disallowed
### `@psalm-mutation-free`
Used to annotate a class method that does not mutate state, either internally or externally of the class's scope.
This requires that the return value depend only on the instance's properties. For example, `random_int` is considered
mutating here because it mutates the random number generator's internal state.
```php
<?php
@ -309,7 +311,7 @@ Used to annotate a class where every property is treated by consumers as `@psalm
abstract class Foo
{
public string $baz;
abstract public function bar(): int;
}
@ -322,7 +324,7 @@ final class ChildClass extends Foo
{
$this->baz = $baz;
}
public function bar(): int
{
return 0;
@ -332,7 +334,7 @@ final class ChildClass extends Foo
$anonymous = new /** @psalm-immutable */ class extends Foo
{
public string $baz = "B";
public function bar(): int
{
return 1;
@ -393,9 +395,9 @@ class Counter {
/**
* @readonly
* @psalm-allow-private-mutation
*/
*/
public int $count = 0;
public function increment() : void {
$this->count++;
}
@ -417,9 +419,9 @@ This is a shorthand for the property annotations `@readonly` and `@psalm-allow-p
class Counter {
/**
* @psalm-readonly-allow-private-mutation
*/
*/
public int $count = 0;
public function increment() : void {
$this->count++;
}
@ -439,7 +441,7 @@ You can use this annotation to trace inferred type (applied to the *next* statem
```php
<?php
/** @psalm-trace $username */
/** @psalm-trace $username */
$username = $_GET['username']; // prints something like "test.php:4 $username: mixed"
```

26
src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php
View File

@ -4106,26 +4106,26 @@ class AssertionFinder
foreach ($type->getAtomicTypes() as $type) {
if (!$type instanceof TNamedObject) {
return 'Variable ' . $name . ' is not an object so assertion cannot be applied';
return 'Variable ' . $name . ' is not an object so the assertion cannot be applied';
}
$class_definition = $class_provider->get($type->value);
$property_definition = $class_definition->properties[$property] ?? null;
if (!$property_definition instanceof PropertyStorage) {
return sprintf(
'Property %s is not defined on variable %s so assertion cannot be applied',
$property,
$name
);
}
$magic_type = $class_definition->pseudo_property_get_types['$' . $property] ?? null;
if ($magic_type === null) {
return sprintf(
'Property %s is not defined on variable %s so the assertion cannot be applied',
$property,
$name
);
}
if (!$property_definition->readonly) {
return sprintf(
'Property %s of variable %s is not read-only/immutable so assertion cannot be applied',
$property,
$name
);
$magic_getter = $class_definition->methods['__get'] ?? null;
if ($magic_getter === null || !$magic_getter->mutation_free) {
return "{$class_definition->name}::__get is not mutation-free, so the assertion cannot be applied";
}
}
}

373
tests/AssertAnnotationTest.php
View File

@ -2,6 +2,9 @@
namespace Psalm\Tests;
use Psalm\Config;
use Psalm\Context;
use Psalm\Exception\CodeException;
use Psalm\Tests\Traits\InvalidCodeAnalysisTestTrait;
use Psalm\Tests\Traits\ValidCodeAnalysisTestTrait;
@ -12,6 +15,82 @@ class AssertAnnotationTest extends TestCase
use ValidCodeAnalysisTestTrait;
use InvalidCodeAnalysisTestTrait;
public function testDontForgetAssertionAfterMutationFreeCall(): void
{
Config::getInstance()->remember_property_assignments_after_call = false;
$this->addFile(
'somefile.php',
'<?php
class Foo
{
public ?string $bar = null;
/** @psalm-mutation-free */
public function mutationFree(): void {}
}
/**
* @psalm-assert-if-true !null $foo->bar
*/
function assertBarNotNull(Foo $foo): bool
{
return $foo->bar !== null;
}
$foo = new Foo();
if (assertBarNotNull($foo)) {
$foo->mutationFree();
requiresString($foo->bar);
}
function requiresString(string $str): void {}
'
);
$this->analyzeFile('somefile.php', new Context());
}
public function testForgetAssertionAfterNonMutationFreeCall(): void
{
$this->expectExceptionMessage('PossiblyNullArgument');
$this->expectException(CodeException::class);
Config::getInstance()->remember_property_assignments_after_call = false;
$this->addFile(
'somefile.php',
'<?php
class Foo
{
public ?string $bar = null;
public function nonMutationFree(): void {}
}
/**
* @psalm-assert-if-true !null $foo->bar
*/
function assertBarNotNull(Foo $foo): bool
{
return $foo->bar !== null;
}
$foo = new Foo();
if (assertBarNotNull($foo)) {
$foo->nonMutationFree();
requiresString($foo->bar);
}
function requiresString(string $_str): void {}
'
);
$this->analyzeFile('somefile.php', new Context());
}
/**
* @return iterable<string,array{string,assertions?:array<string,string>,error_levels?:string[]}>
*/
@ -1719,6 +1798,129 @@ class AssertAnnotationTest extends TestCase
}
}',
],
'dontForgetAssertionAfterIrrelevantNonMutationFreeCall' => [
'<?php
class Foo
{
public ?string $bar = null;
public function nonMutationFree(): void {}
}
/**
* @psalm-assert-if-true !null $foo->bar
*/
function assertBarNotNull(Foo $foo): bool
{
return $foo->bar !== null;
}
$foo = new Foo();
if (assertBarNotNull($foo)) {
$foo->nonMutationFree();
requiresString($foo->bar);
}
function requiresString(string $_str): void {}
',
],
'SKIPPED-applyAssertionsToReferences' => [ // See #7254
'<?php
class Foo
{
public ?string $bar = null;
}
/**
* @psalm-assert-if-true !null $foo->bar
*/
function assertBarNotNull(Foo $foo): bool
{
return $foo->bar !== null;
}
$foo = new Foo();
$bar = &$foo;
if (assertBarNotNull($foo)) {
requiresString($bar->bar);
}
function requiresString(string $_str): void {}
',
],
'SKIPPED-applyAssertionsFromReferences' => [ // See #7254
'<?php
class Foo
{
public ?string $bar = null;
}
/**
* @psalm-assert-if-true !null $foo->bar
*/
function assertBarNotNull(Foo $foo): bool
{
return $foo->bar !== null;
}
$foo = new Foo();
$bar = &$foo;
if (assertBarNotNull($bar)) {
requiresString($foo->bar);
}
function requiresString(string $_str): void {}
',
],
'SKIPPED-applyAssertionsToReferencesWithConditionalOperator' => [ // See #7254
'<?php
class Foo
{
public ?string $bar = null;
}
/**
* @psalm-assert-if-true !null $foo->bar
*/
function assertBarNotNull(Foo $foo): bool
{
return $foo->bar !== null;
}
$foo = new Foo();
$bar = &$foo;
requiresString(assertBarNotNull($foo) ? $bar->bar : "bar");
function requiresString(string $_str): void {}
',
],
'assertionOnMagicProperty' => [
'<?php
/**
* @property ?string $b
*/
class A {
/** @psalm-mutation-free */
public function __get(string $key) {return "";}
public function __set(string $key, string $value): void {}
}
$a = new A;
/** @psalm-assert-if-true string $arg->b */
function assertString(A $arg): bool {return $arg->b !== null;}
if (assertString($a)) {
requiresString($a->b);
}
function requiresString(string $_str): void {}
',
],
];
}
@ -1984,66 +2186,157 @@ class AssertAnnotationTest extends TestCase
}',
'error_message' => 'PossiblyNullReference',
],
'onPropertyOfMutableArgument' => [
'forgetAssertionAfterRelevantNonMutationFreeCall' => [
'<?php
class Aclass {
public ?string $b;
public function __construct(?string $b) {
$this->b = $b;
class Foo
{
public ?string $bar = null;
public function nonMutationFree(): void
{
$this->bar = null;
}
}
/** @psalm-assert !null $item->b */
function c(\Aclass $item): void {
if (null === $item->b) {
throw new \InvalidArgumentException("");
}
/**
* @psalm-assert-if-true !null $foo->bar
*/
function assertBarNotNull(Foo $foo): bool
{
return $foo->bar !== null;
}
/** @var \Aclass $a */
c($a);
echo strlen($a->b);',
'error_message' => 'InvalidDocblock',
$foo = new Foo();
if (assertBarNotNull($foo)) {
$foo->nonMutationFree();
requiresString($foo->bar);
}
function requiresString(string $_str): void {}
',
'error_message' => 'PossiblyNullArgument',
],
'ifTrueOnPropertyOfMutableArgument' => [
'SKIPPED-forgetAssertionAfterRelevantNonMutationFreeCallOnReference' => [ // See #7254
'<?php
class Aclass {
public ?string $b;
public function __construct(?string $b) {
$this->b = $b;
class Foo
{
public ?string $bar = null;
public function nonMutationFree(): void
{
$this->bar = null;
}
}
/** @psalm-assert-if-true !null $item->b */
function c(\Aclass $item): bool {
return null !== $item->b;
/**
* @psalm-assert-if-true !null $foo->bar
*/
function assertBarNotNull(Foo $foo): bool
{
return $foo->bar !== null;
}
/** @var \Aclass $a */
if (c($a)) {
echo strlen($a->b);
}',
'error_message' => 'InvalidDocblock',
$foo = new Foo();
$fooRef = &$foo;
if (assertBarNotNull($foo)) {
$fooRef->nonMutationFree();
requiresString($foo->bar);
}
function requiresString(string $_str): void {}
',
'error_message' => 'PossiblyNullArgument',
],
'ifFalseOnPropertyOfMutableArgument' => [
'SKIPPED-forgetAssertionAfterReferenceModification' => [ // See #7254
'<?php
class Aclass {
public ?string $b;
public function __construct(?string $b) {
$this->b = $b;
class Foo
{
public ?string $bar = null;
}
/**
* @psalm-assert-if-true !null $foo->bar
*/
function assertBarNotNull(Foo $foo): bool
{
return $foo->bar !== null;
}
$foo = new Foo();
$barRef = &$foo->bar;
if (assertBarNotNull($foo)) {
$barRef = null;
requiresString($foo->bar);
}
function requiresString(string $_str): void {}
',
'error_message' => 'PossiblyNullArgument',
],
'assertionOnMagicPropertyWithoutMutationFreeGet' => [
'<?php
/**
* @property ?string $b
*/
class A {
public function __get(string $key) {return "";}
public function __set(string $key, string $value): void {}
}
$a = new A;
/** @psalm-assert-if-true string $arg->b */
function assertString(A $arg): bool {return $arg->b !== null;}
if (assertString($a)) {
requiresString($a->b);
}
function requiresString(string $_str): void {}
',
'error_message' => 'A::__get is not mutation-free',
],
'randomValueFromMagicGetterIsNotMutationFree' => [
'<?php
/**
* @property int<1, 10> $b
*/
class A {
/** @psalm-mutation-free */
public function __get(string $key)
{
if ($key === "b") {
return random_int(1, 10);
}
return null;
}
public function __set(string $key, string $value): void
{
throw new \Exception("Setting not supported!");
}
}
/** @psalm-assert-if-false !null $item->b */
function c(\Aclass $item): bool {
return null === $item->b;
$a = new A;
/** @psalm-assert-if-true =1 $arg->b */
function assertBIsOne(A $arg): bool
{
return $arg->b === 1;
}
/** @var \Aclass $a */
if (!c($a)) {
echo strlen($a->b);
}',
'error_message' => 'InvalidDocblock',
if (assertBIsOne($a)) {
takesOne($a->b);
}
/** @param 1 $_arg */
function takesOne(int $_arg): void {}
',
'error_message' => 'ImpureFunctionCall - src' . DIRECTORY_SEPARATOR . 'somefile.php:10:40',
],
];
}