Skip to content

rector: CombineIfRector #4880

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
+1,246 −1,771
Draft

rector: CombineIfRector #4880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
da038ef
rector: `CombineIfRector`
sreichel Jul 8, 2025
42ae3ba
baseline - shifting errors
sreichel Jul 8, 2025
403737a
cs
sreichel Jul 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions .phpstan.dist.baseline.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,12 @@ parameters:
count: 2
path: app/code/core/Mage/Adminhtml/Block/Widget/Form.php

-
message: '#^Call to function is_array\(\) with array\<mixed\> will always evaluate to true\.$#'
identifier: function.alreadyNarrowedType
count: 1
path: app/code/core/Mage/Adminhtml/Block/Widget/Grid/Column.php

-
message: '#^Parameter \#1 \$emptyLabel of method Mage_Directory_Model_Resource_Country_Collection\:\:toOptionArray\(\) expects string, false given\.$#'
identifier: argument.type
Expand Down Expand Up @@ -4935,7 +4941,7 @@ parameters:
-
message: '#^If condition is always false\.$#'
identifier: if.alwaysFalse
count: 3
count: 2
path: app/code/core/Mage/Tax/Helper/Data.php

-
Expand All @@ -4959,7 +4965,7 @@ parameters:
-
message: '#^Right side of && is always false\.$#'
identifier: booleanAnd.rightAlwaysFalse
count: 1
count: 2
path: app/code/core/Mage/Tax/Helper/Data.php

-
Expand Down
1 change: 0 additions & 1 deletion .rector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
CodeQuality\FunctionLike\SimplifyUselessVariableRector::class, # todo: TMP
CodeQuality\Identical\SimplifyBoolIdenticalTrueRector::class, # todo: TMP
CodeQuality\Identical\SimplifyConditionsRector::class, # todo: TMP
CodeQuality\If_\CombineIfRector::class, # todo: TMP<
CodeQuality\If_\CompleteMissingIfElseBracketRector::class, # todo: TMP (!?!)
CodeQuality\If_\ExplicitBoolCompareRector::class, # todo: TMP
CodeQuality\If_\SimplifyIfElseToTernaryRector::class,
Expand Down
14 changes: 6 additions & 8 deletions app/code/core/Mage/Adminhtml/Block/Catalog/Category/Tab/General.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,12 @@ public function _prepareLayout()

$this->_setFieldset($this->getCategory()->getAttributes(true), $fieldset);

