Corrected fieldValidation annotation#380
Conversation
| * @param string $fieldName Name of field to be validated. | ||
| * @param array $metaData metadata for field | ||
| * @param string $primaryKey The primary key field | ||
| * @param array $primaryKey The primary key field |
There was a problem hiding this comment.
Can't primary key also be a string?
Maybe we should do string|array and cast inside the in_array() call?
There was a problem hiding this comment.
I see the cast is happening before calling this method: $primaryKey = (array)$schema->primaryKey();
Then you can just add the array typehint in the signature IMO.
dmromanov
force-pushed
the
feature/fix-annotations
branch
from
434f2e5 to
a62abd4
Compare
Codecov Report
@@ Coverage Diff @@
## master #380 +/- ##
=========================================
Coverage 92.12% 92.12%
Complexity 664 664
=========================================
Files 25 25
Lines 2184 2184
=========================================
Hits 2012 2012
Misses 172 172
Continue to review full report at Codecov.
|
| * @return array Array of validation for the field. | ||
| */ | ||
| public function fieldValidation($schema, $fieldName, array $metaData, $primaryKey) | ||
| public function fieldValidation($schema, $fieldName, array $metaData, array $primaryKey) |
There was a problem hiding this comment.
I don't think this is a backwards compatible change. Type hint change for $primaryKey var will cause error if method is overridden in an extending class with old signature.
There was a problem hiding this comment.
In this case this is probably fair enough as it was always internally expected to be an array (or crash).
See code. This just puts the error more clearly to the beginning of the method.
There was a problem hiding this comment.
Even if one is correctly passing an array he would still get an error due to method signature mismatch in child class, which shouldn't happen.
5 participants
No description provided.