Skip to content

Commit

Permalink
Too wide @param-out type - consider all execution ends at once
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Mar 13, 2024
1 parent 3cc1a54 commit d1bcf78
Show file tree
Hide file tree
Showing 10 changed files with 345 additions and 65 deletions.
9 changes: 7 additions & 2 deletions conf/config.level4.neon
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ conditionalTags:
phpstan.rules.rule: %featureToggles.notAnalysedTrait%
PHPStan\Rules\Comparison\LogicalXorConstantConditionRule:
phpstan.rules.rule: %featureToggles.logicalXor%
PHPStan\Rules\TooWideTypehints\TooWideParameterOutTypeRule:
PHPStan\Rules\TooWideTypehints\TooWideFunctionParameterOutTypeRule:
phpstan.rules.rule: %featureToggles.paramOutType%
PHPStan\Rules\TooWideTypehints\TooWideMethodParameterOutTypeRule:
phpstan.rules.rule: %featureToggles.paramOutType%

parameters:
Expand Down Expand Up @@ -241,4 +243,7 @@ services:
- phpstan.rules.rule

-
class: PHPStan\Rules\TooWideTypehints\TooWideParameterOutTypeRule
class: PHPStan\Rules\TooWideTypehints\TooWideFunctionParameterOutTypeRule

-
class: PHPStan\Rules\TooWideTypehints\TooWideMethodParameterOutTypeRule
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Node\ExecutionEndNode;
use PHPStan\Reflection\ExtendedMethodReflection;
use PHPStan\Node\FunctionReturnStatementsNode;
use PHPStan\Reflection\FunctionReflection;
use PHPStan\Reflection\ParameterReflectionWithPhpDocs;
use PHPStan\Reflection\ParametersAcceptorSelector;
Expand All @@ -17,24 +16,40 @@
use function sprintf;

