contao
Test the service arguments
This is a first step at validating service parameters to make sure https://github.com/contao/contao/pull/6492 does not happen again.
@contao/developers Is this correct as a functional test or should it be a service in the tools/ folder like our service linter?
I don't think this is a tool, it runs within the applications and accesses the instantiated container objects.
PHPStan still reports some errors. And what about the TODO on line 120? Does this need to be addressed before we merge the PR or is it a long-running task?
phpstan is fixed.
And what about the TODO on line 120?
Its a long running task, something that could be investigated on later. But this PR would already catch a lot of potential issues I think.
The code works now, but it generates a lot of unnecessary warnings about things we can't change:
- Service was removed.
- Argument does not allow NULL, assuming this parameter is set at runtime.
- Cannot yet handle tagged values.
We cannot change any of this, so we shouldn‘t warn about it.
- Cannot validate argument, because it does not have a supported type hint.
This is only helpful if we control the class. Currently it warns me about Symfony\Cmf\Component\Routing\DynamicRouter, but that's not our class, so again there's nothing we can do.
So you would suggest to silently ignore the warnings (for now)? Regarding the argument validation, we could check for a namespace or something thelike?
Tests updated, also added tests for TaggedValue. Failing Require checker checks seem unrelated.
There are still warnings and a lot of skipped tests.
It almost seems like the former warnings have been turned into skipped tests now.
That is correct. Tests are skipped if they cannot be performed. Is that a problem?
@ausi had the idea to simply skip tests in the dataProvider if the service does not exist 😅 should be better now.
I have decided to rebase this onto Contao 5.3, because of our upstream discussion and because we could not correctly validate union types in PHP 7.