Rector: CQ - SimplifyIfReturnBoolRector #4152
Conversation
* Rector: CQ - UnusedForeachValueToArrayKeysRector See Rector\CodeQuality\Rector\Foreach_\UnusedForeachValueToArrayKeysRector * fixes + phpstan See fix at rector: rectorphp/rector-src#6164
This reverts commit 3d7eaf6.
github-actions
bot
added
Component: PayPal
| if (!$this->canShipPartiallyItem() || !$this->canShipPartially()) { | ||
| return false; | ||
| } | ||
| return true; | ||
| return !(!$this->canShipPartiallyItem() || !$this->canShipPartially()); |
There was a problem hiding this comment.
Apply DeMorgan's Law: !(A || B) == (!A && !B)
| if (!$this->canShipPartiallyItem() || !$this->canShipPartially()) { | |
| return false; | |
| } | |
| return true; | |
| return !(!$this->canShipPartiallyItem() || !$this->canShipPartially()); | |
| return $this->canShipPartiallyItem() && $this->canShipPartially(); |
| if (!is_null($value) && !$value) { | ||
| return false; | ||
| } | ||
| return true; | ||
| return !(!is_null($value) && !$value); |
There was a problem hiding this comment.
DeMorgan's Law !(A && B) == (!A || !B):
| if (!is_null($value) && !$value) { | |
| return false; | |
| } | |
| return true; | |
| return !(!is_null($value) && !$value); | |
| return is_null($value) || $value; |
|
|
||
| if (!($secretKey = $this->getRequest()->getParam(Mage_Adminhtml_Model_Url::SECRET_KEY_PARAM_NAME, null)) | ||
| || !hash_equals(Mage::getSingleton('adminhtml/url')->getSecretKey(), $secretKey) | ||
| ) { | ||
| return false; | ||
| } | ||
| return true; | ||
| return !(!($secretKey = $this->getRequest()->getParam(Mage_Adminhtml_Model_Url::SECRET_KEY_PARAM_NAME, null)) | ||
| || !hash_equals(Mage::getSingleton('adminhtml/url')->getSecretKey(), $secretKey)); |
There was a problem hiding this comment.
| if (!($secretKey = $this->getRequest()->getParam(Mage_Adminhtml_Model_Url::SECRET_KEY_PARAM_NAME, null)) | |
| || !hash_equals(Mage::getSingleton('adminhtml/url')->getSecretKey(), $secretKey) | |
| ) { | |
| return false; | |
| } | |
| return true; | |
| return !(!($secretKey = $this->getRequest()->getParam(Mage_Adminhtml_Model_Url::SECRET_KEY_PARAM_NAME, null)) | |
| || !hash_equals(Mage::getSingleton('adminhtml/url')->getSecretKey(), $secretKey)); | |
| return ($secretKey = $this->getRequest()->getParam(Mage_Adminhtml_Model_Url::SECRET_KEY_PARAM_NAME, null)) | |
| && hash_equals(Mage::getSingleton('adminhtml/url')->getSecretKey(), $secretKey); |
There was a problem hiding this comment.
Continue on the conversation on chaining rector rules, I don't like the idea that it requires another PR to clean up the code above. This example from rector is really bad. It should be clean up before committing. Don't you think?
There was a problem hiding this comment.
I think rector did right. With that change it was possible to make use of DeMorgans law, that's another rule.
It's no must to have only one rule per PR, but I'd like to keep it as simple as possible to review.
DeMorgans rule added, let's wait for you bugreport.
There was a problem hiding this comment.
I'd prefer to revert last commit and add the DeMorgans rule separately.
Let's keep it simple. It affects some more files and makes reviews harder.
There was a problem hiding this comment.
I tried to apply the DeMorgan's rule to !(!is_null($value) && !$value), not sure if I made the test correctly here: https://getrector.com/demo/6a07c84e-3a20-45c6-9513-d20329994eb8
Did I make a mistake in the test? It doesn't work, no code change. If the above test is correct and if this PR is committed, there is no guarantee the convoluted code can be cleaned up by the rule later. So, now what?
There was a problem hiding this comment.
OK, so rector won't fix the convoluted bool expression now:
So, should we apply DeMorgan's manually?
There was a problem hiding this comment.
No. I'd take it as it is and apply that rule later. I dont think its worth to change it manually, same pattern is already there.
# Conflicts: # dev/tests/functional/tests/app/Mage/Adminhtml/Test/Block/Widget/Grid.php # dev/tests/functional/tests/app/Mage/Paypal/Test/Block/Login.php # dev/tests/functional/tests/app/Mage/Paypal/Test/Block/NewLogin.php
# Conflicts: # composer.lock
# Conflicts: # composer.lock # errors/report.php
# Conflicts: # .github/workflows/check-files.yml # composer.lock
See:
Rector;