From 2418b2db6ef825c40db3c9e0d9858bdcb4dd3198 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 29 Mar 2022 13:49:37 +0200 Subject: [PATCH] Bleeding edge - use lighter NodeConnectingVisitor --- conf/bleedingEdge.neon | 1 + conf/config.neon | 29 ++++++++ src/Analyser/MutatingScope.php | 43 ++--------- .../ConditionalTagsExtension.php | 2 + src/Parser/ArrayFilterArgVisitor.php | 25 +++++++ src/Parser/ArrayMapArgVisitor.php | 30 ++++++++ src/Parser/NewAssignedToPropertyVisitor.php | 21 ++++++ src/Parser/ParentStmtTypesVisitor.php | 48 ++++++++++++ src/Parser/RichParser.php | 14 ++-- src/Parser/TryCatchTypeVisitor.php | 73 +++++++++++++++++++ src/Reflection/ParametersAcceptorSelector.php | 34 ++------- src/Rules/Comparison/MatchExpressionRule.php | 21 +----- ...ngCheckedExceptionInFunctionThrowsRule.php | 8 +- ...singCheckedExceptionInMethodThrowsRule.php | 8 +- .../MissingCheckedExceptionInThrowsCheck.php | 48 +----------- .../Keywords/ContinueBreakInLoopRule.php | 42 ++++++----- tests/PHPStan/Analyser/AnalyserTest.php | 5 +- .../ParentStmtTypesRule.php} | 14 ++-- .../PHPStan/Node/ParentStmtTypesRuleTest.php | 29 ++++++++ tests/PHPStan/Node/TryCatchTypeRule.php | 40 ++++++++++ tests/PHPStan/Node/TryCatchTypeRuleTest.php | 45 ++++++++++++ .../data/parent-stmt-types.php} | 0 tests/PHPStan/Node/data/try-catch-type.php | 29 ++++++++ .../PHPStan/Rules/NodeConnectingRuleTest.php | 28 ------- 24 files changed, 434 insertions(+), 203 deletions(-) create mode 100644 src/Parser/ArrayFilterArgVisitor.php create mode 100644 src/Parser/ArrayMapArgVisitor.php create mode 100644 src/Parser/NewAssignedToPropertyVisitor.php create mode 100644 src/Parser/ParentStmtTypesVisitor.php create mode 100644 src/Parser/TryCatchTypeVisitor.php rename tests/PHPStan/{Rules/NodeConnectingRule.php => Node/ParentStmtTypesRule.php} (55%) create mode 100644 tests/PHPStan/Node/ParentStmtTypesRuleTest.php create mode 100644 tests/PHPStan/Node/TryCatchTypeRule.php create mode 100644 tests/PHPStan/Node/TryCatchTypeRuleTest.php rename tests/PHPStan/{Rules/data/node-connecting.php => Node/data/parent-stmt-types.php} (100%) create mode 100644 tests/PHPStan/Node/data/try-catch-type.php delete mode 100644 tests/PHPStan/Rules/NodeConnectingRuleTest.php diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index d0bd30b478..44b135d3e3 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -5,5 +5,6 @@ parameters: explicitMixedInUnknownGenericNew: true arrayFilter: true arrayUnpacking: true + nodeConnectingVisitorCompatibility: false stubFiles: - ../stubs/bleedingEdge/Countable.stub diff --git a/conf/config.neon b/conf/config.neon index c7dd311a25..d3553901e5 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -30,6 +30,7 @@ parameters: explicitMixedInUnknownGenericNew: false arrayFilter: false arrayUnpacking: false + nodeConnectingVisitorCompatibility: true fileExtensions: - php checkAdvancedIsset: false @@ -215,6 +216,7 @@ parametersSchema: explicitMixedInUnknownGenericNew: bool(), arrayFilter: bool(), arrayUnpacking: bool(), + nodeConnectingVisitorCompatibility: bool(), ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() @@ -344,6 +346,8 @@ conditionalTags: phpstan.rules.rule: %exceptions.check.tooWideThrowType% PHPStan\Rules\Exceptions\TooWideMethodThrowTypeRule: phpstan.rules.rule: %exceptions.check.tooWideThrowType% + PhpParser\NodeVisitor\NodeConnectingVisitor: + phpstan.parser.richParserNodeVisitor: %featureToggles.nodeConnectingVisitorCompatibility% services: - @@ -358,6 +362,31 @@ services: options: preserveOriginalNames: true + - + class: PHPStan\Parser\ArrayFilterArgVisitor + tags: + - phpstan.parser.richParserNodeVisitor + + - + class: PHPStan\Parser\ArrayMapArgVisitor + tags: + - phpstan.parser.richParserNodeVisitor + + - + class: PHPStan\Parser\NewAssignedToPropertyVisitor + tags: + - phpstan.parser.richParserNodeVisitor + + - + class: PHPStan\Parser\ParentStmtTypesVisitor + tags: + - phpstan.parser.richParserNodeVisitor + + - + class: PHPStan\Parser\TryCatchTypeVisitor + tags: + - phpstan.parser.richParserNodeVisitor + - class: PhpParser\NodeVisitor\NodeConnectingVisitor diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 309257c31d..1051a96115 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -1642,32 +1642,11 @@ private function resolveType(Expr $node): Type } $callableParameters = null; - $arg = $node->getAttribute('parent'); - if ($arg instanceof Arg) { - $funcCall = $arg->getAttribute('parent'); - $argOrder = $arg->getAttribute('expressionOrder'); - if ($funcCall instanceof FuncCall && $funcCall->name instanceof Name) { - $functionName = $this->reflectionProvider->resolveFunctionName($funcCall->name, $this); - if ( - $functionName === 'array_map' - && $argOrder === 0 - && isset($funcCall->getArgs()[1]) - ) { - if (!isset($funcCall->getArgs()[2])) { - $callableParameters = [ - new DummyParameter('item', $this->getType($funcCall->getArgs()[1]->value)->getIterableValueType(), false, PassedByReference::createNo(), false, null), - ]; - } else { - $callableParameters = []; - foreach ($funcCall->getArgs() as $i => $funcCallArg) { - if ($i === 0) { - continue; - } - - $callableParameters[] = new DummyParameter('item', $this->getType($funcCallArg->value)->getIterableValueType(), false, PassedByReference::createNo(), false, null); - } - } - } + $arrayMapArgs = $node->getAttribute('arrayMapArgs'); + if ($arrayMapArgs !== null) { + $callableParameters = []; + foreach ($arrayMapArgs as $funcCallArg) { + $callableParameters[] = new DummyParameter('item', $this->getType($funcCallArg->value)->getIterableValueType(), false, PassedByReference::createNo(), false, null); } } @@ -5745,14 +5724,8 @@ private function exactInstantiation(New_ $node, string $className): ?Type return $objectType; } - $parentNode = $node->getAttribute('parent'); - if ( - ( - $parentNode instanceof Expr\Assign - || $parentNode instanceof Expr\AssignRef - ) - && $parentNode->var instanceof PropertyFetch - ) { + $assignedToProperty = $node->getAttribute('assignedToProperty'); + if ($assignedToProperty !== null) { $constructorVariant = ParametersAcceptorSelector::selectSingle($constructorMethod->getVariants()); $classTemplateTypes = $classReflection->getTemplateTypeMap()->getTypes(); $originalClassTemplateTypes = $classTemplateTypes; @@ -5771,7 +5744,7 @@ private function exactInstantiation(New_ $node, string $className): ?Type } if (count($classTemplateTypes) === count($originalClassTemplateTypes)) { - $propertyType = $this->getType($parentNode->var); + $propertyType = $this->getType($assignedToProperty); if ($objectType->isSuperTypeOf($propertyType)->yes()) { return $propertyType; } diff --git a/src/DependencyInjection/ConditionalTagsExtension.php b/src/DependencyInjection/ConditionalTagsExtension.php index bd2e50c706..42e547d27c 100644 --- a/src/DependencyInjection/ConditionalTagsExtension.php +++ b/src/DependencyInjection/ConditionalTagsExtension.php @@ -7,6 +7,7 @@ use Nette\Schema\Expect; use PHPStan\Analyser\TypeSpecifierFactory; use PHPStan\Broker\BrokerFactory; +use PHPStan\Parser\RichParser; use PHPStan\PhpDoc\TypeNodeResolverExtension; use PHPStan\Rules\RegistryFactory; use PHPStan\ShouldNotHappenException; @@ -31,6 +32,7 @@ public function getConfigSchema(): Nette\Schema\Schema TypeSpecifierFactory::FUNCTION_TYPE_SPECIFYING_EXTENSION_TAG => $bool, TypeSpecifierFactory::METHOD_TYPE_SPECIFYING_EXTENSION_TAG => $bool, TypeSpecifierFactory::STATIC_METHOD_TYPE_SPECIFYING_EXTENSION_TAG => $bool, + RichParser::VISITOR_SERVICE_TAG => $bool, ])->min(1)); } diff --git a/src/Parser/ArrayFilterArgVisitor.php b/src/Parser/ArrayFilterArgVisitor.php new file mode 100644 index 0000000000..1854bd451a --- /dev/null +++ b/src/Parser/ArrayFilterArgVisitor.php @@ -0,0 +1,25 @@ +name instanceof Node\Name) { + $functionName = $node->name->toLowerString(); + if ($functionName === 'array_filter') { + $args = $node->getArgs(); + if (isset($args[0])) { + $args[0]->setAttribute('isArrayFilterArg', $args); + } + } + } + return null; + } + +} diff --git a/src/Parser/ArrayMapArgVisitor.php b/src/Parser/ArrayMapArgVisitor.php new file mode 100644 index 0000000000..a2d826cd0e --- /dev/null +++ b/src/Parser/ArrayMapArgVisitor.php @@ -0,0 +1,30 @@ +name instanceof Node\Name) { + $functionName = $node->name->toLowerString(); + if ($functionName === 'array_map') { + $args = $node->getArgs(); + if (isset($args[0])) { + $slicedArgs = array_slice($args, 1); + if (count($slicedArgs) > 0) { + $args[0]->value->setAttribute('arrayMapArgs', $slicedArgs); + } + } + } + } + return null; + } + +} diff --git a/src/Parser/NewAssignedToPropertyVisitor.php b/src/Parser/NewAssignedToPropertyVisitor.php new file mode 100644 index 0000000000..b99cd5b1e4 --- /dev/null +++ b/src/Parser/NewAssignedToPropertyVisitor.php @@ -0,0 +1,21 @@ +var instanceof Node\Expr\PropertyFetch && $node->expr instanceof Node\Expr\New_) { + $node->expr->setAttribute('assignedToProperty', $node->var); + } + } + return null; + } + +} diff --git a/src/Parser/ParentStmtTypesVisitor.php b/src/Parser/ParentStmtTypesVisitor.php new file mode 100644 index 0000000000..9f9f55814d --- /dev/null +++ b/src/Parser/ParentStmtTypesVisitor.php @@ -0,0 +1,48 @@ +> */ + private array $typeStack = []; + + public function beforeTraverse(array $nodes): ?array + { + $this->typeStack = []; + return null; + } + + public function enterNode(Node $node): ?Node + { + if (!$node instanceof Node\Stmt && !$node instanceof Node\Expr\Closure) { + return null; + } + + if (count($this->typeStack) > 0) { + $node->setAttribute('parentStmtTypes', $this->typeStack); + } + $this->typeStack[] = get_class($node); + + return null; + } + + public function leaveNode(Node $node): ?Node + { + if (!$node instanceof Node\Stmt && !$node instanceof Node\Expr\Closure) { + return null; + } + + array_pop($this->typeStack); + + return null; + } + +} diff --git a/src/Parser/RichParser.php b/src/Parser/RichParser.php index 51ae991940..483229d8f0 100644 --- a/src/Parser/RichParser.php +++ b/src/Parser/RichParser.php @@ -7,9 +7,8 @@ use PhpParser\Node; use PhpParser\NodeTraverser; use PhpParser\NodeVisitor\NameResolver; -use PhpParser\NodeVisitor\NodeConnectingVisitor; +use PHPStan\DependencyInjection\Container; use PHPStan\File\FileReader; -use PHPStan\NodeVisitor\StatementOrderVisitor; use PHPStan\ShouldNotHappenException; use function is_string; use function strpos; @@ -20,12 +19,13 @@ class RichParser implements Parser { + public const VISITOR_SERVICE_TAG = 'phpstan.parser.richParserNodeVisitor'; + public function __construct( private \PhpParser\Parser $parser, private Lexer $lexer, private NameResolver $nameResolver, - private NodeConnectingVisitor $nodeConnectingVisitor, - private StatementOrderVisitor $statementOrderVisitor, + private Container $container, ) { } @@ -60,8 +60,10 @@ public function parseString(string $sourceCode): array $nodeTraverser = new NodeTraverser(); $nodeTraverser->addVisitor($this->nameResolver); - $nodeTraverser->addVisitor($this->nodeConnectingVisitor); - $nodeTraverser->addVisitor($this->statementOrderVisitor); + + foreach ($this->container->getServicesByTag(self::VISITOR_SERVICE_TAG) as $visitor) { + $nodeTraverser->addVisitor($visitor); + } /** @var array */ $nodes = $nodeTraverser->traverse($nodes); diff --git a/src/Parser/TryCatchTypeVisitor.php b/src/Parser/TryCatchTypeVisitor.php new file mode 100644 index 0000000000..7cc0c1e585 --- /dev/null +++ b/src/Parser/TryCatchTypeVisitor.php @@ -0,0 +1,73 @@ +|null> */ + private array $typeStack = []; + + public function beforeTraverse(array $nodes): ?array + { + $this->typeStack = []; + return null; + } + + public function enterNode(Node $node): ?Node + { + if ($node instanceof Node\Stmt || $node instanceof Node\Expr\Match_) { + if (count($this->typeStack) > 0) { + $node->setAttribute('tryCatchTypes', $this->typeStack[count($this->typeStack) - 1]); + } + } + + if ($node instanceof Node\FunctionLike) { + $this->typeStack[] = null; + } + + if ($node instanceof Node\Stmt\TryCatch) { + $types = []; + foreach (array_reverse($this->typeStack) as $stackTypes) { + if ($stackTypes === null) { + break; + } + + foreach ($stackTypes as $type) { + $types[] = $type; + } + } + foreach ($node->catches as $catch) { + foreach ($catch->types as $type) { + $types[] = $type->toString(); + } + } + + $this->typeStack[] = $types; + } + + return null; + } + + public function leaveNode(Node $node): ?Node + { + if ( + !$node instanceof Node\Stmt\TryCatch + && !$node instanceof Node\FunctionLike + && !$node instanceof Node\Expr\Match_ + ) { + return null; + } + + array_pop($this->typeStack); + + return null; + } + +} diff --git a/src/Reflection/ParametersAcceptorSelector.php b/src/Reflection/ParametersAcceptorSelector.php index cdfa607ec9..10c8218a43 100644 --- a/src/Reflection/ParametersAcceptorSelector.php +++ b/src/Reflection/ParametersAcceptorSelector.php @@ -3,8 +3,6 @@ namespace PHPStan\Reflection; use PhpParser\Node; -use PhpParser\Node\Expr\FuncCall; -use PhpParser\Node\Name; use PHPStan\Analyser\Scope; use PHPStan\Reflection\Native\NativeParameterReflection; use PHPStan\Reflection\Php\DummyParameter; @@ -73,30 +71,13 @@ public static function selectFromArgs( count($args) > 0 && count($parametersAcceptors) > 0 ) { - $functionName = null; - $argParent = $args[0]->getAttribute('parent'); - if ($argParent instanceof FuncCall && $argParent->name instanceof Name) { - $functionName = $argParent->name->toLowerString(); - } - if ( - $functionName === 'array_map' - && isset($args[1]) - ) { + $arrayMapArgs = $args[0]->value->getAttribute('arrayMapArgs'); + if ($arrayMapArgs !== null) { $acceptor = $parametersAcceptors[0]; $parameters = $acceptor->getParameters(); - if (!isset($args[2])) { - $callbackParameters = [ - new DummyParameter('item', $scope->getType($args[1]->value)->getIterableValueType(), false, PassedByReference::createNo(), false, null), - ]; - } else { - $callbackParameters = []; - foreach ($args as $i => $arg) { - if ($i === 0) { - continue; - } - - $callbackParameters[] = new DummyParameter('item', $scope->getType($arg->value)->getIterableValueType(), false, PassedByReference::createNo(), false, null); - } + $callbackParameters = []; + foreach ($arrayMapArgs as $arg) { + $callbackParameters[] = new DummyParameter('item', $scope->getType($arg->value)->getIterableValueType(), false, PassedByReference::createNo(), false, null); } $parameters[0] = new NativeParameterReflection( $parameters[0]->getName(), @@ -120,10 +101,7 @@ public static function selectFromArgs( ]; } - if ( - $functionName === 'array_filter' - && isset($args[0]) - ) { + if (isset($args[0]) && (bool) $args[0]->getAttribute('isArrayFilterArg')) { if (isset($args[2])) { $mode = $scope->getType($args[2]->value); if ($mode instanceof ConstantIntegerType) { diff --git a/src/Rules/Comparison/MatchExpressionRule.php b/src/Rules/Comparison/MatchExpressionRule.php index 5593f84237..d9c230d871 100644 --- a/src/Rules/Comparison/MatchExpressionRule.php +++ b/src/Rules/Comparison/MatchExpressionRule.php @@ -103,27 +103,14 @@ public function processNode(Node $node, Scope $scope): array private function isUnhandledMatchErrorCaught(Node $node): bool { - $tryCatchNode = $node->getAttribute('parent'); - while ( - $tryCatchNode !== null && - !$tryCatchNode instanceof Node\FunctionLike && - !$tryCatchNode instanceof Node\Stmt\TryCatch - ) { - $tryCatchNode = $tryCatchNode->getAttribute('parent'); - } - - if ($tryCatchNode === null || $tryCatchNode instanceof Node\FunctionLike) { + $tryCatchTypes = $node->getAttribute('tryCatchTypes'); + if ($tryCatchTypes === null) { return false; } - foreach ($tryCatchNode->catches as $catch) { - $catchType = TypeCombinator::union(...array_map(static fn (Node\Name $class): ObjectType => new ObjectType($class->toString()), $catch->types)); - if ($catchType->isSuperTypeOf(new ObjectType(UnhandledMatchError::class))->yes()) { - return true; - } - } + $tryCatchType = TypeCombinator::union(...array_map(static fn (string $class) => new ObjectType($class), $tryCatchTypes)); - return $this->isUnhandledMatchErrorCaught($tryCatchNode); + return $tryCatchType->isSuperTypeOf(new ObjectType(UnhandledMatchError::class))->yes(); } private function hasUnhandledMatchErrorThrowsTag(Scope $scope): bool diff --git a/src/Rules/Exceptions/MissingCheckedExceptionInFunctionThrowsRule.php b/src/Rules/Exceptions/MissingCheckedExceptionInFunctionThrowsRule.php index c41710b918..aa97cb9fdd 100644 --- a/src/Rules/Exceptions/MissingCheckedExceptionInFunctionThrowsRule.php +++ b/src/Rules/Exceptions/MissingCheckedExceptionInFunctionThrowsRule.php @@ -35,7 +35,7 @@ public function processNode(Node $node, Scope $scope): array } $errors = []; - foreach ($this->check->check($functionReflection->getThrowType(), $statementResult->getThrowPoints()) as [$className, $throwPointNode, $newCatchPosition]) { + foreach ($this->check->check($functionReflection->getThrowType(), $statementResult->getThrowPoints()) as [$className, $throwPointNode]) { $errors[] = RuleErrorBuilder::message(sprintf( 'Function %s() throws checked exception %s but it\'s missing from the PHPDoc @throws tag.', $functionReflection->getName(), @@ -43,12 +43,6 @@ public function processNode(Node $node, Scope $scope): array )) ->line($throwPointNode->getLine()) ->identifier('exceptions.missingThrowsTag') - ->metadata([ - 'exceptionName' => $className, - 'newCatchPosition' => $newCatchPosition, - 'statementDepth' => $throwPointNode->getAttribute('statementDepth'), - 'statementOrder' => $throwPointNode->getAttribute('statementOrder'), - ]) ->build(); } diff --git a/src/Rules/Exceptions/MissingCheckedExceptionInMethodThrowsRule.php b/src/Rules/Exceptions/MissingCheckedExceptionInMethodThrowsRule.php index be58255d26..40a398e25e 100644 --- a/src/Rules/Exceptions/MissingCheckedExceptionInMethodThrowsRule.php +++ b/src/Rules/Exceptions/MissingCheckedExceptionInMethodThrowsRule.php @@ -35,7 +35,7 @@ public function processNode(Node $node, Scope $scope): array } $errors = []; - foreach ($this->check->check($methodReflection->getThrowType(), $statementResult->getThrowPoints()) as [$className, $throwPointNode, $newCatchPosition]) { + foreach ($this->check->check($methodReflection->getThrowType(), $statementResult->getThrowPoints()) as [$className, $throwPointNode]) { $errors[] = RuleErrorBuilder::message(sprintf( 'Method %s::%s() throws checked exception %s but it\'s missing from the PHPDoc @throws tag.', $methodReflection->getDeclaringClass()->getDisplayName(), @@ -44,12 +44,6 @@ public function processNode(Node $node, Scope $scope): array )) ->line($throwPointNode->getLine()) ->identifier('exceptions.missingThrowsTag') - ->metadata([ - 'exceptionName' => $className, - 'newCatchPosition' => $newCatchPosition, - 'statementDepth' => $throwPointNode->getAttribute('statementDepth'), - 'statementOrder' => $throwPointNode->getAttribute('statementOrder'), - ]) ->build(); } diff --git a/src/Rules/Exceptions/MissingCheckedExceptionInThrowsCheck.php b/src/Rules/Exceptions/MissingCheckedExceptionInThrowsCheck.php index f3dfd9d0fb..254b3d91ec 100644 --- a/src/Rules/Exceptions/MissingCheckedExceptionInThrowsCheck.php +++ b/src/Rules/Exceptions/MissingCheckedExceptionInThrowsCheck.php @@ -7,12 +7,10 @@ use PHPStan\Type\NeverType; use PHPStan\Type\ObjectType; use PHPStan\Type\Type; -use PHPStan\Type\TypeCombinator; use PHPStan\Type\TypeUtils; use PHPStan\Type\TypeWithClassName; use PHPStan\Type\VerbosityLevel; use Throwable; -use function array_map; class MissingCheckedExceptionInThrowsCheck { @@ -23,7 +21,7 @@ public function __construct(private ExceptionTypeResolver $exceptionTypeResolver /** * @param ThrowPoint[] $throwPoints - * @return array + * @return array */ public function check(?Type $throwType, array $throwPoints): array { @@ -52,53 +50,11 @@ public function check(?Type $throwType, array $throwPoints): array continue; } - $classes[] = [$throwPointType->describe(VerbosityLevel::typeOnly()), $throwPoint->getNode(), $this->getNewCatchPosition($throwPointType, $throwPoint->getNode())]; + $classes[] = [$throwPointType->describe(VerbosityLevel::typeOnly()), $throwPoint->getNode()]; } } return $classes; } - private function getNewCatchPosition(Type $throwPointType, Node $throwPointNode): ?int - { - if ($throwPointType instanceof TypeWithClassName) { - // to get rid of type subtraction - $throwPointType = new ObjectType($throwPointType->getClassName()); - } - $tryCatch = $this->findTryCatch($throwPointNode); - if ($tryCatch === null) { - return null; - } - - $position = 0; - foreach ($tryCatch->catches as $catch) { - $type = TypeCombinator::union(...array_map(static fn (Node\Name $class): ObjectType => new ObjectType($class->toString()), $catch->types)); - if (!$throwPointType->isSuperTypeOf($type)->yes()) { - continue; - } - - $position++; - } - - return $position; - } - - private function findTryCatch(Node $node): ?Node\Stmt\TryCatch - { - if ($node instanceof Node\FunctionLike) { - return null; - } - - if ($node instanceof Node\Stmt\TryCatch) { - return $node; - } - - $parent = $node->getAttribute('parent'); - if ($parent === null) { - return null; - } - - return $this->findTryCatch($parent); - } - } diff --git a/src/Rules/Keywords/ContinueBreakInLoopRule.php b/src/Rules/Keywords/ContinueBreakInLoopRule.php index 0ccfea7527..bf709ebe40 100644 --- a/src/Rules/Keywords/ContinueBreakInLoopRule.php +++ b/src/Rules/Keywords/ContinueBreakInLoopRule.php @@ -7,7 +7,7 @@ use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; -use PHPStan\ShouldNotHappenException; +use function array_reverse; use function sprintf; /** @@ -33,13 +33,15 @@ public function processNode(Node $node, Scope $scope): array $value = $node->num->value; } - $parent = $node->getAttribute('parent'); - while ($value > 0) { + $parentStmtTypes = array_reverse($node->getAttribute('parentStmtTypes')); + foreach ($parentStmtTypes as $parentStmtType) { + if ($parentStmtType === Stmt\Case_::class) { + continue; + } if ( - $parent === null - || $parent instanceof Stmt\Function_ - || $parent instanceof Stmt\ClassMethod - || $parent instanceof Node\Expr\Closure + $parentStmtType === Stmt\Function_::class + || $parentStmtType === Stmt\ClassMethod::class + || $parentStmtType === Node\Expr\Closure::class ) { return [ RuleErrorBuilder::message(sprintf( @@ -49,22 +51,26 @@ public function processNode(Node $node, Scope $scope): array ]; } if ( - $parent instanceof Stmt\For_ - || $parent instanceof Stmt\Foreach_ - || $parent instanceof Stmt\Do_ - || $parent instanceof Stmt\While_ + $parentStmtType === Stmt\For_::class + || $parentStmtType === Stmt\Foreach_::class + || $parentStmtType === Stmt\Do_::class + || $parentStmtType === Stmt\While_::class + || $parentStmtType === Stmt\Switch_::class ) { $value--; } - if ($parent instanceof Stmt\Case_) { - $value--; - $parent = $parent->getAttribute('parent'); - if (!$parent instanceof Stmt\Switch_) { - throw new ShouldNotHappenException(); - } + if ($value === 0) { + break; } + } - $parent = $parent->getAttribute('parent'); + if ($value > 0) { + return [ + RuleErrorBuilder::message(sprintf( + 'Keyword %s used outside of a loop or a switch statement.', + $node instanceof Stmt\Continue_ ? 'continue' : 'break', + ))->nonIgnorable()->build(), + ]; } return []; diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index 55be9d0d26..275a7849b6 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -4,13 +4,11 @@ use PhpParser\Lexer; use PhpParser\NodeVisitor\NameResolver; -use PhpParser\NodeVisitor\NodeConnectingVisitor; use PhpParser\Parser\Php7; use PhpParser\PrettyPrinter\Standard; use PHPStan\Dependency\DependencyResolver; use PHPStan\Dependency\ExportedNodeResolver; use PHPStan\DependencyInjection\Type\DynamicThrowTypeExtensionProvider; -use PHPStan\NodeVisitor\StatementOrderVisitor; use PHPStan\Parser\RichParser; use PHPStan\Php\PhpVersion; use PHPStan\PhpDoc\PhpDocInheritanceResolver; @@ -497,8 +495,7 @@ private function createAnalyser(bool $reportUnmatchedIgnoredErrors): Analyser new Php7($lexer), $lexer, new NameResolver(), - new NodeConnectingVisitor(), - new StatementOrderVisitor(), + self::getContainer(), ), new DependencyResolver($fileHelper, $reflectionProvider, new ExportedNodeResolver($fileTypeMapper, $printer)), $reportUnmatchedIgnoredErrors, diff --git a/tests/PHPStan/Rules/NodeConnectingRule.php b/tests/PHPStan/Node/ParentStmtTypesRule.php similarity index 55% rename from tests/PHPStan/Rules/NodeConnectingRule.php rename to tests/PHPStan/Node/ParentStmtTypesRule.php index 86dd273d89..1483f6b99d 100644 --- a/tests/PHPStan/Rules/NodeConnectingRule.php +++ b/tests/PHPStan/Node/ParentStmtTypesRule.php @@ -1,16 +1,18 @@ */ -class NodeConnectingRule implements Rule +class ParentStmtTypesRule implements Rule { public function getNodeType(): string @@ -22,10 +24,8 @@ public function processNode(Node $node, Scope $scope): array { return [ sprintf( - 'Parent: %s, previous: %s, next: %s', - get_class($node->getAttribute('parent')), - get_class($node->getAttribute('previous')), - get_class($node->getAttribute('next')), + 'Parents: %s', + implode(', ', array_reverse($node->getAttribute('parentStmtTypes'))), ), ]; } diff --git a/tests/PHPStan/Node/ParentStmtTypesRuleTest.php b/tests/PHPStan/Node/ParentStmtTypesRuleTest.php new file mode 100644 index 0000000000..320a26465f --- /dev/null +++ b/tests/PHPStan/Node/ParentStmtTypesRuleTest.php @@ -0,0 +1,29 @@ + + */ +class ParentStmtTypesRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new ParentStmtTypesRule(); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/parent-stmt-types.php'], [ + [ + 'Parents: PhpParser\Node\Stmt\If_, PhpParser\Node\Stmt\Function_, PhpParser\Node\Stmt\Namespace_', + 11, + ], + ]); + } + +} diff --git a/tests/PHPStan/Node/TryCatchTypeRule.php b/tests/PHPStan/Node/TryCatchTypeRule.php new file mode 100644 index 0000000000..b356792a60 --- /dev/null +++ b/tests/PHPStan/Node/TryCatchTypeRule.php @@ -0,0 +1,40 @@ + + */ +class TryCatchTypeRule implements Rule +{ + + public function getNodeType(): string + { + return Node\Stmt\Echo_::class; + } + + public function processNode(Node $node, Scope $scope): array + { + $tryCatchTypes = $node->getAttribute('tryCatchTypes'); + $type = null; + if ($tryCatchTypes !== null) { + $type = TypeCombinator::union(...array_map(static fn (string $name) => new ObjectType($name), $tryCatchTypes)); + } + return [ + sprintf( + 'Try catch type: %s', + $type !== null ? $type->describe(VerbosityLevel::precise()) : 'nothing', + ), + ]; + } + +} diff --git a/tests/PHPStan/Node/TryCatchTypeRuleTest.php b/tests/PHPStan/Node/TryCatchTypeRuleTest.php new file mode 100644 index 0000000000..e1e649fc1d --- /dev/null +++ b/tests/PHPStan/Node/TryCatchTypeRuleTest.php @@ -0,0 +1,45 @@ + + */ +class TryCatchTypeRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + return new TryCatchTypeRule(); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/try-catch-type.php'], [ + [ + 'Try catch type: nothing', + 10, + ], + [ + 'Try catch type: LogicException|RuntimeException', + 12, + ], + [ + 'Try catch type: nothing', + 14, + ], + [ + 'Try catch type: LogicException|RuntimeException|TypeError', + 17, + ], + [ + 'Try catch type: LogicException|RuntimeException', + 21, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/data/node-connecting.php b/tests/PHPStan/Node/data/parent-stmt-types.php similarity index 100% rename from tests/PHPStan/Rules/data/node-connecting.php rename to tests/PHPStan/Node/data/parent-stmt-types.php diff --git a/tests/PHPStan/Node/data/try-catch-type.php b/tests/PHPStan/Node/data/try-catch-type.php new file mode 100644 index 0000000000..150cdc95f7 --- /dev/null +++ b/tests/PHPStan/Node/data/try-catch-type.php @@ -0,0 +1,29 @@ + - */ -class NodeConnectingRuleTest extends RuleTestCase -{ - - protected function getRule(): Rule - { - return new NodeConnectingRule(); - } - - public function testRule(): void - { - $this->analyse([__DIR__ . '/data/node-connecting.php'], [ - [ - 'Parent: PhpParser\Node\Stmt\If_, previous: PhpParser\Node\Stmt\Switch_, next: PhpParser\Node\Stmt\Foreach_', - 11, - ], - ]); - } - -}