One solution
tmp pr. just want to know the results of the integration tests.
This is one possible solution I can think of.
-
treatPhpDocTypesAsCertainis kept inMutatingScope, but can only be switched from rules -
treatPhpDocTypesAsCertain: falsemakesgetTypeto return native types -
nativeTypesuses https://github.com/phpstan/phpstan-src/pull/1805/files#diff-dc817f2bab8672057a95375a542e68599164f6ab2380e4cccd1b50583e295d85R2116 which should be mostly correct and least an improvement, because the currentgetNativeTypeis falling back to full types.
The failing test in pocketmine for example https://github.com/phpstan/phpstan-src/actions/runs/3214922744/jobs/5255662194 is showing that the native types and specification that wasn't working before, is working with this pull request.
This isn't the direction I want to go for. It breaks some of the ground rules I set in comments of the other PR, and is less useful for my goals.
For example - making getType return only native type when the setting is set to false, it completely breaks user expectation. Basically it says "don't use PHPDocs". Which isn't what the setting is about. We still want to infer types using PHPDocs, we just don't want to complain if the user double-checks the passed type in function arguments.
I've got some ideas about this, I'll trynto explore them soon 😊
Can you please tell me which place in NodeScopeResolver has to care about getType() vs getNativeType()?
@ondrejmirtes
Can you please tell me which place in NodeScopeResolver has to care about getType() vs getNativeType()?
For example, The test failure of https://github.com/phpstan/phpstan-src/pull/1802 https://github.com/phpstan/phpstan-src/actions/runs/3227859202/jobs/5283202887
- PHPStan\Rules\Comparison\UnreachableIfBranchesRuleTest::testDoNotReportPhpDoc Failed asserting that two strings are identical. --- Expected +++ Actual @@ @@ '18: Elseif branch is unreachable because previous condition is always true. 28: Else branch is unreachable because previous condition is always true. 38: Elseif branch is unreachable because previous condition is always true. +54: Else branch is unreachable because previous condition is always true. +64: Elseif branch is unreachable because previous condition is always true.
Is caused by getType in this line
https://github.com/phpstan/phpstan-src/blob/e42952ecc182f1cff8c17761d1efba5598555b24/src/Analyser/NodeScopeResolver.php#L704
$alwaysTerminating is determined by the $conditionType which uses full types and causing the above error.
About UnreachableIfBranchesRuleTest - I don't think so because UnreachableIfBranchesRule already asks correctly about it and the code in NodeScopeResolver doesn't influence that:
$conditionType = $this->treatPhpDocTypesAsCertain ? $scope->getType($condition) : $scope->getNativeType($condition);
$conditionBooleanType = $conditionType->toBoolean();
I think the problem is in some mixup of TypeSpecifier/SpecifiedTypes.
I was wrong with my explanation. The problem is caused by isAlwaysTrue
The scope merge is performed based on isAlwaysTrue of full types, which is causing wrong type for phpdoc types
simple example
https://phpstan.org/r/a8479355-de29-4e8c-9a08-7a2612ee94e1
https://github.com/phpstan/phpstan-src/pull/1805#issuecomment-1276968300 I'll try if this example works with this solution tonight, adding as test.
I now realized that this solution is not correct. I think we need a separate two scopes for each full type and native type?
What's the purpose of this PR when compared to https://github.com/phpstan/phpstan-src/pull/1802 and in context of my plan at https://github.com/phpstan/phpstan/issues/8191? Thank you :)
If there's something in here that still makes sense, please post it as a new PR with this base: https://github.com/phpstan/phpstan-src/pull/1943 Thank you!