From 5f019cef537087d69bfda9a318f4e9a092f0391b Mon Sep 17 00:00:00 2001 From: Niclas van Eyk Date: Mon, 19 Oct 2020 21:08:18 +0200 Subject: [PATCH] Initial proposal for psalm-require-{extends, implements} (#4361) * initial implementation of psalm-require-extends * Added @psalm-require-implements * Added shortcode for ExtensionRequirementViolation * Docs & cofig entries for @pasalm-require-{implements,extends} * Added requirement violations to issues.md --- config.xsd | 2 + docs/running_psalm/issues.md | 2 + .../issues/ExtensionRequirementViolation.md | 20 +++++ .../ImplementationRequirementViolation.md | 22 +++++ src/Psalm/DocComment.php | 4 +- src/Psalm/Internal/Analyzer/ClassAnalyzer.php | 41 +++++++++ .../Internal/Analyzer/CommentAnalyzer.php | 21 ++++- .../Internal/PhpVisitor/ReflectorVisitor.php | 28 +++++++ .../Scanner/ClassLikeDocblockComment.php | 10 +++ .../Issue/ExtensionRequirementViolation.php | 9 ++ .../ImplementationRequirementViolation.php | 9 ++ src/Psalm/Storage/ClassLikeStorage.php | 10 +++ tests/ExtensionRequirementTest.php | 67 +++++++++++++++ tests/ImplementationRequirementTest.php | 84 +++++++++++++++++++ 14 files changed, 327 insertions(+), 2 deletions(-) create mode 100644 docs/running_psalm/issues/ExtensionRequirementViolation.md create mode 100644 docs/running_psalm/issues/ImplementationRequirementViolation.md create mode 100644 src/Psalm/Issue/ExtensionRequirementViolation.php create mode 100644 src/Psalm/Issue/ImplementationRequirementViolation.php create mode 100644 tests/ExtensionRequirementTest.php create mode 100644 tests/ImplementationRequirementTest.php diff --git a/config.xsd b/config.xsd index 8d54e0ab8..72416f403 100644 --- a/config.xsd +++ b/config.xsd @@ -197,11 +197,13 @@ + + diff --git a/docs/running_psalm/issues.md b/docs/running_psalm/issues.md index 2e1233eda..3b65c49a3 100644 --- a/docs/running_psalm/issues.md +++ b/docs/running_psalm/issues.md @@ -21,11 +21,13 @@ - [DuplicateMethod](issues/DuplicateMethod.md) - [DuplicateParam](issues/DuplicateParam.md) - [EmptyArrayAccess](issues/EmptyArrayAccess.md) + - [ExtensionRequirementViolation](issues/ExtensionRequirementViolation.md) - [FalsableReturnStatement](issues/FalsableReturnStatement.md) - [FalseOperand](issues/FalseOperand.md) - [ForbiddenCode](issues/ForbiddenCode.md) - [ForbiddenEcho](issues/ForbiddenEcho.md) - [IfThisIsMismatch](issues/IfThisIsMismatch.md) + - [ImplementationRequirementViolation](issues/ImplementationRequirementViolation.md) - [ImplementedParamTypeMismatch](issues/ImplementedParamTypeMismatch.md) - [ImplementedReturnTypeMismatch](issues/ImplementedReturnTypeMismatch.md) - [ImplicitToStringCast](issues/ImplicitToStringCast.md) diff --git a/docs/running_psalm/issues/ExtensionRequirementViolation.md b/docs/running_psalm/issues/ExtensionRequirementViolation.md new file mode 100644 index 000000000..0c540e1b9 --- /dev/null +++ b/docs/running_psalm/issues/ExtensionRequirementViolation.md @@ -0,0 +1,20 @@ +# ExtensionRequirementViolation + +Emitted when a using class of a trait does not extend the class specified using `@psalm-require-extends`. + +```php +extension_requirement !== null) { + $extension_requirement = $codebase->classlikes->getUnAliasedName( + $trait_storage->extension_requirement + ); + $extensionRequirementMet = in_array($extension_requirement, $storage->parent_classes); + + if (!$extensionRequirementMet) { + if (IssueBuffer::accepts( + new ExtensionRequirementViolation( + $fq_trait_name . ' requires using class to extend ' . $extension_requirement + . ', but ' . $storage->name . ' does not', + new CodeLocation($previous_trait_analyzer ?: $this, $trait_name) + ), + $storage->suppressed_issues + $this->getSuppressedIssues() + )) { + // fall through + } + } + } + + foreach ($trait_storage->implementation_requirements as $implementation_requirement) { + $implementation_requirement = $codebase->classlikes->getUnAliasedName($implementation_requirement); + $implementationRequirementMet = in_array($implementation_requirement, $storage->class_implements); + + if (!$implementationRequirementMet) { + if (IssueBuffer::accepts( + new ImplementationRequirementViolation( + $fq_trait_name . ' requires using class to implement ' + . $implementation_requirement . ', but ' . $storage->name . ' does not', + new CodeLocation($previous_trait_analyzer ?: $this, $trait_name) + ), + $storage->suppressed_issues + $this->getSuppressedIssues() + )) { + // fall through + } + } + } + if ($storage->mutation_free && !$trait_storage->mutation_free) { if (IssueBuffer::accepts( new MutableDependency( diff --git a/src/Psalm/Internal/Analyzer/CommentAnalyzer.php b/src/Psalm/Internal/Analyzer/CommentAnalyzer.php index 7c9dc60fd..c627a48a4 100644 --- a/src/Psalm/Internal/Analyzer/CommentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/CommentAnalyzer.php @@ -17,7 +17,6 @@ use Psalm\Internal\Type\ParseTreeCreator; use Psalm\Internal\Type\TypeAlias; use Psalm\Internal\Type\TypeParser; use Psalm\Internal\Type\TypeTokenizer; -use Psalm\Type; use function array_unique; use function trim; use function substr_count; @@ -40,6 +39,7 @@ use function explode; use function array_merge; use const PREG_OFFSET_CAPTURE; use function rtrim; +use function array_key_first; /** * @internal @@ -889,6 +889,25 @@ class CommentAnalyzer } } + if (isset($parsed_docblock->tags['psalm-require-extends']) + && count($extension_requirements = $parsed_docblock->tags['psalm-require-extends']) > 0) { + $info->extension_requirement = trim(preg_replace( + '@^[ \t]*\*@m', + '', + $extension_requirements[array_key_first($extension_requirements)] + )); + } + + if (isset($parsed_docblock->tags['psalm-require-implements'])) { + foreach ($parsed_docblock->tags['psalm-require-implements'] as $implementation_requirement) { + $info->implementation_requirements[] = trim(preg_replace( + '@^[ \t]*\*@m', + '', + $implementation_requirement + )); + } + } + if (isset($parsed_docblock->combined_tags['implements'])) { foreach ($parsed_docblock->combined_tags['implements'] as $template_line) { $info->template_implements[] = trim(preg_replace('@^[ \t]*\*@m', '', $template_line)); diff --git a/src/Psalm/Internal/PhpVisitor/ReflectorVisitor.php b/src/Psalm/Internal/PhpVisitor/ReflectorVisitor.php index baf6b81f2..e94fcb3f3 100644 --- a/src/Psalm/Internal/PhpVisitor/ReflectorVisitor.php +++ b/src/Psalm/Internal/PhpVisitor/ReflectorVisitor.php @@ -1277,6 +1277,34 @@ class ReflectorVisitor extends PhpParser\NodeVisitorAbstract implements FileSour } } + if ($docblock_info->extension_requirement !== null) { + $storage->extension_requirement = (string) TypeParser::parseTokens( + TypeTokenizer::getFullyQualifiedTokens( + $docblock_info->extension_requirement, + $this->aliases, + $this->class_template_types, + $this->type_aliases + ), + null, + $this->class_template_types, + $this->type_aliases + ); + } + + foreach ($docblock_info->implementation_requirements as $implementation_requirement) { + $storage->implementation_requirements[] = (string) TypeParser::parseTokens( + TypeTokenizer::getFullyQualifiedTokens( + $implementation_requirement, + $this->aliases, + $this->class_template_types, + $this->type_aliases + ), + null, + $this->class_template_types, + $this->type_aliases + ); + } + $storage->sealed_properties = $docblock_info->sealed_properties; $storage->sealed_methods = $docblock_info->sealed_methods; diff --git a/src/Psalm/Internal/Scanner/ClassLikeDocblockComment.php b/src/Psalm/Internal/Scanner/ClassLikeDocblockComment.php index e8c945052..0df5cd943 100644 --- a/src/Psalm/Internal/Scanner/ClassLikeDocblockComment.php +++ b/src/Psalm/Internal/Scanner/ClassLikeDocblockComment.php @@ -127,4 +127,14 @@ class ClassLikeDocblockComment /** @var bool */ public $stub_override = false; + + /** + * @var null|string + */ + public $extension_requirement; + + /** + * @var array + */ + public $implementation_requirements = []; } diff --git a/src/Psalm/Issue/ExtensionRequirementViolation.php b/src/Psalm/Issue/ExtensionRequirementViolation.php new file mode 100644 index 000000000..4fce9cdd4 --- /dev/null +++ b/src/Psalm/Issue/ExtensionRequirementViolation.php @@ -0,0 +1,9 @@ + + */ + public $implementation_requirements = []; + public function __construct(string $name) { $this->name = $name; diff --git a/tests/ExtensionRequirementTest.php b/tests/ExtensionRequirementTest.php new file mode 100644 index 000000000..235164a27 --- /dev/null +++ b/tests/ExtensionRequirementTest.php @@ -0,0 +1,67 @@ +addFile( + 'base.php', + 'addFile( + 'trait.php', + ' [ + ' [ + ' 'requires using class to extend' + ] + ]; + } +} diff --git a/tests/ImplementationRequirementTest.php b/tests/ImplementationRequirementTest.php new file mode 100644 index 000000000..2984e8a9f --- /dev/null +++ b/tests/ImplementationRequirementTest.php @@ -0,0 +1,84 @@ +addFile( + 'base.php', + 'addFile( + 'trait.php', + ' [ + ' [ + ' 'requires using class to implement' + ], + 'onlyImplementsOneRequirement' => [ + ' 'requires using class to implement' + ] + ]; + } +}