Skip to content

Commit

Permalink
Too wide private method return type - level 4 (bleeding edge)
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Jun 9, 2019
1 parent 78e04aa commit 178953d
Show file tree
Hide file tree
Showing 6 changed files with 221 additions and 1 deletion.
5 changes: 5 additions & 0 deletions conf/config.level4.neon
Expand Up @@ -30,6 +30,8 @@ conditionalTags:
phpstan.rules.rule: %featureToggles.tooWideTypehints%
PHPStan\Rules\TooWideTypehints\TooWideFunctionReturnTypehintRule:
phpstan.rules.rule: %featureToggles.tooWideTypehints%
PHPStan\Rules\TooWideTypehints\TooWidePrivateMethodReturnTypehintRule:
phpstan.rules.rule: %featureToggles.tooWideTypehints%

services:
-
Expand Down Expand Up @@ -81,3 +83,6 @@ services:

-
class: PHPStan\Rules\TooWideTypehints\TooWideFunctionReturnTypehintRule

-
class: PHPStan\Rules\TooWideTypehints\TooWidePrivateMethodReturnTypehintRule
16 changes: 15 additions & 1 deletion src/Analyser/NodeScopeResolver.php
Expand Up @@ -53,6 +53,7 @@
use PHPStan\Node\InClassMethodNode;
use PHPStan\Node\LiteralArrayItem;
use PHPStan\Node\LiteralArrayNode;
use PHPStan\Node\MethodReturnStatementsNode;
use PHPStan\Node\ReturnStatement;
use PHPStan\Node\UnreachableStatementNode;
use PHPStan\Parser\Parser;
Expand Down Expand Up @@ -369,7 +370,20 @@ private function processStmtNode(
$nodeCallback(new InClassMethodNode($stmt), $methodScope);

if ($stmt->stmts !== null) {
$this->processStmtNodes($stmt, $stmt->stmts, $methodScope, $nodeCallback);
$gatheredReturnStatements = [];
$statementResult = $this->processStmtNodes($stmt, $stmt->stmts, $methodScope, static function (\PhpParser\Node $node, Scope $scope) use ($nodeCallback, &$gatheredReturnStatements): void {
$nodeCallback($node, $scope);
if (!$node instanceof Return_) {
return;
}

$gatheredReturnStatements[] = new ReturnStatement($scope, $node);
});
$nodeCallback(new MethodReturnStatementsNode(
$stmt,
$gatheredReturnStatements,
$statementResult
), $methodScope);
}
} elseif ($stmt instanceof Echo_) {
$hasYield = false;
Expand Down
57 changes: 57 additions & 0 deletions src/Node/MethodReturnStatementsNode.php
@@ -0,0 +1,57 @@
<?php declare(strict_types = 1);

namespace PHPStan\Node;

use PhpParser\Node\Stmt\ClassMethod;
use PhpParser\NodeAbstract;
use PHPStan\Analyser\StatementResult;

class MethodReturnStatementsNode extends NodeAbstract implements VirtualNode
{

/** @var \PHPStan\Node\ReturnStatement[] */
private $returnStatements;

/** @var StatementResult */
private $statementResult;

/**
* @param \PhpParser\Node\Stmt\ClassMethod $method
* @param \PHPStan\Node\ReturnStatement[] $returnStatements
* @param \PHPStan\Analyser\StatementResult $statementResult
*/
public function __construct(
ClassMethod $method,
array $returnStatements,
StatementResult $statementResult
)
{
parent::__construct($method->getAttributes());
$this->returnStatements = $returnStatements;
$this->statementResult = $statementResult;
}

/**
* @return \PHPStan\Node\ReturnStatement[]
*/
public function getReturnStatements(): array
{
return $this->returnStatements;
}

public function getStatementResult(): StatementResult
{
return $this->statementResult;
}

public function getType(): string
{
return 'PHPStan_Node_FunctionReturnStatementsNode';
}

public function getSubNodeNames(): array
{
return [];
}

}
@@ -0,0 +1,80 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\TooWideTypehints;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Node\MethodReturnStatementsNode;
use PHPStan\Reflection\MethodReflection;
use PHPStan\Reflection\ParametersAcceptorSelector;
use PHPStan\Rules\Rule;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\UnionType;
use PHPStan\Type\VerbosityLevel;

class TooWidePrivateMethodReturnTypehintRule implements Rule
{

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

/**
* @param \PHPStan\Node\MethodReturnStatementsNode $node
* @param \PHPStan\Analyser\Scope $scope
* @return string[]
*/
public function processNode(Node $node, Scope $scope): array
{
$method = $scope->getFunction();
if (!$method instanceof MethodReflection) {
throw new \PHPStan\ShouldNotHappenException();
}
if (!$method->isPrivate()) {

This comment has been minimized.

Copy link
@pepakriz

pepakriz Jun 9, 2019

Contributor

@ondrejmirtes protected/public final method can be checked too, right? Of course, you should check the parent class for potential conflict.

This comment has been minimized.

Copy link
@ondrejmirtes

ondrejmirtes Jun 9, 2019

Author Member

I'd want to check all public and protected methods but until the covariance RFC is implemented, I can't be sure about usages in child classes (and even outside of analysed repository).

It could be implemented for final methods and methods in final classes, but I feel like it's too small subset to be worth it.

return [];
}

$methodReturnType = ParametersAcceptorSelector::selectSingle($method->getVariants())->getReturnType();
if (!$methodReturnType instanceof UnionType) {
return [];
}
$statementResult = $node->getStatementResult();
if ($statementResult->hasYield()) {
return [];
}

$returnStatements = $node->getReturnStatements();
if (count($returnStatements) === 0) {
return [];
}

$returnTypes = [];
foreach ($returnStatements as $returnStatement) {
$returnNode = $returnStatement->getReturnNode();
if ($returnNode->expr === null) {
continue;
}

$returnTypes[] = $returnStatement->getScope()->getType($returnNode->expr);
}

if (count($returnTypes) === 0) {
return [];
}

$returnType = TypeCombinator::union(...$returnTypes);

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

$messages[] = sprintf('Private method %s::%s() never returns %s so it can be removed from the return typehint.', $method->getDeclaringClass()->getDisplayName(), $method->getName(), $type->describe(VerbosityLevel::typeOnly()));
}

return $messages;
}

}
@@ -0,0 +1,30 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\TooWideTypehints;

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

class TooWidePrivateMethodReturnTypehintRuleTest extends RuleTestCase
{

protected function getRule(): Rule
{
return new TooWidePrivateMethodReturnTypehintRule();
}

public function testRule(): void
{
$this->analyse([__DIR__ . '/data/tooWidePrivateMethodReturnType.php'], [
[
'Private method TooWidePrivateMethodReturnType\Foo::bar() never returns string so it can be removed from the return typehint.',
14,
],
[
'Private method TooWidePrivateMethodReturnType\Foo::baz() never returns null so it can be removed from the return typehint.',
18,
],
]);
}

}
@@ -0,0 +1,34 @@
<?php

namespace TooWidePrivateMethodReturnType;

class Foo
{

private function foo(): \Generator {
yield 1;
yield 2;
return 3;
}

private function bar(): ?string {
return null;
}

private function baz(): ?string {
return 'foo';
}

private function lorem(): ?string {
if (rand(0, 1)) {
return '1';
}

return null;
}

public function ipsum(): ?string {
return null;
}

}

0 comments on commit 178953d

Please sign in to comment.