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

Ignore non-explicit NEVER in purity check #3243

Merged
merged 6 commits into from
Jul 16, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -4330,7 +4330,13 @@ public function createCallableParameters(Scope $scope, Expr $closureExpr, ?array
{
$callableParameters = null;
if ($args !== null) {
$acceptors = $scope->getType($closureExpr)->getCallableParametersAcceptors($scope);
$closureType = $scope->getType($closureExpr);

if ($closureType->isCallable()->no()) {
return null;
}

$acceptors = $closureType->getCallableParametersAcceptors($scope);
if (count($acceptors) === 1) {
$callableParameters = $acceptors[0]->getParameters();

Expand All @@ -4356,6 +4362,10 @@ public function createCallableParameters(Scope $scope, Expr $closureExpr, ?array
$passedToType->getTypes(),
static fn (Type $type) => $type->isCallable()->yes(),
));

if ($passedToType->isCallable()->no()) {
return null;
}
}

$acceptors = $passedToType->getCallableParametersAcceptors($scope);
Expand Down
5 changes: 2 additions & 3 deletions src/Type/NeverType.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
use PHPStan\Reflection\ConstantReflection;
use PHPStan\Reflection\ExtendedMethodReflection;
use PHPStan\Reflection\PropertyReflection;
use PHPStan\Reflection\TrivialParametersAcceptor;
use PHPStan\Reflection\Type\UnresolvedMethodPrototypeReflection;
use PHPStan\Reflection\Type\UnresolvedPropertyPrototypeReflection;
use PHPStan\ShouldNotHappenException;
Expand Down Expand Up @@ -334,12 +333,12 @@ public function shuffleArray(): Type

public function isCallable(): TrinaryLogic
{
return TrinaryLogic::createYes();
return TrinaryLogic::createNo();
}

public function getCallableParametersAcceptors(ClassMemberAccessAnswerer $scope): array
{
return [new TrivialParametersAcceptor()];
throw new ShouldNotHappenException();
}

public function isCloneable(): TrinaryLogic
Expand Down
27 changes: 27 additions & 0 deletions tests/PHPStan/Rules/Pure/PureMethodRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,21 @@
class PureMethodRuleTest extends RuleTestCase
{

private bool $treatPhpDocTypesAsCertain;

public function getRule(): Rule
{
return new PureMethodRule(new FunctionPurityCheck());
}

protected function shouldTreatPhpDocTypesAsCertain(): bool
{
return $this->treatPhpDocTypesAsCertain;
}

public function testRule(): void
{
$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/data/pure-method.php'], [
[
'Method PureMethod\Foo::doFoo() is marked as pure but parameter $p is passed by reference.',
Expand Down Expand Up @@ -141,6 +149,7 @@ public function testPureConstructor(): void
$this->markTestSkipped('Test requires PHP 8.0.');
}

$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/data/pure-constructor.php'], [
[
'Impure property assignment in pure method PureConstructor\Foo::__construct().',
Expand All @@ -159,6 +168,7 @@ public function testPureConstructor(): void

public function testImpureAssignRef(): void
{
$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/data/impure-assign-ref.php'], [
[
'Possibly impure property assignment by reference in pure method ImpureAssignRef\HelloWorld::bar6().',
Expand All @@ -167,4 +177,21 @@ public function testImpureAssignRef(): void
]);
}

/**
* @dataProvider dataBug11207
*/
public function testBug11207(bool $treatPhpDocTypesAsCertain): void
{
$this->treatPhpDocTypesAsCertain = $treatPhpDocTypesAsCertain;
$this->analyse([__DIR__ . '/data/bug-11207.php'], []);
}

public function dataBug11207(): array
{
return [
[true],
[false],
];
}

}
38 changes: 38 additions & 0 deletions tests/PHPStan/Rules/Pure/data/bug-11207.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<?php // lint >= 8.0

namespace Bug11207;

final class FilterData
{
/**
* @phpstan-pure
*/
private function __construct(
public ?int $type,
public bool $hasValue,
public mixed $value = null
) {
}

/**
* @param array{type?: int|numeric-string|null, value?: mixed} $data
* @phpstan-pure
*/
public static function fromArray(array $data): self
{
if (isset($data['type'])) {
if (!\is_int($data['type']) && (!\is_string($data['type']) || !is_numeric($data['type']))) {
throw new \InvalidArgumentException(sprintf(
'The "type" parameter MUST be of type "integer" or "null", "%s" given.',
\gettype($data['type'])
));
}

$type = (int) $data['type'];
} else {
$type = null;
}

return new self($type, \array_key_exists('value', $data), $data['value'] ?? null);
}
}
Loading