Skip to content

specify switch-condition in switch-branch-scope#1602

Closed
staabm wants to merge 2 commits intophpstan:1.8.xfrom
staabm:switch-cond
Closed

specify switch-condition in switch-branch-scope#1602
staabm wants to merge 2 commits intophpstan:1.8.xfrom
staabm:switch-cond

Conversation

@staabm
Copy link
Copy Markdown
Contributor

@staabm staabm commented Aug 7, 2022

killing some implicit mixed types, see https://phpstan.org/r/493a6a00-051c-4646-bf58-f508b3d7930e

Comment thread tests/PHPStan/Analyser/data/if.php Outdated
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

had to add this case, as otherwise phpstan detects after this PR, that the Exception would always be thrown and $nullOverwrittenInSwitchToOne can never be reached

@staabm staabm marked this pull request as ready for review August 7, 2022 08:19
@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented Aug 7, 2022

looked into the test failures. I think these are related to the new ValueType stuff and unrelated to this PR

$throwPoints = array_merge($throwPoints, $caseResult->getThrowPoints());
$branchScope = $scopeForBranches->filterByTruthyValue($condExpr);

$branchScope = $branchScope->assignExpression($stmt->cond, $branchScope->getType($caseNode->cond));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't agree with this, switch uses == semantics. So if you have:

switch ($foo) {
    case true: doFoo();
    break;
}

$foo doesn't have to be true, it can be any truthy-string for example...

The way to achieve this is to make PHPStan aware of these semantics in TypeSpecifier, to make it learn the whole table "Loose comparisons with ==" here https://www.php.net/manual/en/types.comparisons.php.

If you want better analysis now, switch to the match expression which uses === semantics and is a much better alternative.

Copy link
Copy Markdown
Contributor Author

@staabm staabm Aug 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for the hint, will look into it.

I also realized that same type switch already works like I expected

https://phpstan.org/r/3899f414-c91a-4a30-9801-07c839e474af

@staabm staabm deleted the switch-cond branch August 8, 2022 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants