From 58860536dcc60bfc86c29ad67711ed1b19046cb0 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 10 Feb 2021 13:39:46 +0100 Subject: [PATCH] Bleeding edge - detect wrong usage of `@var` PHPDoc tag --- conf/bleedingEdge.neon | 1 + conf/config.level2.neon | 7 +++- conf/config.neon | 4 +- .../PhpDoc/WrongVariableNameInVarTagRule.php | 39 +++++++++++++++++-- .../WrongVariableNameInVarTagRuleTest.php | 15 ++++++- .../PhpDoc/data/wrong-variable-name-var.php | 20 ++++++++++ 6 files changed, 80 insertions(+), 6 deletions(-) diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index ccd0874ee4..ebc00780c4 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -12,3 +12,4 @@ parameters: checkLogicalAndConstantCondition: true checkLogicalOrConstantCondition: true checkMissingTemplateTypeInParameter: true + wrongVarUsage: true diff --git a/conf/config.level2.neon b/conf/config.level2.neon index 9ff871b3d4..40f4384efc 100644 --- a/conf/config.level2.neon +++ b/conf/config.level2.neon @@ -30,7 +30,6 @@ rules: - PHPStan\Rules\PhpDoc\InvalidPhpDocTagValueRule - PHPStan\Rules\PhpDoc\InvalidPHPStanDocTagRule - PHPStan\Rules\PhpDoc\InvalidThrowsPhpDocValueRule - - PHPStan\Rules\PhpDoc\WrongVariableNameInVarTagRule services: - @@ -52,3 +51,9 @@ services: checkMissingVarTagTypehint: %checkMissingVarTagTypehint% tags: - phpstan.rules.rule + - + class: PHPStan\Rules\PhpDoc\WrongVariableNameInVarTagRule + arguments: + checkWrongVarUsage: %featureToggles.wrongVarUsage% + tags: + - phpstan.rules.rule diff --git a/conf/config.neon b/conf/config.neon index 31a55ca063..232395d884 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -25,6 +25,7 @@ parameters: checkLogicalAndConstantCondition: false checkLogicalOrConstantCondition: false checkMissingTemplateTypeInParameter: false + wrongVarUsage: false fileExtensions: - php checkAlwaysTrueCheckTypeFunctionCall: false @@ -176,7 +177,8 @@ parametersSchema: detectDuplicateStubFiles: bool(), checkLogicalAndConstantCondition: bool(), checkLogicalOrConstantCondition: bool(), - checkMissingTemplateTypeInParameter: bool() + checkMissingTemplateTypeInParameter: bool(), + wrongVarUsage: bool() ]) fileExtensions: listOf(string()) checkAlwaysTrueCheckTypeFunctionCall: bool() diff --git a/src/Rules/PhpDoc/WrongVariableNameInVarTagRule.php b/src/Rules/PhpDoc/WrongVariableNameInVarTagRule.php index fe6d0f5cf7..18c95f0bc4 100644 --- a/src/Rules/PhpDoc/WrongVariableNameInVarTagRule.php +++ b/src/Rules/PhpDoc/WrongVariableNameInVarTagRule.php @@ -6,7 +6,10 @@ use PhpParser\Node; use PhpParser\Node\Expr; use PHPStan\Analyser\Scope; -use PHPStan\Node\UnreachableStatementNode; +use PHPStan\Node\InClassMethodNode; +use PHPStan\Node\InClassNode; +use PHPStan\Node\InFunctionNode; +use PHPStan\Node\VirtualNode; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Type\FileTypeMapper; @@ -19,9 +22,15 @@ class WrongVariableNameInVarTagRule implements Rule private FileTypeMapper $fileTypeMapper; - public function __construct(FileTypeMapper $fileTypeMapper) + private bool $checkWrongVarUsage; + + public function __construct( + FileTypeMapper $fileTypeMapper, + bool $checkWrongVarUsage = false + ) { $this->fileTypeMapper = $fileTypeMapper; + $this->checkWrongVarUsage = $checkWrongVarUsage; } public function getNodeType(): string @@ -36,7 +45,7 @@ public function processNode(Node $node, Scope $scope): array || $node instanceof Node\Stmt\PropertyProperty || $node instanceof Node\Stmt\ClassConst || $node instanceof Node\Stmt\Const_ - || $node instanceof UnreachableStatementNode + || ($node instanceof VirtualNode && !$node instanceof InFunctionNode && !$node instanceof InClassMethodNode && !$node instanceof InClassNode) ) { return []; } @@ -83,6 +92,30 @@ public function processNode(Node $node, Scope $scope): array return $this->processGlobal($scope, $node, $varTags); } + if ($node instanceof InClassNode || $node instanceof InClassMethodNode || $node instanceof InFunctionNode) { + if ($this->checkWrongVarUsage) { + $description = 'a function'; + $originalNode = $node->getOriginalNode(); + if ($originalNode instanceof Node\Stmt\Interface_) { + $description = 'an interface'; + } elseif ($originalNode instanceof Node\Stmt\Class_) { + $description = 'a class'; + } elseif ($originalNode instanceof Node\Stmt\Trait_) { + throw new \PHPStan\ShouldNotHappenException(); + } elseif ($originalNode instanceof Node\Stmt\ClassMethod) { + $description = 'a method'; + } + return [ + RuleErrorBuilder::message(sprintf( + 'PHPDoc tag @var above %s has no effect.', + $description + ))->build(), + ]; + } + + return []; + } + return $this->processStmt($scope, $varTags, null); } diff --git a/tests/PHPStan/Rules/PhpDoc/WrongVariableNameInVarTagRuleTest.php b/tests/PHPStan/Rules/PhpDoc/WrongVariableNameInVarTagRuleTest.php index dea4d453ac..05d47dbc03 100644 --- a/tests/PHPStan/Rules/PhpDoc/WrongVariableNameInVarTagRuleTest.php +++ b/tests/PHPStan/Rules/PhpDoc/WrongVariableNameInVarTagRuleTest.php @@ -15,7 +15,8 @@ class WrongVariableNameInVarTagRuleTest extends RuleTestCase protected function getRule(): Rule { return new WrongVariableNameInVarTagRule( - self::getContainer()->getByType(FileTypeMapper::class) + self::getContainer()->getByType(FileTypeMapper::class), + true ); } @@ -110,6 +111,18 @@ public function testRule(): void 'Variable $slots in PHPDoc tag @var does not match assigned variable $itemSlots.', 280, ], + [ + 'PHPDoc tag @var above a class has no effect.', + 300, + ], + [ + 'PHPDoc tag @var above a method has no effect.', + 304, + ], + [ + 'PHPDoc tag @var above a function has no effect.', + 312, + ], ]); } diff --git a/tests/PHPStan/Rules/PhpDoc/data/wrong-variable-name-var.php b/tests/PHPStan/Rules/PhpDoc/data/wrong-variable-name-var.php index 4ece594f48..ceffc3b89c 100644 --- a/tests/PHPStan/Rules/PhpDoc/data/wrong-variable-name-var.php +++ b/tests/PHPStan/Rules/PhpDoc/data/wrong-variable-name-var.php @@ -293,3 +293,23 @@ public function doSit(): void } } + +/** + * @var string + */ +class VarInWrongPlaces +{ + + /** @var int $a */ + public function doFoo($a) + { + + } + +} + +/** @var int */ +function doFoo(): void +{ + +}