Skip to content

Commit

Permalink
Keep other rules working via 'keepVoid' => true node attribute appr…
Browse files Browse the repository at this point in the history
…oach
  • Loading branch information
herndlm committed Nov 29, 2023
1 parent 6e8d6af commit 7dbe5a8
Show file tree
Hide file tree
Showing 5 changed files with 180 additions and 14 deletions.
24 changes: 19 additions & 5 deletions src/Analyser/MutatingScope.php
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,10 @@ private function getNodeKey(Expr $node): string
$key .= '/*' . $node->getAttribute('startFilePos') . '*/';
}

if ($node->hasAttribute('keepVoid')) {
$key .= '/*keepVoid*/';
}

return $key;
}

Expand Down Expand Up @@ -1483,6 +1487,9 @@ private function resolveType(string $exprString, Expr $node): Type
$matchScope = $this;
foreach ($node->arms as $arm) {
if ($arm->conds === null) {
if ($node->hasAttribute('keepVoid')) {
$arm->body->setAttribute('keepVoid', $node->getAttribute('keepVoid'));
}
$types[] = $matchScope->getType($arm->body);
continue;
}
Expand Down Expand Up @@ -1512,6 +1519,9 @@ private function resolveType(string $exprString, Expr $node): Type

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

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

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

return new MixedType();
Expand Down Expand Up @@ -1977,8 +1987,12 @@ private function getNullsafeShortCircuitingType(Expr $expr, Type $type): Type
return $type;
}

private function transformVoidToNull(Type $type): Type
private function transformVoidToNull(Type $type, Node $node): Type
{
if ($node->getAttribute('keepVoid') === true) {
return $type;
}

return TypeTraverser::map($type, static function (Type $type, callable $traverse): Type {
if ($type instanceof UnionType || $type instanceof IntersectionType) {
return $traverse($type);
Expand Down Expand Up @@ -5109,7 +5123,7 @@ private function methodCallReturnType(Type $typeWithMethod, string $methodName,
$normalizedMethodCall = ArgumentsNormalizer::reorderStaticCallArguments($parametersAcceptor, $methodCall);
}
if ($normalizedMethodCall === null) {
return $this->transformVoidToNull($parametersAcceptor->getReturnType());
return $this->transformVoidToNull($parametersAcceptor->getReturnType(), $methodCall);
}

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

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

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

/** @api */
Expand Down
4 changes: 3 additions & 1 deletion src/Rules/Comparison/UsageOfVoidMatchExpressionRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ public function getNodeType(): string
public function processNode(Node $node, Scope $scope): array
{
if (!$scope->isInFirstLevelStatement()) {
$matchResultType = $scope->getType($node);
$clonedNode = clone $node;
$clonedNode->setAttribute('keepVoid', true);
$matchResultType = $scope->getType($clonedNode);
if ($matchResultType->isVoid()->yes()) {
return [RuleErrorBuilder::message('Result of match expression (void) is used.')->build()];
}
Expand Down
12 changes: 6 additions & 6 deletions src/Rules/FunctionCallParametersCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -207,12 +207,12 @@ public function check(
}
}

if (
!$funcCall instanceof Node\Expr\New_
&& !$scope->isInFirstLevelStatement()
&& $scope->getType($funcCall)->isVoid()->yes()
) {
$errors[] = RuleErrorBuilder::message($messages[7])->line($funcCall->getLine())->build();
if (!$funcCall instanceof Node\Expr\New_ && !$scope->isInFirstLevelStatement()) {
$clonedFuncCall = clone $funcCall;
$clonedFuncCall->setAttribute('keepVoid', true);
if ($scope->getType($clonedFuncCall)->isVoid()->yes()) {
$errors[] = RuleErrorBuilder::message($messages[7])->line($funcCall->getLine())->build();
}
}

[$addedErrors, $argumentsWithParameters] = $this->processArguments($parametersAcceptor, $funcCall->getLine(), $isBuiltin, $arguments, $hasNamedArguments, $messages[10], $messages[11]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@ protected function getRule(): Rule

public function testRule(): void
{
$this->analyse([__DIR__ . '/data/void-match.php'], []);
$this->analyse([__DIR__ . '/data/void-match.php'], [
[
'Result of match expression (void) is used.',
21,
],
]);
}

}
147 changes: 146 additions & 1 deletion tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,70 @@ public function testCallMethods(): void
63,
'Learn more at https://phpstan.org/user-guide/discovering-symbols',
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
66,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
68,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
70,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
72,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
75,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
76,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
77,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
78,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
79,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
81,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
83,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
84,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
85,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
86,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
90,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
91,
],
[
'Call to an undefined method ArrayObject<int, stdClass>::doFoo().',
108,
Expand Down Expand Up @@ -140,6 +204,10 @@ public function testCallMethods(): void
'Method DateTimeZone::getTransitions() invoked with 3 parameters, 0-2 required.',
214,
],
[
'Result of method Test\ReturningSomethingFromConstructor::__construct() (void) is used.',
234,
],
[
'Cannot call method foo() on int|string.',
254,
Expand Down Expand Up @@ -522,6 +590,70 @@ public function testCallMethodsOnThisOnly(): void
'Method Test\Foo::test() invoked with 0 parameters, 1 required.',
46,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
66,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
68,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
70,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
72,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
75,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
76,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
77,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
78,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
79,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
81,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
83,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
84,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
85,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
86,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
90,
],
[
'Result of method Test\Bar::returnsVoid() (void) is used.',
91,
],
[
'Parameter #1 $bar of method Test\ClassWithNullableProperty::doBar() is passed by reference, so it expects variables only.',
167,
Expand Down Expand Up @@ -1327,6 +1459,10 @@ public function testClosureCallInvocations(): void
'Method Closure::call() invoked with 3 parameters, 2 required.',
14,
],
[
'Result of method Closure::call() (void) is used.',
18,
],
]);
}

Expand Down Expand Up @@ -1604,7 +1740,16 @@ public function testMatchExpressionVoidIsUsed(): void
$this->checkThisOnly = false;
$this->checkNullables = true;
$this->checkUnionTypes = true;
$this->analyse([__DIR__ . '/data/match-expr-void-used.php'], []);
$this->analyse([__DIR__ . '/data/match-expr-void-used.php'], [
[
'Result of method MatchExprVoidUsed\Foo::doLorem() (void) is used.',
10,
],
[
'Result of method MatchExprVoidUsed\Foo::doBar() (void) is used.',
11,
],
]);
}

public function testNullSafe(): void
Expand Down

0 comments on commit 7dbe5a8

Please sign in to comment.