Skip to content

Commit b62441d

Browse files
staabmstaabm
authored
Bleeding edge - report bool type as too-wide when only true or false is assigned
1 parent a1d56e2 commit b62441d

24 files changed

+540
-44
lines changed

‎conf/bleedingEdge.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,4 @@ parameters:
1010
internalTag: true
1111
newStaticInAbstractClassStaticMethod: true
1212
checkExtensionsForComparisonOperators: true
13+
reportTooWideBool: true

‎conf/config.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ parameters:
3434
internalTag: false
3535
newStaticInAbstractClassStaticMethod: false
3636
checkExtensionsForComparisonOperators: false
37+
reportTooWideBool: false
3738
fileExtensions:
3839
- php
3940
checkAdvancedIsset: false

‎conf/parametersSchema.neon‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ parametersSchema:
3737
internalTag: bool()
3838
newStaticInAbstractClassStaticMethod: bool()
3939
checkExtensionsForComparisonOperators: bool()
40+
reportTooWideBool: bool()
4041
])
4142
fileExtensions: listOf(string())
4243
checkAdvancedIsset: bool()

‎src/Php/PhpVersions.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,9 @@ public function supportsNamedArgumentAfterUnpackedArgument(): TrinaryLogic
4343
return IntegerRangeType::fromInterval(80100, null)->isSuperTypeOf($this->phpVersions)->result;
4444
}
4545

46+
public function supportsTrueAndFalseStandaloneType(): TrinaryLogic
47+
{
48+
return IntegerRangeType::fromInterval(80200, null)->isSuperTypeOf($this->phpVersions)->result;
49+
}
50+
4651
}

‎src/Reflection/BetterReflection/SourceLocator/FileReadTrapStreamWrapper.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,9 @@ public function stream_eof(): bool
212212
return $this->readFromFile;
213213
}
214214

215+
/**
216+
* @return true
217+
*/
215218
public function stream_flush(): bool
216219
{
217220
return true;
@@ -254,6 +257,8 @@ public function stream_seek($offset, $whence): bool
254257
* @param int $option
255258
* @param int $arg1
256259
* @param int $arg2
260+
*
261+
* @return false
257262
*/
258263
public function stream_set_option($option, $arg1, $arg2): bool
259264
{

‎src/Rules/TooWideTypehints/TooWideFunctionReturnTypehintRule.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,16 @@ public function processNode(Node $node, Scope $scope): array
3131
{
3232
$function = $node->getFunctionReflection();
3333

34-
$functionReturnType = $function->getReturnType();
3534
return $this->check->checkFunction(
3635
$node,
37-
$functionReturnType,
36+
$function->getReturnType(),
37+
$function->getPhpDocReturnType(),
3838
sprintf(
3939
'Function %s()',
4040
$function->getName(),
4141
),
4242
false,
43+
$scope,
4344
);
4445
}
4546

‎src/Rules/TooWideTypehints/TooWideMethodReturnTypehintRule.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,16 +49,17 @@ public function processNode(Node $node, Scope $scope): array
4949
}
5050
}
5151

52-
$methodReturnType = $method->getReturnType();
5352
return $this->check->checkFunction(
5453
$node,
55-
$methodReturnType,
54+
$method->getReturnType(),
55+
$method->getPhpDocReturnType(),
5656
sprintf(
5757
'Method %s::%s()',
5858
$method->getDeclaringClass()->getDisplayName(),
5959
$method->getName(),
6060
),
6161
!$isFirstDeclaration && !$method->isPrivate(),
62+
$scope,
6263
);
6364
}
6465

‎src/Rules/TooWideTypehints/TooWidePropertyTypeRule.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
use PHPStan\Rules\Properties\ReadWritePropertiesExtensionProvider;
1212
use PHPStan\Rules\Rule;
1313
use PHPStan\Type\TypeCombinator;
14-
use PHPStan\Type\UnionType;
1514
use function count;
1615
use function sprintf;
1716

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

