From 703a1e1698dfe0feedd30604395e2d832f911076 Mon Sep 17 00:00:00 2001 From: Jack Worman Date: Thu, 22 Dec 2022 08:43:55 -0600 Subject: [PATCH] @psalm-api --- composer.json | 2 +- docs/annotating_code/supported_annotations.md | 17 ++- .../issues/PossiblyUnusedMethod.md | 6 +- .../issues/PossiblyUnusedProperty.md | 6 +- docs/running_psalm/issues/UnusedClass.md | 5 +- docs/running_psalm/issues/UnusedMethod.md | 6 +- docs/running_psalm/issues/UnusedProperty.md | 6 +- phpunit.xml.dist | 4 + psalm-baseline.xml | 9 +- src/Psalm/Config.php | 8 ++ src/Psalm/DocComment.php | 1 + src/Psalm/Internal/Codebase/ClassLikes.php | 123 +++++++++++++----- .../Reflector/ClassLikeDocblockParser.php | 2 + .../Reflector/ClassLikeNodeScanner.php | 2 + .../Scanner/ClassLikeDocblockComment.php | 2 + src/Psalm/Storage/ClassLikeStorage.php | 2 + tests/UnusedCodeTest.php | 87 +++++++++++++ 17 files changed, 246 insertions(+), 42 deletions(-) diff --git a/composer.json b/composer.json index 1314b2699e6..914304006aa 100644 --- a/composer.json +++ b/composer.json @@ -117,7 +117,7 @@ "phpunit" ], "verify-callmap": "phpunit tests/Internal/Codebase/InternalCallMapHandlerTest.php", - "psalm": "@php ./psalm --find-dead-code", + "psalm": "@php ./psalm", "tests": [ "@lint", "@cs", diff --git a/docs/annotating_code/supported_annotations.md b/docs/annotating_code/supported_annotations.md index 5393cca2e7a..922f5baa053 100644 --- a/docs/annotating_code/supported_annotations.md +++ b/docs/annotating_code/supported_annotations.md @@ -630,7 +630,7 @@ class Success implements Promise { * @return Promise */ function fetch(): Promise { - return new Success('{"data":[]}'); + return new Success('{"data":[]}'); } function (): Generator { @@ -642,6 +642,21 @@ function (): Generator { ``` This annotation supports only generic types, meaning that e.g. `@psalm-yield string` would be ignored. +### `@psalm-api` + +Used to tell Psalm that a class is used, even if no references to it can be +found. Unused issues will be suppressed. + +For example, in frameworks, controllers are often invoked "magically" without +any explicit references to them in your code. You should mark these classes with +`@psalm-api`. +```php +/** + * @psalm-api + */ +class UnreferencedClass {} +``` + ## Type Syntax Psalm supports PHPDoc’s [type syntax](https://docs.phpdoc.org/latest/guide/guides/types.html), and also the [proposed PHPDoc PSR type syntax](https://github.com/php-fig/fig-standards/blob/master/proposed/phpdoc.md#appendix-a-types). diff --git a/docs/running_psalm/issues/PossiblyUnusedMethod.md b/docs/running_psalm/issues/PossiblyUnusedMethod.md index f4a973256cd..fcaaeebbee5 100644 --- a/docs/running_psalm/issues/PossiblyUnusedMethod.md +++ b/docs/running_psalm/issues/PossiblyUnusedMethod.md @@ -1,6 +1,10 @@ # PossiblyUnusedMethod -Emitted when `--find-dead-code` is turned on and Psalm cannot find any calls to a public or protected method. +Emitted when `--find-dead-code` is turned on and Psalm cannot find any calls to +a public or protected method. + +If this method is used and part of the public API, annotate the containing class +with `@psalm-api`. ```php tests + + + + diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 4e28bc73e20..4270844759e 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1,5 +1,5 @@ - + $comment_block->tags['variablesfrom'][0] @@ -308,6 +308,13 @@ $type > 4 + + + $capabilities + $processId + $rootPath + + $pair[1] diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php index 048b51c53ae..e40eb680bd1 100644 --- a/src/Psalm/Config.php +++ b/src/Psalm/Config.php @@ -60,6 +60,7 @@ use function class_exists; use function clearstatcache; use function count; +use function defined; use function dirname; use function explode; use function extension_loaded; @@ -69,6 +70,7 @@ use function filetype; use function flock; use function fopen; +use function fwrite; use function get_class; use function get_defined_constants; use function get_defined_functions; @@ -122,6 +124,7 @@ use const PHP_VERSION_ID; use const PSALM_VERSION; use const SCANDIR_SORT_NONE; +use const STDERR; /** * @psalm-suppress PropertyNotSetInConstructor @@ -441,6 +444,8 @@ class Config public $forbidden_functions = []; /** + * TODO: Psalm 6: Update default to be true and remove warning. + * * @var bool */ public $find_unused_code = false; @@ -1090,6 +1095,9 @@ private static function fromXmlAndPaths( $attribute_text = (string) $config_xml['findUnusedCode']; $config->find_unused_code = $attribute_text === 'true' || $attribute_text === '1'; $config->find_unused_variables = $config->find_unused_code; + } elseif (!defined('__IS_TEST_ENV__')) { + fwrite(STDERR, 'Warning: "findUnusedCode" will be defaulted to "true" in Psalm 6. You should explicitly' + . ' enable or disable this setting.' . PHP_EOL); } if (isset($config_xml['findUnusedVariablesAndParams'])) { diff --git a/src/Psalm/DocComment.php b/src/Psalm/DocComment.php index 3adb8312e62..3c4d09ad586 100644 --- a/src/Psalm/DocComment.php +++ b/src/Psalm/DocComment.php @@ -33,6 +33,7 @@ final class DocComment 'taint-unescape', 'self-out', 'consistent-constructor', 'stub-override', 'require-extends', 'require-implements', 'param-out', 'ignore-var', 'consistent-templates', 'if-this-is', 'this-out', 'check-type', 'check-type-exact', + 'api', ]; /** diff --git a/src/Psalm/Internal/Codebase/ClassLikes.php b/src/Psalm/Internal/Codebase/ClassLikes.php index 6e6508d1ffc..08e43ac4c1b 100644 --- a/src/Psalm/Internal/Codebase/ClassLikes.php +++ b/src/Psalm/Internal/Codebase/ClassLikes.php @@ -851,7 +851,12 @@ public function consolidateAnalyzedData(Methods $methods, ?Progress $progress, b && !$classlike_storage->is_trait ) { if ($find_unused_code) { - if (!$this->file_reference_provider->isClassReferenced($fq_class_name_lc)) { + if ($classlike_storage->public_api + || $this->file_reference_provider->isClassReferenced($fq_class_name_lc) + ) { + $this->checkMethodReferences($classlike_storage, $methods); + $this->checkPropertyReferences($classlike_storage); + } else { IssueBuffer::maybeAdd( new UnusedClass( 'Class ' . $classlike_storage->name . ' is never used', @@ -860,10 +865,8 @@ public function consolidateAnalyzedData(Methods $methods, ?Progress $progress, b ), $classlike_storage->suppressed_issues, ); - } else { - $this->checkMethodReferences($classlike_storage, $methods); - $this->checkPropertyReferences($classlike_storage); } + $this->checkMethodParamReferences($classlike_storage); } $this->findPossibleMethodParamTypes($classlike_storage); @@ -1690,6 +1693,16 @@ private function checkMethodReferences(ClassLikeStorage $classlike_storage, Meth $method_id = $declaring_method_id; } + if ($classlike_storage->public_api + && ($method_storage->visibility === ClassLikeAnalyzer::VISIBILITY_PUBLIC + || ($method_storage->visibility === ClassLikeAnalyzer::VISIBILITY_PROTECTED + && !$classlike_storage->final + ) + ) + ) { + continue; + } + if ($method_storage->location && !$project_analyzer->canReportIssues($method_storage->location->file_path) && !$codebase->analyzer->canReportIssues($method_storage->location->file_path) @@ -1715,7 +1728,7 @@ private function checkMethodReferences(ClassLikeStorage $classlike_storage, Meth && $method_name !== '__unserialize' && $method_name !== '__set_state' && $method_name !== '__debuginfo' - && $method_name !== '__tostring' // can be called in array_unique) + && $method_name !== '__tostring' // can be called in array_unique ) { $method_location = $method_storage->location; @@ -1900,36 +1913,68 @@ private function checkMethodReferences(ClassLikeStorage $classlike_storage, Meth } } } + } + } + } - if ($method_storage->visibility !== ClassLikeAnalyzer::VISIBILITY_PRIVATE - && !$classlike_storage->is_interface - ) { - foreach ($method_storage->params as $offset => $param_storage) { - if (empty($classlike_storage->overridden_method_ids[$method_name]) - && $param_storage->location - && !$param_storage->promoted_property - && !$this->file_reference_provider->isMethodParamUsed( - strtolower((string) $method_id), - $offset, - ) - ) { - if ($method_storage->final) { - IssueBuffer::maybeAdd( - new UnusedParam( - 'Param #' . ($offset + 1) . ' is never referenced in this method', - $param_storage->location, - ), - $method_storage->suppressed_issues, - ); - } else { - IssueBuffer::maybeAdd( - new PossiblyUnusedParam( - 'Param #' . ($offset + 1) . ' is never referenced in this method', - $param_storage->location, - ), - $method_storage->suppressed_issues, - ); - } + + private function checkMethodParamReferences(ClassLikeStorage $classlike_storage): void + { + foreach ($classlike_storage->appearing_method_ids as $method_name => $appearing_method_id) { + $appearing_fq_classlike_name = $appearing_method_id->fq_class_name; + + if ($appearing_fq_classlike_name !== $classlike_storage->name) { + continue; + } + + $method_id = $appearing_method_id; + + if (isset($classlike_storage->methods[$method_name])) { + $method_storage = $classlike_storage->methods[$method_name]; + } else { + $declaring_method_id = $classlike_storage->declaring_method_ids[$method_name]; + + $declaring_fq_classlike_name = $declaring_method_id->fq_class_name; + $declaring_method_name = $declaring_method_id->method_name; + + try { + $declaring_classlike_storage = $this->classlike_storage_provider->get($declaring_fq_classlike_name); + } catch (InvalidArgumentException $e) { + continue; + } + + $method_storage = $declaring_classlike_storage->methods[$declaring_method_name]; + $method_id = $declaring_method_id; + } + + if ($method_storage->visibility !== ClassLikeAnalyzer::VISIBILITY_PRIVATE + && !$classlike_storage->is_interface + ) { + foreach ($method_storage->params as $offset => $param_storage) { + if (empty($classlike_storage->overridden_method_ids[$method_name]) + && $param_storage->location + && !$param_storage->promoted_property + && !$this->file_reference_provider->isMethodParamUsed( + strtolower((string) $method_id), + $offset, + ) + ) { + if ($method_storage->final) { + IssueBuffer::maybeAdd( + new UnusedParam( + 'Param #' . ($offset + 1) . ' is never referenced in this method', + $param_storage->location, + ), + $method_storage->suppressed_issues, + ); + } else { + IssueBuffer::maybeAdd( + new PossiblyUnusedParam( + 'Param #' . ($offset + 1) . ' is never referenced in this method', + $param_storage->location, + ), + $method_storage->suppressed_issues, + ); } } } @@ -2064,6 +2109,16 @@ private function checkPropertyReferences(ClassLikeStorage $classlike_storage): v $codebase = $project_analyzer->getCodebase(); foreach ($classlike_storage->properties as $property_name => $property_storage) { + if ($classlike_storage->public_api + && ($property_storage->visibility === ClassLikeAnalyzer::VISIBILITY_PUBLIC + || ($property_storage->visibility === ClassLikeAnalyzer::VISIBILITY_PROTECTED + && !$classlike_storage->final + ) + ) + ) { + continue; + } + $referenced_property_name = strtolower($classlike_storage->name) . '::$' . $property_name; $property_referenced = $this->file_reference_provider->isClassPropertyReferenced( $referenced_property_name, diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeDocblockParser.php b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeDocblockParser.php index 4f9deaaceed..59805870186 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeDocblockParser.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeDocblockParser.php @@ -474,6 +474,8 @@ public static function parse( $info->description = $parsed_docblock->description; } + $info->public_api = isset($parsed_docblock->tags['psalm-api']) || isset($parsed_docblock->tags['api']); + self::addMagicPropertyToInfo($comment, $info, $parsed_docblock->tags, 'property'); self::addMagicPropertyToInfo($comment, $info, $parsed_docblock->tags, 'psalm-property'); self::addMagicPropertyToInfo($comment, $info, $parsed_docblock->tags, 'property-read'); diff --git a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php index eeceb5057b5..52821adfb91 100644 --- a/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php +++ b/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php @@ -681,6 +681,8 @@ public function start(PhpParser\Node\Stmt\ClassLike $node): ?bool if ($docblock_info->description) { $storage->description = $docblock_info->description; } + + $storage->public_api = $docblock_info->public_api; } foreach ($node->stmts as $node_stmt) { diff --git a/src/Psalm/Internal/Scanner/ClassLikeDocblockComment.php b/src/Psalm/Internal/Scanner/ClassLikeDocblockComment.php index 0b865ba01b9..fc0a83c91d5 100644 --- a/src/Psalm/Internal/Scanner/ClassLikeDocblockComment.php +++ b/src/Psalm/Internal/Scanner/ClassLikeDocblockComment.php @@ -101,4 +101,6 @@ class ClassLikeDocblockComment public array $implementation_requirements = []; public ?string $description = null; + + public bool $public_api = false; } diff --git a/src/Psalm/Storage/ClassLikeStorage.php b/src/Psalm/Storage/ClassLikeStorage.php index aaacfc67555..6d6d6f1b5cd 100644 --- a/src/Psalm/Storage/ClassLikeStorage.php +++ b/src/Psalm/Storage/ClassLikeStorage.php @@ -462,6 +462,8 @@ final class ClassLikeStorage implements HasAttributesInterface */ public $description; + public bool $public_api = false; + public function __construct(string $name) { $this->name = $name; diff --git a/tests/UnusedCodeTest.php b/tests/UnusedCodeTest.php index 28ab6edf88c..68b3bd194e7 100644 --- a/tests/UnusedCodeTest.php +++ b/tests/UnusedCodeTest.php @@ -1229,6 +1229,33 @@ class A { $a->bar[$a->foo] = "bar"; print_r($a->bar);', ], + 'psalm-api with unused class' => [ + 'code' => <<<'PHP' + [ + 'code' => <<<'PHP' + [ + 'code' => <<<'PHP' + 'InaccessibleProperty', ], + 'psalm-api with unused private property' => [ + 'code' => <<<'PHP' + 'UnusedProperty', + ], + 'psalm-api with final class and unused protected property' => [ + 'code' => <<<'PHP' + 'PossiblyUnusedProperty', + ], + 'psalm-api with unused private method' => [ + 'code' => <<<'PHP' + 'UnusedMethod', + ], + 'psalm-api with final class and unused protected method' => [ + 'code' => <<<'PHP' + 'PossiblyUnusedMethod', + ], + 'psalm-api with unused class and unused param' => [ + 'code' => <<<'PHP' + 'PossiblyUnusedParam', + ], + 'unused param' => [ + 'code' => <<<'PHP' + 'PossiblyUnusedParam', + ], ]; } }