if ($this->getCategory()->getId()) {
if ($this->getCategory()->getLevel() == 1) {
$fieldset->removeField('url_key');
$fieldset->addField('url_key', 'hidden', [
'name' => 'url_key',
'value' => $this->getCategory()->getUrlKey(),
]);
}
if ($this->getCategory()->getId() && $this->getCategory()->getLevel() == 1) {
$fieldset->removeField('url_key');
$fieldset->addField('url_key', 'hidden', [
'name' => 'url_key',
'value' => $this->getCategory()->getUrlKey(),
]);
}

$form->addValues($this->getCategory()->getData());
Expand Down
10 changes: 2 additions & 8 deletions app/code/core/Mage/Adminhtml/Block/Catalog/Form/Renderer/Fieldset/Element.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,8 @@ public function getAttributeCode()
*/
public function canDisplayUseDefault()
{
if ($attribute = $this->getAttribute()) {
if (!$attribute->isScopeGlobal()
&& $this->getDataObject()
&& $this->getDataObject()->getId()
&& $this->getDataObject()->getStoreId()
) {
return true;
}
if (($attribute = $this->getAttribute()) && (!$attribute->isScopeGlobal() && $this->getDataObject() && $this->getDataObject()->getId() && $this->getDataObject()->getStoreId())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a long one. Also, it can simplify to return $condition

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. This doesnt enhance readability.

return true;
}
return false;
}
Expand Down
6 changes: 2 additions & 4 deletions app/code/core/Mage/Adminhtml/Block/Catalog/Product/Edit/Tab/Inventory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,8 @@ public function getFieldValueAsFloat(string $field): float

public function getConfigFieldValue($field)
{
if ($this->getStockItem()) {
if ($this->getStockItem()->getData('use_config_' . $field) == 0) {
return $this->getStockItem()->getData($field);
}
if ($this->getStockItem() && $this->getStockItem()->getData('use_config_' . $field) == 0) {
return $this->getStockItem()->getData($field);
}

return Mage::getStoreConfig(Mage_CatalogInventory_Model_Stock_Item::XML_PATH_ITEM . $field);
Expand Down
39 changes: 17 additions & 22 deletions app/code/core/Mage/Adminhtml/Block/Catalog/Product/Edit/Tabs.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,29 +123,24 @@ protected function _prepareLayout()
}

if ($this->getRequest()->getParam('id', false)) {
if ($this->isModuleEnabled('Mage_Review', 'catalog')) {
if (Mage::getSingleton('admin/session')->isAllowed('admin/catalog/reviews_ratings')) {
$this->addTab('reviews', [
'label' => Mage::helper('catalog')->__('Product Reviews'),
'url' => $this->getUrl('*/*/reviews', ['_current' => true]),
'class' => 'ajax',
]);
}
if ($this->isModuleEnabled('Mage_Review', 'catalog') && Mage::getSingleton('admin/session')->isAllowed('admin/catalog/reviews_ratings')) {
$this->addTab('reviews', [
'label' => Mage::helper('catalog')->__('Product Reviews'),
'url' => $this->getUrl('*/*/reviews', ['_current' => true]),
'class' => 'ajax',
]);
}
if ($this->isModuleEnabled('Mage_Tag', 'catalog')) {
if (Mage::getSingleton('admin/session')->isAllowed('admin/catalog/tag')) {
$this->addTab('tags', [
'label' => Mage::helper('catalog')->__('Product Tags'),
'url' => $this->getUrl('*/*/tagGrid', ['_current' => true]),
'class' => 'ajax',
]);

$this->addTab('customers_tags', [
'label' => Mage::helper('catalog')->__('Customers Tagged Product'),
'url' => $this->getUrl('*/*/tagCustomerGrid', ['_current' => true]),
'class' => 'ajax',
]);
}
if ($this->isModuleEnabled('Mage_Tag', 'catalog') && Mage::getSingleton('admin/session')->isAllowed('admin/catalog/tag')) {
$this->addTab('tags', [
'label' => Mage::helper('catalog')->__('Product Tags'),
'url' => $this->getUrl('*/*/tagGrid', ['_current' => true]),
'class' => 'ajax',
]);
$this->addTab('customers_tags', [
'label' => Mage::helper('catalog')->__('Customers Tagged Product'),
'url' => $this->getUrl('*/*/tagCustomerGrid', ['_current' => true]),
'class' => 'ajax',
]);
}
}

Expand Down
20 changes: 9 additions & 11 deletions app/code/core/Mage/Adminhtml/Block/Catalog/Product/Grid.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,17 +113,15 @@ protected function _prepareCollection()

protected function _addColumnFilterToCollection($column)
{
if ($this->getCollection()) {
if ($column->getId() === 'websites') {
$this->getCollection()->joinField(
'websites',
'catalog/product_website',
'website_id',
'product_id=entity_id',
null,
'left',
);
}
if ($this->getCollection() && $column->getId() === 'websites') {
$this->getCollection()->joinField(
'websites',
'catalog/product_website',
'website_id',
'product_id=entity_id',
null,
'left',
);
}
return parent::_addColumnFilterToCollection($column);
}
Expand Down
8 changes: 3 additions & 5 deletions app/code/core/Mage/Adminhtml/Block/Catalog/Product/Helper/Form/Price.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,9 @@ public function getAfterElementHtml()
}
$store = Mage::app()->getStore($storeId);
$html .= '<strong>[' . (string) $store->getBaseCurrencyCode() . ']</strong>';
if (Mage::helper('tax')->priceIncludesTax($store)) {
if ($attribute->getAttributeCode() !== 'cost') {
$addJsObserver = true;
$html .= ' <strong>[' . Mage::helper('tax')->__('Inc. Tax') . '<span id="dynamic-tax-' . $attribute->getAttributeCode() . '"></span>]</strong>';
}
if (Mage::helper('tax')->priceIncludesTax($store) && $attribute->getAttributeCode() !== 'cost') {
$addJsObserver = true;
$html .= ' <strong>[' . Mage::helper('tax')->__('Inc. Tax') . '<span id="dynamic-tax-' . $attribute->getAttributeCode() . '"></span>]</strong>';
}
}
if ($addJsObserver) {
Expand Down
8 changes: 3 additions & 5 deletions app/code/core/Mage/Adminhtml/Block/Permissions/Tab/Rolesedit.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,9 @@ public function __construct()
/** @var Mage_Admin_Model_Rules $item */
foreach ($rules->getItems() as $item) {
$itemResourceId = $item->getResource_id();
if (array_key_exists(strtolower($itemResourceId), $resources)) {
if ($item->isAllowed()) {
$resources[$itemResourceId]['checked'] = true;
$selrids[] = $itemResourceId;
}
if (array_key_exists(strtolower($itemResourceId), $resources) && $item->isAllowed()) {
$resources[$itemResourceId]['checked'] = true;
$selrids[] = $itemResourceId;
}
}

Expand Down
6 changes: 2 additions & 4 deletions app/code/core/Mage/Adminhtml/Block/Report/Grid/Abstract.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,8 @@ protected function _prepareCollection()
$storeIds = $this->_getStoreIds();

$orderStatuses = $filterData->getData('order_statuses');
if (is_array($orderStatuses)) {
if (count($orderStatuses) == 1 && str_contains($orderStatuses[0], ',')) {
$filterData->setData('order_statuses', explode(',', $orderStatuses[0]));
}
if (is_array($orderStatuses) && (count($orderStatuses) == 1 && str_contains($orderStatuses[0], ','))) {
$filterData->setData('order_statuses', explode(',', $orderStatuses[0]));
}

/** @var Mage_Sales_Model_Resource_Report_Collection_Abstract $resourceCollection */
Expand Down
20 changes: 9 additions & 11 deletions app/code/core/Mage/Adminhtml/Block/Sales/Order/Create/Items/Grid.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,15 @@ public function getItems()
// To dispatch inventory event sales_quote_item_qty_set_after, set item qty
$item->setQty($item->getQty());
$stockItem = $item->getProduct()->getStockItem();
if ($stockItem instanceof Mage_CatalogInventory_Model_Stock_Item) {
// This check has been performed properly in Inventory observer, so it has no sense
/*
$check = $stockItem->checkQuoteItemQty($item->getQty(), $item->getQty(), $item->getQty());
$item->setMessage($check->getMessage());
$item->setHasError($check->getHasError());
*/
if ($item->getProduct()->getStatus() == Mage_Catalog_Model_Product_Status::STATUS_DISABLED) {
$item->setMessage(Mage::helper('adminhtml')->__('This product is currently disabled.'));
$item->setHasError(true);
}
// This check has been performed properly in Inventory observer, so it has no sense
/*
$check = $stockItem->checkQuoteItemQty($item->getQty(), $item->getQty(), $item->getQty());
$item->setMessage($check->getMessage());
$item->setHasError($check->getHasError());
*/
if ($stockItem instanceof Mage_CatalogInventory_Model_Stock_Item && $item->getProduct()->getStatus() == Mage_Catalog_Model_Product_Status::STATUS_DISABLED) {
$item->setMessage(Mage::helper('adminhtml')->__('This product is currently disabled.'));
$item->setHasError(true);
}
}
$this->getQuote()->setIsSuperMode($oldSuperMode);
Expand Down
20 changes: 9 additions & 11 deletions app/code/core/Mage/Adminhtml/Block/Sales/Order/Creditmemo/Create/Items.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,19 +160,17 @@ public function canReturnToStock()
*/
public function canReturnItemsToStock()
{
if (is_null($this->_canReturnToStock)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh boy.. setting something in a if and also in a setter.

if ($this->_canReturnToStock = Mage::getStoreConfig(Mage_CatalogInventory_Model_Stock_Item::XML_PATH_CAN_SUBTRACT)) {
$canReturnToStock = false;
foreach ($this->getCreditmemo()->getAllItems() as $item) {
$product = Mage::getModel('catalog/product')->load($item->getOrderItem()->getProductId());
if ($product->getId() && $product->getStockItem()->getManageStock()) {
$item->setCanReturnToStock($canReturnToStock = true);
} else {
$item->setCanReturnToStock(false);
}
if (is_null($this->_canReturnToStock) && $this->_canReturnToStock = Mage::getStoreConfig(Mage_CatalogInventory_Model_Stock_Item::XML_PATH_CAN_SUBTRACT)) {
$canReturnToStock = false;
foreach ($this->getCreditmemo()->getAllItems() as $item) {
$product = Mage::getModel('catalog/product')->load($item->getOrderItem()->getProductId());
if ($product->getId() && $product->getStockItem()->getManageStock()) {
$item->setCanReturnToStock($canReturnToStock = true);
} else {
$item->setCanReturnToStock(false);
}
$this->getCreditmemo()->getOrder()->setCanReturnToStock($this->_canReturnToStock = $canReturnToStock);
}
$this->getCreditmemo()->getOrder()->setCanReturnToStock($this->_canReturnToStock = $canReturnToStock);
}
return $this->_canReturnToStock;
}
Expand Down
18 changes: 6 additions & 12 deletions app/code/core/Mage/Adminhtml/Block/Sales/Order/Invoice/View.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,12 @@ public function __construct()

$orderPayment = $this->getInvoice()->getOrder()->getPayment();

if ($this->_isAllowedAction('creditmemo') && $this->getInvoice()->getOrder()->canCreditmemo()) {
if (($orderPayment->canRefundPartialPerInvoice()
&& $this->getInvoice()->canRefund()
&& $orderPayment->getAmountPaid() > $orderPayment->getAmountRefunded())
|| ($orderPayment->canRefund() && !$this->getInvoice()->getIsUsedForRefund())
) {
$this->_addButton('capture', [ // capture?
'label' => Mage::helper('sales')->__('Credit Memo'),
'class' => 'go',
'onclick' => Mage::helper('core/js')->getSetLocationJs($this->getCreditMemoUrl()),
]);
}
if ($this->_isAllowedAction('creditmemo') && $this->getInvoice()->getOrder()->canCreditmemo() && ($orderPayment->canRefundPartialPerInvoice() && $this->getInvoice()->canRefund() && $orderPayment->getAmountPaid() > $orderPayment->getAmountRefunded() || $orderPayment->canRefund() && !$this->getInvoice()->getIsUsedForRefund())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also way too long to process. Can refactor do these in a seperate line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll check all long lines later.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. It would be nice if after a certain character cutoff Rector could stack conditions vertically for better readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hoped php-cs-fixer would jump in to fix it, but seems i have to do it manually.

$this->_addButton('capture', [ // capture?
'label' => Mage::helper('sales')->__('Credit Memo'),
'class' => 'go',
'onclick' => Mage::helper('core/js')->getSetLocationJs($this->getCreditMemoUrl()),
]);
}

if ($this->_isAllowedAction('capture') && $this->getInvoice()->canCapture()) {
Expand Down
6 changes: 2 additions & 4 deletions app/code/core/Mage/Adminhtml/Block/System/Convert/Gui/Edit/Tab/Wizard.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,8 @@ public function getAttributes($entityType)
*/
public function getValue($key, $default = '', $defaultNew = null)
{
if ($defaultNew !== null) {
if ($this->getProfileId() == 0) {
$default = $defaultNew;
}
if ($defaultNew !== null && $this->getProfileId() == 0) {
$default = $defaultNew;
}

$value = $this->getData($key);
Expand Down
6 changes: 2 additions & 4 deletions app/code/core/Mage/Adminhtml/Block/Widget/Grid/Column.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,8 @@ public function getRowFieldExport(Varien_Object $row)
*/
protected function &_applyDecorators($value, $decorators)
{
if (!is_array($decorators)) {
if (is_string($decorators)) {
$decorators = explode(' ', $decorators);
}
if (!is_array($decorators) && is_string($decorators)) {
$decorators = explode(' ', $decorators);
}
if ((!is_array($decorators)) || empty($decorators)) {
return $value;
Expand Down
25 changes: 11 additions & 14 deletions app/code/core/Mage/Adminhtml/Controller/Sales/Creditmemo.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,21 +70,18 @@ public function viewAction()
*/
public function emailAction()
{
if ($creditmemoId = $this->getRequest()->getParam('creditmemo_id')) {
if ($creditmemo = Mage::getModel('sales/order_creditmemo')->load($creditmemoId)) {
$creditmemo->sendEmail();
$historyItem = Mage::getResourceModel('sales/order_status_history_collection')
->getUnnotifiedForInstance($creditmemo, Mage_Sales_Model_Order_Creditmemo::HISTORY_ENTITY_NAME);
if ($historyItem) {
$historyItem->setIsCustomerNotified(1);
$historyItem->save();
}

$this->_getSession()->addSuccess(Mage::helper('sales')->__('The message was sent.'));
$this->_redirect('*/sales_order_creditmemo/view', [
'creditmemo_id' => $creditmemoId,
]);
if (($creditmemoId = $this->getRequest()->getParam('creditmemo_id')) && $creditmemo = Mage::getModel('sales/order_creditmemo')->load($creditmemoId)) {
$creditmemo->sendEmail();
$historyItem = Mage::getResourceModel('sales/order_status_history_collection')
->getUnnotifiedForInstance($creditmemo, Mage_Sales_Model_Order_Creditmemo::HISTORY_ENTITY_NAME);
if ($historyItem) {
$historyItem->setIsCustomerNotified(1);
$historyItem->save();
}
$this->_getSession()->addSuccess(Mage::helper('sales')->__('The message was sent.'));
$this->_redirect('*/sales_order_creditmemo/view', [
'creditmemo_id' => $creditmemoId,
]);
}
}

Expand Down
26 changes: 12 additions & 14 deletions app/code/core/Mage/Adminhtml/Controller/Sales/Invoice.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,21 +82,19 @@ public function viewAction()
*/
public function emailAction()
{
if ($invoiceId = $this->getRequest()->getParam('invoice_id')) {
if ($invoice = Mage::getModel('sales/order_invoice')->load($invoiceId)) {
$invoice->sendEmail();
$historyItem = Mage::getResourceModel('sales/order_status_history_collection')
->getUnnotifiedForInstance($invoice, Mage_Sales_Model_Order_Invoice::HISTORY_ENTITY_NAME);
if ($historyItem) {
$historyItem->setIsCustomerNotified(1);
$historyItem->save();
}
$this->_getSession()->addSuccess(Mage::helper('sales')->__('The message has been sent.'));
$this->_redirect('*/sales_invoice/view', [
'order_id' => $invoice->getOrder()->getId(),
'invoice_id' => $invoiceId,
]);
if (($invoiceId = $this->getRequest()->getParam('invoice_id')) && $invoice = Mage::getModel('sales/order_invoice')->load($invoiceId)) {
$invoice->sendEmail();
$historyItem = Mage::getResourceModel('sales/order_status_history_collection')
->getUnnotifiedForInstance($invoice, Mage_Sales_Model_Order_Invoice::HISTORY_ENTITY_NAME);
if ($historyItem) {
$historyItem->setIsCustomerNotified(1);
$historyItem->save();
}
$this->_getSession()->addSuccess(Mage::helper('sales')->__('The message has been sent.'));
$this->_redirect('*/sales_invoice/view', [
'order_id' => $invoice->getOrder()->getId(),
'invoice_id' => $invoiceId,
]);
}
}

Expand Down
Loading
Loading