Hunt down invalid doc blocks #10476
Hunt down invalid doc blocks #10476
Conversation
greg0ire
requested review from
SenseException and
derrabus
and removed request for
SenseException
|
🤔 there are some more occurrences of |
greg0ire
force-pushed
the
remove-duplicate-phpdoc
branch
from
715eeec to
3283adb
Compare
greg0ire
force-pushed
the
remove-duplicate-phpdoc
branch
from
3283adb to
00b0e25
Compare
|
ping @derrabus 🙂 |
| @@ -922,6 +922,8 @@ private function cacheToArray(array $cacheMapping): array | |||
| $usage = (int) constant('Doctrine\ORM\Mapping\ClassMetadata::CACHE_USAGE_' . $usage); | |||
| } | |||
|
|
|||
| assert($usage !== '0' && $usage !== ''); | |||
There was a problem hiding this comment.
Can we be sure? I think we should either map all falsy values to null or make sure falsy values other than null also trigger an InvalidArgumentException.
There was a problem hiding this comment.
I think you're right, but let's maybe not do this in a patch release?
There was a problem hiding this comment.
Is a suddenly failing assert() better in a patch release? 😬
There was a problem hiding this comment.
Provided assertions are not enabled in production, I'd say yes.
There was a problem hiding this comment.
Not a doc block (as the title suggests) though, but in comparison it screams at dev runtime when something's wrong. :-)
There was a problem hiding this comment.
OK, I'll remove that line and ignore whatever SA errors come up then.
greg0ire
force-pushed
the
remove-duplicate-phpdoc
branch
from
00b0e25 to
bafd023
Compare
It is OK to ignore some of the errors we get, but not this one.
greg0ire
force-pushed
the
remove-duplicate-phpdoc
branch
from
bafd023 to
271d399
Compare
It is OK to ignore some of the errors we get, but not this one.