[Validator] Check cascasdedGroups for being countable #21155
Conversation
Prevents notice for PHP 7.2
|
Not sure if AppVeyor fail is related (https://ci.appveyor.com/project/fabpot/symfony/build/1.0.16476#L492) |
|
Btw why not add the nightly php builds as allowed-failure travus matrix entry? I think this would expose these failure points a bit earlier |
|
@scaytrase because nighlty builds create false positive failures when then break. We could add then as "allowed failures", but that would consume a Travis job thus slow down everyone. |
| @@ -746,7 +746,7 @@ private function validateGenericNode($value, $object, $cacheKey, MetadataInterfa | |||
| // The $cascadedGroups property is set, if the "Default" group is | |||
| // overridden by a group sequence | |||
| // See validateClassNode() | |||
| $cascadedGroups = count($cascadedGroups) > 0 | |||
| $cascadedGroups = ((is_array($cascadedGroups) || $cascadedGroups instanceof \Countable) && count($cascadedGroups) > 0) | |||
There was a problem hiding this comment.
surrounding brackets should be removed
|
@nicolas-grekas by slowing down you just mean that we consume one more container per build or making the build being longer? if second - AFAIK travis marks build green as soon as every mandatory job finishes green. if first we can make the travis to skip the job if branch is not 2.x or master. Or not a tag. Thus we will check only merged commits or tags only an we will collect at least one run on nigtly build per release |
|
Travis failed this time: https://travis-ci.org/symfony/symfony/jobs/189505947#L2114 Fatal error: Call to undefined method PHPUnit_Framework_TestResult::addWarning() in /home/travis/build/symfony/symfony/vendor/symfony/phpunit-bridge/SymfonyTestsListener.php on line 215 Seems unrelated |
|
Why not checking then that the value is not equal to |
|
@xabbuh I have the same thoughts on the start. But the if we have an If you sure, that |
|
We always rely on the fact that argument types comply with the ones specified in the docblock when we cannot use type hints (for example, we don't have additional checks for scalar types). If you don't respect this contract, you are basically on your own. |
|
@xabbuh sounds reasonable, updated the check |
|
👍 Status: Reviewed |
|
Thank you @scaytrase. |
…ytrase) This PR was squashed before being merged into the 2.7 branch (closes #21155). Discussion ---------- [Validator] Check cascasdedGroups for being countable Prevents notice for PHP 7.2 | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | Just repeated for this place #20859 Waiting travis to fill in the `Tests pass` field `cascasdedGroups` can be null at this point (according to failures and phpdoc) Commits ------- 8fa45a1 [Validator] Check cascasdedGroups for being countable
Prevents notice for PHP 7.2
Just repeated for this place #20859
Waiting travis to fill in the
Tests passfieldcascasdedGroupscan be null at this point (according to failures and phpdoc)