Sonataadminbundle: Add psalm to v4
Feature Request
Since v4 is BC you may want to add some strictness to the project by adding psalm.
What will be possible to do with it
1) make internal code more strict
2) add generics, so I, as a end user, can benefit from it
vladyslavstartsev
All 6 comments
We already use phpstan to add some strictness
core23
on 19 Jun 2020
@core23 yes, I seen. I think psalm and phpstan complement each other
vladyslavstartsev
on 19 Jun 2020
Wouldn't it be better to try to use a stricter level with phpstan before adding another tool ?
VincentLanglet
on 19 Jun 2020
I see that there is no phpstan in 3.x branch and also I look at the PR that adds phpstan to v4 https://github.com/sonata-project/SonataAdminBundle/pull/5766
And I don't understand why "I am targeting this branch, because this is BC, but should only be done for the next major version.". I don't understand why adding phpstan is BC break.
What if we
1) also add phpstan to 3.x and and fix things only on PhpDoc level?
2) Then merge up to master.
3) After phpstan level is max (with strict rules of all kinds) we add psalm.
vladyslavstartsev
on 20 Jun 2020
And I don't understand why "I am targeting this branch, because this is BC, but should only be done for the next major version.". I don't understand why adding phpstan is BC break.
He didn't say it was a BC-break.
Since we're prioritizing the next major, I understand he wanted to start only after the release. Some code will be removed/changed between 3.x and 4.0. Some of these have been already done.
Maybe the goal was to avoid some extra-work or some conflict.
What if we
- also add phpstan to 3.x and and fix things only on PhpDoc level?
- Then merge up to master.
- After phpstan level is max (with strict rules of all kinds) we add psalm.
Starting on 3.x could still be useful. Especially since we're adding typehint in 4.0. A correct phpdoc will help to add a correct typehint.
Do you want to make some PR ? I recommend to not fix everything in a single PR ; it will be easier to read multiple PR and we'll see with the first PRs if there is a lot of conflict in master.
We could also use the @phpstan-template annotation for Admin
VincentLanglet
on 20 Jun 2020
greg0ire
on 20 Jun 2020
Related issues
SlimenTN
路
3Comments
axzx
路
3Comments
mdstorage
路
3Comments
tiagojsag
路
4Comments
peter-gribanov
路
3Comments
Most helpful comment
He didn't say it was a BC-break.
Since we're prioritizing the next major, I understand he wanted to start only after the release. Some code will be removed/changed between 3.x and 4.0. Some of these have been already done.
Maybe the goal was to avoid some extra-work or some conflict.
Starting on 3.x could still be useful. Especially since we're adding typehint in 4.0. A correct phpdoc will help to add a correct typehint.
Do you want to make some PR ? I recommend to not fix everything in a single PR ; it will be easier to read multiple PR and we'll see with the first PRs if there is a lot of conflict in master.
We could also use the
@phpstan-templateannotation for Admin