Skip to content
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.

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

Rector: CQ - SimplifyIfReturnBoolRector #4152

Closed
wants to merge 40 commits into from
Closed
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
3d7eaf6
Rector: CQ - UnusedForeachValueToArrayKeysRector (#1)
sreichel Jul 22, 2024
53ee2f8
Merge branch 'OpenMage:main' into main
sreichel Jul 24, 2024
89cc37c
Merge branch 'OpenMage:main' into main
sreichel Jul 24, 2024
e939180
Merge branch 'OpenMage:main' into main
sreichel Jul 31, 2024
35c91ef
Merge branch 'OpenMage:main' into main
sreichel Jul 31, 2024
a7fe45c
Merge branch 'OpenMage:main' into main
sreichel Aug 1, 2024
c7d6aeb
Merge branch 'OpenMage:main' into main
sreichel Aug 7, 2024
30fb790
Added rector to composer dev requirements
sreichel Aug 12, 2024
3904b0f
Updated DDEV command
sreichel Aug 12, 2024
f76521e
Added rector.php with first rule
sreichel Aug 12, 2024
14468be
Updated labeler [skip ci]
sreichel Aug 12, 2024
e788ea7
Added rector to workflow
sreichel Aug 12, 2024
28f12c8
Revert "Rector: CQ - UnusedForeachValueToArrayKeysRector (#1)"
sreichel Aug 12, 2024
3fd569d
Typo
sreichel Aug 12, 2024
954fcdd
Applied rule
sreichel Aug 12, 2024
4fe6c68
Added SimplifyDeMorganBinaryRector rule
sreichel Aug 12, 2024
392d0a8
Revert "Added SimplifyDeMorganBinaryRector rule"
sreichel Aug 12, 2024
f4c7890
Simpler config [skip ci]
sreichel Aug 12, 2024
b094298
CompleteMissingIfElseBracketRector
sreichel Aug 13, 2024
79cd0e9
Merge branch 'main' into rector-2-cq
sreichel Sep 4, 2024
333d3f8
Merge branch 'main' into rector-2-cq
sreichel Sep 6, 2024
ba7e439
Merge branch 'main' into rector-2-cq
sreichel Sep 6, 2024
fb3621a
Merge remote-tracking branch 'origin/rector-2-cq' into rector-2-cq
sreichel Sep 6, 2024
e20935e
Merge branch 'main' into rector-2-cq
sreichel Sep 9, 2024
1fb4750
Merge branch 'main' into rector-2-cq
sreichel Sep 12, 2024
b3cdb0a
Merge branch 'main' into rector-2-cq
sreichel Sep 12, 2024
fce8547
Merge remote-tracking branch 'origin/rector-2-cq' into rector-2-cq
sreichel Sep 17, 2024
575046a
Merge branch 'main' into rector-2-cq
sreichel Sep 17, 2024
ea44e92
Merge branch 'main' into rector-2-cq
sreichel Sep 17, 2024
030a69c
Added already applied rules
sreichel Sep 18, 2024
1a84a3b
Merge branch 'main' into rector-2-cq
sreichel Sep 18, 2024
2db67c9
Merge branch 'main' into rector-2-cq
sreichel Sep 18, 2024
1a5704e
Fixed errors
sreichel Sep 19, 2024
005833e
Merge branch 'main' into rector-2-cq
sreichel Sep 19, 2024
ae0df9e
Ignore dev shell-scripts
sreichel Sep 19, 2024
5696be4
Merge remote-tracking branch 'origin/rector-2-cq' into rector-2-cq
sreichel Sep 19, 2024
6110b0d
Merge branch 'main' into rector-2-cq
sreichel Sep 22, 2024
488acc8
Rectored fixes
sreichel Sep 22, 2024
7f09ae1
Merge branch 'main' into rector-2-cq
sreichel Sep 23, 2024
0fc002f
Merge branch 'main' into rector-2-cq
sreichel Sep 23, 2024
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
1 change: 0 additions & 1 deletion .ddev/commands/web/rector
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,4 @@
## Usage: rector
## Example: ddev rector <path-to-files>

cp -n vendor/sreichel/openmage-rector/rector.php rector.php
php vendor/bin/rector process "$@"
8 changes: 8 additions & 0 deletions .github/labeler.yml
Original file line number Diff line number Diff line change
Expand Up @@ -897,6 +897,7 @@
dev/test/*,
dev/phpunit*,
dev/sonar*,
phpunit*,
.github/workflows/phpunit.yml,
.github/workflows/sonar.yml
]
Expand All @@ -907,3 +908,10 @@
.ddev/*,
.ddev/**/*
]

