Skip to content
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
24 changes: 24 additions & 0 deletions src/Analyser/TypeSpecifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -2231,6 +2231,30 @@
}
}

if (
$expr instanceof FuncCall
&& !$expr->name instanceof Name
) {
$nameType = $scope->getType($expr->name);
if ($nameType->isCallable()->yes()) {

Check warning on line 2239 in src/Analyser/TypeSpecifier.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ && !$expr->name instanceof Name ) { $nameType = $scope->getType($expr->name); - if ($nameType->isCallable()->yes()) { + if (!$nameType->isCallable()->no()) { $isPure = null; foreach ($nameType->getCallableParametersAcceptors($scope) as $variant) { $variantIsPure = $variant->isPure();

Check warning on line 2239 in src/Analyser/TypeSpecifier.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ && !$expr->name instanceof Name ) { $nameType = $scope->getType($expr->name); - if ($nameType->isCallable()->yes()) { + if (!$nameType->isCallable()->no()) { $isPure = null; foreach ($nameType->getCallableParametersAcceptors($scope) as $variant) { $variantIsPure = $variant->isPure();
$isPure = null;
foreach ($nameType->getCallableParametersAcceptors($scope) as $variant) {
$variantIsPure = $variant->isPure();
$isPure = $isPure === null ? $variantIsPure : $isPure->and($variantIsPure);
}

if ($isPure !== null) {
if ($isPure->no()) {

Check warning on line 2247 in src/Analyser/TypeSpecifier.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ } if ($isPure !== null) { - if ($isPure->no()) { + if (!$isPure->yes()) { return new SpecifiedTypes([], []); }

Check warning on line 2247 in src/Analyser/TypeSpecifier.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ } if ($isPure !== null) { - if ($isPure->no()) { + if (!$isPure->yes()) { return new SpecifiedTypes([], []); }
return new SpecifiedTypes([], []);
}

if (!$this->rememberPossiblyImpureFunctionValues && !$isPure->yes()) {
return new SpecifiedTypes([], []);
}
}
}
}

if (
$expr instanceof MethodCall
&& $expr->name instanceof Node\Identifier
Expand Down
63 changes: 63 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-12686.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?php declare(strict_types = 1);

namespace Bug12686;

use function PHPStan\Testing\assertType;

/** @phpstan-impure */
$f = function (): bool {
return (bool) rand(0,1);
};

if ($f()) {
assertType('bool', $f());
}

// Pure closure should still have narrowing
$h = function (): bool {
return true;
};

if ($h()) {
assertType('true', $h());
}

// Multiple callable parameter acceptors (union of closures)
// When one variant is impure, the combined result should be impure
/** @phpstan-impure */
$impure = function (): bool {
return (bool) rand(0, 1);
};

$pure = function (): bool {
return true;
};

if (rand(0, 1)) {
$g = $impure;
} else {
$g = $pure;
}

if ($g()) {
assertType('bool', $g());
}

// Multiple callable parameter acceptors where all are pure
$pure1 = function (): bool {
return true;
};

$pure2 = function (): bool {
return true;
};

if (rand(0, 1)) {
$p = $pure1;
} else {
$p = $pure2;
}

if ($p()) {
assertType('true', $p());
}
27 changes: 27 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-3770.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php declare(strict_types = 1);

namespace Bug3770;

use function PHPStan\Testing\assertType;

// PHPDoc on closures should be respected for purity

/** @phpstan-impure */
$f = static function (string $input): bool {
return strlen($input) > rand(0, 10);
};

if ($f('hello')) {
// Should not narrow to true because closure is impure
assertType('bool', $f('hello'));
}

// Closure with @phpstan-pure should allow narrowing
/** @phpstan-pure */
$g = static function (string $input): bool {
return strlen($input) > 5;
};

if ($g('hello world')) {
assertType('true', $g('hello world'));
}
36 changes: 36 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-6822.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php declare(strict_types = 1);

namespace Bug6822;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

bug 6822 was about a false-positive rule error. we need a rule test for this file to prove it no longer errors.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done. Added a rule test for bug 6822 in IfConstantConditionRuleTest that verifies impure closures (@phpstan-impure) don't produce false-positive "If condition is always true" errors when used in nested conditions. The test data file at tests/PHPStan/Rules/Comparison/data/bug-6822.php covers both bool-returning and int-returning impure closures.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

re-use the tests/PHPStan/Analyser/nsrt/bug-6822.php for the new rule-test instead of adding another new one

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Full test suite passed too. All done — the review feedback has been addressed, tests and static analysis are green, and the commit is pushed.


use function PHPStan\Testing\assertType;

// Closures marked as @phpstan-impure should not have their return type narrowed

/** @phpstan-impure */
$closure = function (): bool {
return (bool) rand(0, 1);
};

assertType('bool', $closure());

if ($closure()) {
assertType('bool', $closure());

if ($closure()) { // should not be reported as "always true"
echo 'yes';
}
}

// Same with an explicit impure closure assigned to a variable
/** @phpstan-impure */
$impureFn = function (): int {
return rand(0, 100);
};

if ($impureFn() > 50) {
assertType('int<0, 100>', $impureFn());

if ($impureFn() > 50) { // should not be reported as "always true"
echo 'yes';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -213,4 +213,10 @@ public function testBug4284(): void
]);
}

public function testBug6822(): void
{
$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-6822.php'], []);
}

}
Loading