PHPStan issue report in gitlab

Hetal.Solanki → made their first commit to this issue’s fork.

@Hetal.Solanki please don't push commit unnecessarily. First assign to yourself and do the needful changes.
The last commit of yours is totally irrelevant.

The MR I've just pushed up includes 📌 Fix the issues reported by phpcs Postponed which we should land first and then this one and we'll then be prevent regression in code quality with gitlab ftw. And people can work on fixing stuff form the PHPStan baseline if they like...

alexpott → changed the visibility of the branch 3451531-phpstan-issue-report to hidden.

I have a few thoughts on this, not exactly sure what to do with it:

* I think we should should only run one phpstan job. Don't really see much point in running it on previous major, it's very confused about the baseline as half the things there don't exist as a problem as it is depending on symfony/core changes. IMHO, that should be the default in Gitlab CI templates, but I haven't been successful in convincing people about that.
* A good chunk of the baseline would be sensible and fairly easy fixes. We took care of the validateForm return value to please phpstan, but this adds the almost identical message about save() to the baseline. the $request->get() calls should be easy to change to $request->query->get(), always should have been. it's basically a deprecation, but the replacement has always been there.
* It's unlikely that people will look at the baseline and actively try to reduce it and fix stuff, contrib is a different world than core. So I think I'd prefer to start with a baseline of things that aren't easy to fix or just false positives. Will make this larger, but that's OK IMHO.
* another good chunk of the baseline is false positives, mostly the property stuff. That's the correct way of doing things, phpstan just doesn't know. So I guess adding it makes sense, it'll just be awkward if core figures out a way tell phpstan that this is ok, then it will start to fail, so we'll have to undo it again.

In short, I think I'm willing to merge this if we clean up the baseline a bit more. But, if it becomes too much of a chore between minor updates then I'll revert the allow failures flag.

Totally agree that running multiple PHPStan jobs is a recipe for confusion and pain.

Will do the rest as asked as it all makes sense to me! Thanks for thinking about this @berdir.

Addressed #11.

Opened 📌 MatchingRouteNotFoundException is deprecated and is not thrown so why are we catching it Active

Merged.

Automatically closed - issue fixed for 2 weeks with no activity.