From 1ff8518888e8c8839f59a54f4fbb30d0cb2de716 Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Tue, 19 Dec 2023 11:07:11 +0100 Subject: [PATCH 1/2] Fix https://github.com/vimeo/psalm/issues/9840 --- .../Expression/Call/ArgumentsAnalyzer.php | 73 +++++++++++++++++++ tests/ImmutableAnnotationTest.php | 63 ++++++++++++++++ 2 files changed, 136 insertions(+) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php index 9ff268abe..97026ac25 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php @@ -7,6 +7,7 @@ use Psalm\CodeLocation; use Psalm\Codebase; use Psalm\Context; use Psalm\Internal\Analyzer\AttributesAnalyzer; +use Psalm\Internal\Analyzer\Statements\Expression\Assignment\InstancePropertyAssignmentAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\AssignmentAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\CallAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\ExpressionIdentifier; @@ -45,6 +46,7 @@ use Psalm\Type\Atomic\TClosure; use Psalm\Type\Atomic\TKeyedArray; use Psalm\Type\Atomic\TList; use Psalm\Type\Atomic\TLiteralString; +use Psalm\Type\Atomic\TNamedObject; use Psalm\Type\Atomic\TNonEmptyArray; use Psalm\Type\Atomic\TTemplateParam; use Psalm\Type\Union; @@ -1253,6 +1255,37 @@ final class ArgumentsAnalyzer return null; } + private static function handleByRefReadonlyArg( + StatementsAnalyzer $statements_analyzer, + Context $context, + PhpParser\Node\Expr\PropertyFetch $stmt, + string $fq_class_name, + string $prop_name + ): void { + $property_id = $fq_class_name . '::$' . $prop_name; + + $codebase = $statements_analyzer->getCodebase(); + $declaring_property_class = (string) $codebase->properties->getDeclaringClassForProperty( + $property_id, + false, # what does this do? @todo + $statements_analyzer, + ); + $declaring_class_storage = $codebase->classlike_storage_provider->get($declaring_property_class); + + if (isset($declaring_class_storage->properties[$prop_name])) { + $property_storage = $declaring_class_storage->properties[$prop_name]; + + InstancePropertyAssignmentAnalyzer::trackPropertyImpurity( + $statements_analyzer, + $stmt, + $property_id, + $property_storage, + $declaring_class_storage, + $context, + ); + } + } + /** * @return false|null */ @@ -1274,6 +1307,46 @@ final class ArgumentsAnalyzer 'reset', 'end', 'next', 'prev', 'array_pop', 'array_shift', ]; + if ($arg->value instanceof PhpParser\Node\Expr\PropertyFetch + && $arg->value->name instanceof PhpParser\Node\Identifier) { + $prop_name = $arg->value->name->name; + if ($statements_analyzer->getFQCLN()) { + $fq_class_name = $statements_analyzer->getFQCLN(); + + self::handleByRefReadonlyArg( + $statements_analyzer, + $context, + $arg->value, + $fq_class_name, + $prop_name, + ); + } else { + // @todo atm only works for simple fetch, $a->foo, not $a->foo->bar + // I guess there's a function to do this, but I couldn't locate it + $var_id = ExpressionIdentifier::getVarId( + $arg->value->var, + $statements_analyzer->getFQCLN(), + $statements_analyzer, + ); + + if ($var_id && isset($context->vars_in_scope[$var_id])) { + foreach ($context->vars_in_scope[$var_id]->getAtomicTypes() as $atomic_type) { + if ($atomic_type instanceof TNamedObject) { + $fq_class_name = $atomic_type->value; + + self::handleByRefReadonlyArg( + $statements_analyzer, + $context, + $arg->value, + $fq_class_name, + $prop_name, + ); + } + } + } + } + } + if (($var_id && isset($context->vars_in_scope[$var_id])) || ($method_id && in_array( diff --git a/tests/ImmutableAnnotationTest.php b/tests/ImmutableAnnotationTest.php index 1aa481b2a..56838b941 100644 --- a/tests/ImmutableAnnotationTest.php +++ b/tests/ImmutableAnnotationTest.php @@ -764,6 +764,69 @@ class ImmutableAnnotationTest extends TestCase }', 'error_message' => 'ImpurePropertyAssignment', ], + 'readonlyByRefInClass' => [ + 'code' => 'values = $values; + } + + public function bar(): mixed + { + return reset($this->values); + } + }', + 'error_message' => 'InaccessibleProperty', + ], + 'readonlyByRef' => [ + 'code' => 'values = $values; + } + } + + $x = new Foo([]); + reset($x->values);', + 'error_message' => 'InaccessibleProperty', + ], + 'readonlyByRefCustomFunction' => [ + 'code' => 'values = $values; + } + } + + /** + * @param string $a + * @param array $b + * @return void + */ + function bar($a, &$b) {} + + $x = new Foo([]); + bar("hello", $x->values);', + 'error_message' => 'InaccessibleProperty', + ], 'preventUnset' => [ 'code' => ' Date: Tue, 19 Dec 2023 11:22:51 +0100 Subject: [PATCH 2/2] make tests work in PHP < 8.2 --- .../Expression/Call/ArgumentsAnalyzer.php | 12 ++- src/Psalm/Type/Atomic/TKeyedArray.php | 2 + tests/ImmutableAnnotationTest.php | 88 +++++++++++-------- 3 files changed, 61 insertions(+), 41 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php index 97026ac25..003355bfc 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php @@ -2,6 +2,7 @@ namespace Psalm\Internal\Analyzer\Statements\Expression\Call; +use InvalidArgumentException; use PhpParser; use Psalm\CodeLocation; use Psalm\Codebase; @@ -1267,10 +1268,15 @@ final class ArgumentsAnalyzer $codebase = $statements_analyzer->getCodebase(); $declaring_property_class = (string) $codebase->properties->getDeclaringClassForProperty( $property_id, - false, # what does this do? @todo + true, $statements_analyzer, ); - $declaring_class_storage = $codebase->classlike_storage_provider->get($declaring_property_class); + + try { + $declaring_class_storage = $codebase->classlike_storage_provider->get($declaring_property_class); + } catch (InvalidArgumentException $_) { + return; + } if (isset($declaring_class_storage->properties[$prop_name])) { $property_storage = $declaring_class_storage->properties[$prop_name]; @@ -1310,7 +1316,7 @@ final class ArgumentsAnalyzer if ($arg->value instanceof PhpParser\Node\Expr\PropertyFetch && $arg->value->name instanceof PhpParser\Node\Identifier) { $prop_name = $arg->value->name->name; - if ($statements_analyzer->getFQCLN()) { + if (!empty($statements_analyzer->getFQCLN())) { $fq_class_name = $statements_analyzer->getFQCLN(); self::handleByRefReadonlyArg( diff --git a/src/Psalm/Type/Atomic/TKeyedArray.php b/src/Psalm/Type/Atomic/TKeyedArray.php index 3bd7e2a65..c1c29b991 100644 --- a/src/Psalm/Type/Atomic/TKeyedArray.php +++ b/src/Psalm/Type/Atomic/TKeyedArray.php @@ -112,6 +112,8 @@ class TKeyedArray extends Atomic if ($cloned->is_list) { $last_k = -1; $had_possibly_undefined = false; + + /** @psalm-suppress InaccessibleProperty */ ksort($cloned->properties); foreach ($cloned->properties as $k => $v) { if (is_string($k) || $last_k !== ($k-1) || ($had_possibly_undefined && !$v->possibly_undefined)) { diff --git a/tests/ImmutableAnnotationTest.php b/tests/ImmutableAnnotationTest.php index 56838b941..da36f660d 100644 --- a/tests/ImmutableAnnotationTest.php +++ b/tests/ImmutableAnnotationTest.php @@ -768,63 +768,75 @@ class ImmutableAnnotationTest extends TestCase 'code' => 'values = $values; - } + public function __construct(array $values) + { + $this->values = $values; + } - public function bar(): mixed - { - return reset($this->values); - } - }', + /** + * @return mixed + */ + public function bar() + { + return reset($this->values); + } + }', 'error_message' => 'InaccessibleProperty', ], 'readonlyByRef' => [ 'code' => 'values = $values; - } - } + public function __construct(array $values) + { + $this->values = $values; + } + } - $x = new Foo([]); - reset($x->values);', + $x = new Foo([]); + reset($x->values);', 'error_message' => 'InaccessibleProperty', ], 'readonlyByRefCustomFunction' => [ 'code' => 'values = $values; - } - } + public function __construct(array $values) + { + $this->values = $values; + } + } - /** - * @param string $a - * @param array $b - * @return void - */ - function bar($a, &$b) {} + /** + * @param string $a + * @param array $b + * @return void + */ + function bar($a, &$b) {} - $x = new Foo([]); - bar("hello", $x->values);', + $x = new Foo([]); + bar("hello", $x->values);', 'error_message' => 'InaccessibleProperty', ], 'preventUnset' => [