From efd17a124bfa1f7f07fe3f4459889b55aae5a1c7 Mon Sep 17 00:00:00 2001 From: kkmuffme <11071985+kkmuffme@users.noreply.github.com> Date: Thu, 28 Mar 2024 01:42:32 +0100 Subject: [PATCH 1/5] add function and consistent error for internal property --- .../InstancePropertyAssignmentAnalyzer.php | 40 ++---- .../Fetch/AtomicPropertyFetchAnalyzer.php | 50 +++++-- .../Fetch/InstancePropertyFetchAnalyzer.php | 8 ++ .../Fetch/StaticPropertyFetchAnalyzer.php | 8 ++ tests/InternalAnnotationTest.php | 124 ++++++++++++++++++ 5 files changed, 192 insertions(+), 38 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php index bedec945c25..501457823f4 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Assignment/InstancePropertyAssignmentAnalyzer.php @@ -14,7 +14,6 @@ use Psalm\Internal\Analyzer\ClassAnalyzer; use Psalm\Internal\Analyzer\ClassLikeAnalyzer; use Psalm\Internal\Analyzer\FunctionLikeAnalyzer; -use Psalm\Internal\Analyzer\NamespaceAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\Call\ClassTemplateParamCollector; use Psalm\Internal\Analyzer\Statements\Expression\Call\MethodCallAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\ExpressionIdentifier; @@ -33,12 +32,9 @@ use Psalm\Internal\Type\TemplateResult; use Psalm\Internal\Type\TemplateStandinTypeReplacer; use Psalm\Internal\Type\TypeExpander; -use Psalm\Issue\DeprecatedProperty; use Psalm\Issue\ImplicitToStringCast; use Psalm\Issue\ImpurePropertyAssignment; use Psalm\Issue\InaccessibleProperty; -use Psalm\Issue\InternalClass; -use Psalm\Issue\InternalProperty; use Psalm\Issue\InvalidPropertyAssignment; use Psalm\Issue\InvalidPropertyAssignmentValue; use Psalm\Issue\LoopInvalidation; @@ -1257,30 +1253,22 @@ private static function analyzeAtomicAssignment( $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]; + AtomicPropertyFetchAnalyzer::checkPropertyDeprecation( + $prop_name, + $declaring_property_class, + $stmt, + $statements_analyzer, + ); - if ($property_storage->deprecated) { - IssueBuffer::maybeAdd( - new DeprecatedProperty( - $property_id . ' is marked deprecated', - new CodeLocation($statements_analyzer->getSource(), $stmt), - $property_id, - ), - $statements_analyzer->getSuppressedIssues(), - ); - } + AtomicPropertyFetchAnalyzer::checkPropertyInternal( + $prop_name, + $declaring_property_class, + $stmt, + $statements_analyzer, + $context, + ); - if ($context->self && !NamespaceAnalyzer::isWithinAny($context->self, $property_storage->internal)) { - IssueBuffer::maybeAdd( - new InternalProperty( - $property_id . ' is internal to ' . InternalClass::listToPhrase($property_storage->internal) - . ' but called from ' . $context->self, - new CodeLocation($statements_analyzer->getSource(), $stmt), - $property_id, - ), - $statements_analyzer->getSuppressedIssues(), - ); - } + $property_storage = $declaring_class_storage->properties[$prop_name]; self::trackPropertyImpurity( $statements_analyzer, diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php index 6ad6983b76e..239d45dd848 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php @@ -456,19 +456,9 @@ public static function analyze( if (isset($declaring_class_storage->properties[$prop_name])) { self::checkPropertyDeprecation($prop_name, $declaring_property_class, $stmt, $statements_analyzer); - $property_storage = $declaring_class_storage->properties[$prop_name]; + self::checkPropertyInternal($prop_name, $declaring_property_class, $stmt, $statements_analyzer, $context); - if ($context->self && !NamespaceAnalyzer::isWithinAny($context->self, $property_storage->internal)) { - IssueBuffer::maybeAdd( - new InternalProperty( - $property_id . ' is internal to ' . InternalClass::listToPhrase($property_storage->internal) - . ' but called from ' . $context->self, - new CodeLocation($statements_analyzer->getSource(), $stmt), - $property_id, - ), - $statements_analyzer->getSuppressedIssues(), - ); - } + $property_storage = $declaring_class_storage->properties[$prop_name]; if ($context->inside_unset) { InstancePropertyAssignmentAnalyzer::trackPropertyImpurity( @@ -572,6 +562,42 @@ public static function checkPropertyDeprecation( } } + /** + * @param PropertyFetch|StaticPropertyFetch $stmt + */ + public static function checkPropertyInternal( + string $prop_name, + string $declaring_property_class, + PhpParser\Node\Expr $stmt, + StatementsAnalyzer $statements_analyzer, + Context $context + ): void { + $property_id = $declaring_property_class . '::$' . $prop_name; + $codebase = $statements_analyzer->getCodebase(); + $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]; + + $caller_identifier = $context->self ?? $statements_analyzer->getNamespace(); + + if ($caller_identifier !== null + && !NamespaceAnalyzer::isWithinAny($caller_identifier, $property_storage->internal)) { + IssueBuffer::maybeAdd( + new InternalProperty( + $property_id . ' is internal to ' . InternalClass::listToPhrase($property_storage->internal) + . ' but called from ' . ($caller_identifier ?: 'root namespace'), + new CodeLocation($statements_analyzer->getSource(), $stmt), + $property_id, + ), + $statements_analyzer->getSuppressedIssues(), + ); + } + } + } + private static function propertyFetchCanBeAnalyzed( StatementsAnalyzer $statements_analyzer, Codebase $codebase, diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php index acf004490c1..72402be2b74 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/InstancePropertyFetchAnalyzer.php @@ -434,6 +434,14 @@ private static function handleScopedProperty( $stmt, $statements_analyzer, ); + + AtomicPropertyFetchAnalyzer::checkPropertyInternal( + $stmt->name->name, + $declaring_property_class, + $stmt, + $statements_analyzer, + $context, + ); } $codebase->properties->propertyExists( diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/StaticPropertyFetchAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/StaticPropertyFetchAnalyzer.php index 790b36b30e7..c28fc9293ee 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/StaticPropertyFetchAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/StaticPropertyFetchAnalyzer.php @@ -295,6 +295,14 @@ public static function analyze( $statements_analyzer, ); + AtomicPropertyFetchAnalyzer::checkPropertyInternal( + $prop_name, + $declaring_property_class, + $stmt, + $statements_analyzer, + $context, + ); + $class_storage = $codebase->classlike_storage_provider->get($declaring_property_class); $property = $class_storage->properties[$prop_name]; diff --git a/tests/InternalAnnotationTest.php b/tests/InternalAnnotationTest.php index 0ae59e81ccc..b7f87c77e41 100644 --- a/tests/InternalAnnotationTest.php +++ b/tests/InternalAnnotationTest.php @@ -900,6 +900,130 @@ public function batBat() : void { }', 'error_message' => 'InternalProperty', ], + 'internalPropertyStatic' => [ + 'code' => ' 'InternalProperty', + ], + 'internalPropertyStaticOutsideClass' => [ + 'code' => ' 'InternalProperty', + ], + 'internalPropertyStaticOutsideClassGlobalNamespace' => [ + 'code' => ' 'InternalProperty', + ], + 'internalPropertyInsideFunction' => [ + 'code' => 'foo; + } + }', + 'error_message' => 'InternalProperty', + ], + 'internalPropertyInsideFunctionGlobalNamespace' => [ + 'code' => 'foo; + } + }', + 'error_message' => 'InternalProperty', + ], + 'internalPropertyOutsideClass' => [ + 'code' => 'foo; + }', + 'error_message' => 'InternalProperty', + ], + 'internalPropertyOutsideClassGlobalNamespace' => [ + 'code' => 'foo; + }', + 'error_message' => 'InternalProperty', + ], 'magicPropertyGetInternalExplicit' => [ 'code' => ' Date: Thu, 28 Mar 2024 02:16:01 +0100 Subject: [PATCH 2/5] same for classes and class constants Fix https://github.com/vimeo/psalm/issues/10025 --- .../Expression/Call/NewAnalyzer.php | 8 +-- .../StaticMethod/AtomicStaticCallAnalyzer.php | 6 +- .../Expression/ClassConstAnalyzer.php | 14 +++-- tests/InternalAnnotationTest.php | 56 +++++++++++++++++++ 4 files changed, 72 insertions(+), 12 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php index 8f35fee56b3..2f3a226d6cf 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php @@ -373,16 +373,16 @@ private static function analyzeNamedConstructor( ); } - - if ($context->self + $caller_identifier = $context->self ?? $statements_analyzer->getNamespace(); + if ($caller_identifier !== null && !$context->collect_initializations && !$context->collect_mutations - && !NamespaceAnalyzer::isWithinAny($context->self, $storage->internal) + && !NamespaceAnalyzer::isWithinAny($caller_identifier, $storage->internal) ) { IssueBuffer::maybeAdd( new InternalClass( $fq_class_name . ' is internal to ' . InternalClass::listToPhrase($storage->internal) - . ' but called from ' . $context->self, + . ' but called from ' . ($caller_identifier ?: 'root namespace'), new CodeLocation($statements_analyzer->getSource(), $stmt), $fq_class_name, ), diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php index c9dce4d1460..de6bb6b2553 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/StaticMethod/AtomicStaticCallAnalyzer.php @@ -840,11 +840,13 @@ private static function handleNamedCall( ); } - if ($context->self && ! NamespaceAnalyzer::isWithinAny($context->self, $class_storage->internal)) { + $caller_identifier = $context->self ?? $statements_analyzer->getNamespace(); + if ($caller_identifier !== null + && !NamespaceAnalyzer::isWithinAny($caller_identifier, $class_storage->internal)) { IssueBuffer::maybeAdd( new InternalClass( $fq_class_name . ' is internal to ' . InternalClass::listToPhrase($class_storage->internal) - . ' but called from ' . $context->self, + . ' but called from ' . ($caller_identifier ?: 'root namespace'), new CodeLocation($statements_analyzer->getSource(), $stmt), $fq_class_name, ), diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/ClassConstAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/ClassConstAnalyzer.php index 41c04ed79c0..10fea953df6 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/ClassConstAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/ClassConstAnalyzer.php @@ -365,16 +365,17 @@ public static function analyzeFetch( } } - if ($context->self + $caller_identifier = $context->self ?? $statements_analyzer->getNamespace(); + if ($caller_identifier !== null && !$context->collect_initializations && !$context->collect_mutations - && !NamespaceAnalyzer::isWithinAny($context->self, $const_class_storage->internal) + && !NamespaceAnalyzer::isWithinAny($caller_identifier, $const_class_storage->internal) ) { IssueBuffer::maybeAdd( new InternalClass( $fq_class_name . ' is internal to ' . InternalClass::listToPhrase($const_class_storage->internal) - . ' but called from ' . $context->self, + . ' but called from ' . ($caller_identifier ?: 'root namespace'), new CodeLocation($statements_analyzer->getSource(), $stmt), $fq_class_name, ), @@ -656,16 +657,17 @@ public static function analyzeFetch( } } - if ($context->self + $caller_identifier = $context->self ?? $statements_analyzer->getNamespace(); + if ($caller_identifier !== null && !$context->collect_initializations && !$context->collect_mutations - && !NamespaceAnalyzer::isWithinAny($context->self, $const_class_storage->internal) + && !NamespaceAnalyzer::isWithinAny($caller_identifier, $const_class_storage->internal) ) { IssueBuffer::maybeAdd( new InternalClass( $fq_class_name . ' is internal to ' . InternalClass::listToPhrase($const_class_storage->internal) - . ' but called from ' . $context->self, + . ' but called from ' . ($caller_identifier ?: 'root namespace'), new CodeLocation($statements_analyzer->getSource(), $stmt), $fq_class_name, ), diff --git a/tests/InternalAnnotationTest.php b/tests/InternalAnnotationTest.php index b7f87c77e41..87c8a1fb750 100644 --- a/tests/InternalAnnotationTest.php +++ b/tests/InternalAnnotationTest.php @@ -1024,6 +1024,62 @@ class Foo { }', 'error_message' => 'InternalProperty', ], + 'internalClassOutsideClass' => [ + 'code' => ' 'InternalClass', + ], + 'internalClassOutsideClassGlobalNamespace' => [ + 'code' => ' 'InternalClass', + ], + 'internalClassInsideFunction' => [ + 'code' => ' 'InternalClass', + ], + 'internalClassInsideFunctionGlobalNamespace' => [ + 'code' => ' 'InternalClass', + ], 'magicPropertyGetInternalExplicit' => [ 'code' => ' Date: Thu, 28 Mar 2024 02:36:19 +0100 Subject: [PATCH 3/5] add support for @internal in functions --- .../Expression/Call/FunctionCallAnalyzer.php | 27 ++++++ tests/InternalAnnotationTest.php | 90 +++++++++++++++++++ 2 files changed, 117 insertions(+) diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php index 98e192c72f7..0d27349bf1c 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php @@ -10,6 +10,7 @@ use Psalm\Internal\Algebra\FormulaGenerator; use Psalm\Internal\Analyzer\AlgebraAnalyzer; use Psalm\Internal\Analyzer\FunctionLikeAnalyzer; +use Psalm\Internal\Analyzer\NamespaceAnalyzer; use Psalm\Internal\Analyzer\Statements\Expression\CallAnalyzer; use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer; use Psalm\Internal\Analyzer\StatementsAnalyzer; @@ -23,6 +24,8 @@ use Psalm\Internal\Type\TypeCombiner; use Psalm\Issue\DeprecatedFunction; use Psalm\Issue\ImpureFunctionCall; +use Psalm\Issue\InternalClass; +use Psalm\Issue\InternalMethod; use Psalm\Issue\InvalidFunctionCall; use Psalm\Issue\MixedFunctionCall; use Psalm\Issue\NullFunctionCall; @@ -402,6 +405,30 @@ public static function analyze( $statements_analyzer->getSuppressedIssues(), ); } + + if (!$context->collect_initializations + && !$context->collect_mutations + && $function_call_info->function_id !== null + ) { + $caller_identifier = $statements_analyzer->getFullyQualifiedFunctionMethodOrNamespaceName(); + if ($caller_identifier !== null + && !NamespaceAnalyzer::isWithinAny( + $caller_identifier, + $function_call_info->function_storage->internal, + )) { + IssueBuffer::maybeAdd( + new InternalMethod( + 'The function ' . $function_call_info->function_id + . ' is internal to ' + . InternalClass::listToPhrase($function_call_info->function_storage->internal) + . ' but called from ' . ($caller_identifier ?: 'root namespace'), + $code_location, + $function_call_info->function_id, + ), + $statements_analyzer->getSuppressedIssues(), + ); + } + } } if ($function_name instanceof PhpParser\Node\Name && $function_call_info->function_id) { diff --git a/tests/InternalAnnotationTest.php b/tests/InternalAnnotationTest.php index 87c8a1fb750..3f9443add34 100644 --- a/tests/InternalAnnotationTest.php +++ b/tests/InternalAnnotationTest.php @@ -1080,6 +1080,96 @@ function hello(): void { }', 'error_message' => 'InternalClass', ], + 'internalFunctionOutsideClass' => [ + 'code' => ' 'InternalMethod', + ], + 'internalFunctionOutsideClassGlobalNamespace' => [ + 'code' => ' 'InternalMethod', + ], + 'internalFunctionInsideFunction' => [ + 'code' => ' 'InternalMethod', + ], + 'internalFunctionInsideFunctionGlobalNamespace' => [ + 'code' => ' 'InternalMethod', + ], + 'internalFunctionInsideClass' => [ + 'code' => ' 'InternalMethod', + ], + 'internalFunctionInsideClassGlobalNamespace' => [ + 'code' => ' 'InternalMethod', + ], 'magicPropertyGetInternalExplicit' => [ 'code' => ' Date: Fri, 29 Mar 2024 00:25:32 +0100 Subject: [PATCH 4/5] Feature: treat legacy PHP `@access private` annotations as `@internal` annotations --- .../Internal/Analyzer/CommentAnalyzer.php | 9 +- .../Reflector/ClassLikeDocblockParser.php | 5 +- .../Reflector/FunctionLikeDocblockParser.php | 5 +- tests/InternalAnnotationTest.php | 84 +++++++++++++++++++ 4 files changed, 100 insertions(+), 3 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/CommentAnalyzer.php b/src/Psalm/Internal/Analyzer/CommentAnalyzer.php index ec06f1878fe..74415858570 100644 --- a/src/Psalm/Internal/Analyzer/CommentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/CommentAnalyzer.php @@ -27,6 +27,7 @@ use function count; use function is_string; +use function preg_grep; use function preg_match; use function preg_replace; use function preg_split; @@ -196,6 +197,8 @@ public static function arrayToDocblocks( if (!$var_comments && (isset($parsed_docblock->tags['deprecated']) || isset($parsed_docblock->tags['internal']) + || (isset($parsed_docblock->tags['access']) + && preg_grep('/^private(?>\s|$)/', $parsed_docblock->tags['access'])) || isset($parsed_docblock->tags['readonly']) || isset($parsed_docblock->tags['psalm-readonly']) || isset($parsed_docblock->tags['psalm-readonly-allow-private-mutation']) @@ -220,7 +223,11 @@ private static function decorateVarDocblockComment( ParsedDocblock $parsed_docblock ): void { $var_comment->deprecated = isset($parsed_docblock->tags['deprecated']); - $var_comment->internal = isset($parsed_docblock->tags['internal']); + if (isset($parsed_docblock->tags['internal']) + || (isset($parsed_docblock->tags['access']) + && preg_grep('/^private(?>\s|$)/', $parsed_docblock->tags['access']))) { + $var_comment->internal = true; + } $var_comment->readonly = isset($parsed_docblock->tags['readonly']) || isset($parsed_docblock->tags['psalm-readonly']) || isset($parsed_docblock->tags['psalm-readonly-allow-private-mutation']); diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeDocblockParser.php b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeDocblockParser.php index 2a6922627c2..38dee763296 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeDocblockParser.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeDocblockParser.php @@ -30,6 +30,7 @@ use function explode; use function implode; use function in_array; +use function preg_grep; use function preg_last_error_msg; use function preg_match; use function preg_replace; @@ -204,7 +205,9 @@ public static function parse( $info->deprecated = true; } - if (isset($parsed_docblock->tags['internal'])) { + if (isset($parsed_docblock->tags['internal']) + || (isset($parsed_docblock->tags['access']) + && preg_grep('/^private(?>\s|$)/', $parsed_docblock->tags['access']))) { $info->internal = true; } diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php index 25fcb44bf21..141ae846a73 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockParser.php @@ -24,6 +24,7 @@ use function implode; use function in_array; use function pathinfo; +use function preg_grep; use function preg_last_error_msg; use function preg_match; use function preg_replace; @@ -436,7 +437,9 @@ public static function parse( $info->deprecated = true; } - if (isset($parsed_docblock->tags['internal'])) { + if (isset($parsed_docblock->tags['internal']) + || (isset($parsed_docblock->tags['access']) + && preg_grep('/^private(?>\s|$)/', $parsed_docblock->tags['access']))) { $info->internal = true; } diff --git a/tests/InternalAnnotationTest.php b/tests/InternalAnnotationTest.php index 3f9443add34..1ee8f387d0d 100644 --- a/tests/InternalAnnotationTest.php +++ b/tests/InternalAnnotationTest.php @@ -1448,6 +1448,90 @@ public function __construct() {} ', 'error_message' => 'InternalMethod', ], + 'accessPrivateClass' => [ + 'code' => ' 'InternalClass', + ], + 'accessPrivateConstructor' => [ + 'code' => ' 'InternalMethod', + ], + 'accessPrivateMethod' => [ + 'code' => 'run(); + } + ', + 'error_message' => 'InternalMethod', + ], + 'accessPrivateFunction' => [ + 'code' => ' 'InternalMethod', + ], + 'accessPrivateProperty' => [ + 'code' => 'foo; + } + ', + 'error_message' => 'InternalProperty', + ], 'psalmInternalClassWithCallClass' => [ 'code' => ' Date: Fri, 29 Mar 2024 01:10:09 +0100 Subject: [PATCH 5/5] Make @internal in global namespace for methods consistent with properties and fix https://github.com/vimeo/psalm/issues/10868 --- .../Internal/Analyzer/NamespaceAnalyzer.php | 8 +- .../Reflector/ClassLikeNodeScanner.php | 2 + .../Reflector/FunctionLikeDocblockScanner.php | 4 + src/Psalm/Issue/InternalClass.php | 6 +- src/Psalm/Storage/ClassLikeStorage.php | 2 +- src/Psalm/Storage/FunctionLikeStorage.php | 2 +- tests/InternalAnnotationTest.php | 108 ++++++++++++++++++ 7 files changed, 128 insertions(+), 4 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/NamespaceAnalyzer.php b/src/Psalm/Internal/Analyzer/NamespaceAnalyzer.php index d5f2ac9e4a7..6b2a304182b 100644 --- a/src/Psalm/Internal/Analyzer/NamespaceAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/NamespaceAnalyzer.php @@ -191,11 +191,17 @@ public static function isWithin(string $calling_identifier, string $identifier): */ public static function isWithinAny(string $calling_identifier, array $identifiers): bool { - if (count($identifiers) === 0) { + if ($identifiers === []) { return true; } foreach ($identifiers as $identifier) { + if ($calling_identifier !== '' && $identifier === '') { + // if we have any non-empty, we ignore the empty one + // therefore continue instead of return + continue; + } + if (self::isWithin($calling_identifier, $identifier)) { return true; } diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php index c8edcf6723e..1fdc9994189 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php @@ -650,6 +650,8 @@ public function start(PhpParser\Node\Stmt\ClassLike $node): ?bool $storage->internal = $docblock_info->psalm_internal; } elseif ($docblock_info->internal && $this->aliases->namespace) { $storage->internal = [NamespaceAnalyzer::getNameSpaceRoot($this->aliases->namespace)]; + } elseif ($docblock_info->internal) { + $storage->internal = ['']; } if ($docblock_info->final && !$storage->final) { diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php index 313a3713af4..faa2e1e7492 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/FunctionLikeDocblockScanner.php @@ -118,6 +118,10 @@ public static function addDocblockInfo( $storage->internal = $docblock_info->psalm_internal; } elseif ($docblock_info->internal && $aliases->namespace) { $storage->internal = [NamespaceAnalyzer::getNameSpaceRoot($aliases->namespace)]; + } elseif ($docblock_info->internal && $classlike_storage && $classlike_storage->name) { + $storage->internal = [NamespaceAnalyzer::getNameSpaceRoot($classlike_storage->name)]; + } elseif ($docblock_info->internal) { + $storage->internal = ['']; } if (($storage->internal || ($classlike_storage && $classlike_storage->internal)) diff --git a/src/Psalm/Issue/InternalClass.php b/src/Psalm/Issue/InternalClass.php index 087eece0201..1ac99433987 100644 --- a/src/Psalm/Issue/InternalClass.php +++ b/src/Psalm/Issue/InternalClass.php @@ -13,10 +13,14 @@ final class InternalClass extends ClassIssue public const ERROR_LEVEL = 4; public const SHORTCODE = 174; - /** @param non-empty-list $words */ + /** @param non-empty-list $words */ public static function listToPhrase(array $words): string { $words = array_unique($words); + if ($words === ['']) { + return 'root namespace'; + } + if (count($words) === 1) { return reset($words); } diff --git a/src/Psalm/Storage/ClassLikeStorage.php b/src/Psalm/Storage/ClassLikeStorage.php index 6aec220047d..47c0eb1c26b 100644 --- a/src/Psalm/Storage/ClassLikeStorage.php +++ b/src/Psalm/Storage/ClassLikeStorage.php @@ -51,7 +51,7 @@ final class ClassLikeStorage implements HasAttributesInterface public $deprecated = false; /** - * @var list + * @var list */ public $internal = []; diff --git a/src/Psalm/Storage/FunctionLikeStorage.php b/src/Psalm/Storage/FunctionLikeStorage.php index 01ab5d13242..e2e706dc18d 100644 --- a/src/Psalm/Storage/FunctionLikeStorage.php +++ b/src/Psalm/Storage/FunctionLikeStorage.php @@ -76,7 +76,7 @@ abstract class FunctionLikeStorage implements HasAttributesInterface public $deprecated; /** - * @var list + * @var list */ public $internal = []; diff --git a/tests/InternalAnnotationTest.php b/tests/InternalAnnotationTest.php index 1ee8f387d0d..4266ea069a1 100644 --- a/tests/InternalAnnotationTest.php +++ b/tests/InternalAnnotationTest.php @@ -646,6 +646,30 @@ public function batBat() : void { } }', ], + 'globalNamespaceInternalValidClass' => [ + 'code' => ' [ + 'code' => ' [ 'code' => ' 'InternalProperty', ], + 'globalNamespaceInternalClass' => [ + 'code' => ' 'InternalClass', + ], + 'globalNamespaceInternalConstructor' => [ + 'code' => ' 'InternalMethod', + ], + 'globalNamespaceInternalMethod' => [ + 'code' => 'run(); + } + ', + 'error_message' => 'InternalMethod', + ], + 'globalNamespaceInternalFunction' => [ + 'code' => ' 'InternalMethod', + ], + 'globalNamespaceInternalProperty' => [ + 'code' => 'foo; + } + ', + 'error_message' => 'InternalProperty', + ], 'psalmInternalClassWithCallClass' => [ 'code' => '