Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

EnforceNativeReturnTypehintRule: try narrowing enforcement #122

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

EnforceNativeReturnTypehintRule: try narrowing enforcement #122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4c12eab
EnforceNativeReturnTypehintRule: try narrowing enforcement
janedbal Dec 23, 2022
53c2455
Some fixes and simplifications
janedbal Jun 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
189 changes: 131 additions & 58 deletions src/Rule/EnforceNativeReturnTypehintRule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
use Generator;
use LogicException;
use PhpParser\Node;
use PhpParser\Node\Stmt\Function_;
use PhpParser\Node\Stmt\Throw_;
use PHPStan\Analyser\Scope;
use PHPStan\Node\ClassMethod;
use PHPStan\Node\ClosureReturnStatementsNode;
use PHPStan\Node\FunctionReturnStatementsNode;
use PHPStan\Node\MethodReturnStatementsNode;
Expand All @@ -16,25 +18,28 @@
use PHPStan\Type\ArrayType;
use PHPStan\Type\BooleanType;
use PHPStan\Type\CallableType;
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\FileTypeMapper;
use PHPStan\Type\FloatType;
use PHPStan\Type\IntegerType;
use PHPStan\Type\IntersectionType;
use PHPStan\Type\IterableType;
use PHPStan\Type\MixedType;
use PHPStan\Type\NeverType;
use PHPStan\Type\NullType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\ObjectWithoutClassType;
use PHPStan\Type\StaticType;
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\UnionType;
use PHPStan\Type\VerbosityLevel;
use PHPStan\Type\VoidType;
use ReflectionClass;
use function count;
use function implode;
use function in_array;
use function sprintf;
use function str_replace;
use function strlen;
use function substr;