'rector':
- changed-files:
- any-glob-to-any-file: [
rector.php,
.github/workflows/rector.yml
]
1 change: 1 addition & 0 deletions .github/workflows/check-files.yml
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ jobs:
**phpcs**
**php-cs-fixer**
**phpstan**
rector.php
dev/tests/
dev/phpunit*
dev/sonar*
Expand Down
37 changes: 37 additions & 0 deletions .github/workflows/rector.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
name: Rector

on:
workflow_call:
# Allow manually triggering the workflow.
workflow_dispatch:

jobs:
rector:
name: Validation
runs-on: [ubuntu-latest]

steps:
- name: Setup PHP
uses: shivammathur/setup-php@v2
with:
php-version: 7.4

- name: Checkout code
uses: actions/checkout@v4

- name: Get composer cache directory
id: composer-cache
run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT

- name: Cache dependencies
uses: actions/cache@v4
with:
path: ${{ steps.composer-cache.outputs.dir }}
key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.lock') }}
restore-keys: ${{ runner.os }}-composer-

- name: Install dependencies
run: composer install --prefer-dist --no-progress --ignore-platform-req=ext-*

- name: Rector
run: php vendor/bin/rector process --dry-run
8 changes: 8 additions & 0 deletions .github/workflows/workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,14 @@ jobs:
if: needs.check.outputs.xml > 0
uses: ./.github/workflows/syntax-xml.yml

rector:
name: Rector
needs: [check, phpcs, php-cs-fixer]
if: |
needs.check.outputs.php > 0 ||
needs.check.outputs.workflow > 0
uses: ./.github/workflows/rector.yml

# DOES NOT run by default
# runs on schedule or when worklfow or unit tests changed
sonar:
Expand Down
6 changes: 1 addition & 5 deletions app/code/core/Mage/Admin/Model/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -696,11 +696,7 @@ public function isResetPasswordLinkTokenExpired()
}

$hoursDifference = floor(($currentTimestamp - $tokenTimestamp) / (60 * 60));
if ($hoursDifference >= $tokenExpirationPeriod) {
return true;
}

return false;
return $hoursDifference >= $tokenExpirationPeriod;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,7 @@ public function getCategoryPath()
public function hasStoreRootCategory()
{
$root = $this->getRoot();
if ($root && $root->getId()) {
return true;
}
return false;
return $root && $root->getId();
}

