diff --git a/docs/annotating_code/supported_annotations.md b/docs/annotating_code/supported_annotations.md index a5d985d1fa9..e1066f04de7 100644 --- a/docs/annotating_code/supported_annotations.md +++ b/docs/annotating_code/supported_annotations.md @@ -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 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 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"; + } } } diff --git a/tests/AssertAnnotationTest.php b/tests/AssertAnnotationTest.php index e46056f8ec3..ade990073f5 100644 --- a/tests/AssertAnnotationTest.php +++ b/tests/AssertAnnotationTest.php @@ -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', + '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', + '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,error_levels?:string[]}> */ @@ -1719,6 +1798,129 @@ function assertSame($expected, $actual): void { } }', ], + 'dontForgetAssertionAfterIrrelevantNonMutationFreeCall' => [ + '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 + '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 + '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 + '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' => [ + '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 @@ function foo(bool $bar) : void { }', 'error_message' => 'PossiblyNullReference', ], - 'onPropertyOfMutableArgument' => [ + 'forgetAssertionAfterRelevantNonMutationFreeCall' => [ '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 '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 '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' => [ + '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' => [ + ' $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', ], ]; }