Remove error suppression to avoid undefined errors on PHP 8.1 #11
Conversation
|
ping @finwe |
|
Include a unit test with a HTML/PHP code example this fixes. @gersonfs |
There was a problem hiding this comment.
I aggree that removing the silencing operator is somewhat cleaner eventhough this could be improved further:
- We do not know what even is causing the missing key
- Is the QR code complete when the issue is present?
- Based on research above, could the suppressed "error" be reported instead of silenced? By throwing an exception on invalid data or at least logging?
src/QrCode.php
Outdated
| if ($remainingBit > $bufferBit) { | ||
| $this->wordData[$wordCount] = ((@$this->wordData[$wordCount] << $bufferBit) | $bufferVal); | ||
| $this->wordData[$wordCount] = (($wordDataValue << $bufferBit) | $bufferVal); |
There was a problem hiding this comment.
why bitshift the value anyway when we know it is null? Is this necessary?
There was a problem hiding this comment.
Good question. I don't work with operators like this, I thought you would know better. Why does the algorithm try to access a non-existent index?
src/QrCode.php
Outdated
| $remainingBit -= $bufferBit; | ||
| $flag = false; | ||
| } else { | ||
| $bufferBit -= $remainingBit; | ||
| $this->wordData[$wordCount] = ((@$this->wordData[$wordCount] << $remainingBit) | ($bufferVal >> $bufferBit)); | ||
| $this->wordData[$wordCount] = (($wordDataValue << $remainingBit) | ($bufferVal >> $bufferBit)); |
|
Oh, and the test code example from the other PR would be completely OK to add here. |
|
Is the QR code complete when the issue is present? Based on research above, could the suppressed "error" be reported instead of silenced? By throwing an exception on invalid data or at least logging?
|
|
Well, after debugging the loop a bit it seems the cleanest way would be to initialize the array key directly with |
|
@finwe do you thing is ok now? |
|
Seems OK, thanks! |
In php 8.1 we are getting an undefined index warning