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.

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

Transform void return to null after call #2778

Merged
merged 7 commits into from
Dec 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
49 changes: 44 additions & 5 deletions src/Analyser/MutatingScope.php
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ class MutatingScope implements Scope

private const BOOLEAN_EXPRESSION_MAX_PROCESS_DEPTH = 4;

private const KEEP_VOID_ATTRIBUTE_NAME = 'keepVoid';

/** @var Type[] */
private array $resolvedTypes = [];

Expand Down Expand Up @@ -676,6 +678,10 @@ private function getNodeKey(Expr $node): string
$key .= '/*' . $node->getAttribute('startFilePos') . '*/';
}

if ($node->getAttribute(self::KEEP_VOID_ATTRIBUTE_NAME) === true) {
$key .= '/*' . self::KEEP_VOID_ATTRIBUTE_NAME . '*/';
herndlm marked this conversation as resolved.
Show resolved Hide resolved
}

return $key;
}

Expand Down Expand Up @@ -1236,7 +1242,7 @@ private function resolveType(string $exprString, Expr $node): Type
new VoidType(),
]);
} else {
$returnType = $arrowScope->getType($node->expr);
$returnType = $arrowScope->getKeepVoidType($node->expr);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it make sense to handle a "void" return type of a arrow function? can arrow functions have a void return type?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to change it and see what fails. I think it's valid, at least for a throw expr or calling never function.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just found #2778 (comment) which answers this question, I guess

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm locally I did not get a test failure after changing it. lets see #2873

if ($node->returnType !== null) {
$returnType = TypehintHelper::decideType($this->getFunctionType($node->returnType, false, false), $returnType);
}
Expand Down Expand Up @@ -1492,6 +1498,9 @@ private function resolveType(string $exprString, Expr $node): Type
$matchScope = $this;
foreach ($node->arms as $arm) {
if ($arm->conds === null) {
if ($node->hasAttribute(self::KEEP_VOID_ATTRIBUTE_NAME)) {
$arm->body->setAttribute(self::KEEP_VOID_ATTRIBUTE_NAME, $node->getAttribute(self::KEEP_VOID_ATTRIBUTE_NAME));
}
$types[] = $matchScope->getType($arm->body);
continue;
}
Expand Down Expand Up @@ -1521,6 +1530,9 @@ private function resolveType(string $exprString, Expr $node): Type

if (!$filteringExprType->isFalse()->yes()) {
$truthyScope = $matchScope->filterByTruthyValue($filteringExpr);
if ($node->hasAttribute(self::KEEP_VOID_ATTRIBUTE_NAME)) {
$arm->body->setAttribute(self::KEEP_VOID_ATTRIBUTE_NAME, $node->getAttribute(self::KEEP_VOID_ATTRIBUTE_NAME));
}
$types[] = $truthyScope->getType($arm->body);
}

Expand Down Expand Up @@ -1946,7 +1958,7 @@ private function resolveType(string $exprString, Expr $node): Type
}
}

return $parametersAcceptor->getReturnType();
return $this->transformVoidToNull($parametersAcceptor->getReturnType(), $node);
}

return new MixedType();
Expand Down Expand Up @@ -1986,6 +1998,25 @@ private function getNullsafeShortCircuitingType(Expr $expr, Type $type): Type
return $type;
}

private function transformVoidToNull(Type $type, Node $node): Type
{
if ($node->getAttribute(self::KEEP_VOID_ATTRIBUTE_NAME) === true) {
herndlm marked this conversation as resolved.
Show resolved Hide resolved
return $type;
}

return TypeTraverser::map($type, static function (Type $type, callable $traverse): Type {
if ($type instanceof UnionType || $type instanceof IntersectionType) {
return $traverse($type);
}

if ($type->isVoid()->yes()) {
return new NullType();
}

return $type;
});
}

/**
* @param callable(Type): ?bool $typeCallback
*/
Expand Down Expand Up @@ -2173,6 +2204,14 @@ public function getNativeType(Expr $expr): Type
return $this->promoteNativeTypes()->getType($expr);
}

public function getKeepVoidType(Expr $node): Type
{
$clonedNode = clone $node;
$clonedNode->setAttribute(self::KEEP_VOID_ATTRIBUTE_NAME, true);

return $this->getType($clonedNode);
}