/**
* @implements Rule<ExecutionEndNode>
* @implements Rule<FunctionReturnStatementsNode>
*/
class TooWideParameterOutTypeRule implements Rule
class TooWideFunctionParameterOutTypeRule implements Rule
{

public function getNodeType(): string
{
return ExecutionEndNode::class;
return FunctionReturnStatementsNode::class;
}

public function processNode(Node $node, Scope $scope): array
{
$inFunction = $scope->getFunction();
if ($inFunction === null) {
return [];
$inFunction = $node->getFunctionReflection();
$finalScope = null;
foreach ($node->getExecutionEnds() as $executionEnd) {
$endScope = $executionEnd->getStatementResult()->getScope();
if ($finalScope === null) {
$finalScope = $endScope;
continue;
}

$finalScope = $finalScope->mergeWith($endScope);
}

if ($scope->isInAnonymousFunction()) {
foreach ($node->getReturnStatements() as $statement) {
if ($finalScope === null) {
$finalScope = $statement->getScope();
continue;
}

$finalScope = $finalScope->mergeWith($statement->getScope());
}

if ($finalScope === null) {
return [];
}

Expand All @@ -46,7 +61,7 @@ public function processNode(Node $node, Scope $scope): array
continue;
}

foreach ($this->processSingleParameter($scope, $inFunction, $parameter) as $error) {
foreach ($this->processSingleParameter($finalScope, $inFunction, $parameter) as $error) {
$errors[] = $error;
}
}
Expand All @@ -59,7 +74,7 @@ public function processNode(Node $node, Scope $scope): array
*/
private function processSingleParameter(
Scope $scope,
FunctionReflection|ExtendedMethodReflection $inFunction,
FunctionReflection $inFunction,
ParameterReflectionWithPhpDocs $parameter,
): array
{
Expand All @@ -77,21 +92,15 @@ private function processSingleParameter(
$variableExpr = new Node\Expr\Variable($parameter->getName());
$variableType = $scope->getType($variableExpr);

if ($inFunction instanceof ExtendedMethodReflection) {
$functionDescription = sprintf('Method %s::%s()', $inFunction->getDeclaringClass()->getDisplayName(), $inFunction->getName());
} else {
$functionDescription = sprintf('Function %s()', $inFunction->getName());
}

$messages = [];
foreach ($outType->getTypes() as $type) {
if (!$type->isSuperTypeOf($variableType)->no()) {
continue;
}

$errorBuilder = RuleErrorBuilder::message(sprintf(
'%s never assigns %s to &$%s so it can be removed from the %s.',
$functionDescription,
'Function %s() never assigns %s to &$%s so it can be removed from the %s.',
$inFunction->getName(),
$type->describe(VerbosityLevel::getRecommendedLevelByType($type)),
$parameter->getName(),
$isParamOutType ? '@param-out type' : 'by-ref type',
Expand Down
119 changes: 119 additions & 0 deletions src/Rules/TooWideTypehints/TooWideMethodParameterOutTypeRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\TooWideTypehints;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Node\MethodReturnStatementsNode;
use PHPStan\Reflection\ExtendedMethodReflection;
use PHPStan\Reflection\ParameterReflectionWithPhpDocs;
use PHPStan\Reflection\ParametersAcceptorSelector;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\UnionType;
use PHPStan\Type\VerbosityLevel;
use function sprintf;

/**
* @implements Rule<MethodReturnStatementsNode>
*/
class TooWideMethodParameterOutTypeRule implements Rule
{

public function getNodeType(): string
{
return MethodReturnStatementsNode::class;
}

public function processNode(Node $node, Scope $scope): array
{
$inMethod = $node->getMethodReflection();
$finalScope = null;
foreach ($node->getExecutionEnds() as $executionEnd) {
$endScope = $executionEnd->getStatementResult()->getScope();
if ($finalScope === null) {
$finalScope = $endScope;
continue;
}

$finalScope = $finalScope->mergeWith($endScope);
}

foreach ($node->getReturnStatements() as $statement) {
if ($finalScope === null) {
$finalScope = $statement->getScope();
continue;
}

$finalScope = $finalScope->mergeWith($statement->getScope());
}

if ($finalScope === null) {
return [];
}

$variant = ParametersAcceptorSelector::selectSingle($inMethod->getVariants());
$parameters = $variant->getParameters();
$errors = [];
foreach ($parameters as $parameter) {
if (!$parameter->passedByReference()->createsNewVariable()) {
continue;
}

foreach ($this->processSingleParameter($finalScope, $inMethod, $parameter) as $error) {
$errors[] = $error;
}
}

return $errors;
}

/**
* @return array<RuleError>
*/
private function processSingleParameter(
Scope $scope,
ExtendedMethodReflection $inMethod,
ParameterReflectionWithPhpDocs $parameter,
): array
{
$isParamOutType = true;
$outType = $parameter->getOutType();
if ($outType === null) {
$isParamOutType = false;
$outType = $parameter->getType();
}

if (!$outType instanceof UnionType) {
return [];
}

$variableExpr = new Node\Expr\Variable($parameter->getName());
$variableType = $scope->getType($variableExpr);

$messages = [];
foreach ($outType->getTypes() as $type) {
if (!$type->isSuperTypeOf($variableType)->no()) {
continue;
}

$errorBuilder = RuleErrorBuilder::message(sprintf(
'Method %s::%s() never assigns %s to &$%s so it can be removed from the %s.',
$inMethod->getDeclaringClass()->getDisplayName(),
$inMethod->getName(),
$type->describe(VerbosityLevel::getRecommendedLevelByType($type)),
$parameter->getName(),
$isParamOutType ? '@param-out type' : 'by-ref type',
));
if (!$isParamOutType) {
$errorBuilder->tip('You can narrow the parameter out type with @param-out PHPDoc tag.');
}

$messages[] = $errorBuilder->build();
}

return $messages;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\TooWideTypehints;

use PHPStan\Rules\Rule as TRule;
use PHPStan\Testing\RuleTestCase;

/**
* @extends RuleTestCase<TooWideFunctionParameterOutTypeRule>
*/
class TooWideFunctionParameterOutTypeRuleTest extends RuleTestCase
{

protected function getRule(): TRule
{
return new TooWideFunctionParameterOutTypeRule();
}

public function testRule(): void
{
$this->analyse([__DIR__ . '/data/too-wide-function-parameter-out.php'], [
[
'Function TooWideFunctionParameterOut\doBar() never assigns null to &$p so it can be removed from the by-ref type.',
10,
'You can narrow the parameter out type with @param-out PHPDoc tag.',
],
[
'Function TooWideFunctionParameterOut\doBaz() never assigns null to &$p so it can be removed from the @param-out type.',
18,
],
[
'Function TooWideFunctionParameterOut\doLorem() never assigns null to &$p so it can be removed from the by-ref type.',
23,
'You can narrow the parameter out type with @param-out PHPDoc tag.',
],
]);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\TooWideTypehints;

use PHPStan\Rules\Rule as TRule;
use PHPStan\Testing\RuleTestCase;

/**
* @extends RuleTestCase<TooWideMethodParameterOutTypeRule>
*/
class TooWideMethodParameterOutTypeRuleTest extends RuleTestCase
{

protected function getRule(): TRule
{
return new TooWideMethodParameterOutTypeRule();
}

public function testRule(): void
{
$this->analyse([__DIR__ . '/data/too-wide-method-parameter-out.php'], [
[
'Method TooWideMethodParameterOut\Foo::doBar() never assigns null to &$p so it can be removed from the by-ref type.',
13,
'You can narrow the parameter out type with @param-out PHPDoc tag.',
],
[
'Method TooWideMethodParameterOut\Foo::doBaz() never assigns null to &$p so it can be removed from the @param-out type.',
21,
],
[
'Method TooWideMethodParameterOut\Foo::doLorem() never assigns null to &$p so it can be removed from the by-ref type.',
26,
'You can narrow the parameter out type with @param-out PHPDoc tag.',
],
]);
}

public function testBug10684(): void
{
$this->analyse([__DIR__ . '/data/bug-10684.php'], []);
}

public function testBug10687(): void
{
$this->analyse([__DIR__ . '/data/bug-10687.php'], []);
}

}

This file was deleted.

Loading

0 comments on commit d1bcf78

Please sign in to comment.