5857
$propertyReflection = $classReflection->getNativeProperty($propertyName);
5958
$propertyType = $propertyReflection->getWritableType();
60-
if (!$propertyType instanceof UnionType) {
59+
$phpdocType = $propertyReflection->getPhpDocType();
60+
61+
$propertyType = $this->check->findTypeToCheck($propertyType, $phpdocType, $scope);
62+
if ($propertyType === null) {
6163
continue;
6264
}
65+
6366
foreach ($this->extensionProvider->getExtensions() as $extension) {
6467
if ($extension->isAlwaysRead($propertyReflection, $propertyName)) {
6568
continue 2;

‎src/Rules/TooWideTypehints/TooWideTypeCheck.php‎

Lines changed: 94 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,18 @@
22

33
namespace PHPStan\Rules\TooWideTypehints;
44

5+
use PHPStan\Analyser\Scope;
6+
use PHPStan\DependencyInjection\AutowiredParameter;
57
use PHPStan\DependencyInjection\AutowiredService;
68
use PHPStan\Node\ClassPropertyNode;
79
use PHPStan\Node\FunctionReturnStatementsNode;
810
use PHPStan\Node\MethodReturnStatementsNode;
911
use PHPStan\Rules\IdentifierRuleError;
1012
use PHPStan\Rules\RuleErrorBuilder;
13+
use PHPStan\Type\Constant\ConstantBooleanType;
1114
use PHPStan\Type\Type;
1215
use PHPStan\Type\TypeCombinator;
16+
use PHPStan\Type\TypehintHelper;
1317
use PHPStan\Type\TypeUtils;
1418
use PHPStan\Type\UnionType;
1519
use PHPStan\Type\VerbosityLevel;
@@ -21,20 +25,28 @@
2125
final class TooWideTypeCheck
2226
{
2327

28+
public function __construct(
29+
#[AutowiredParameter(ref: '%featureToggles.reportTooWideBool%')]
30+
private bool $reportTooWideBool,
31+
)
32+
{
33+
}
34+
2435
/**
2536
* @return list<IdentifierRuleError>
2637
*/
2738
public function checkProperty(
2839
ClassPropertyNode $property,
29-
UnionType $propertyType,
40+
Type $propertyType,
3041
string $propertyDescription,
3142
Type $assignedType,
3243
): array
3344
{
3445
$errors = [];
3546

3647
$verbosityLevel = VerbosityLevel::getRecommendedLevelByType($propertyType, $assignedType);
37-
foreach ($propertyType->getTypes() as $type) {
48+
$propertyTypes = $propertyType instanceof UnionType ? $propertyType->getTypes() : $propertyType->getFiniteTypes();
49+
foreach ($propertyTypes as $type) {
3850
if (!$type->isSuperTypeOf($assignedType)->no()) {
3951
continue;
4052
}
@@ -43,6 +55,23 @@ public function checkProperty(
4355
continue;
4456
}
4557

58+
if ($propertyType->isBoolean()->yes()) {
59+
$suggestedType = $type->isTrue()->yes() ? new ConstantBooleanType(false) : new ConstantBooleanType(true);
60+
61+
$errors[] = RuleErrorBuilder::message(sprintf(
62+
'%s (%s) is never assigned %s so the property type can be changed to %s.',
63+
$propertyDescription,
64+
$propertyType->describe($verbosityLevel),
65+
$type->describe($verbosityLevel),
66+
$suggestedType->describe($verbosityLevel),
67+
))
68+
->identifier('property.tooWideBool')
69+
->line($property->getStartLine())
70+
->build();
71+
72+
continue;
73+
}
74+
4675
$errors[] = RuleErrorBuilder::message(sprintf(
4776
'%s (%s) is never assigned %s so it can be removed from the property type.',
4877
$propertyDescription,
@@ -62,15 +91,18 @@ public function checkProperty(
6291
*/
6392
public function checkFunction(
6493
MethodReturnStatementsNode|FunctionReturnStatementsNode $node,
65-
Type $functionReturnType,
94+
Type $nativeFunctionReturnType,
95+
Type $phpdocFunctionReturnType,
6696
string $functionDescription,
6797
bool $checkDescendantClass,
98+
Scope $scope,
6899
): array
69100
{
70-
$functionReturnType = TypeUtils::resolveLateResolvableTypes($functionReturnType);
71-
if (!$functionReturnType instanceof UnionType) {
101+
$functionReturnType = $this->findTypeToCheck($nativeFunctionReturnType, $phpdocFunctionReturnType, $scope);
102+
if ($functionReturnType === null) {
72103
return [];
73104
}
105+
74106
$statementResult = $node->getStatementResult();
75107
if ($statementResult->hasYield()) {
76108
return [];
@@ -114,7 +146,8 @@ public function checkFunction(
114146
}
115147

116148
$messages = [];
117-
foreach ($functionReturnType->getTypes() as $type) {
149+
$functionReturnTypes = $functionReturnType instanceof UnionType ? $functionReturnType->getTypes() : $functionReturnType->getFiniteTypes();
150+
foreach ($functionReturnTypes as $type) {
118151
if (!$type->isSuperTypeOf($returnType)->no()) {
119152
continue;
120153
}
@@ -129,6 +162,19 @@ public function checkFunction(
129162
}
130163
}
131164

165+
if ($functionReturnType->isBoolean()->yes()) {
166+
$suggestedType = $type->isTrue()->yes() ? new ConstantBooleanType(false) : new ConstantBooleanType(true);
167+
168+
$messages[] = RuleErrorBuilder::message(sprintf(
169+
'%s never returns %s so the return type can be changed to %s.',
170+
$functionDescription,
171+
$type->describe(VerbosityLevel::getRecommendedLevelByType($type)),
172+
$suggestedType->describe(VerbosityLevel::getRecommendedLevelByType($suggestedType)),
173+
))->identifier('return.tooWideBool')->build();
174+
175+
continue;
176+
}
177+
132178
$messages[] = RuleErrorBuilder::message(sprintf(
133179
'%s never returns %s so it can be removed from the return type.',
134180
$functionDescription,
@@ -165,4 +211,46 @@ public function checkAnonymousFunction(
165211
return $messages;
166212
}
167213

214+
/**
215+
* Returns null when type should not be checked, e.g. because it would be too annoying.
216+
*/
217+
public function findTypeToCheck(
218+
Type $nativeType,
219+
Type $phpdocType,
220+
Scope $scope,
221+
): ?Type
222+
{
223+
$combinedType = TypeUtils::resolveLateResolvableTypes(TypehintHelper::decideType($nativeType, $phpdocType));
224+
if ($combinedType instanceof UnionType) {
225+
return $combinedType;
226+
}
227+
228+
if (!$this->reportTooWideBool) {
229+
return null;
230+
}
231+
232+
if (
233+
$phpdocType->isBoolean()->yes()
234+
) {
235+
if (
236+
!$phpdocType->isTrue()->yes()
237+
&& !$phpdocType->isFalse()->yes()
238+
) {
239+
return $combinedType;
240+
}
241+
} elseif (
242+
$scope->getPhpVersion()->supportsTrueAndFalseStandaloneType()->yes()
243+
&& $nativeType->isBoolean()->yes()
244+
) {
245+
if (
246+
!$nativeType->isTrue()->yes()
247+
&& !$nativeType->isFalse()->yes()
248+
) {
249+
return $combinedType;
250+
}
251+
}
252+
253+
return null;
254+
}
255+
168256
}

‎tests/PHPStan/Analyser/AnalyserIntegrationTest.php‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -437,14 +437,17 @@ public function testBug4715(): void
437437
$this->assertNoErrors($errors);
438438
}
439439

440+
#[RequiresPhp('>= 8.2')]
440441
public function testBug4734(): void
441442
{
442443
$errors = $this->runAnalyse(__DIR__ . '/data/bug-4734.php');
443-
$this->assertCount(3, $errors);
444+
$this->assertCount(5, $errors); // could be 3
444445

445-
$this->assertSame('Unsafe access to private property Bug4734\Foo::$httpMethodParameterOverride through static::.', $errors[0]->getMessage());
446-
$this->assertSame('Access to an undefined static property static(Bug4734\Foo)::$httpMethodParameterOverride3.', $errors[1]->getMessage());
447-
$this->assertSame('Access to an undefined property Bug4734\Foo::$httpMethodParameterOverride4.', $errors[2]->getMessage());
446+
$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
447+
$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
448+
$this->assertSame('Unsafe access to private property Bug4734\Foo::$httpMethodParameterOverride through static::.', $errors[2]->getMessage());
449+
$this->assertSame('Access to an undefined static property static(Bug4734\Foo)::$httpMethodParameterOverride3.', $errors[3]->getMessage());
450+
$this->assertSame('Access to an undefined property Bug4734\Foo::$httpMethodParameterOverride4.', $errors[4]->getMessage());
448451
}
449452

450453
public function testBug5231(): void
@@ -1092,19 +1095,27 @@ public function testBug8376(): void
10921095
$this->assertNoErrors($errors);
10931096
}
10941097

1095-
#[RequiresPhp('>= 8.0')]
1098+
#[RequiresPhp('>= 8.2')]
10961099
public function testAssertDocblock(): void
10971100
{
10981101
$errors = $this->runAnalyse(__DIR__ . '/nsrt/assert-docblock.php');
1099-
$this->assertCount(4, $errors);
1100-
$this->assertSame('Call to method AssertDocblock\A::testInt() with string will always evaluate to false.', $errors[0]->getMessage());
1101-
$this->assertSame(218, $errors[0]->getLine());
1102-
$this->assertSame('Call to method AssertDocblock\A::testNotInt() with string will always evaluate to true.', $errors[1]->getMessage());
1103-
$this->assertSame(224, $errors[1]->getLine());
1104-
$this->assertSame('Call to method AssertDocblock\A::testInt() with int will always evaluate to true.', $errors[2]->getMessage());
1105-
$this->assertSame(232, $errors[2]->getLine());
1106-
$this->assertSame('Call to method AssertDocblock\A::testNotInt() with int will always evaluate to false.', $errors[3]->getMessage());
1107-
$this->assertSame(238, $errors[3]->getLine());
1102+
$this->assertCount(8, $errors);
1103+
$this->assertSame('Function AssertDocblock\validateStringArrayIfTrue() never returns false so the return type can be changed to true.', $errors[0]->getMessage());
1104+
$this->assertSame(17, $errors[0]->getLine());
1105+
$this->assertSame('Function AssertDocblock\validateStringArrayIfFalse() never returns true so the return type can be changed to false.', $errors[1]->getMessage());
1106+
$this->assertSame(25, $errors[1]->getLine());
1107+
$this->assertSame('Function AssertDocblock\validateStringOrIntArray() never returns true so the return type can be changed to false.', $errors[2]->getMessage());
1108+
$this->assertSame(34, $errors[2]->getLine());
1109+
$this->assertSame('Function AssertDocblock\validateStringOrNonEmptyIntArray() never returns true so the return type can be changed to false.', $errors[3]->getMessage());
1110+
$this->assertSame(44, $errors[3]->getLine());
1111+
$this->assertSame('Call to method AssertDocblock\A::testInt() with string will always evaluate to false.', $errors[4]->getMessage());
1112+
$this->assertSame(218, $errors[4]->getLine());
1113+
$this->assertSame('Call to method AssertDocblock\A::testNotInt() with string will always evaluate to true.', $errors[5]->getMessage());
1114+
$this->assertSame(224, $errors[5]->getLine());
1115+
$this->assertSame('Call to method AssertDocblock\A::testInt() with int will always evaluate to true.', $errors[6]->getMessage());
1116+
$this->assertSame(232, $errors[6]->getLine());
1117+
$this->assertSame('Call to method AssertDocblock\A::testNotInt() with int will always evaluate to false.', $errors[7]->getMessage());
1118+
$this->assertSame(238, $errors[7]->getLine());
11081119
}
11091120

11101121
#[RequiresPhp('>= 8.0')]

0 commit comments

Comments
 (0)