Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions conf/bleedingEdge.neon
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@ parameters:
checkDynamicConstantNameValues: true
unusedLabel: true
newOnNonObject: true
switchConditionAlwaysFalse: true
7 changes: 7 additions & 0 deletions conf/config.level4.neon
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ conditionalTags:
phpstan.rules.rule: %exceptions.check.tooWideThrowType%
PHPStan\Rules\Keywords\UnusedLabelRule:
phpstan.rules.rule: %featureToggles.unusedLabel%
PHPStan\Rules\Comparison\SwitchConditionRule:
phpstan.rules.rule: %featureToggles.switchConditionAlwaysFalse%

parameters:
checkAdvancedIsset: true
Expand All @@ -35,3 +37,8 @@ services:

-
class: PHPStan\Rules\Keywords\UnusedLabelRule

-
class: PHPStan\Rules\Comparison\SwitchConditionRule
arguments:
treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%
1 change: 1 addition & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ parameters:
checkDynamicConstantNameValues: false
unusedLabel: false
newOnNonObject: false
switchConditionAlwaysFalse: false
fileExtensions:
- php
checkAdvancedIsset: false
Expand Down
1 change: 1 addition & 0 deletions conf/parametersSchema.neon
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ parametersSchema:
checkDynamicConstantNameValues: bool()
unusedLabel: bool()
newOnNonObject: bool()
switchConditionAlwaysFalse: bool()
])
fileExtensions: listOf(string())
checkAdvancedIsset: bool()
Expand Down
23 changes: 22 additions & 1 deletion src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@
use PHPStan\Node\PropertyHookStatementNode;
use PHPStan\Node\ReturnStatement;
use PHPStan\Node\StaticMethodCallableNode;
use PHPStan\Node\SwitchConditionArm;
use PHPStan\Node\SwitchConditionNode;
use PHPStan\Node\UnreachableStatementNode;
use PHPStan\Node\VariableAssignNode;
use PHPStan\Node\VarTagChangedExpressionTypeNode;
Expand Down Expand Up @@ -2020,7 +2022,16 @@ public function processStmtNode(
$throwPoints = $condResult->getThrowPoints();
$impurePoints = $condResult->getImpurePoints();
$fullCondExpr = null;
foreach ($stmt->cases as $caseNode) {
$switchConditionArms = [];
$lastNonDefaultCaseKey = null;
foreach ($stmt->cases as $caseKey => $caseNode) {
if ($caseNode->cond === null) {
continue;
}

$lastNonDefaultCaseKey = $caseKey;
}
foreach ($stmt->cases as $caseKey => $caseNode) {
if ($caseNode->cond !== null) {
$condExpr = new BinaryOp\Equal($stmt->cond, $caseNode->cond);
$fullCondExpr = $fullCondExpr === null ? $condExpr : new BooleanOr($fullCondExpr, $condExpr);
Expand All @@ -2029,6 +2040,12 @@ public function processStmtNode(
$hasYield = $hasYield || $caseResult->hasYield();
$throwPoints = array_merge($throwPoints, $caseResult->getThrowPoints());
$impurePoints = array_merge($impurePoints, $caseResult->getImpurePoints());
$switchConditionArms[] = new SwitchConditionArm(
$caseNode->cond,
$scopeForBranches,
$caseNode->cond->getStartLine(),
$caseKey === $lastNonDefaultCaseKey,
);
$branchScope = $caseResult->getScope()->filterByTruthyValue($condExpr);
} else {
$hasDefaultCase = true;
Expand Down Expand Up @@ -2066,6 +2083,10 @@ public function processStmtNode(
}
}

if ($switchConditionArms !== []) {
$this->callNodeCallback($nodeCallback, new SwitchConditionNode($stmt->cond, $switchConditionArms, $stmt), $scope, $storage);
}

$exhaustive = $scopeForBranches->getType($stmt->cond) instanceof NeverType;

if (!$hasDefaultCase && !$exhaustive) {
Expand Down
53 changes: 53 additions & 0 deletions src/Node/SwitchConditionArm.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php declare(strict_types = 1);

namespace PHPStan\Node;

use PhpParser\Node\Expr;
use PHPStan\Analyser\Scope;

/**
* A single non-default `case` of a `switch`, paired with the scope captured
* right after the case condition was processed (which already excludes the
* values matched by earlier terminating cases).
*
* @api
*/
final class SwitchConditionArm
{

public function __construct(
private Expr $caseCondition,
private Scope $scope,
private int $line,
private bool $isLast,
)
{
}

public function getCaseCondition(): Expr
{
return $this->caseCondition;
}

public function getScope(): Scope
{
return $this->scope;
}

public function getLine(): int
{
return $this->line;
}

/**
* Whether this is the last non-default `case` of the `switch` (only a
* `default` may follow it), in which case an always-true comparison is fine
* because it does not make any subsequent `case` unreachable. A trailing
* `default` is not considered a `case` it would make unreachable.
*/
public function isLast(): bool
{
return $this->isLast;
}

}
61 changes: 61 additions & 0 deletions src/Node/SwitchConditionNode.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php declare(strict_types = 1);

namespace PHPStan\Node;

use Override;
use PhpParser\Node\Expr;
use PhpParser\Node\Stmt\Switch_;
use PhpParser\NodeAbstract;

/**
* Virtual node emitted once per `switch` statement. It pairs the switch subject
* with each non-default `case` condition so rules can inspect the loose `==`
* comparison the `switch` performs, using the scope captured at each case
* (which already excludes the values matched by earlier cases).
*
* @api
*/
final class SwitchConditionNode extends NodeAbstract implements VirtualNode
{

/**
* @param SwitchConditionArm[] $arms
*/
public function __construct(
private Expr $subject,
private array $arms,
Switch_ $originalNode,
)
{
parent::__construct($originalNode->getAttributes());
}

public function getSubject(): Expr
{
return $this->subject;
}

/**
* @return SwitchConditionArm[]
*/
public function getArms(): array
{
return $this->arms;
}

#[Override]
public function getType(): string
{
return 'PHPStan_Node_SwitchCondition';
}

/**
* @return string[]
*/
#[Override]
public function getSubNodeNames(): array
{
return [];
}

}
163 changes: 163 additions & 0 deletions src/Rules/Comparison/SwitchConditionRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Comparison;

use PhpParser\Node;
use PhpParser\Node\Expr\BinaryOp\Equal;
use PHPStan\Analyser\CollectedDataEmitter;
use PHPStan\Analyser\NodeCallbackInvoker;
use PHPStan\Analyser\Scope;
use PHPStan\Node\Printer\ExprPrinter;
use PHPStan\Node\SwitchConditionNode;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\Type;
use PHPStan\Type\VerbosityLevel;
use function count;
use function sprintf;

/**
* @implements Rule<SwitchConditionNode>
*/
final class SwitchConditionRule implements Rule
{

public function __construct(
private ConstantConditionRuleHelper $constantConditionRuleHelper,
private PossiblyImpureTipHelper $possiblyImpureTipHelper,
private ConstantConditionInTraitHelper $constantConditionInTraitHelper,
private ExprPrinter $exprPrinter,
private bool $treatPhpDocTypesAsCertain,
)
{
}

public function getNodeType(): string
{
return SwitchConditionNode::class;
}

public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array
{
$subject = $node->getSubject();
$errors = [];
$nextCaseIsDeadForType = false;
$nextCaseIsDeadForNativeType = false;
$seenCases = [];

foreach ($node->getArms() as $arm) {
if (
$nextCaseIsDeadForNativeType
|| ($nextCaseIsDeadForType && $this->treatPhpDocTypesAsCertain)
) {
continue;
}

$armScope = $arm->getScope();
$caseCondition = $arm->getCaseCondition();

$caseConditionType = $armScope->getType($caseCondition);
$finiteTypes = $caseConditionType->getFiniteTypes();
if (count($finiteTypes) === 1) {
$caseValueType = $finiteTypes[0];
$firstSeen = null;
foreach ($seenCases as $seenCase) {
if ($seenCase['type']->equals($caseValueType)) {
$firstSeen = $seenCase;
break;
}
}

if ($firstSeen !== null) {
$errors[] = RuleErrorBuilder::message(sprintf(
'Case %s in switch is a duplicate of case %s on line %d.',
$this->exprPrinter->printExpr($caseCondition),
$firstSeen['printed'],
$firstSeen['line'],
))->line($arm->getLine())->identifier('switch.duplicateCase')->build();
continue;
}

$seenCases[] = [
'type' => $caseValueType,
'printed' => $this->exprPrinter->printExpr($caseCondition),
'line' => $arm->getLine(),
];
}

$conditionExpr = new Equal($subject, $caseCondition);

$conditionType = $armScope->getType($conditionExpr);
if (!$this->isConstantBoolean($conditionType)) {
$this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr);
continue;
}
if ($conditionType->isTrue()->yes()) {

Check warning on line 95 in src/Rules/Comparison/SwitchConditionRule.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\LooseBooleanMutator": @@ @@ $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr); continue; } - if ($conditionType->isTrue()->yes()) { + if ($conditionType->toBoolean()->isTrue()->yes()) { $nextCaseIsDeadForType = true; }

Check warning on line 95 in src/Rules/Comparison/SwitchConditionRule.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr); continue; } - if ($conditionType->isTrue()->yes()) { + if (!$conditionType->toBoolean()->isTrue()->no()) { $nextCaseIsDeadForType = true; }
$nextCaseIsDeadForType = true;
}

if (!$this->treatPhpDocTypesAsCertain) {
$conditionNativeType = $armScope->getNativeType($conditionExpr);
if (!$this->isConstantBoolean($conditionNativeType)) {
$this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr);
continue;
}
if ($conditionNativeType->isTrue()->yes()) {

Check warning on line 105 in src/Rules/Comparison/SwitchConditionRule.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr); continue; } - if ($conditionNativeType->isTrue()->yes()) { + if (!$conditionNativeType->toBoolean()->isTrue()->no()) { $nextCaseIsDeadForNativeType = true; } }

Check warning on line 105 in src/Rules/Comparison/SwitchConditionRule.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\LooseBooleanMutator": @@ @@ $this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr); continue; } - if ($conditionNativeType->isTrue()->yes()) { + if ($conditionNativeType->toBoolean()->isTrue()->yes()) { $nextCaseIsDeadForNativeType = true; } }
$nextCaseIsDeadForNativeType = true;
}
}

$subjectType = $armScope->getType($subject);
if ($this->isConstantBoolean($subjectType)) {
$caseConditionStandaloneType = $this->constantConditionRuleHelper->getBooleanType($armScope, $caseCondition);
if (!$this->isConstantBoolean($caseConditionStandaloneType)) {
$this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr);
continue;
}
}

if ($conditionType->isFalse()->yes()) {

Check warning on line 119 in src/Rules/Comparison/SwitchConditionRule.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ } } - if ($conditionType->isFalse()->yes()) { + if (!$conditionType->toBoolean()->isFalse()->no()) { $errorBuilder = RuleErrorBuilder::message(sprintf( 'Switch condition comparison between %s and %s is always false.', $subjectType->describe(VerbosityLevel::value()),

Check warning on line 119 in src/Rules/Comparison/SwitchConditionRule.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\LooseBooleanMutator": @@ @@ } } - if ($conditionType->isFalse()->yes()) { + if ($conditionType->toBoolean()->isFalse()->yes()) { $errorBuilder = RuleErrorBuilder::message(sprintf( 'Switch condition comparison between %s and %s is always false.', $subjectType->describe(VerbosityLevel::value()),

Check warning on line 119 in src/Rules/Comparison/SwitchConditionRule.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ } } - if ($conditionType->isFalse()->yes()) { + if (!$conditionType->toBoolean()->isFalse()->no()) { $errorBuilder = RuleErrorBuilder::message(sprintf( 'Switch condition comparison between %s and %s is always false.', $subjectType->describe(VerbosityLevel::value()),
$errorBuilder = RuleErrorBuilder::message(sprintf(
'Switch condition comparison between %s and %s is always false.',
$subjectType->describe(VerbosityLevel::value()),
$caseConditionType->describe(VerbosityLevel::value()),
))->line($arm->getLine())->identifier('switch.alwaysFalse');
$this->possiblyImpureTipHelper->addTip($armScope, $conditionExpr, $errorBuilder);
$ruleError = $errorBuilder->build();
if ($scope->isInTrait()) {
$this->constantConditionInTraitHelper->emitError(self::class, $scope, $conditionExpr, false, $ruleError);
} else {
$errors[] = $ruleError;
}
continue;
}

if ($arm->isLast()) {
$this->constantConditionInTraitHelper->emitNoError(self::class, $scope, $conditionExpr);
continue;
}

$errorBuilder = RuleErrorBuilder::message(sprintf(
'Switch condition comparison between %s and %s is always true.',
$subjectType->describe(VerbosityLevel::value()),
$armScope->getType($caseCondition)->describe(VerbosityLevel::value()),
))->line($arm->getLine())->identifier('switch.alwaysTrue')
->tip('Remove remaining cases below this one and this error will disappear too.');
$this->possiblyImpureTipHelper->addTip($armScope, $conditionExpr, $errorBuilder);
$ruleError = $errorBuilder->build();
if ($scope->isInTrait()) {
$this->constantConditionInTraitHelper->emitError(self::class, $scope, $conditionExpr, true, $ruleError);
} else {
$errors[] = $ruleError;
}
}

return $errors;
}

private function isConstantBoolean(Type $type): bool
{
return $type->isTrue()->yes() || $type->isFalse()->yes();

Check warning on line 160 in src/Rules/Comparison/SwitchConditionRule.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\LooseBooleanMutator": @@ @@ private function isConstantBoolean(Type $type): bool { - return $type->isTrue()->yes() || $type->isFalse()->yes(); + return $type->isTrue()->yes() || $type->toBoolean()->isFalse()->yes(); } }

Check warning on line 160 in src/Rules/Comparison/SwitchConditionRule.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\LooseBooleanMutator": @@ @@ private function isConstantBoolean(Type $type): bool { - return $type->isTrue()->yes() || $type->isFalse()->yes(); + return $type->isTrue()->yes() || $type->toBoolean()->isFalse()->yes(); } }
}

}
Loading
Loading