-
-
Notifications
You must be signed in to change notification settings - Fork 708
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CountOnNullRector] Should understand array/countable variable in trait method #2650
[CountOnNullRector] Should understand array/countable variable in trait method #2650
Conversation
Could this be related to phpstan/phpstan#1601 (fixed in the new release by PHPStan 0.12.5) ? |
We might guess, but testing would answer us. |
I tried to re-run checks, but I think you'll have to rebase |
bdad270
to
13ed224
Compare
Last push force is the rebase on master. We'll see. |
@TomasVotruba Not related to PHPStan 0.12.5. Any idea ? |
@gnutix What do you mean? Link might help |
I mean that I've rebased this PR on master, which includes PHPStan 0.12.15, and it didn't fix the broken test. So the issue remains, and I have no lead / clue on why this is broken nor how to fix this. So I'm asking for your help / guidance. :) |
This might be more tricky. Traits sucks hard. Are you ok with merging as it is? |
Sure! |
Both passes after merge 👍 probably fixed by type refactoring |
Or not... false positive: paths have changed from |
After 30-40 minutes of digging, traits are impossible to analyse without class by PHPStan. It provides many false positives on standalone analysis, so we better skip them. If you find a stable way of using PHPStan for traits here, I'm open to the PR. |
rectorphp/rector-src@7e1c2bb [TypeDeclaration] Skip Type modified between Assign and Return_ on ReturnTypeFromReturnNewRector (#2650)
This PR adds two fixtures relating to
trait
handling byCountOnNullRector
. Currently :PropertyWithinTraitMethod
: passes 👍VariableWithinTraitMethod
: fails 👎If I change the Fixture from a
trait
to aclass
they both pass, so this is related to how a variable within a trait is handled.I spent some time digging and tried to use
\Rector\NodeTypeResolver\PerNodeTypeResolver\VariableTypeResolver::resolve()
that seems to be written to handle resolution of variables (though it's used nowhere in Rector's code, only in its own Test file 🤔 ) by calling :resolveTypesFromScope($node)
, which calls...resolveNodeScope($node)
, which calls on\Rector\NodeTypeResolver\PHPStan\Collector\TraitNodeScopeCollector
...getScopeForTraitAndNode($traitName, $node)
)But it returned
null
, which was resolved toMixedType
inresolveTypesFromScope
and bubbled up back to\Rector\Php71\Rector\FuncCall\CountOnNullRector::refactor
which passes theisNullableType
check and thus refactors the code.@TomasVotruba Any idea how to deal with this one ?