public function getStore()
Expand Down
5 changes: 1 addition & 4 deletions app/code/core/Mage/Adminhtml/Block/Catalog/Product.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,6 @@ public function getGridHtml()
*/
public function isSingleStoreMode()
{
if (!Mage::app()->isSingleStoreMode()) {
return false;
}
return true;
return Mage::app()->isSingleStoreMode();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,7 @@ public function isReadonly()

public function isNew()
{
if ($this->getProduct()->getId()) {
return false;
}
return true;
return !$this->getProduct()->getId();
}

public function getFieldSuffix()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,10 +309,7 @@ public function isScopeGlobal()
*/
public function isShowWebsiteColumn()
{
if ($this->isScopeGlobal() || Mage::app()->isSingleStoreMode()) {
return false;
}
return true;
return !($this->isScopeGlobal() || Mage::app()->isSingleStoreMode());
}

/**
Expand All @@ -322,9 +319,6 @@ public function isShowWebsiteColumn()
*/
public function isAllowChangeWebsite()
{
if (!$this->isShowWebsiteColumn() || $this->getProduct()->getStoreId()) {
return false;
}
return true;
return !(!$this->isShowWebsiteColumn() || $this->getProduct()->getStoreId());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,7 @@ public function getLabel()
*/
public function canDisplayUseDefault($attribute)
{
if (!$attribute->isScopeGlobal() && $this->getDataObject()->getStoreId()) {
return true;
}

return false;
return !$attribute->isScopeGlobal() && $this->getDataObject()->getStoreId();
}

/**
Expand Down Expand Up @@ -161,12 +157,7 @@ public function getAttributeReadonly($attribute)
if (is_object($attribute)) {
$attribute = $attribute->getAttributeCode();
}

if ($this->getDataObject()->isLockedAttribute($attribute)) {
return true;
}

return false;
return $this->getDataObject()->isLockedAttribute($attribute);
}

public function toHtml()
Expand Down
10 changes: 2 additions & 8 deletions app/code/core/Mage/Adminhtml/Block/Customer/Edit/Tab/View.php
Original file line number Diff line number Diff line change
Expand Up @@ -224,21 +224,15 @@ public function getTabTitle()
*/
public function canShowTab()
{
if (Mage::registry('current_customer')->getId()) {
return true;
}
return false;
return (bool) Mage::registry('current_customer')->getId();
}

/**
* @return bool
*/
public function isHidden()
{
if (Mage::registry('current_customer')->getId()) {
return false;
}
return true;
return !Mage::registry('current_customer')->getId();
}

/**
Expand Down
5 changes: 1 addition & 4 deletions app/code/core/Mage/Adminhtml/Block/Denied.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ class Mage_Adminhtml_Block_Denied extends Mage_Adminhtml_Block_Template
public function hasAvailaleResources()
{
$user = Mage::getSingleton('admin/session')->getUser();
if ($user && $user->hasAvailableResources()) {
return true;
}
return false;
return $user && $user->hasAvailableResources();
}
}
9 changes: 2 additions & 7 deletions app/code/core/Mage/Adminhtml/Block/Notification/Toolbar.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,8 @@ public function isShow()
if ($this->getRequest()->getControllerName() === 'notification') {
return false;
}
if ($this->getCriticalCount() == 0 && $this->getMajorCount() == 0 && $this->getMinorCount() == 0
&& $this->getNoticeCount() == 0
) {
return false;
}

return true;
return !($this->getCriticalCount() == 0 && $this->getMajorCount() == 0 && $this->getMinorCount() == 0
&& $this->getNoticeCount() == 0);
Comment on lines +56 to +57
Copy link
Contributor

Choose a reason for hiding this comment

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

We can simplify with DeMorgan's Law !(A && B) == (!A || !B):

Suggested change
return !($this->getCriticalCount() == 0 && $this->getMajorCount() == 0 && $this->getMinorCount() == 0
&& $this->getNoticeCount() == 0);
return $this->getCriticalCount() || $this->getMajorCount() || $this->getMinorCount() || $this->getNoticeCount();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@kiatng kiatng Aug 12, 2024

Choose a reason for hiding this comment

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

Oh...so rector rules cannot be chained? It has to be worst before better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With rulessets (chained rules) the PRs would grow to large. Mhhh.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I created an issue: rectorphp/rector#8781

}

/**
Expand Down
15 changes: 3 additions & 12 deletions app/code/core/Mage/Adminhtml/Block/Sales/Items/Abstract.php
Original file line number Diff line number Diff line change
Expand Up @@ -562,10 +562,7 @@ public function canShipPartially($order = null)
$order = Mage::registry('current_shipment')->getOrder();
}
$value = $order->getCanShipPartially();
if (!is_null($value) && !$value) {
return false;
}
return true;
return !(!is_null($value) && !$value);
Copy link
Contributor

Choose a reason for hiding this comment

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

DeMorgan's Law !(A && B) == (!A || !B):

Suggested change
if (!is_null($value) && !$value) {
return false;
}
return true;
return !(!is_null($value) && !$value);
return is_null($value) || $value;

}

/**
Expand All @@ -580,17 +577,11 @@ public function canShipPartiallyItem($order = null)
$order = Mage::registry('current_shipment')->getOrder();
}
$value = $order->getCanShipPartiallyItem();
if (!is_null($value) && !$value) {
return false;
}
return true;
return !(!is_null($value) && !$value);
Comment on lines -577 to +574
Copy link
Contributor

Choose a reason for hiding this comment

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

DeMorgan's Law !(A && B) == (!A || !B):

Suggested change
if (!is_null($value) && !$value) {
return false;
}
return true;
return !(!is_null($value) && !$value);
return is_null($value) || $value;

}

public function isShipmentRegular()
{
if (!$this->canShipPartiallyItem() || !$this->canShipPartially()) {
return false;
}
return true;
return !(!$this->canShipPartiallyItem() || !$this->canShipPartially());
Comment on lines -585 to +579
Copy link
Contributor

@kiatng kiatng Aug 12, 2024

Choose a reason for hiding this comment

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

Apply DeMorgan's Law: !(A || B) == (!A && !B)

Suggested change
if (!$this->canShipPartiallyItem() || !$this->canShipPartially()) {
return false;
}
return true;
return !(!$this->canShipPartiallyItem() || !$this->canShipPartially());
return $this->canShipPartiallyItem() && $this->canShipPartially();

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,7 @@ protected function _canUseMethod($method)
public function hasMethods()
{
$methods = $this->getMethods();
if (is_array($methods) && count($methods)) {
return true;
}
return false;
return is_array($methods) && count($methods);
}

/**
Expand Down Expand Up @@ -86,9 +83,6 @@ public function hasSsCardType()
{
$availableTypes = explode(',', $this->getQuote()->getPayment()->getMethod()->getConfigData('cctypes'));
$ssPresenations = array_intersect(['SS', 'SM', 'SO'], $availableTypes);
if ($availableTypes && count($ssPresenations) > 0) {
return true;
}
return false;
return $availableTypes && count($ssPresenations) > 0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ public function getCommentNote()
public function getNoteNotify()
{
$notify = $this->getQuote()->getCustomerNoteNotify();
if (is_null($notify) || $notify) {
return true;
}
return false;
return is_null($notify) || $notify;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,7 @@ public function hasInvoiceShipmentTypeMismatch()
public function canShipPartiallyItem()
{
$value = $this->getOrder()->getCanShipPartiallyItem();
if (!is_null($value) && !$value) {
return false;
}
return true;
return !(!is_null($value) && !$value);
Copy link
Contributor

Choose a reason for hiding this comment

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

DeMorgan's Law !(A && B) == (!A || !B):

Suggested change
if (!is_null($value) && !$value) {
return false;
}
return true;
return !(!is_null($value) && !$value);
return is_null($value) || $value;

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,10 +231,7 @@ public function displayCustomsValue()
$storeId
);
$recipientAddressCountryCode = $address->getCountryId();
if ($shipperAddressCountryCode != $recipientAddressCountryCode) {
return true;
}
return false;
return $shipperAddressCountryCode != $recipientAddressCountryCode;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,7 @@ public function displayCustomsValue()
$storeId
);
$recipientAddressCountryCode = $address->getCountryId();
if ($shipperAddressCountryCode != $recipientAddressCountryCode) {
return true;
}
return false;
return $shipperAddressCountryCode != $recipientAddressCountryCode;
}

/**
Expand Down
10 changes: 2 additions & 8 deletions app/code/core/Mage/Adminhtml/Block/System/Config/Form.php
Original file line number Diff line number Diff line change
Expand Up @@ -565,10 +565,7 @@ public function canUseDefaultValue($field)
if ($this->getScope() == self::SCOPE_STORES && $field) {
return true;
}
if ($this->getScope() == self::SCOPE_WEBSITES && $field) {
return true;
}
return false;
return $this->getScope() == self::SCOPE_WEBSITES && $field;
}

/**
Expand All @@ -577,10 +574,7 @@ public function canUseDefaultValue($field)
*/
public function canUseWebsiteValue($field)
{
if ($this->getScope() == self::SCOPE_STORES && $field) {
return true;
}
return false;
return $this->getScope() == self::SCOPE_STORES && $field;
}

/**
Expand Down
10 changes: 2 additions & 8 deletions app/code/core/Mage/Adminhtml/Block/Widget/Grid.php
Original file line number Diff line number Diff line change
Expand Up @@ -1395,10 +1395,7 @@ public function getExcel($filename = '')
*/
public function canDisplayContainer()
{
if ($this->getRequest()->getQuery('ajax')) {
return false;
}
return true;
return !$this->getRequest()->getQuery('ajax');
}

/**
Expand Down Expand Up @@ -1817,10 +1814,7 @@ public function shouldRenderCell($item, $column)
if ($this->isColumnGrouped($column) && $item->getIsEmpty()) {
return true;
}
if (!$item->getIsEmpty()) {
return true;
}
return false;
return !$item->getIsEmpty();
}

/**
Expand Down
Loading
Loading