/**
* @implements Rule<ReturnStatementsNode>
Expand All @@ -48,15 +53,19 @@ class EnforceNativeReturnTypehintRule implements Rule

private bool $treatPhpDocTypesAsCertain;

private bool $enforceNarrowestTypehint;

public function __construct(
FileTypeMapper $fileTypeMapper,
PhpVersion $phpVersion,
bool $treatPhpDocTypesAsCertain
bool $treatPhpDocTypesAsCertain,
bool $enforceNarrowestTypehint = true
)
{
$this->fileTypeMapper = $fileTypeMapper;
$this->phpVersion = $phpVersion;
$this->treatPhpDocTypesAsCertain = $treatPhpDocTypesAsCertain;
$this->enforceNarrowestTypehint = $enforceNarrowestTypehint;
}

public function getNodeType(): string
Expand All @@ -74,10 +83,6 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

if ($this->hasNativeReturnTypehint($node)) {
return [];
}

if (!$scope->isInAnonymousFunction() && in_array($scope->getFunctionName(), ['__construct', '__destruct', '__clone'], true)) {
return [];
}
Expand All @@ -86,6 +91,12 @@ public function processNode(Node $node, Scope $scope): array
return []; // return may easily differ for each usage
}

$hasNativeReturnType = $this->hasNativeReturnTypehint($node);

if ($hasNativeReturnType && !$this->enforceNarrowestTypehint) {
return [];
}

$phpDocReturnType = $this->getPhpDocReturnType($node, $scope);
$returnType = $phpDocReturnType ?? $this->getTypeOfReturnStatements($node);
$alwaysThrows = $this->alwaysThrowsException($node);
Expand All @@ -96,9 +107,28 @@ public function processNode(Node $node, Scope $scope): array
return [];
}

return [
sprintf('Missing native return typehint %s', $typeHint),
];
if (!$hasNativeReturnType) {
return ["Missing native return typehint {$this->toTypehint($typeHint)}"];
}

if ($this->enforceNarrowestTypehint) {
$actualReturnType = $this->getTypeOfReturnStatements($node);
$nativeReturnType = $this->getNativeReturnTypehint($node, $scope);

$typeHintFromActualReturns = $this->getTypehintByType($actualReturnType, $scope, $phpDocReturnType !== null, $alwaysThrows, true);
$typeHintFromNativeTypehint = $this->getTypehintByType($nativeReturnType, $scope, $phpDocReturnType !== null, $alwaysThrows, true);

if (
$typeHintFromActualReturns !== null
&& $typeHintFromNativeTypehint !== null
&& $typeHintFromNativeTypehint->isSuperTypeOf($typeHintFromActualReturns)->yes()
&& !$typeHintFromNativeTypehint->equals($typeHintFromActualReturns)
) {
return ["Native return typehint is {$this->toTypehint($typeHintFromNativeTypehint)}, but can be narrowed to {$this->toTypehint($typeHintFromActualReturns)}"];
}
}

return [];
}

private function getTypehintByType(
Expand All @@ -107,27 +137,27 @@ private function getTypehintByType(
bool $typeFromPhpDoc,
bool $alwaysThrowsException,
bool $topLevel
): ?string
): ?Type
{
if ($type instanceof MixedType) {
return $this->phpVersion->getVersionId() >= 80_000 ? 'mixed' : null;
return $this->phpVersion->getVersionId() >= 80_000 ? new MixedType() : null;
}

if ($type->isVoid()->yes()) {
return 'void';
return new VoidType();
}

if ($type instanceof NeverType) {
if (($typeFromPhpDoc || $alwaysThrowsException) && $this->phpVersion->getVersionId() >= 80_100) {
return 'never';
return new NeverType();
}

return 'void';
return new VoidType();
}

if ($type->isNull()->yes()) {
if (!$topLevel || $this->phpVersion->getVersionId() >= 80_200) {
return 'null';
return new NullType();
}

return null;
Expand All @@ -137,47 +167,49 @@ private function getTypehintByType(
$typeHint = null;

if ((new BooleanType())->accepts($typeWithoutNull, $scope->isDeclareStrictTypes())->yes()) {
if (($typeWithoutNull->isTrue()->yes() || $typeWithoutNull->isFalse()->yes()) && $this->phpVersion->getVersionId() >= 80_200) {
$typeHint = $typeWithoutNull->describe(VerbosityLevel::typeOnly());
$supportsStandaloneTrue = $this->phpVersion->getVersionId() >= 80_200;

if ($supportsStandaloneTrue && $typeWithoutNull->isTrue()) {
$typeHint = new ConstantBooleanType(true);
} elseif ($supportsStandaloneTrue && $typeWithoutNull->isFalse()) {
$typeHint = new ConstantBooleanType(false);
} else {
$typeHint = 'bool';
$typeHint = new BooleanType();
}
} elseif ((new IntegerType())->accepts($typeWithoutNull, $scope->isDeclareStrictTypes())->yes()) {
$typeHint = 'int';
$typeHint = new IntegerType();

} elseif ((new FloatType())->accepts($typeWithoutNull, $scope->isDeclareStrictTypes())->yes()) {
$typeHint = 'float';
$typeHint = new FloatType();

} elseif ((new ArrayType(new MixedType(), new MixedType()))->accepts($typeWithoutNull, $scope->isDeclareStrictTypes())->yes()) {
$typeHint = 'array';
$typeHint = new ArrayType(new MixedType(), new MixedType());

} elseif ((new StringType())->accepts($typeWithoutNull, $scope->isDeclareStrictTypes())->yes()) {
$typeHint = 'string';
} elseif ($typeWithoutNull instanceof StaticType) {
if ($this->phpVersion->getVersionId() < 80_000) {
$typeHint = 'self';
} else {
$typeHint = 'static';
}
$typeHint = new StringType();

} elseif (count($typeWithoutNull->getObjectClassNames()) === 1) {
$className = $typeWithoutNull->getObjectClassNames()[0];
$typeHint = new ObjectType('\\' . $className);

if ($className === $this->getClassName($scope)) {
$typeHint = 'self';
} else {
$typeHint = '\\' . $className;
}
} elseif ((new CallableType())->accepts($typeWithoutNull, $scope->isDeclareStrictTypes())->yes()) {
$typeHint = 'callable';
$typeHint = new CallableType();

} elseif ((new IterableType(new MixedType(), new MixedType()))->accepts($typeWithoutNull, $scope->isDeclareStrictTypes())->yes()) {
$typeHint = 'iterable';
$typeHint = new IterableType(new MixedType(), new MixedType());

} elseif ($this->getUnionTypehint($type, $scope, $typeFromPhpDoc, $alwaysThrowsException) !== null) {
return $this->getUnionTypehint($type, $scope, $typeFromPhpDoc, $alwaysThrowsException);

} elseif ($this->getIntersectionTypehint($type, $scope, $typeFromPhpDoc, $alwaysThrowsException) !== null) {
return $this->getIntersectionTypehint($type, $scope, $typeFromPhpDoc, $alwaysThrowsException);

} elseif ((new ObjectWithoutClassType())->accepts($typeWithoutNull, $scope->isDeclareStrictTypes())->yes()) {
$typeHint = 'object';
$typeHint = new ObjectWithoutClassType();
}

if ($typeHint !== null && TypeCombinator::containsNull($type)) {
$typeHint = '?' . $typeHint;
$typeHint = TypeCombinator::addNull($typeHint);
}

return $typeHint;
Expand All @@ -202,6 +234,37 @@ private function getTypeOfReturnStatements(ReturnStatementsNode $node): Type
return TypeCombinator::union(...$types);
}

private function getNativeReturnTypehint(ReturnStatementsNode $node, Scope $scope): Type
{
$reflection = new ReflectionClass($node);

if ($node instanceof MethodReturnStatementsNode) { // @phpstan-ignore-line ignore bc warning
$classMethodReflection = $reflection->getProperty('classMethod');
$classMethodReflection->setAccessible(true);
/** @var ClassMethod $classMethod */
$classMethod = $classMethodReflection->getValue($node);
return $scope->getFunctionType($classMethod->returnType, $classMethod->returnType === null, false);
}

