Skip to content

Commit

Permalink
Merge pull request #10765 from theodorejb/avoid-duplication
Browse files Browse the repository at this point in the history
  • Loading branch information
weirdan committed Mar 2, 2024
2 parents d3e070c + e357b97 commit cabcb02
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 93 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,8 @@
use Psalm\Issue\DocblockTypeContradiction;
use Psalm\Issue\RedundantCondition;
use Psalm\Issue\RedundantConditionGivenDocblockType;
use Psalm\Issue\RiskyTruthyFalsyComparison;
use Psalm\Issue\TypeDoesNotContainType;
use Psalm\IssueBuffer;
use Psalm\Type\Atomic\TBool;
use Psalm\Type\Reconciler;

use function array_diff_key;
Expand Down Expand Up @@ -371,33 +369,7 @@ public static function handleParadoxicalCondition(
} elseif (!($stmt instanceof PhpParser\Node\Expr\BinaryOp\NotIdentical)
&& !($stmt instanceof PhpParser\Node\Expr\BinaryOp\Identical)
&& !($stmt instanceof PhpParser\Node\Expr\BooleanNot)) {
if (count($type->getAtomicTypes()) > 1) {
$has_truthy_or_falsy_exclusive_type = false;
$both_types = $type->getBuilder();
foreach ($both_types->getAtomicTypes() as $key => $atomic_type) {
if ($atomic_type->isTruthy()
|| $atomic_type->isFalsy()
|| $atomic_type instanceof TBool) {
$both_types->removeType($key);
$has_truthy_or_falsy_exclusive_type = true;
}
}

if (count($both_types->getAtomicTypes()) > 0 && $has_truthy_or_falsy_exclusive_type) {
$both_types = $both_types->freeze();
IssueBuffer::maybeAdd(
new RiskyTruthyFalsyComparison(
'Operand of type ' . $type->getId() . ' contains ' .
'type' . (count($both_types->getAtomicTypes()) > 1 ? 's' : '') . ' ' .
$both_types->getId() . ', which can be falsy and truthy. ' .
'This can cause possibly unexpected behavior. Use strict comparison instead.',
new CodeLocation($statements_analyzer, $stmt),
$type->getId(),
),
$statements_analyzer->getSuppressedIssues(),
);
}
}
ExpressionAnalyzer::checkRiskyTruthyFalsyComparison($type, $statements_analyzer, $stmt);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,15 @@
namespace Psalm\Internal\Analyzer\Statements\Expression;

use PhpParser;
use Psalm\CodeLocation;
use Psalm\Context;
use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer;
use Psalm\Internal\Analyzer\StatementsAnalyzer;
use Psalm\Issue\RiskyTruthyFalsyComparison;
use Psalm\IssueBuffer;
use Psalm\Type;
use Psalm\Type\Atomic\TBool;
use Psalm\Type\Atomic\TFalse;
use Psalm\Type\Atomic\TTrue;
use Psalm\Type\Union;

use function count;

/**
* @internal
*/
Expand Down Expand Up @@ -45,34 +40,7 @@ public static function analyze(
} elseif ($expr_type->isAlwaysFalsy()) {
$stmt_type = new TTrue($expr_type->from_docblock);
} else {
if (count($expr_type->getAtomicTypes()) > 1) {
$has_truthy_or_falsy_exclusive_type = false;
$both_types = $expr_type->getBuilder();
foreach ($both_types->getAtomicTypes() as $key => $atomic_type) {
if ($atomic_type->isTruthy()
|| $atomic_type->isFalsy()
|| $atomic_type instanceof TBool) {
$both_types->removeType($key);
$has_truthy_or_falsy_exclusive_type = true;
}
}

if (count($both_types->getAtomicTypes()) > 0 && $has_truthy_or_falsy_exclusive_type) {
$both_types = $both_types->freeze();
IssueBuffer::maybeAdd(
new RiskyTruthyFalsyComparison(
'Operand of type ' . $expr_type->getId() . ' contains ' .
'type' . (count($both_types->getAtomicTypes()) > 1 ? 's' : '') . ' ' .
$both_types->getId() . ', which can be falsy and truthy. ' .
'This can cause possibly unexpected behavior. Use strict comparison instead.',
new CodeLocation($statements_analyzer, $stmt),
$expr_type->getId(),
),
$statements_analyzer->getSuppressedIssues(),
);
}
}

ExpressionAnalyzer::checkRiskyTruthyFalsyComparison($expr_type, $statements_analyzer, $stmt);
$stmt_type = new TBool();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,17 @@
use PhpParser;
use Psalm\CodeLocation;
use Psalm\Context;
use Psalm\Internal\Analyzer\Statements\ExpressionAnalyzer;
use Psalm\Internal\Analyzer\StatementsAnalyzer;
use Psalm\Issue\ForbiddenCode;
use Psalm\Issue\InvalidArgument;
use Psalm\Issue\RiskyTruthyFalsyComparison;
use Psalm\IssueBuffer;
use Psalm\Type;
use Psalm\Type\Atomic\TBool;
use Psalm\Type\Atomic\TFalse;
use Psalm\Type\Atomic\TTrue;
use Psalm\Type\Union;

use function count;

/**
* @internal
*/
Expand Down Expand Up @@ -64,34 +62,7 @@ public static function analyze(
} elseif ($expr_type->isAlwaysFalsy()) {
$stmt_type = new TTrue($expr_type->from_docblock);
} else {
if (count($expr_type->getAtomicTypes()) > 1) {
$has_truthy_or_falsy_exclusive_type = false;
$both_types = $expr_type->getBuilder();
foreach ($both_types->getAtomicTypes() as $key => $atomic_type) {
if ($atomic_type->isTruthy()
|| $atomic_type->isFalsy()
|| $atomic_type instanceof TBool) {
$both_types->removeType($key);
$has_truthy_or_falsy_exclusive_type = true;
}
}

if (count($both_types->getAtomicTypes()) > 0 && $has_truthy_or_falsy_exclusive_type) {
$both_types = $both_types->freeze();
IssueBuffer::maybeAdd(
new RiskyTruthyFalsyComparison(
'Operand of type ' . $expr_type->getId() . ' contains ' .
'type' . (count($both_types->getAtomicTypes()) > 1 ? 's' : '') . ' ' .
$both_types->getId() . ', which can be falsy and truthy. ' .
'This can cause possibly unexpected behavior. Use strict comparison instead.',
new CodeLocation($statements_analyzer, $stmt),
$expr_type->getId(),
),
$statements_analyzer->getSuppressedIssues(),
);
}
}

ExpressionAnalyzer::checkRiskyTruthyFalsyComparison($expr_type, $statements_analyzer, $stmt);
$stmt_type = new TBool();
}

Expand Down
37 changes: 37 additions & 0 deletions src/Psalm/Internal/Analyzer/Statements/ExpressionAnalyzer.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
use Psalm\Internal\Analyzer\StatementsAnalyzer;
use Psalm\Internal\FileManipulation\FileManipulationBuffer;
use Psalm\Internal\Type\TemplateResult;
use Psalm\Issue\RiskyTruthyFalsyComparison;
use Psalm\Issue\UnrecognizedExpression;
use Psalm\Issue\UnsupportedReferenceUsage;
use Psalm\IssueBuffer;
Expand All @@ -54,7 +55,9 @@
use Psalm\Plugin\EventHandler\Event\AfterExpressionAnalysisEvent;
use Psalm\Plugin\EventHandler\Event\BeforeExpressionAnalysisEvent;
use Psalm\Type;
use Psalm\Type\Atomic\TBool;

use function count;
use function get_class;
use function in_array;
use function strtolower;
Expand Down Expand Up @@ -135,6 +138,40 @@ public static function analyze(
return true;
}

public static function checkRiskyTruthyFalsyComparison(
Type\Union $type,
StatementsAnalyzer $statements_analyzer,
PhpParser\Node\Expr $stmt
): void {
if (count($type->getAtomicTypes()) > 1) {
$has_truthy_or_falsy_exclusive_type = false;
$both_types = $type->getBuilder();
foreach ($both_types->getAtomicTypes() as $key => $atomic_type) {
if ($atomic_type->isTruthy()
|| $atomic_type->isFalsy()
|| $atomic_type instanceof TBool) {
$both_types->removeType($key);
$has_truthy_or_falsy_exclusive_type = true;
}
}

if (count($both_types->getAtomicTypes()) > 0 && $has_truthy_or_falsy_exclusive_type) {
$both_types = $both_types->freeze();
IssueBuffer::maybeAdd(
new RiskyTruthyFalsyComparison(
'Operand of type ' . $type->getId() . ' contains ' .
'type' . (count($both_types->getAtomicTypes()) > 1 ? 's' : '') . ' ' .
$both_types->getId() . ', which can be falsy and truthy. ' .
'This can cause possibly unexpected behavior. Use strict comparison instead.',
new CodeLocation($statements_analyzer, $stmt),
$type->getId(),
),
$statements_analyzer->getSuppressedIssues(),
);
}
}
}

/**
* @param bool $assigned_to_reference This is set to true when the expression being analyzed
* here is being assigned to another variable by reference.
Expand Down

0 comments on commit cabcb02

Please sign in to comment.