Skip to content
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 src/Node/Expr/AlwaysRememberedExpr.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use PHPStan\Node\VirtualNode;
use PHPStan\Type\Type;

/** Wraps an expression so its type is always remembered in the scope, bypassing impurity checks. */
final class AlwaysRememberedExpr extends Expr implements VirtualNode
{

Expand Down
9 changes: 6 additions & 3 deletions src/Type/Php/ClassExistsFunctionTypeSpecifyingExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@
use PHPStan\Analyser\TypeSpecifierAwareExtension;
use PHPStan\Analyser\TypeSpecifierContext;
use PHPStan\DependencyInjection\AutowiredService;
use PHPStan\Node\Expr\AlwaysRememberedExpr;
use PHPStan\Reflection\FunctionReflection;
use PHPStan\Type\BooleanType;
use PHPStan\Type\ClassStringType;
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\Constant\ConstantStringType;
Expand Down Expand Up @@ -47,10 +49,11 @@ public function specifyTypes(FunctionReflection $functionReflection, FuncCall $n
$args = $node->getArgs();
$argType = $scope->getType($args[0]->value);
if ($argType instanceof ConstantStringType) {
$funcCall = new FuncCall(new FullyQualified('class_exists'), [
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.

per PR title I expect a tests (and fixes) also for interface_exists, trait_exists, and enum_exists

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.

Pushed. Here's what was done to address staabm's review:

1. Added interface_exists/trait_exists/enum_exists tests (tests/PHPStan/Rules/Classes/data/bug-8579.php): Added instantiation guards using all four *_exists functions, not just class_exists. The InstantiationDoNotRememberPossiblyImpureValuesRuleTest now covers all four functions with rememberPossiblyImpureFunctionValues: false.

2. Fixed nsrt assertions to actually test the fix (tests/PHPStan/Analyser/data/do-not-remember-possibly-impure-function-values.php): The previous assertions used unqualified class_exists(...) which produced a different expression key (class_exists(...)) than what the extension stores (\class_exists(...) via FullyQualified), so they always got bool regardless of the fix. Changed to \class_exists(...) (fully qualified) so the key matches. The truthy branch now correctly asserts 'true' — verified this fails before the fix and passes after.

All 12000 tests pass and make phpstan reports no errors.

new Arg(new String_(ltrim($argType->getValue(), '\\'))),
]);
return $this->typeSpecifier->create(
new FuncCall(new FullyQualified('class_exists'), [
new Arg(new String_(ltrim($argType->getValue(), '\\'))),
]),
new AlwaysRememberedExpr($funcCall, new BooleanType(), new BooleanType()),
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.

I always wondered what the use-case is for AlwaysRememberedExpr and when it is expected to be used. would be great this could be documented in the AlwaysRememberedExpr-class

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.

I processed this review but have nothing to report.

new ConstantBooleanType(true),
$context,
$scope,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,50 @@ function test(): void
assertType('int', impure());
}
}

function testClassExistsRemembered(): void
{
if (\class_exists('Bug8579RememberedA')) {
assertType('true', \class_exists('Bug8579RememberedA'));
} else {
assertType('bool', \class_exists('Bug8579RememberedA'));
}

assertType('bool', \class_exists('Bug8579RememberedA'));
}

function testClassExistsFalseNotRemembered(): void
{
if (!\class_exists('Bug8579FalseNotRememberedA')) {
assertType('bool', \class_exists('Bug8579FalseNotRememberedA'));
}

assertType('bool', \class_exists('Bug8579FalseNotRememberedA'));
}

function testInterfaceExistsFalseNotRemembered(): void
{
if (!\interface_exists('Bug8579FalseNotRememberedC')) {
assertType('bool', \interface_exists('Bug8579FalseNotRememberedC'));
}

assertType('bool', \interface_exists('Bug8579FalseNotRememberedC'));
}

function testTraitExistsFalseNotRemembered(): void
{
if (!\trait_exists('Bug8579FalseNotRememberedD')) {
assertType('bool', \trait_exists('Bug8579FalseNotRememberedD'));
}

assertType('bool', \trait_exists('Bug8579FalseNotRememberedD'));
}

function testEnumExistsFalseNotRemembered(): void
{
if (!\enum_exists('Bug8579FalseNotRememberedE')) {
assertType('bool', \enum_exists('Bug8579FalseNotRememberedE'));
}

assertType('bool', \enum_exists('Bug8579FalseNotRememberedE'));
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Classes;

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use function array_merge;

/**
* @extends RuleTestCase<InstantiationRule>
*/
class InstantiationDoNotRememberPossiblyImpureValuesRuleTest extends RuleTestCase
{

protected function getRule(): Rule
{
return self::getContainer()->getByType(InstantiationRule::class);
}

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

public static function getAdditionalConfigFiles(): array
{
return array_merge(
parent::getAdditionalConfigFiles(),
[
__DIR__ . '/doNotRememberPossiblyImpureValues.neon',
],
);
}

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

if (!class_exists('NonexistentClassBug8579')) throw new \Exception('nonexistentclass');
$x = new \NonexistentClassBug8579();

if (!interface_exists('NonexistentInterfaceBug8579')) throw new \Exception('nonexistentinterface');
$x = new \NonexistentInterfaceBug8579();

if (!trait_exists('NonexistentTraitBug8579')) throw new \Exception('nonexistenttrait');
$x = new \NonexistentTraitBug8579();

if (!enum_exists('NonexistentEnumBug8579')) throw new \Exception('nonexistentenum');
$x = new \NonexistentEnumBug8579();
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
parameters:
rememberPossiblyImpureFunctionValues: false
Loading