Check fixtures with PHPStan #5026
Conversation
|
is this what you had in mind? |
|
On second looks, this might be useful 🙂 Could you fix the static errors and make PR pass? |
I think so, too. I just wanted to make sure we talk about the same errors. should all test Fixtures/Sources have a namespace or do certain rules/tests exist which should not have a namespace?
sure |
|
Only those that should exist. Some are missing namespace on purpose e.g. Should be clear from rule context:) |
staabm
force-pushed
the
fixtures-stan
branch
from
e01fc9a
to
4fec488
Compare
|
fixed a bunch of namespace errors. the only thing left is #5026 (comment) |
phpstan-for-fixtures.neon
Outdated
| excludePaths: | ||
| - '*/Fixture/*' | ||
| - '*/RenameClassRector/*' | ||
| - '*/Source/phpstan.phar/*' | ||
| - '*/ImportFullyQualifiedNamesRector/*' | ||
| - '*/TagValueNodeReprint/*' |
There was a problem hiding this comment.
this list now includes those which I was not sure how/whether to resolve.
|
@TomasVotruba I think this one should be good to go now |
|
Looks good, thanks 👍 The paths |
clxmstaab
force-pushed
the
fixtures-stan
branch
from
894bf40
to
c7ea24e
Compare
|
would be great someone could look into the few remaining errors //cc @samsonasik maybe? |
| @@ -1,6 +1,6 @@ | |||
| <?php | |||
|
|
|||
| namespace Rector\Comments\Tests\CommentRemover\Fixture; | |||
| // intentional without namespace | |||
There was a problem hiding this comment.
you can create a class/function and move the content inside a class to ensure no cache conflict phpstan for anonymous function
There was a problem hiding this comment.
I guess this test uses intentionally no namespace and therefore intentionally does not define a function or class, so it will not define stuff in the global namespace.
clxmstaab
force-pushed
the
fixtures-stan
branch
from
ef1a540
to
725cc6f
Compare
|
@TomasVotruba finally green. I think this is good enough for now. |
This reverts commit d857343.
…moverRector/Fixture/fixture2.php.inc
staabm
force-pushed
the
fixtures-stan
branch
from
cab54dc
to
43ffad9
Compare
|
@TomasVotruba @samsonasik would be great to get it merged, as it would easily conflict with other PRs which get merged in the meantime |
|
Thank you @staabm |
|
Thanks 👏 |
as discussed in #5018 (comment)