diff --git a/docs/running_psalm/issues/RedundantIdentityWithTrue.md b/docs/running_psalm/issues/RedundantIdentityWithTrue.md index 0ce985095..86cbee73c 100644 --- a/docs/running_psalm/issues/RedundantIdentityWithTrue.md +++ b/docs/running_psalm/issues/RedundantIdentityWithTrue.md @@ -12,4 +12,8 @@ function returnsABool(): bool { if (returnsABool() === true) { echo "hi!"; } + +if (returnsABool() !== false) { + echo "hi!"; +} ``` diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php b/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php index 1d667e98d..4a19eccf3 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/AssertionFinder.php @@ -2076,41 +2076,58 @@ class AssertionFinder if ($codebase && $source instanceof StatementsAnalyzer && ($var_type = $source->node_data->getType($base_conditional)) + && $conditional instanceof PhpParser\Node\Expr\BinaryOp\NotIdentical ) { - if ($conditional instanceof PhpParser\Node\Expr\BinaryOp\NotIdentical) { - $false_type = Type::getFalse(); + $config = $source->getCodebase()->config; - if (!UnionTypeComparator::isContainedBy( - $codebase, - $var_type, - $false_type - ) && !UnionTypeComparator::isContainedBy( - $codebase, - $false_type, - $var_type + if ($config->strict_binary_operands + && $var_type->isSingle() + && $var_type->hasBool() + && !$var_type->from_docblock + ) { + if (IssueBuffer::accepts( + new RedundantIdentityWithTrue( + 'The "!== false" part of this comparison is redundant', + new CodeLocation($source, $conditional) + ), + $source->getSuppressedIssues() )) { - if ($var_type->from_docblock) { - if (IssueBuffer::accepts( - new RedundantConditionGivenDocblockType( - 'Docblock-defined type ' . $var_type . ' can never contain false', - new CodeLocation($source, $conditional), - $var_type->getId() . ' false' - ), - $source->getSuppressedIssues() - )) { - // fall through - } - } else { - if (IssueBuffer::accepts( - new RedundantCondition( - $var_type . ' can never contain false', - new CodeLocation($source, $conditional), - $var_type->getId() . ' false' - ), - $source->getSuppressedIssues() - )) { - // fall through - } + // fall through + } + } + + $false_type = Type::getFalse(); + + if (!UnionTypeComparator::isContainedBy( + $codebase, + $var_type, + $false_type + ) && !UnionTypeComparator::isContainedBy( + $codebase, + $false_type, + $var_type + )) { + if ($var_type->from_docblock) { + if (IssueBuffer::accepts( + new RedundantConditionGivenDocblockType( + 'Docblock-defined type ' . $var_type . ' can never contain false', + new CodeLocation($source, $conditional), + $var_type->getId() . ' false' + ), + $source->getSuppressedIssues() + )) { + // fall through + } + } else { + if (IssueBuffer::accepts( + new RedundantCondition( + $var_type . ' can never contain false', + new CodeLocation($source, $conditional), + $var_type->getId() . ' false' + ), + $source->getSuppressedIssues() + )) { + // fall through } } } diff --git a/tests/BinaryOperationTest.php b/tests/BinaryOperationTest.php index 76a3b2fa6..0cb9ce124 100644 --- a/tests/BinaryOperationTest.php +++ b/tests/BinaryOperationTest.php @@ -107,6 +107,29 @@ class BinaryOperationTest extends TestCase $this->analyzeFile('somefile.php', new \Psalm\Context()); } + public function testStringFalseInequivalence(): void + { + $config = \Psalm\Config::getInstance(); + $config->strict_binary_operands = true; + + $this->addFile( + 'somefile.php', + 'expectException(\Psalm\Exception\CodeException::class); + $this->expectExceptionMessage('RedundantIdentityWithTrue'); + + $this->analyzeFile('somefile.php', new \Psalm\Context()); + } + /** * @return iterable,error_levels?:string[]}> */ @@ -532,7 +555,7 @@ class BinaryOperationTest extends TestCase ], 'IntOverflowPlus' => [ ' [ '$a' => 'int',