mirror of
https://github.com/danog/psalm.git
synced 2024-11-27 04:45:20 +01:00
Merge pull request #6518 from orklah/fix-truthy
improvements of alwaysTruthy/alwaysFalsy
This commit is contained in:
commit
0e83afdb86
@ -5,8 +5,11 @@ Emitted when a paradox is encountered in your programs logic that could not be c
|
||||
```php
|
||||
<?php
|
||||
|
||||
function foo($a) : void {
|
||||
if ($a) return;
|
||||
if ($a) echo "cannot happen";
|
||||
function foo($a, $b) : void {
|
||||
if ($a && $b) {
|
||||
echo "a";
|
||||
} elseif ($a && $b) {
|
||||
echo "cannot happen";
|
||||
}
|
||||
}
|
||||
```
|
||||
|
@ -685,12 +685,9 @@ class ClassAnalyzer extends ClassLikeAnalyzer
|
||||
}
|
||||
}
|
||||
|
||||
if ((($property_storage->signature_type
|
||||
&& !$guide_property_storage->signature_type)
|
||||
|| (!$property_storage->signature_type
|
||||
&& $guide_property_storage->signature_type)
|
||||
if ((($property_storage->signature_type && !$guide_property_storage->signature_type)
|
||||
|| (!$property_storage->signature_type && $guide_property_storage->signature_type)
|
||||
|| ($property_storage->signature_type
|
||||
&& $guide_property_storage->signature_type
|
||||
&& !$property_storage->signature_type->equals(
|
||||
$guide_property_storage->signature_type
|
||||
)))
|
||||
|
@ -110,7 +110,7 @@ class CommentAnalyzer
|
||||
);
|
||||
$description = $parsed_docblock->description;
|
||||
|
||||
if ($line_parts && $line_parts[0]) {
|
||||
if ($line_parts[0]) {
|
||||
$type_start = $offset;
|
||||
$type_end = $type_start + strlen($line_parts[0]);
|
||||
|
||||
|
@ -215,59 +215,7 @@ class IfConditionalAnalyzer
|
||||
)
|
||||
);
|
||||
|
||||
$cond_type = $statements_analyzer->node_data->getType($cond);
|
||||
|
||||
if ($cond_type !== null) {
|
||||
if ($cond_type->isAlwaysFalsy()) {
|
||||
if ($cond_type->from_docblock) {
|
||||
if (IssueBuffer::accepts(
|
||||
new DocblockTypeContradiction(
|
||||
'if (false) is impossible',
|
||||
new CodeLocation($statements_analyzer, $cond),
|
||||
'false falsy'
|
||||
),
|
||||
$statements_analyzer->getSuppressedIssues()
|
||||
)) {
|
||||
// fall through
|
||||
}
|
||||
} else {
|
||||
if (IssueBuffer::accepts(
|
||||
new TypeDoesNotContainType(
|
||||
'if (false) is impossible',
|
||||
new CodeLocation($statements_analyzer, $cond),
|
||||
'false falsy'
|
||||
),
|
||||
$statements_analyzer->getSuppressedIssues()
|
||||
)) {
|
||||
// fall through
|
||||
}
|
||||
}
|
||||
} elseif ($cond_type->isAlwaysTruthy()) {
|
||||
if ($cond_type->from_docblock) {
|
||||
if (IssueBuffer::accepts(
|
||||
new RedundantConditionGivenDocblockType(
|
||||
'if (true) is redundant',
|
||||
new CodeLocation($statements_analyzer, $cond),
|
||||
'true falsy'
|
||||
),
|
||||
$statements_analyzer->getSuppressedIssues()
|
||||
)) {
|
||||
// fall through
|
||||
}
|
||||
} else {
|
||||
if (IssueBuffer::accepts(
|
||||
new RedundantCondition(
|
||||
'if (true) is redundant',
|
||||
new CodeLocation($statements_analyzer, $cond),
|
||||
'true falsy'
|
||||
),
|
||||
$statements_analyzer->getSuppressedIssues()
|
||||
)) {
|
||||
// fall through
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
self::handleParadoxicalCondition($statements_analyzer, $cond);
|
||||
|
||||
// get all the var ids that were referenced in the conditional, but not assigned in it
|
||||
$cond_referenced_var_ids = array_diff_key($cond_referenced_var_ids, $assigned_in_conditional_var_ids);
|
||||
@ -370,4 +318,63 @@ class IfConditionalAnalyzer
|
||||
|
||||
return $stmt;
|
||||
}
|
||||
|
||||
public static function handleParadoxicalCondition(
|
||||
StatementsAnalyzer $statements_analyzer,
|
||||
PhpParser\Node\Expr $stmt
|
||||
): void {
|
||||
$type = $statements_analyzer->node_data->getType($stmt);
|
||||
|
||||
if ($type !== null) {
|
||||
if ($type->isAlwaysFalsy()) {
|
||||
if ($type->from_docblock) {
|
||||
if (IssueBuffer::accepts(
|
||||
new DocblockTypeContradiction(
|
||||
'Operand of type ' . $type->getId() . ' is always false',
|
||||
new CodeLocation($statements_analyzer, $stmt),
|
||||
'false falsy'
|
||||
),
|
||||
$statements_analyzer->getSuppressedIssues()
|
||||
)) {
|
||||
// fall through
|
||||
}
|
||||
} else {
|
||||
if (IssueBuffer::accepts(
|
||||
new TypeDoesNotContainType(
|
||||
'Operand of type ' . $type->getId() . ' is always false',
|
||||
new CodeLocation($statements_analyzer, $stmt),
|
||||
'false falsy'
|
||||
),
|
||||
$statements_analyzer->getSuppressedIssues()
|
||||
)) {
|
||||
// fall through
|
||||
}
|
||||
}
|
||||
} elseif ($type->isAlwaysTruthy() && !$stmt instanceof PhpParser\Node\Expr\Assign) {
|
||||
if ($type->from_docblock) {
|
||||
if (IssueBuffer::accepts(
|
||||
new RedundantConditionGivenDocblockType(
|
||||
'Operand of type ' . $type->getId() . ' is always true',
|
||||
new CodeLocation($statements_analyzer, $stmt),
|
||||
'true falsy'
|
||||
),
|
||||
$statements_analyzer->getSuppressedIssues()
|
||||
)) {
|
||||
// fall through
|
||||
}
|
||||
} else {
|
||||
if (IssueBuffer::accepts(
|
||||
new RedundantCondition(
|
||||
'Operand of type ' . $type->getId() . ' is always true',
|
||||
new CodeLocation($statements_analyzer, $stmt),
|
||||
'true falsy'
|
||||
),
|
||||
$statements_analyzer->getSuppressedIssues()
|
||||
)) {
|
||||
// fall through
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -6,6 +6,7 @@ use Psalm\CodeLocation;
|
||||
use Psalm\Context;
|
||||
use Psalm\Internal\Algebra;
|
||||
use Psalm\Internal\Algebra\FormulaGenerator;
|
||||
use Psalm\Internal\Analyzer\Statements\Block\IfConditionalAnalyzer;
|
||||
use Psalm\Internal\Analyzer\Statements\Block\IfElseAnalyzer;
|
||||
use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer;
|
||||
use Psalm\Internal\Analyzer\StatementsAnalyzer;
|
||||
@ -62,6 +63,8 @@ class AndAnalyzer
|
||||
return false;
|
||||
}
|
||||
|
||||
IfConditionalAnalyzer::handleParadoxicalCondition($statements_analyzer, $stmt->left);
|
||||
|
||||
$codebase = $statements_analyzer->getCodebase();
|
||||
|
||||
$left_cond_id = \spl_object_id($stmt->left);
|
||||
@ -157,6 +160,8 @@ class AndAnalyzer
|
||||
return false;
|
||||
}
|
||||
|
||||
IfConditionalAnalyzer::handleParadoxicalCondition($statements_analyzer, $stmt->right);
|
||||
|
||||
$context->referenced_var_ids = array_merge(
|
||||
$right_context->referenced_var_ids,
|
||||
$left_context->referenced_var_ids
|
||||
|
@ -100,6 +100,8 @@ class OrAnalyzer
|
||||
return false;
|
||||
}
|
||||
|
||||
IfConditionalAnalyzer::handleParadoxicalCondition($statements_analyzer, $stmt->left);
|
||||
|
||||
foreach ($left_context->vars_in_scope as $var_id => $type) {
|
||||
if (!isset($context->vars_in_scope[$var_id])) {
|
||||
if (isset($left_context->assigned_var_ids[$var_id])) {
|
||||
@ -263,6 +265,8 @@ class OrAnalyzer
|
||||
return false;
|
||||
}
|
||||
|
||||
IfConditionalAnalyzer::handleParadoxicalCondition($statements_analyzer, $stmt->right);
|
||||
|
||||
$right_referenced_var_ids = $right_context->referenced_var_ids;
|
||||
$right_context->referenced_var_ids = array_merge($pre_referenced_var_ids, $right_referenced_var_ids);
|
||||
|
||||
|
@ -14,8 +14,7 @@ class BooleanNotAnalyzer
|
||||
PhpParser\Node\Expr\BooleanNot $stmt,
|
||||
Context $context
|
||||
) : bool {
|
||||
$stmt_type = Type::getBool();
|
||||
$statements_analyzer->node_data->setType($stmt, $stmt_type);
|
||||
|
||||
|
||||
$inside_negation = $context->inside_negation;
|
||||
|
||||
@ -27,10 +26,20 @@ class BooleanNotAnalyzer
|
||||
|
||||
$expr_type = $statements_analyzer->node_data->getType($stmt->expr);
|
||||
|
||||
$stmt_type = Type::getBool();
|
||||
if ($expr_type) {
|
||||
if ($expr_type->isAlwaysTruthy()) {
|
||||
$stmt_type = Type::getFalse();
|
||||
} elseif ($expr_type->isAlwaysFalsy()) {
|
||||
$stmt_type = Type::getTrue();
|
||||
}
|
||||
|
||||
$stmt_type->from_docblock = $expr_type->from_docblock;
|
||||
$stmt_type->parent_nodes = $expr_type->parent_nodes;
|
||||
}
|
||||
|
||||
$statements_analyzer->node_data->setType($stmt, $stmt_type);
|
||||
|
||||
return $result;
|
||||
}
|
||||
}
|
||||
|
@ -132,7 +132,7 @@ class ArgumentsAnalyzer
|
||||
|
||||
$by_ref_type = null;
|
||||
|
||||
if ($by_ref && $param) {
|
||||
if ($by_ref) {
|
||||
$by_ref_type = $param->type ? clone $param->type : Type::getMixed();
|
||||
}
|
||||
|
||||
|
@ -307,6 +307,7 @@ class ArrayFetchAnalyzer
|
||||
$stmt_type->possibly_undefined = false;
|
||||
}
|
||||
|
||||
/** @psalm-suppress RedundantCondition can be empty after removing above */
|
||||
if ($context->inside_isset && $dim_var_id && $new_offset_type && $new_offset_type->getAtomicTypes()) {
|
||||
$context->vars_in_scope[$dim_var_id] = $new_offset_type;
|
||||
}
|
||||
|
@ -828,7 +828,7 @@ class Methods
|
||||
if ((!$old_contained_by_new && !$new_contained_by_old)
|
||||
|| ($old_contained_by_new && $new_contained_by_old)
|
||||
) {
|
||||
if ($old_contained_by_new && $new_contained_by_old) {
|
||||
if ($old_contained_by_new) { //implicitly $new_contained_by_old as well
|
||||
$attempted_intersection = Type::intersectUnionTypes(
|
||||
$candidate_type,
|
||||
$overridden_storage->return_type,
|
||||
|
@ -144,6 +144,7 @@ class ArrayFilterReturnTypeProvider implements \Psalm\Plugin\EventHandler\Functi
|
||||
$key_type->addType(new Type\Atomic\TInt);
|
||||
}
|
||||
|
||||
/** @psalm-suppress TypeDoesNotContainType can be empty after removing above */
|
||||
if (!$inner_type->getAtomicTypes()) {
|
||||
return Type::getEmptyArray();
|
||||
}
|
||||
@ -296,6 +297,7 @@ class ArrayFilterReturnTypeProvider implements \Psalm\Plugin\EventHandler\Functi
|
||||
]);
|
||||
}
|
||||
|
||||
/** @psalm-suppress TypeDoesNotContainType can be empty after removing above */
|
||||
if (!$inner_type->getAtomicTypes()) {
|
||||
return Type::getEmptyArray();
|
||||
}
|
||||
|
@ -576,7 +576,7 @@ abstract class Type
|
||||
return $type_1;
|
||||
}
|
||||
|
||||
if ($type_1_mixed && !$type_2_mixed) {
|
||||
if ($type_1_mixed) {
|
||||
$combined_type = clone $type_2;
|
||||
$intersection_performed = true;
|
||||
} elseif ($type_2_mixed) {
|
||||
|
@ -208,6 +208,7 @@ class Reconciler
|
||||
$negated
|
||||
);
|
||||
|
||||
/** @psalm-suppress TypeDoesNotContainType can be empty after removing above */
|
||||
if (!$result_type_candidate->getAtomicTypes()) {
|
||||
$result_type_candidate->addType(new TEmpty);
|
||||
}
|
||||
@ -1092,6 +1093,7 @@ class Reconciler
|
||||
}
|
||||
}
|
||||
|
||||
/** @psalm-suppress TypeDoesNotContainType can be empty after removing above */
|
||||
if (!$key_type->getAtomicTypes()) {
|
||||
// this should ideally prompt some sort of error
|
||||
$key_type->addType(new Type\Atomic\TArrayKey());
|
||||
|
@ -978,13 +978,25 @@ class Union implements TypeNode
|
||||
if ($atomic_type instanceof Type\Atomic\TLiteralString &&
|
||||
($atomic_type->value === '' || $atomic_type->value === '0')
|
||||
) {
|
||||
continue;
|
||||
continue;
|
||||
}
|
||||
|
||||
if ($atomic_type instanceof Type\Atomic\TNull) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if ($atomic_type instanceof Type\Atomic\TEmptyMixed) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if ($atomic_type instanceof Type\Atomic\TEmptyNumeric) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if ($atomic_type instanceof Type\Atomic\TEmptyScalar) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if ($atomic_type instanceof TTemplateParam && $atomic_type->as->isAlwaysFalsy()) {
|
||||
continue;
|
||||
}
|
||||
@ -1010,6 +1022,10 @@ class Union implements TypeNode
|
||||
|
||||
public function isAlwaysTruthy(): bool
|
||||
{
|
||||
if ($this->possibly_undefined || $this->possibly_undefined_from_try) {
|
||||
return false;
|
||||
}
|
||||
|
||||
foreach ($this->getAtomicTypes() as $atomic_type) {
|
||||
if ($atomic_type instanceof Type\Atomic\TTrue) {
|
||||
continue;
|
||||
@ -1029,7 +1045,11 @@ class Union implements TypeNode
|
||||
continue;
|
||||
}
|
||||
|
||||
if ($atomic_type instanceof Type\Atomic\TNonFalsyString) {
|
||||
if ($atomic_type instanceof Type\Atomic\TNonEmptyString) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if ($atomic_type instanceof Type\Atomic\TNonEmptyNonspecificLiteralString) {
|
||||
continue;
|
||||
}
|
||||
|
||||
@ -1065,10 +1085,18 @@ class Union implements TypeNode
|
||||
continue;
|
||||
}
|
||||
|
||||
if ($atomic_type instanceof Type\Atomic\TTraitString) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if ($atomic_type instanceof Type\Atomic\TResource) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if ($atomic_type instanceof Type\Atomic\TKeyedArray) {
|
||||
foreach ($atomic_type->properties as $property) {
|
||||
if ($property->possibly_undefined === false) {
|
||||
continue;
|
||||
continue 2;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -3086,6 +3086,23 @@ class ConditionalTest extends \Psalm\Tests\TestCase
|
||||
}',
|
||||
'error_message' => 'TypeDoesNotContainType',
|
||||
],
|
||||
'BooleanNotOfAlwaysTruthyisFalse' => [
|
||||
'<?php
|
||||
class a
|
||||
{
|
||||
public function fluent(): self
|
||||
{
|
||||
return $this;
|
||||
}
|
||||
}
|
||||
|
||||
$a = new a();
|
||||
if (!$a->fluent()) {
|
||||
echo "always";
|
||||
}
|
||||
',
|
||||
'error_message' => 'TypeDoesNotContainType',
|
||||
],
|
||||
];
|
||||
}
|
||||
}
|
||||
|
@ -943,14 +943,14 @@ class RedundantConditionTest extends \Psalm\Tests\TestCase
|
||||
'<?php
|
||||
$a = rand(0, 10) > 5;
|
||||
if ($a || $a) {}',
|
||||
'error_message' => 'ParadoxicalCondition',
|
||||
'error_message' => 'TypeDoesNotContainType',
|
||||
],
|
||||
'typeResolutionRepeatingOredConditionWithVarInMiddle' => [
|
||||
'<?php
|
||||
$a = rand(0, 10) > 5;
|
||||
$b = rand(0, 10) > 5;
|
||||
if ($a || $b || $a) {}',
|
||||
'error_message' => 'ParadoxicalCondition',
|
||||
'error_message' => 'TypeDoesNotContainType',
|
||||
],
|
||||
'typeResolutionIsIntAndIsNumeric' => [
|
||||
'<?php
|
||||
@ -1416,6 +1416,26 @@ class RedundantConditionTest extends \Psalm\Tests\TestCase
|
||||
}',
|
||||
'error_message' => 'DocblockTypeContradiction',
|
||||
],
|
||||
'OrTrue' => [
|
||||
'<?php
|
||||
if(rand(0,1) || true){}',
|
||||
'error_message' => 'RedundantCondition',
|
||||
],
|
||||
'AndTrue' => [
|
||||
'<?php
|
||||
if(rand(0,1) && true){}',
|
||||
'error_message' => 'RedundantCondition',
|
||||
],
|
||||
'OrFalse' => [
|
||||
'<?php
|
||||
if(rand(0,1) || false){}',
|
||||
'error_message' => 'TypeDoesNotContainType',
|
||||
],
|
||||
'AndFalse' => [
|
||||
'<?php
|
||||
if(rand(0,1) && false){}',
|
||||
'error_message' => 'TypeDoesNotContainType',
|
||||
]
|
||||
];
|
||||
}
|
||||
}
|
||||
|
@ -45,7 +45,7 @@ class ScopeTest extends \Psalm\Tests\TestCase
|
||||
],
|
||||
'functionExists' => [
|
||||
'<?php
|
||||
if (true && function_exists("flabble")) {
|
||||
if (rand(0,1) && function_exists("flabble")) {
|
||||
flabble();
|
||||
}',
|
||||
],
|
||||
|
@ -1200,7 +1200,7 @@ class TypeAlgebraTest extends \Psalm\Tests\TestCase
|
||||
if (!$a) return $b;
|
||||
return $a;
|
||||
}',
|
||||
'error_message' => 'ParadoxicalCondition',
|
||||
'error_message' => 'TypeDoesNotContainType',
|
||||
],
|
||||
'twoVarLogicNotNestedWithElseifIncorrectlyReinforcedInIf' => [
|
||||
'<?php
|
||||
|
Loading…
Reference in New Issue
Block a user