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
1 change: 1 addition & 0 deletions conf/bleedingEdge.neon
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ parameters:
internalTag: true
newStaticInAbstractClassStaticMethod: true
checkExtensionsForComparisonOperators: true
reportTooWideBool: true
1 change: 1 addition & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ parameters:
internalTag: false
newStaticInAbstractClassStaticMethod: false
checkExtensionsForComparisonOperators: false
reportTooWideBool: false
fileExtensions:
- php
checkAdvancedIsset: false
Expand Down
1 change: 1 addition & 0 deletions conf/parametersSchema.neon
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ parametersSchema:
internalTag: bool()
newStaticInAbstractClassStaticMethod: bool()
checkExtensionsForComparisonOperators: bool()
reportTooWideBool: bool()
])
fileExtensions: listOf(string())
checkAdvancedIsset: bool()
Expand Down
5 changes: 5 additions & 0 deletions src/Php/PhpVersions.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,9 @@ public function supportsNamedArgumentAfterUnpackedArgument(): TrinaryLogic
return IntegerRangeType::fromInterval(80100, null)->isSuperTypeOf($this->phpVersions)->result;
}

public function supportsTrueAndFalseStandaloneType(): TrinaryLogic
{
return IntegerRangeType::fromInterval(80200, null)->isSuperTypeOf($this->phpVersions)->result;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,9 @@ public function stream_eof(): bool
return $this->readFromFile;
}

/**
* @return true
*/
public function stream_flush(): bool
{
return true;
Expand Down Expand Up @@ -254,6 +257,8 @@ public function stream_seek($offset, $whence): bool
* @param int $option
* @param int $arg1
* @param int $arg2
*
* @return false
*/
public function stream_set_option($option, $arg1, $arg2): bool
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,16 @@ public function processNode(Node $node, Scope $scope): array
{
$function = $node->getFunctionReflection();

$functionReturnType = $function->getReturnType();
return $this->check->checkFunction(
$node,
$functionReturnType,
$function->getReturnType(),
$function->getPhpDocReturnType(),
sprintf(
'Function %s()',
$function->getName(),
),
false,
$scope,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,17 @@ public function processNode(Node $node, Scope $scope): array
}
}

$methodReturnType = $method->getReturnType();
return $this->check->checkFunction(
$node,
$methodReturnType,
$method->getReturnType(),
$method->getPhpDocReturnType(),
sprintf(
'Method %s::%s()',
$method->getDeclaringClass()->getDisplayName(),
$method->getName(),
),
!$isFirstDeclaration && !$method->isPrivate(),
$scope,
);
}

Expand Down
7 changes: 5 additions & 2 deletions src/Rules/TooWideTypehints/TooWidePropertyTypeRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
use PHPStan\Rules\Properties\ReadWritePropertiesExtensionProvider;
use PHPStan\Rules\Rule;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\UnionType;
use function count;
use function sprintf;

Expand Down Expand Up @@ -57,9 +56,13 @@ public function processNode(Node $node, Scope $scope): array

