Skip to content

Introduce checkTooWideParameterOutInProtectedAndPublicMethods #4229

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

Open
wants to merge 1 commit into
base: 2.1.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ parameters:
checkPhpDocMethodSignatures: false
checkExtraArguments: false
checkMissingTypehints: false
checkTooWideParameterOutInProtectedAndPublicMethods: false
checkTooWideReturnTypesInProtectedAndPublicMethods: false
checkUninitializedProperties: false
checkDynamicProperties: false
Expand Down
1 change: 1 addition & 0 deletions conf/parametersSchema.neon
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ parametersSchema:
checkPhpDocMethodSignatures: bool()
checkExtraArguments: bool()
checkMissingTypehints: bool()
checkTooWideParameterOutInProtectedAndPublicMethods: bool()
checkTooWideReturnTypesInProtectedAndPublicMethods: bool()
checkUninitializedProperties: bool()
checkDynamicProperties: bool()
Expand Down
11 changes: 11 additions & 0 deletions src/Rules/TooWideTypehints/TooWideMethodParameterOutTypeRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\DependencyInjection\AutowiredParameter;
use PHPStan\DependencyInjection\RegisteredRule;
use PHPStan\Node\MethodReturnStatementsNode;
use PHPStan\Rules\Rule;
Expand All @@ -18,6 +19,8 @@ final class TooWideMethodParameterOutTypeRule implements Rule

public function __construct(
private TooWideParameterOutTypeCheck $check,
#[AutowiredParameter(ref: '%checkTooWideParameterOutInProtectedAndPublicMethods%')]
private bool $checkProtectedAndPublicMethods,
)
{
}
Expand All @@ -31,6 +34,14 @@ public function processNode(Node $node, Scope $scope): array
{
$inMethod = $node->getMethodReflection();

if (!$inMethod->isPrivate()) {
if (!$inMethod->getDeclaringClass()->isFinal() && !$inMethod->isFinal()->yes()) {
if (!$this->checkProtectedAndPublicMethods) {
return [];
}
}
}

return $this->check->check(
$node->getExecutionEnds(),
$node->getReturnStatements(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
class TooWideMethodParameterOutTypeRuleTest extends RuleTestCase
{

private bool $checkProtectedAndPublicMethods = true;

protected function getRule(): TRule
{
return new TooWideMethodParameterOutTypeRule(new TooWideParameterOutTypeCheck());
return new TooWideMethodParameterOutTypeRule(new TooWideParameterOutTypeCheck(), $this->checkProtectedAndPublicMethods);
}

public function testRule(): void
Expand All @@ -37,6 +39,69 @@ public function testRule(): void
'Method TooWideMethodParameterOut\Foo::bug10699() never assigns 20 to &$out so it can be removed from the @param-out type.',
37,
],
[
'Method TooWideMethodParameterOut\Foo::finalDoBaz() never assigns null to &$p so it can be removed from the @param-out type.',
45,
],
[
'Method TooWideMethodParameterOut\Foo::doBazProtected() never assigns null to &$p so it can be removed from the @param-out type.',
53,
],
[
'Method TooWideMethodParameterOut\Foo::doBazPrivate() never assigns null to &$p so it can be removed from the @param-out type.',
61,
],
[
'Method TooWideMethodParameterOut\FinalFoo::doBar() never assigns null to &$p so it can be removed from the by-ref type.',
76,
'You can narrow the parameter out type with @param-out PHPDoc tag.',
],
[
'Method TooWideMethodParameterOut\FinalFoo::doBaz() never assigns null to &$p so it can be removed from the @param-out type.',
84,
],
[
'Method TooWideMethodParameterOut\FinalFoo::doLorem() never assigns null to &$p so it can be removed from the by-ref type.',
89,
'You can narrow the parameter out type with @param-out PHPDoc tag.',
],
[
'Method TooWideMethodParameterOut\FinalFoo::bug10699() never assigns 20 to &$out so it can be removed from the @param-out type.',
100,
],
]);
}

public function testRuleWithoutProtectedAndPublic(): void
{
$this->checkProtectedAndPublicMethods = false;
$this->analyse([__DIR__ . '/data/too-wide-method-parameter-out.php'], [
[
'Method TooWideMethodParameterOut\Foo::finalDoBaz() never assigns null to &$p so it can be removed from the @param-out type.',
45,
],
[
'Method TooWideMethodParameterOut\Foo::doBazPrivate() never assigns null to &$p so it can be removed from the @param-out type.',
61,
],
[
'Method TooWideMethodParameterOut\FinalFoo::doBar() never assigns null to &$p so it can be removed from the by-ref type.',
76,
'You can narrow the parameter out type with @param-out PHPDoc tag.',
],
[
'Method TooWideMethodParameterOut\FinalFoo::doBaz() never assigns null to &$p so it can be removed from the @param-out type.',
84,
],
[
'Method TooWideMethodParameterOut\FinalFoo::doLorem() never assigns null to &$p so it can be removed from the by-ref type.',
89,
'You can narrow the parameter out type with @param-out PHPDoc tag.',
],
[
'Method TooWideMethodParameterOut\FinalFoo::bug10699() never assigns 20 to &$out so it can be removed from the @param-out type.',
100,
],
]);
}

Expand All @@ -50,4 +115,10 @@ public function testBug10687(): void
$this->analyse([__DIR__ . '/data/bug-10687.php'], []);
}

public function testBug12080(): void
{
$this->checkProtectedAndPublicMethods = false;
$this->analyse([__DIR__ . '/data/bug-12080.php'], []);
}

}
19 changes: 19 additions & 0 deletions tests/PHPStan/Rules/TooWideTypehints/data/bug-12080.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

namespace Bug12080;

class C {
/**
* @param mixed $x
* @param-out string|int $x
*/
public function foo(&$x): void {
$x = "foo";
}
}

class D extends C {
public function foo(&$x): void {
$x = 42;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,5 +39,67 @@ function bug10699(int $flags, &$out): void

}

/**
* @param-out string|null $p
*/
final public function finalDoBaz(?string &$p): void
{
$p = 'foo';
}

/**
* @param-out string|null $p
*/
protected function doBazProtected(?string &$p): void
{
$p = 'foo';
}

/**
* @param-out string|null $p
*/
private function doBazPrivate(?string &$p): void
{
$p = 'foo';
}

}

final class FinalFoo
{

public function doFoo(?string &$p): void
{

}

public function doBar(?string &$p): void
{
$p = 'foo';
}

/**
* @param-out string|null $p
*/
public function doBaz(?string &$p): void
{
$p = 'foo';
}

public function doLorem(?string &$p): void
{
$p = 'foo';
}

/**
* @param int $flags
* @param 10 $out
*
* @param-out ($flags is 2 ? 20 : 10) $out
*/
function bug10699(int $flags, &$out): void
{

}

}
Loading