Skip to content

rector: RemoveUnusedVariableInCatchRector #4834

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

Jump to bottom
Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

rector: RemoveUnusedVariableInCatchRector #4834

wants to merge 7 commits into from

Conversation

sreichel
Copy link
Contributor

@github-actions github-actions bot added Component: PayPal Relates to Mage_Paypal Component: Core Relates to Mage_Core Component: Catalog Relates to Mage_Catalog Component: Reports Relates to Mage_Reports Component: CatalogInventory Relates to Mage_CatalogInventory Component: Checkout Relates to Mage_Checkout Component: AdminNotification Relates to Mage_AdminNotification Component: lib/Varien Relates to lib/Varien Component: Sales Relates to Mage_Sales Component: Usa Relates to Mage_Usa Component: Customer Relates to Mage_Customer Component: Eav Relates to Mage_Eav Component: lib/Mage Relates to lib/Mage Component: Adminhtml Relates to Mage_Adminhtml Mage.php Relates to app/Mage.php Component: Api PageRelates to Mage_Api Component: Tag Relates to Mage_Tag Component: Admin Relates to Mage_Admin Component: Wishlist Relates to Mage_Wishlist Component: Widget Relates to Mage_Widget Component: Rule Relates to Mage_Rule Component: Review Relates to Mage_Review Component: Newsletter Relates to Mage_Newsletter Component: Index Relates to Mage_Index Component: Downloadable Relates to Mage_Downloadable Component: Api2 Relates to Mage_Api2 Component: ImportExport Relates to Mage_ImportExport Component: Directory Relates to Mage_Directory Component: Centinel Relates to Mage_Centinel labels Jun 20, 2025
@github-actions github-actions bot added Component: Api2 Relates to Mage_Api2 Component: ImportExport Relates to Mage_ImportExport Component: Directory Relates to Mage_Directory Component: Centinel Relates to Mage_Centinel Component: Dataflow Relates to Mage_Dataflow Component: Install Relates to Mage_Install Component: lib/* Relates to lib/* rector labels Jun 20, 2025
@sreichel sreichel added the chore label Jun 20, 2025
@sreichel sreichel added this to the 20.15.0 milestone Jun 20, 2025
@sreichel
Copy link
Contributor Author

Pls ignore Sonar.

@sreichel sreichel requested review from Hanmac, addison74 and kiatng July 6, 2025 00:42
@github-actions github-actions bot removed the rector label Jul 6, 2025
@Hanmac
Copy link
Contributor

Hanmac commented Jul 6, 2025

i'm more curious why the Exceptions aren't used, not even logged with Mage::logException?
And in most of the cases where i would throw a different exception, i would use the old exception as previous one.

Also since most of Mage core stuff does throw Throwable instead of Exception now,
wouldn't it be better to catch Throwable instead, or is that too dangerous?

@sreichel
Copy link
Contributor Author

sreichel commented Jul 6, 2025

I think there is some space for improvements. Want to oen a new issue or discussion for that?

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
8.1% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

kiatng
kiatng reviewed Jul 11, 2025
View reviewed changes
app/Mage.php
} catch (Mage_Core_Model_Session_Exception) {
header('Location: ' . self::getBaseUrl());
die;
} catch (Mage_Core_Model_Store_Exception $e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not understand the rationale of this PR. Why $e isn't remove here?

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 don't know. Think I have to report it to rector.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe it is passed to next catch-statement. I dont know. Ignore it for now?

@sreichel sreichel modified the milestones: 20.15.0, 20.16.0 Jul 12, 2025
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Oct 1, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
5.6% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@sreichel
Copy link
Contributor Author

sreichel commented Oct 1, 2025

Pls ignnore sonar.

(in free version it is not possible to adjust that limit)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kiatng kiatng kiatng left review comments

@addison74 addison74 addison74 approved these changes

@Hanmac Hanmac Awaiting requested review from Hanmac

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
chore Component: Admin Relates to Mage_Admin Component: Adminhtml Relates to Mage_Adminhtml Component: AdminNotification Relates to Mage_AdminNotification Component: Api PageRelates to Mage_Api Component: Api2 Relates to Mage_Api2 Component: Catalog Relates to Mage_Catalog Component: CatalogInventory Relates to Mage_CatalogInventory Component: Centinel Relates to Mage_Centinel Component: Checkout Relates to Mage_Checkout Component: Core Relates to Mage_Core Component: Customer Relates to Mage_Customer Component: Dataflow Relates to Mage_Dataflow Component: Directory Relates to Mage_Directory Component: Downloadable Relates to Mage_Downloadable Component: Eav Relates to Mage_Eav Component: ImportExport Relates to Mage_ImportExport Component: Index Relates to Mage_Index Component: Install Relates to Mage_Install Component: lib/Mage Relates to lib/Mage Component: lib/Varien Relates to lib/Varien Component: lib/* Relates to lib/* Component: Newsletter Relates to Mage_Newsletter Component: PayPal Relates to Mage_Paypal Component: Reports Relates to Mage_Reports Component: Review Relates to Mage_Review Component: Rule Relates to Mage_Rule Component: Sales Relates to Mage_Sales Component: Tag Relates to Mage_Tag Component: Usa Relates to Mage_Usa Component: Widget Relates to Mage_Widget Component: Wishlist Relates to Mage_Wishlist Mage.php Relates to app/Mage.php PHP 8 Related to PHP8
Projects
None yet
Milestone
20.16.0
Development

Successfully merging this pull request may close these issues.
4 participants
@sreichel @Hanmac @kiatng @addison74