/**
* @api
* @deprecated Use getNativeType()
Expand Down Expand Up @@ -5103,7 +5142,7 @@ private function methodCallReturnType(Type $typeWithMethod, string $methodName,
$normalizedMethodCall = ArgumentsNormalizer::reorderStaticCallArguments($parametersAcceptor, $methodCall);
}
if ($normalizedMethodCall === null) {
return $parametersAcceptor->getReturnType();
return $this->transformVoidToNull($parametersAcceptor->getReturnType(), $methodCall);
}

$resolvedTypes = [];
Expand Down Expand Up @@ -5142,10 +5181,10 @@ private function methodCallReturnType(Type $typeWithMethod, string $methodName,
}

if (count($resolvedTypes) > 0) {
return TypeCombinator::union(...$resolvedTypes);
return $this->transformVoidToNull(TypeCombinator::union(...$resolvedTypes), $methodCall);
}

return $parametersAcceptor->getReturnType();
return $this->transformVoidToNull($parametersAcceptor->getReturnType(), $methodCall);
}

/** @api */
Expand Down
2 changes: 2 additions & 0 deletions src/Analyser/Scope.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ public function getType(Expr $node): Type;

public function getNativeType(Expr $expr): Type;

public function getKeepVoidType(Expr $node): Type;

/**
* @deprecated Use getNativeType()
*/
Expand Down
2 changes: 1 addition & 1 deletion src/Rules/Comparison/UsageOfVoidMatchExpressionRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public function getNodeType(): string
public function processNode(Node $node, Scope $scope): array
{
if (!$scope->isInFirstLevelStatement()) {
$matchResultType = $scope->getType($node);
$matchResultType = $scope->getKeepVoidType($node);
if ($matchResultType->isVoid()->yes()) {
return [RuleErrorBuilder::message('Result of match expression (void) is used.')->build()];
}
Expand Down
2 changes: 1 addition & 1 deletion src/Rules/FunctionCallParametersCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ public function check(
if (
!$funcCall instanceof Node\Expr\New_
&& !$scope->isInFirstLevelStatement()
&& $scope->getType($funcCall)->isVoid()->yes()
&& $scope->getKeepVoidType($funcCall)->isVoid()->yes()
) {
$errors[] = RuleErrorBuilder::message($messages[7])->line($funcCall->getLine())->build();
}
Expand Down
6 changes: 3 additions & 3 deletions tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6072,15 +6072,15 @@ public function dataVoid(): array
{
return [
[
'void',
'null',
'$this->doFoo()',
],
[
'void',
'null',
'$this->doBar()',
],
[
'void',
'null',
'$this->doConflictingVoid()',
],
];
Expand Down
4 changes: 2 additions & 2 deletions tests/PHPStan/Analyser/data/conditional-types.php
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,9 @@ abstract public function maybeNever(int $option): void;

public function testMaybeNever(): void
{
assertType('void', $this->maybeNever(0));
assertType('null', $this->maybeNever(0));
assertType('never', $this->maybeNever(1));
assertType('void', $this->maybeNever(2));
assertType('null', $this->maybeNever(2));
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/data/emptyiterator.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public function doFoo(\EmptyIterator $it): void
assertType('EmptyIterator', $it);
assertType('never', $it->key());
assertType('never', $it->current());
assertType('void', $it->next());
assertType('null', $it->next());
assertType('false', $it->valid());
}

Expand Down
4 changes: 2 additions & 2 deletions tests/PHPStan/Analyser/data/self-out.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,12 +69,12 @@ function () {
$i = new a(123);
// OK - $i is a<123>
assertType('SelfOut\\a<int>', $i);
assertType('void', $i->test());
assertType('null', $i->test());

$i->addData(321);
// OK - $i is a<123|321>
assertType('SelfOut\\a<int>', $i);
assertType('void', $i->test());
assertType('null', $i->test());

$i->setData("test");
// IfThisIsMismatch - Class is not a<int> as required
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1584,6 +1584,11 @@ public function testBug10171(): void
]);
}

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

public function testBug8659(): void
{
$this->analyse([__DIR__ . '/data/bug-8659.php'], []);
Expand Down
11 changes: 11 additions & 0 deletions tests/PHPStan/Rules/Functions/data/bug-6720.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php declare(strict_types = 1);

namespace Bug6720;

/** @return string|void */
function a() {}

function b(?string $a): void {}

$a = a();
b($a);
9 changes: 9 additions & 0 deletions tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -974,4 +974,13 @@ public function testWrongListTip(): void
]);
}

public function testArrowFunctionReturningVoidClosure(): void
{
if (PHP_VERSION_ID < 70400) {
$this->markTestSkipped('Test requires PHP 7.4.');
}

$this->analyse([__DIR__ . '/data/arrow-function-returning-void-closure.php'], []);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

namespace ArrowFunctionReturningVoidClosure;

class ArrowFunctionReturningVoidClosure
{
/**
* @return \Closure(): void
*/
public function getClosure(): \Closure
{
return fn () => $this->returnVoid();
}

public function returnVoid(): void
{

}
}