diff --git a/src/Rules/DeadCode/CallToConstructorStatementWithoutImpurePointsRule.php b/src/Rules/DeadCode/CallToConstructorStatementWithoutImpurePointsRule.php index 71673e8e43b..d2ecd7ae61c 100644 --- a/src/Rules/DeadCode/CallToConstructorStatementWithoutImpurePointsRule.php +++ b/src/Rules/DeadCode/CallToConstructorStatementWithoutImpurePointsRule.php @@ -33,13 +33,13 @@ public function processNode(Node $node, Scope $scope): array $errors = []; foreach ($node->get(PossiblyPureNewCollector::class) as $filePath => $data) { - foreach ($data as [$class, $line]) { + foreach ($data as [$class, $line, $hasEmptyConstructorBody]) { $lowerClass = strtolower($class); - if (!array_key_exists($lowerClass, $classesWithConstructors)) { + if (!$hasEmptyConstructorBody && !array_key_exists($lowerClass, $classesWithConstructors)) { continue; } - $originalClassName = $classesWithConstructors[$lowerClass]; + $originalClassName = $classesWithConstructors[$lowerClass] ?? $class; $errors[] = RuleErrorBuilder::message(sprintf( 'Call to new %s() on a separate line has no effect.', $originalClassName, diff --git a/src/Rules/DeadCode/CallToFunctionStatementWithoutImpurePointsRule.php b/src/Rules/DeadCode/CallToFunctionStatementWithoutImpurePointsRule.php index 06d710a83a7..149cf8201cf 100644 --- a/src/Rules/DeadCode/CallToFunctionStatementWithoutImpurePointsRule.php +++ b/src/Rules/DeadCode/CallToFunctionStatementWithoutImpurePointsRule.php @@ -33,13 +33,13 @@ public function processNode(Node $node, Scope $scope): array $errors = []; foreach ($node->get(PossiblyPureFuncCallCollector::class) as $filePath => $data) { - foreach ($data as [$func, $line]) { + foreach ($data as [$func, $line, $hasEmptyBody]) { $lowerFunc = strtolower($func); - if (!array_key_exists($lowerFunc, $functions)) { + if (!$hasEmptyBody && !array_key_exists($lowerFunc, $functions)) { continue; } - $originalFunctionName = $functions[$lowerFunc]; + $originalFunctionName = $functions[$lowerFunc] ?? $func; $errors[] = RuleErrorBuilder::message(sprintf( 'Call to function %s() on a separate line has no effect.', $originalFunctionName, diff --git a/src/Rules/DeadCode/CallToStaticMethodStatementWithoutImpurePointsRule.php b/src/Rules/DeadCode/CallToStaticMethodStatementWithoutImpurePointsRule.php index f8938547b5a..7c8953c0e9c 100644 --- a/src/Rules/DeadCode/CallToStaticMethodStatementWithoutImpurePointsRule.php +++ b/src/Rules/DeadCode/CallToStaticMethodStatementWithoutImpurePointsRule.php @@ -36,20 +36,23 @@ public function processNode(Node $node, Scope $scope): array $errors = []; foreach ($node->get(PossiblyPureStaticCallCollector::class) as $filePath => $data) { - foreach ($data as [$className, $method, $line]) { + foreach ($data as [$className, $method, $line, $classDisplayName, $hasEmptyBody]) { $lowerClassName = strtolower($className); + $lowerMethod = strtolower($method); - if (!array_key_exists($lowerClassName, $methods)) { - continue; - } + if ( + !array_key_exists($lowerClassName, $methods) + || !array_key_exists($lowerMethod, $methods[$lowerClassName]) + ) { + if (!$hasEmptyBody) { + continue; + } - $lowerMethod = strtolower($method); - if (!array_key_exists($lowerMethod, $methods[$lowerClassName])) { - continue; + $originalMethodName = $classDisplayName . '::' . $method; + } else { + $originalMethodName = $methods[$lowerClassName][$lowerMethod]; } - $originalMethodName = $methods[$lowerClassName][$lowerMethod]; - $errors[] = RuleErrorBuilder::message(sprintf( 'Call to %s() on a separate line has no effect.', $originalMethodName, diff --git a/src/Rules/DeadCode/EmptyBodyCallableDetector.php b/src/Rules/DeadCode/EmptyBodyCallableDetector.php new file mode 100644 index 00000000000..8e40d373487 --- /dev/null +++ b/src/Rules/DeadCode/EmptyBodyCallableDetector.php @@ -0,0 +1,207 @@ + */ + private array $parsedFiles = []; + + public function __construct( + #[AutowiredParameter(ref: '@currentPhpVersionRichParser')] + private Parser $parser, + ) + { + } + + public function hasEmptyFunctionBody(?string $fileName, string $functionName): bool + { + $nodes = $this->parseFile($fileName); + if ($nodes === null) { + return false; + } + + $functionNode = $this->findFunctionNode($functionName, $nodes); + if ($functionNode === null) { + return false; + } + + return $this->hasEmptyBody($functionNode); + } + + public function hasEmptyMethodBody(?string $fileName, string $className, string $methodName): bool + { + $nodes = $this->parseFile($fileName); + if ($nodes === null) { + return false; + } + + $classNode = $this->findClassNode($className, $nodes); + if ($classNode === null) { + return false; + } + + $methodNode = $this->findMethodNode($methodName, $classNode->stmts); + if ($methodNode === null) { + return false; + } + + return $this->hasEmptyBody($methodNode); + } + + /** + * @return Node[]|null + */ + private function parseFile(?string $fileName): ?array + { + if ($fileName === null) { + return null; + } + + if (array_key_exists($fileName, $this->parsedFiles)) { + return $this->parsedFiles[$fileName]; + } + + try { + return $this->parsedFiles[$fileName] = $this->parser->parseFile($fileName); + } catch (Throwable) { + return $this->parsedFiles[$fileName] = null; + } + } + + private function hasEmptyBody(ClassMethod|Function_ $node): bool + { + if ($node->stmts === null || count($node->stmts) !== 0) { + return false; + } + + foreach ($node->params as $param) { + // promoted properties assign to $this, by-reference params create new variables + if ($param->flags !== 0 || $param->byRef) { + return false; + } + } + + return true; + } + + /** + * @param Node[] $nodes + */ + private function findFunctionNode(string $functionName, array $nodes): ?Function_ + { + foreach ($nodes as $node) { + if ( + $node instanceof Function_ + && $node->namespacedName !== null + && strtolower($node->namespacedName->toString()) === strtolower($functionName) + ) { + return $node; + } + if ( + !$node instanceof Namespace_ + && !$node instanceof Declare_ + ) { + continue; + } + $result = $this->findFunctionNode($functionName, $this->getChildStatements($node)); + if ($result !== null) { + return $result; + } + } + return null; + } + + /** + * @param Node[] $nodes + */ + private function findClassNode(string $className, array $nodes): ?Class_ + { + foreach ($nodes as $node) { + if ( + $node instanceof Class_ + && $node->namespacedName !== null + && $node->namespacedName->toString() === $className + ) { + return $node; + } + if ( + !$node instanceof Namespace_ + && !$node instanceof Declare_ + ) { + continue; + } + $result = $this->findClassNode($className, $this->getChildStatements($node)); + if ($result !== null) { + return $result; + } + } + return null; + } + + /** + * @param Node\Stmt[] $classStatements + */ + private function findMethodNode(string $methodName, array $classStatements): ?ClassMethod + { + foreach ($classStatements as $statement) { + if ( + $statement instanceof ClassMethod + && strtolower($statement->name->toString()) === strtolower($methodName) + ) { + return $statement; + } + } + return null; + } + + /** + * @return Node[] + */ + private function getChildStatements(Namespace_|Declare_ $node): array + { + $statements = []; + foreach ($node->getSubNodeNames() as $subNodeName) { + $subNode = $node->{$subNodeName}; + if (!is_array($subNode)) { + $subNode = [$subNode]; + } + foreach ($subNode as $childNode) { + if (!$childNode instanceof Node) { + continue; + } + $statements[] = $childNode; + } + } + return $statements; + } + +} diff --git a/src/Rules/DeadCode/PossiblyPureFuncCallCollector.php b/src/Rules/DeadCode/PossiblyPureFuncCallCollector.php index 237b09cb643..6578a5598aa 100644 --- a/src/Rules/DeadCode/PossiblyPureFuncCallCollector.php +++ b/src/Rules/DeadCode/PossiblyPureFuncCallCollector.php @@ -8,15 +8,19 @@ use PHPStan\Collectors\Collector; use PHPStan\DependencyInjection\RegisteredCollector; use PHPStan\Reflection\ReflectionProvider; +use function count; /** - * @implements Collector + * @implements Collector */ #[RegisteredCollector(level: 4)] final class PossiblyPureFuncCallCollector implements Collector { - public function __construct(private ReflectionProvider $reflectionProvider) + public function __construct( + private ReflectionProvider $reflectionProvider, + private EmptyBodyCallableDetector $emptyBodyCallableDetector, + ) { } @@ -61,7 +65,11 @@ public function processNode(Node $node, Scope $scope) return null; } - return [$functionReflection->getName(), $node->getStartLine()]; + $hasEmptyBody = !$functionReflection->isBuiltin() + && count($functionReflection->getAsserts()->getAll()) === 0 + && $this->emptyBodyCallableDetector->hasEmptyFunctionBody($functionReflection->getFileName(), $functionReflection->getName()); + + return [$functionReflection->getName(), $node->getStartLine(), $hasEmptyBody]; } } diff --git a/src/Rules/DeadCode/PossiblyPureNewCollector.php b/src/Rules/DeadCode/PossiblyPureNewCollector.php index 54d8e1da7a1..250c6b6bc22 100644 --- a/src/Rules/DeadCode/PossiblyPureNewCollector.php +++ b/src/Rules/DeadCode/PossiblyPureNewCollector.php @@ -7,17 +7,22 @@ use PHPStan\Analyser\Scope; use PHPStan\Collectors\Collector; use PHPStan\DependencyInjection\RegisteredCollector; +use PHPStan\Reflection\ExtendedMethodReflection; use PHPStan\Reflection\ReflectionProvider; +use function count; use function strtolower; /** - * @implements Collector + * @implements Collector */ #[RegisteredCollector(level: 4)] final class PossiblyPureNewCollector implements Collector { - public function __construct(private ReflectionProvider $reflectionProvider) + public function __construct( + private ReflectionProvider $reflectionProvider, + private EmptyBodyCallableDetector $emptyBodyCallableDetector, + ) { } @@ -56,7 +61,30 @@ public function processNode(Node $node, Scope $scope) return null; } - return [$constructor->getDeclaringClass()->getName(), $node->getStartLine()]; + return [ + $constructor->getDeclaringClass()->getName(), + $node->getStartLine(), + $this->hasEmptyConstructorBody($constructor), + ]; + } + + private function hasEmptyConstructorBody(ExtendedMethodReflection $constructor): bool + { + if (count($constructor->getAsserts()->getAll()) !== 0) { + return false; + } + + $declaringClass = $constructor->getDeclaringClass(); + // built-in classes are reflected from stubs with empty bodies that don't reflect reality + if ($declaringClass->isBuiltin()) { + return false; + } + + return $this->emptyBodyCallableDetector->hasEmptyMethodBody( + $declaringClass->getFileName(), + $declaringClass->getName(), + $constructor->getName(), + ); } } diff --git a/src/Rules/DeadCode/PossiblyPureStaticCallCollector.php b/src/Rules/DeadCode/PossiblyPureStaticCallCollector.php index 782bf82cfdb..5db901cd775 100644 --- a/src/Rules/DeadCode/PossiblyPureStaticCallCollector.php +++ b/src/Rules/DeadCode/PossiblyPureStaticCallCollector.php @@ -7,15 +7,19 @@ use PHPStan\Analyser\Scope; use PHPStan\Collectors\Collector; use PHPStan\DependencyInjection\RegisteredCollector; +use function count; +use function strtolower; /** - * @implements Collector + * @implements Collector */ #[RegisteredCollector(level: 4)] final class PossiblyPureStaticCallCollector implements Collector { - public function __construct() + public function __construct( + private EmptyBodyCallableDetector $emptyBodyCallableDetector, + ) { } @@ -63,7 +67,28 @@ public function processNode(Node $node, Scope $scope) return null; } - return [$methodReflection->getDeclaringClass()->getName(), $methodReflection->getName(), $node->getStartLine()]; + $declaringClass = $methodReflection->getDeclaringClass(); + + // late static binding (static::) is polymorphic, the runtime method might be overridden; + // constructors (parent::__construct()) are excluded just like in MethodWithoutImpurePointsCollector; + // built-in classes are reflected from stubs with empty bodies that don't reflect reality + $hasEmptyBody = strtolower($expr->class->toString()) !== 'static' + && strtolower($methodReflection->getName()) !== '__construct' + && !$declaringClass->isBuiltin() + && count($methodReflection->getAsserts()->getAll()) === 0 + && $this->emptyBodyCallableDetector->hasEmptyMethodBody( + $declaringClass->getFileName(), + $declaringClass->getName(), + $methodReflection->getName(), + ); + + return [ + $declaringClass->getName(), + $methodReflection->getName(), + $node->getStartLine(), + $declaringClass->getDisplayName(), + $hasEmptyBody, + ]; } } diff --git a/tests/PHPStan/Rules/DeadCode/CallToConstructorStatementWithoutImpurePointsRuleTest.php b/tests/PHPStan/Rules/DeadCode/CallToConstructorStatementWithoutImpurePointsRuleTest.php index c4dd2583f3d..9eb7523b705 100644 --- a/tests/PHPStan/Rules/DeadCode/CallToConstructorStatementWithoutImpurePointsRuleTest.php +++ b/tests/PHPStan/Rules/DeadCode/CallToConstructorStatementWithoutImpurePointsRuleTest.php @@ -26,10 +26,23 @@ public function testRule(): void ]); } + public function testBug14757(): void + { + $this->analyse([__DIR__ . '/data/bug-14757.php'], [ + [ + 'Call to new PHPStan\Type\StringType() on a separate line has no effect.', + 8, + ], + ]); + } + protected function getCollectors(): array { return [ - new PossiblyPureNewCollector(self::createReflectionProvider()), + new PossiblyPureNewCollector( + self::createReflectionProvider(), + self::getContainer()->getByType(EmptyBodyCallableDetector::class), + ), new ConstructorWithoutImpurePointsCollector(), ]; } diff --git a/tests/PHPStan/Rules/DeadCode/CallToFunctionStatementWithoutImpurePointsRuleTest.php b/tests/PHPStan/Rules/DeadCode/CallToFunctionStatementWithoutImpurePointsRuleTest.php index 41c7e9915c3..0b9e460dae9 100644 --- a/tests/PHPStan/Rules/DeadCode/CallToFunctionStatementWithoutImpurePointsRuleTest.php +++ b/tests/PHPStan/Rules/DeadCode/CallToFunctionStatementWithoutImpurePointsRuleTest.php @@ -27,6 +27,17 @@ public function testRule(): void ]); } + public function testBug14757(): void + { + require_once __DIR__ . '/data/bug-14757-function-definition.php'; + $this->analyse([__DIR__ . '/data/bug-14757-function-call.php'], [ + [ + 'Call to function Bug14757Func\emptyFunc() on a separate line has no effect.', + 6, + ], + ]); + } + #[RequiresPhp('>= 8.5.0')] public function testPipeOperator(): void { @@ -45,7 +56,10 @@ public function testPipeOperator(): void protected function getCollectors(): array { return [ - new PossiblyPureFuncCallCollector(self::createReflectionProvider()), + new PossiblyPureFuncCallCollector( + self::createReflectionProvider(), + self::getContainer()->getByType(EmptyBodyCallableDetector::class), + ), new FunctionWithoutImpurePointsCollector(), ]; } diff --git a/tests/PHPStan/Rules/DeadCode/CallToStaticMethodStatementWithoutImpurePointsRuleTest.php b/tests/PHPStan/Rules/DeadCode/CallToStaticMethodStatementWithoutImpurePointsRuleTest.php index 4a0ac6b38f9..f3b7626ebcb 100644 --- a/tests/PHPStan/Rules/DeadCode/CallToStaticMethodStatementWithoutImpurePointsRuleTest.php +++ b/tests/PHPStan/Rules/DeadCode/CallToStaticMethodStatementWithoutImpurePointsRuleTest.php @@ -59,6 +59,17 @@ public function testRule(): void ]); } + public function testBug14757(): void + { + require_once __DIR__ . '/data/bug-14757-static-definition.php'; + $this->analyse([__DIR__ . '/data/bug-14757-static-call.php'], [ + [ + 'Call to Bug14757Static\Utils::emptyStatic() on a separate line has no effect.', + 6, + ], + ]); + } + #[RequiresPhp('>= 8.5.0')] public function testPipeOperator(): void { @@ -77,7 +88,9 @@ public function testPipeOperator(): void protected function getCollectors(): array { return [ - new PossiblyPureStaticCallCollector(), + new PossiblyPureStaticCallCollector( + self::getContainer()->getByType(EmptyBodyCallableDetector::class), + ), new MethodWithoutImpurePointsCollector(), ]; } diff --git a/tests/PHPStan/Rules/DeadCode/data/bug-14757-function-call.php b/tests/PHPStan/Rules/DeadCode/data/bug-14757-function-call.php new file mode 100644 index 00000000000..c52996c2241 --- /dev/null +++ b/tests/PHPStan/Rules/DeadCode/data/bug-14757-function-call.php @@ -0,0 +1,7 @@ +