Skip to content

Commit

Permalink
StrictComparisonOfDifferentTypesRule - tip about treatPhpDocTypesAsCe…
Browse files Browse the repository at this point in the history 
…rtain
  • Loading branch information
@ondrejmirtes
ondrejmirtes committed Mar 29, 2023
1 parent ea2670a commit 40400ae
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 4 deletions.
21 changes: 17 additions & 4 deletions src/Rules/Comparison/StrictComparisonOfDifferentTypesRule.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,27 +44,40 @@ public function processNode(Node $node, Scope $scope): array
$leftType = $this->treatPhpDocTypesAsCertain ? $scope->getType($node->left) : $scope->getNativeType($node->left);
$rightType = $this->treatPhpDocTypesAsCertain ? $scope->getType($node->right) : $scope->getNativeType($node->right);

$addTip = function (RuleErrorBuilder $ruleErrorBuilder) use ($scope, $node): RuleErrorBuilder {
if (!$this->treatPhpDocTypesAsCertain) {
return $ruleErrorBuilder;
}

$instanceofTypeWithoutPhpDocs = $scope->getNativeType($node);
if ($instanceofTypeWithoutPhpDocs instanceof ConstantBooleanType) {
return $ruleErrorBuilder;
}

return $ruleErrorBuilder->tip('Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.');
};