$propertyReflection = $classReflection->getNativeProperty($propertyName);
$propertyType = $propertyReflection->getWritableType();
if (!$propertyType instanceof UnionType) {
$phpdocType = $propertyReflection->getPhpDocType();

$propertyType = $this->check->findTypeToCheck($propertyType, $phpdocType, $scope);
if ($propertyType === null) {
continue;
}

foreach ($this->extensionProvider->getExtensions() as $extension) {
if ($extension->isAlwaysRead($propertyReflection, $propertyName)) {
continue 2;
Expand Down
100 changes: 94 additions & 6 deletions src/Rules/TooWideTypehints/TooWideTypeCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,18 @@

namespace PHPStan\Rules\TooWideTypehints;

use PHPStan\Analyser\Scope;
use PHPStan\DependencyInjection\AutowiredParameter;
use PHPStan\DependencyInjection\AutowiredService;
use PHPStan\Node\ClassPropertyNode;
use PHPStan\Node\FunctionReturnStatementsNode;
use PHPStan\Node\MethodReturnStatementsNode;
use PHPStan\Rules\IdentifierRuleError;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\TypehintHelper;
use PHPStan\Type\TypeUtils;
use PHPStan\Type\UnionType;
use PHPStan\Type\VerbosityLevel;
Expand All @@ -21,20 +25,28 @@
final class TooWideTypeCheck
{

public function __construct(
#[AutowiredParameter(ref: '%featureToggles.reportTooWideBool%')]
private bool $reportTooWideBool,
)
{
}

/**
* @return list<IdentifierRuleError>
*/
public function checkProperty(
ClassPropertyNode $property,
UnionType $propertyType,
Type $propertyType,
string $propertyDescription,
Type $assignedType,
): array
{
$errors = [];

$verbosityLevel = VerbosityLevel::getRecommendedLevelByType($propertyType, $assignedType);
foreach ($propertyType->getTypes() as $type) {
$propertyTypes = $propertyType instanceof UnionType ? $propertyType->getTypes() : $propertyType->getFiniteTypes();
foreach ($propertyTypes as $type) {
if (!$type->isSuperTypeOf($assignedType)->no()) {
continue;
}
Expand All @@ -43,6 +55,23 @@ public function checkProperty(
continue;
}

if ($propertyType->isBoolean()->yes()) {
$suggestedType = $type->isTrue()->yes() ? new ConstantBooleanType(false) : new ConstantBooleanType(true);

$errors[] = RuleErrorBuilder::message(sprintf(
'%s (%s) is never assigned %s so the property type can be changed to %s.',
$propertyDescription,
$propertyType->describe($verbosityLevel),
$type->describe($verbosityLevel),
$suggestedType->describe($verbosityLevel),
))
->identifier('property.tooWideBool')
->line($property->getStartLine())
->build();

continue;
}

$errors[] = RuleErrorBuilder::message(sprintf(
'%s (%s) is never assigned %s so it can be removed from the property type.',
$propertyDescription,
Expand All @@ -62,15 +91,18 @@ public function checkProperty(
*/
public function checkFunction(
MethodReturnStatementsNode|FunctionReturnStatementsNode $node,
Type $functionReturnType,
Type $nativeFunctionReturnType,
Type $phpdocFunctionReturnType,
string $functionDescription,
bool $checkDescendantClass,
Scope $scope,
): array
{
$functionReturnType = TypeUtils::resolveLateResolvableTypes($functionReturnType);
if (!$functionReturnType instanceof UnionType) {
$functionReturnType = $this->findTypeToCheck($nativeFunctionReturnType, $phpdocFunctionReturnType, $scope);
if ($functionReturnType === null) {
return [];
}

$statementResult = $node->getStatementResult();
if ($statementResult->hasYield()) {
return [];
Expand Down Expand Up @@ -114,7 +146,8 @@ public function checkFunction(
}

$messages = [];
foreach ($functionReturnType->getTypes() as $type) {
$functionReturnTypes = $functionReturnType instanceof UnionType ? $functionReturnType->getTypes() : $functionReturnType->getFiniteTypes();
foreach ($functionReturnTypes as $type) {
if (!$type->isSuperTypeOf($returnType)->no()) {
continue;
}
Expand All @@ -129,6 +162,19 @@ public function checkFunction(
}
}

if ($functionReturnType->isBoolean()->yes()) {
$suggestedType = $type->isTrue()->yes() ? new ConstantBooleanType(false) : new ConstantBooleanType(true);

$messages[] = RuleErrorBuilder::message(sprintf(
'%s never returns %s so the return type can be changed to %s.',
$functionDescription,
$type->describe(VerbosityLevel::getRecommendedLevelByType($type)),
$suggestedType->describe(VerbosityLevel::getRecommendedLevelByType($suggestedType)),
))->identifier('return.tooWideBool')->build();

continue;
}

$messages[] = RuleErrorBuilder::message(sprintf(
'%s never returns %s so it can be removed from the return type.',
$functionDescription,
Expand Down Expand Up @@ -165,4 +211,46 @@ public function checkAnonymousFunction(
return $messages;
}

/**
* Returns null when type should not be checked, e.g. because it would be too annoying.
*/
public function findTypeToCheck(
Type $nativeType,
Type $phpdocType,
Scope $scope,
): ?Type
{
$combinedType = TypeUtils::resolveLateResolvableTypes(TypehintHelper::decideType($nativeType, $phpdocType));
if ($combinedType instanceof UnionType) {
return $combinedType;
}

if (!$this->reportTooWideBool) {
return null;
}

if (
$phpdocType->isBoolean()->yes()
) {
if (
!$phpdocType->isTrue()->yes()
&& !$phpdocType->isFalse()->yes()
) {
return $combinedType;
}
} elseif (
$scope->getPhpVersion()->supportsTrueAndFalseStandaloneType()->yes()
&& $nativeType->isBoolean()->yes()
) {
if (
!$nativeType->isTrue()->yes()
&& !$nativeType->isFalse()->yes()
) {
return $combinedType;
}
}

return null;
}

}
39 changes: 25 additions & 14 deletions tests/PHPStan/Analyser/AnalyserIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -437,14 +437,17 @@ public function testBug4715(): void
$this->assertNoErrors($errors);
}

#[RequiresPhp('>= 8.2')]
public function testBug4734(): void
{
$errors = $this->runAnalyse(__DIR__ . '/data/bug-4734.php');
$this->assertCount(3, $errors);
$this->assertCount(5, $errors); // could be 3
Copy link
Contributor Author

@staabm staabm Aug 30, 2025

Choose a reason for hiding this comment

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

this is a topic for a followup: NodeScopeResolver's ClassStatementsGatherer works on a per class level, which means access to private properties from a closure-scope (from within a different class) are not seen when analyzing the class which declares the property

Copy link
Member

Choose a reason for hiding this comment

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

Yeah of course, static analysis is not fit for these shenanigans 😊 Especially for private properties.


$this->assertSame('Unsafe access to private property Bug4734\Foo::$httpMethodParameterOverride through static::.', $errors[0]->getMessage());
$this->assertSame('Access to an undefined static property static(Bug4734\Foo)::$httpMethodParameterOverride3.', $errors[1]->getMessage());
$this->assertSame('Access to an undefined property Bug4734\Foo::$httpMethodParameterOverride4.', $errors[2]->getMessage());
$this->assertSame('Static property Bug4734\Foo::$httpMethodParameterOverride (bool) is never assigned false so the property type can be changed to true.', $errors[0]->getMessage()); // should not error
$this->assertSame('Property Bug4734\Foo::$httpMethodParameterOverride2 (bool) is never assigned false so the property type can be changed to true.', $errors[1]->getMessage()); // should not error
$this->assertSame('Unsafe access to private property Bug4734\Foo::$httpMethodParameterOverride through static::.', $errors[2]->getMessage());
$this->assertSame('Access to an undefined static property static(Bug4734\Foo)::$httpMethodParameterOverride3.', $errors[3]->getMessage());
$this->assertSame('Access to an undefined property Bug4734\Foo::$httpMethodParameterOverride4.', $errors[4]->getMessage());
}

public function testBug5231(): void
Expand Down Expand Up @@ -1092,19 +1095,27 @@ public function testBug8376(): void
$this->assertNoErrors($errors);
}

#[RequiresPhp('>= 8.0')]
#[RequiresPhp('>= 8.2')]
public function testAssertDocblock(): void
{
$errors = $this->runAnalyse(__DIR__ . '/nsrt/assert-docblock.php');
$this->assertCount(4, $errors);
$this->assertSame('Call to method AssertDocblock\A::testInt() with string will always evaluate to false.', $errors[0]->getMessage());
$this->assertSame(218, $errors[0]->getLine());
$this->assertSame('Call to method AssertDocblock\A::testNotInt() with string will always evaluate to true.', $errors[1]->getMessage());
$this->assertSame(224, $errors[1]->getLine());
$this->assertSame('Call to method AssertDocblock\A::testInt() with int will always evaluate to true.', $errors[2]->getMessage());
$this->assertSame(232, $errors[2]->getLine());
$this->assertSame('Call to method AssertDocblock\A::testNotInt() with int will always evaluate to false.', $errors[3]->getMessage());
$this->assertSame(238, $errors[3]->getLine());
$this->assertCount(8, $errors);
$this->assertSame('Function AssertDocblock\validateStringArrayIfTrue() never returns false so the return type can be changed to true.', $errors[0]->getMessage());
$this->assertSame(17, $errors[0]->getLine());
$this->assertSame('Function AssertDocblock\validateStringArrayIfFalse() never returns true so the return type can be changed to false.', $errors[1]->getMessage());
$this->assertSame(25, $errors[1]->getLine());
$this->assertSame('Function AssertDocblock\validateStringOrIntArray() never returns true so the return type can be changed to false.', $errors[2]->getMessage());
$this->assertSame(34, $errors[2]->getLine());
$this->assertSame('Function AssertDocblock\validateStringOrNonEmptyIntArray() never returns true so the return type can be changed to false.', $errors[3]->getMessage());
$this->assertSame(44, $errors[3]->getLine());
$this->assertSame('Call to method AssertDocblock\A::testInt() with string will always evaluate to false.', $errors[4]->getMessage());
$this->assertSame(218, $errors[4]->getLine());
$this->assertSame('Call to method AssertDocblock\A::testNotInt() with string will always evaluate to true.', $errors[5]->getMessage());
$this->assertSame(224, $errors[5]->getLine());
$this->assertSame('Call to method AssertDocblock\A::testInt() with int will always evaluate to true.', $errors[6]->getMessage());
$this->assertSame(232, $errors[6]->getLine());
$this->assertSame('Call to method AssertDocblock\A::testNotInt() with int will always evaluate to false.', $errors[7]->getMessage());
$this->assertSame(238, $errors[7]->getLine());
}

#[RequiresPhp('>= 8.0')]
Expand Down
12 changes: 12 additions & 0 deletions tests/PHPStan/Rules/Comparison/IfConstantConditionRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -182,4 +182,16 @@ public function testBug8926(): void
$this->analyse([__DIR__ . '/data/bug-8926.php'], []);
}

#[RequiresPhp('>= 8.0')]
public function testBug13384b(): void
{
$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/../TooWideTypehints/data/bug-13384b.php'], [
[
'If condition is always false.',
23,
],
]);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
class LogicalXorConstantConditionRuleTest extends RuleTestCase
{

private bool $reportAlwaysTrueInLastCondition = false;

protected function getRule(): TRule
{
return new LogicalXorConstantConditionRule(
Expand All @@ -26,7 +24,7 @@ protected function getRule(): TRule
$this->shouldTreatPhpDocTypesAsCertain(),
),
$this->shouldTreatPhpDocTypesAsCertain(),
$this->reportAlwaysTrueInLastCondition,
false,
true,
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,9 @@
class TooWideFunctionThrowTypeRuleTest extends RuleTestCase
{

private bool $implicitThrows = true;

protected function getRule(): Rule
{
return new TooWideFunctionThrowTypeRule(new TooWideThrowTypeCheck($this->implicitThrows));
return new TooWideFunctionThrowTypeRule(new TooWideThrowTypeCheck(true));
}

public function testRule(): void
Expand Down
Loading
Loading