if ($node instanceof FunctionReturnStatementsNode) { // @phpstan-ignore-line ignore bc warning
$functionReflection = $reflection->getProperty('function');
$functionReflection->setAccessible(true);
/** @var Function_ $function */
$function = $functionReflection->getValue($node);
return $scope->getFunctionType($function->returnType, $function->returnType === null, false);
}

if ($node instanceof ClosureReturnStatementsNode) { // @phpstan-ignore-line ignore bc warning
$closureReflection = $reflection->getProperty('closureExpr');
$closureReflection->setAccessible(true);
/** @var Closure $closure */
$closure = $closureReflection->getValue($node);
return $scope->getFunctionType($closure->returnType, $closure->returnType === null, false);
}

throw new LogicException('Unexpected subtype');
}

/**
* To be removed once we bump phpstan version to 1.9.5+ (https://github.com/phpstan/phpstan-src/pull/2141)
*/
Expand Down Expand Up @@ -261,7 +324,7 @@ private function getUnionTypehint(
Scope $scope,
bool $typeFromPhpDoc,
bool $alwaysThrowsException
): ?string
): ?Type
{
if (!$type instanceof UnionType) {
return null;
Expand All @@ -274,14 +337,10 @@ private function getUnionTypehint(
$typehintParts = [];

foreach ($type->getTypes() as $subtype) {
$wrap = false;

if ($subtype instanceof IntersectionType) { // @phpstan-ignore-line ignore instanceof intersection
if ($this->phpVersion->getVersionId() < 80_200) { // DNF
return null;
}

$wrap = true;
}

$subtypeHint = $this->getTypehintByType($subtype, $scope, $typeFromPhpDoc, $alwaysThrowsException, false);
Expand All @@ -294,18 +353,18 @@ private function getUnionTypehint(
continue;
}

$typehintParts[] = $wrap ? "($subtypeHint)" : $subtypeHint;
$typehintParts[] = $subtypeHint;
}

return implode('|', $typehintParts);
return TypeCombinator::union(...$typehintParts);
}

private function getIntersectionTypehint(
Type $type,
Scope $scope,
bool $typeFromPhpDoc,
bool $alwaysThrowsException
): ?string
): ?Type
{
if (!$type instanceof IntersectionType) { // @phpstan-ignore-line ignore instanceof intersection
return null;
Expand All @@ -318,14 +377,10 @@ private function getIntersectionTypehint(
$typehintParts = [];

foreach ($type->getTypes() as $subtype) {
$wrap = false;

if ($subtype instanceof UnionType) {
if ($this->phpVersion->getVersionId() < 80_200) { // DNF
return null;
}

$wrap = true;
}

$subtypeHint = $this->getTypehintByType($subtype, $scope, $typeFromPhpDoc, $alwaysThrowsException, false);
Expand All @@ -338,23 +393,41 @@ private function getIntersectionTypehint(
continue;
}

$typehintParts[] = $wrap ? "($subtypeHint)" : $subtypeHint;
$typehintParts[] = $subtypeHint;
}

return implode('&', $typehintParts);
return TypeCombinator::intersect(...$typehintParts);
}

private function alwaysThrowsException(ReturnStatementsNode $node): bool
{
$exitPoints = $node->getStatementResult()->getExitPoints();
if (count($node->getReturnStatements()) > 0) {
return false;
}

foreach ($exitPoints as $exitPoint) {
if (!$exitPoint->getStatement() instanceof Throw_) {
foreach ($node->getExecutionEnds() as $executionEnd) {
if (!$executionEnd->getNode() instanceof Throw_) {
return false;
}
}

return $exitPoints !== [];
return $node->getExecutionEnds() !== [];
}

private function toTypehint(Type $type): string
{
if (TypeCombinator::containsNull($type) && $type instanceof UnionType && count($type->getTypes()) === 2) {
$typeWithoutNull = TypeCombinator::removeNull($type);
return '?' . $typeWithoutNull->toPhpDocNode();
}

$typeHint = str_replace(' ', '', (string) $type->toPhpDocNode());

if ($typeHint[0] === '(' && $typeHint[strlen($typeHint) - 1] === ')') {
return substr($typeHint, 1, strlen($typeHint) - 2);
}

return $typeHint;
}

}
1 change: 1 addition & 0 deletions tests/Rule/EnforceNativeReturnTypehintRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ protected function getRule(): Rule
self::getContainer()->getByType(FileTypeMapper::class),
$this->phpVersion,
true,
true,
);
}

Expand Down
Loading