if (!$nodeType->getValue()) {
return [
RuleErrorBuilder::message(sprintf(
$addTip(RuleErrorBuilder::message(sprintf(
'Strict comparison using %s between %s and %s will always evaluate to false.',
$node instanceof Node\Expr\BinaryOp\Identical ? '===' : '!==',
$leftType->describe(VerbosityLevel::value()),
$rightType->describe(VerbosityLevel::value()),
))->build(),
)))->build(),
];
} elseif ($this->checkAlwaysTrueStrictComparison) {
$isLast = $node->getAttribute(LastConditionVisitor::ATTRIBUTE_NAME);
if ($isLast === true && !$this->reportAlwaysTrueInLastCondition) {
return [];
}

$errorBuilder = RuleErrorBuilder::message(sprintf(
$errorBuilder = $addTip(RuleErrorBuilder::message(sprintf(
'Strict comparison using %s between %s and %s will always evaluate to true.',
$node instanceof Node\Expr\BinaryOp\Identical ? '===' : '!==',
$leftType->describe(VerbosityLevel::value()),
$rightType->describe(VerbosityLevel::value()),
));
)));
if ($isLast === false && !$this->reportAlwaysTrueInLastCondition) {
$errorBuilder->tip('Remove remaining cases below this one and this error will disappear too.');
}
Expand Down
33 changes: 33 additions & 0 deletions tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ protected function shouldTreatPhpDocTypesAsCertain(): bool
public function testStrictComparison(): void
{
$this->checkAlwaysTrueStrictComparison = true;
$tipText = 'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.';
$this->analyse(
[__DIR__ . '/data/strict-comparison.php'],
[
Expand All @@ -58,6 +59,7 @@ public function testStrictComparison(): void
[
'Strict comparison using === between 1 and array<StrictComparison\Foo>|bool|StrictComparison\Collection will always evaluate to false.',
19,
$tipText,
],
[
'Strict comparison using === between true and false will always evaluate to false.',
Expand Down Expand Up @@ -110,10 +112,12 @@ public function testStrictComparison(): void
[
'Strict comparison using !== between StrictComparison\Node|null and false will always evaluate to true.',
212,
$tipText,
],
[
'Strict comparison using !== between StrictComparison\Node|null and false will always evaluate to true.',
255,
$tipText,
],
[
'Strict comparison using !== between stdClass and null will always evaluate to true.',
Expand Down Expand Up @@ -182,6 +186,7 @@ public function testStrictComparison(): void
[
'Strict comparison using === between int<0, 1> and 100 will always evaluate to false.',
622,
$tipText,
],
[
'Strict comparison using === between 100 and \'foo\' will always evaluate to false.',
Expand Down Expand Up @@ -267,6 +272,7 @@ public function testStrictComparison(): void
public function testStrictComparisonWithoutAlwaysTrue(): void
{
$this->checkAlwaysTrueStrictComparison = false;
$tipText = 'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.';
$this->analyse(
[__DIR__ . '/data/strict-comparison.php'],
[
Expand All @@ -285,6 +291,7 @@ public function testStrictComparisonWithoutAlwaysTrue(): void
[
'Strict comparison using === between 1 and array<StrictComparison\Foo>|bool|StrictComparison\Collection will always evaluate to false.',
19,
$tipText,
],
[
'Strict comparison using === between true and false will always evaluate to false.',
Expand Down Expand Up @@ -377,6 +384,7 @@ public function testStrictComparisonWithoutAlwaysTrue(): void
[
'Strict comparison using === between int<0, 1> and 100 will always evaluate to false.',
622,
$tipText,
],
[
'Strict comparison using === between 100 and \'foo\' will always evaluate to false.',
Expand Down Expand Up @@ -572,6 +580,7 @@ public function testBug7555(): void
[
'Strict comparison using === between 2 and 2 will always evaluate to true.',
11,
'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.',
],
]);
}
Expand Down Expand Up @@ -693,14 +702,17 @@ public function testBug4242(): void
public function testBug3633(): void
{
$this->checkAlwaysTrueStrictComparison = true;
$tipText = 'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.';
$this->analyse([__DIR__ . '/data/bug-3633.php'], [
[
'Strict comparison using === between class-string<Bug3633\HelloWorld> and \'Bug3633\\\OtherClass\' will always evaluate to false.',
37,
$tipText,
],
[
'Strict comparison using === between \'Bug3633\\\HelloWorld\' and \'Bug3633\\\HelloWorld\' will always evaluate to true.',
41,
$tipText,
],
[
'Strict comparison using === between \'Bug3633\\\HelloWorld\' and \'Bug3633\\\OtherClass\' will always evaluate to false.',
Expand All @@ -709,38 +721,47 @@ public function testBug3633(): void
[
'Strict comparison using === between class-string<Bug3633\OtherClass> and \'Bug3633\\\HelloWorld\' will always evaluate to false.',
64,
$tipText,
],
[
'Strict comparison using === between \'Bug3633\\\OtherClass\' and \'Bug3633\\\HelloWorld\' will always evaluate to false.',
71,
$tipText,
],
[
'Strict comparison using === between \'Bug3633\\\OtherClass\' and \'Bug3633\\\OtherClass\' will always evaluate to true.',
74,
$tipText,
],
[
'Strict comparison using === between class-string<Bug3633\FinalClass> and \'Bug3633\\\HelloWorld\' will always evaluate to false.',
93,
$tipText,
],
[
'Strict comparison using === between class-string<Bug3633\FinalClass> and \'Bug3633\\\OtherClass\' will always evaluate to false.',
96,
$tipText,
],
[
'Strict comparison using === between \'Bug3633\\\FinalClass\' and \'Bug3633\\\FinalClass\' will always evaluate to true.',
102,
$tipText,
],
[
'Strict comparison using === between \'Bug3633\\\FinalClass\' and \'Bug3633\\\HelloWorld\' will always evaluate to false.',
106,
$tipText,
],
[
'Strict comparison using === between \'Bug3633\\\FinalClass\' and \'Bug3633\\\OtherClass\' will always evaluate to false.',
109,
$tipText,
],
[
'Strict comparison using !== between \'Bug3633\\\FinalClass\' and \'Bug3633\\\FinalClass\' will always evaluate to false.',
112,
$tipText,
],
[
'Strict comparison using === between \'Bug3633\\\FinalClass\' and \'Bug3633\\\FinalClass\' will always evaluate to true.',
Expand Down Expand Up @@ -903,4 +924,16 @@ public function testBug5978(): void
$this->analyse([__DIR__ . '/data/bug-5978.php'], $expectedErrors);
}

public function testBug9104(): void
{
$this->checkAlwaysTrueStrictComparison = true;
$this->analyse([__DIR__ . '/data/bug-9104.php'], [
[
'Strict comparison using === between int<1, max> and 0 will always evaluate to false.',
12,
'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.',
],
]);
}

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

namespace Bug9104;

class ArrayLibrary
{
/**
* @param non-empty-list<int> $list
*/
public function getFirst(array $list): int
{
if (count($list) === 0) {
throw new \LogicException('empty array');
}

return $list[0];
}
}

0 comments on commit 40400ae

Please sign in to comment.