Repository navigation
Readonly classes cannot be combined with #[AllowDynamicProperties] - #3738
Conversation
f56a765 to
ddb564c
Compare
|
This pull request has been marked as ready for review. |
ondrejmirtes
left a comment
ondrejmirtes
left a comment
There was a problem hiding this comment.
We also need to error when this attribute is used above interfaces. Also please check what happens above enums and traits (I'm not sure).
I have tested this cases and linked the 3v4l.org examples in phpstan/phpstan#12281 (comment) TL;DR:
what do you think about adding a separate |
ondrejmirtes
left a comment
ondrejmirtes
left a comment
There was a problem hiding this comment.
Also:
- Please do new TraitAttributesRule similar to ClassAttributesRule that will call AttributesCheck. We need the rule to hook onto Trait_ and fetch the reflection ourselves from ReflectionProvider.
- Add AllowDynamicProperties check there too.
83a0ea6 to
af3d0af
Compare
| } | ||
|
|
||
| $errors = $this->attributesCheck->check( | ||
| $scope, |
There was a problem hiding this comment.
We need to do something with $scope here. Please recreate this problem 3447391 but with a trait.
We probably need to do $scope->enterTrait() here before calling the check. Just do if !$scope instanceof MutatingScope then throw ShouldNotHappen.
There was a problem hiding this comment.
I have adjusted the PR. I think I have reproduced the problem you are after.
adding $scope->enterTrait($classReflection) didn't work and throws:
2) PHPStan\Rules\Traits\TraitAttributesRuleTest::testBug12011
PHPStan\ShouldNotHappenException: Internal error.
C:\dvl\Workspace\phpstan-src-staabm\src\Analyser\ScopeContext.php:44
C:\dvl\Workspace\phpstan-src-staabm\src\Analyser\MutatingScope.php:2935
C:\dvl\Workspace\phpstan-src-staabm\src\Rules\Traits\TraitAttributesRule.php:49
C:\dvl\Workspace\phpstan-src-staabm\src\Analyser\FileAnalyser.php:111
C:\dvl\Workspace\phpstan-src-staabm\src\Analyser\NodeScopeResolver.php:482
C:\dvl\Workspace\phpstan-src-staabm\src\Analyser\NodeScopeResolver.php:354
C:\dvl\Workspace\phpstan-src-staabm\src\Analyser\NodeScopeResolver.php:842
C:\dvl\Workspace\phpstan-src-staabm\src\Analyser\NodeScopeResolver.php:307
C:\dvl\Workspace\phpstan-src-staabm\src\Analyser\FileAnalyser.php:212
C:\dvl\Workspace\phpstan-src-staabm\src\Analyser\Analyser.php:75
C:\dvl\Workspace\phpstan-src-staabm\src\Testing\RuleTestCase.php:177
C:\dvl\Workspace\phpstan-src-staabm\src\Testing\RuleTestCase.php:138
C:\dvl\Workspace\phpstan-src-staabm\tests\PHPStan\Rules\Traits\TraitAttributesRuleTest.php:78
at this point I have no idea what todo. could you fix the remaining thing?
There was a problem hiding this comment.
let me fix the build real quick
There was a problem hiding this comment.
Push the code that's crashing like that please, I will take a look
There was a problem hiding this comment.
Oh right, we need to be in a class first before we can enter a trait.
I'd try two things: try to put the trait reflection into enterClass (but a different ShouldNotHappen might be thrown, not sure).
If that fails, we need to come up with some fake class to put into enterClass before we can enterTrait. It might be okay to simulate some anonymous Class_ node to use.
If it's too much work, don't worry and I'll merge the original code that doesn't use MutatingScope at all.
There was a problem hiding this comment.
I have class-faking without a thrown exception working. the test-case is still failling though
Will have another look after sleeping over it
|
Please keep this on 2.0.x. We don't need to be adding new features to 1.12.x anymore. |
05dfb04 to
4fc9574
Compare
4fc9574 to
88402dc
Compare
88402dc to
b96f17b
Compare
Co-authored-by: Markus Staab <markus.staab@redaxo.de>
5220c97 to
ff83891
Compare
|
I changed my mind, I'm going to adjust it myself, thanks! |
ff83891 to
0999d0f
Compare
Some attribute arguments actually depend on using class instead of the trait So we have to analyse the attributes in context of each class that uses the trait
0999d0f to
b2b833c
Compare
|
See my commit and reasoning in description: b1cebc2 |
b2b833c to
4d7274d
Compare
|
Please stop force-pushing here, I said I'm taking over. Thanks. |
b85805f to
b2b833c
Compare
oh sorry, did not see your comments. I got it working just a moment ago ;) |
|
Thank you! |
|
thanks |
closes phpstan/phpstan#12281