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: automatic code refactoring (part 1) #2879

Merged
merged 18 commits into from
Jan 2, 2023
Merged

rector: automatic code refactoring (part 1) #2879

merged 18 commits into from
Jan 2, 2023

Conversation

sreichel
Copy link
Contributor

@sreichel sreichel commented Jan 1, 2023

Description (*)

We had a lot of code-style improvements made manually (or with help of phpstorm in my case). E.g. Tenery operators. A lot of changes were only made when the file has been touched for some other reason.

Why not automate this and fix all occourences?

I had time to test rector and i am positively surprised!

I tried ~50 rules step by step and ... there is not soo much to change. (stopped with ~500 changed files ... incl. lib)

  1. finish rulset
  2. apply changes in small steps
  3. add OM specific rules (to update 3rd-party-code)

Manual testing scenarios (*)

  1. composer require --dev sreichel/openmage-rector
  2. cp -n vendor/sreichel/openmage-rector/rector.php rector.php
  3. php vendor/bin/rector process <path-to-files> --dry-run

Note: running w/o path (whole project) takes some memory.

Questions or comments

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

@github-actions github-actions bot added Component: Api PageRelates to Mage_Api Component: Catalog Relates to Mage_Catalog Component: Core Relates to Mage_Core Component: lib/Mage Relates to lib/Mage Component: lib/* Relates to lib/* Component: Oauth Relates to Mage_Oauth phpstan labels Jan 1, 2023
@github-actions github-actions bot added the ddev label Jan 1, 2023
@github-actions github-actions bot added the Component: lib/Varien Relates to lib/Varien label Jan 1, 2023
@github-actions github-actions bot added Component: ImportExport Relates to Mage_ImportExport Component: PayPal Relates to Mage_Paypal labels Jan 1, 2023
@github-actions github-actions bot added Component: Adminhtml Relates to Mage_Adminhtml Component: Sales Relates to Mage_Sales labels Jan 1, 2023
@github-actions github-actions bot added the Component: Eav Relates to Mage_Eav label Jan 2, 2023
@github-actions github-actions bot added the Component: CatalogInventory Relates to Mage_CatalogInventory label Jan 2, 2023
@sreichel sreichel changed the title [wip] automatic code refactoring rector: automatic code refactoring (part 1) Jan 2, 2023
This error goes and comes ...
@github-actions github-actions bot added the Component: Downloadable Relates to Mage_Downloadable label Jan 2, 2023
@github-actions github-actions bot added the Component: Dataflow Relates to Mage_Dataflow label Jan 2, 2023
@elidrissidev
Copy link
Member

Cool stuff. But why are certain CI workflows being skipped?

Copy link
Contributor

@fballiano fballiano left a comment

Choose a reason for hiding this comment

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

beautiful

@sreichel
Copy link
Contributor Author

sreichel commented Jan 2, 2023

Cool stuff. But why are certain CI workflows being skipped?

Some checks run on schedule at the moment ... https://github.com/OpenMage/magento-lts/actions?query=event%3Aschedule

@elidrissidev elidrissidev merged commit aff70df into OpenMage:1.9.4.x Jan 2, 2023
@sreichel sreichel deleted the rector-1 branch January 2, 2023 19:42
fballiano pushed a commit that referenced this pull request Jan 3, 2023
* Add "never" return-type for methods that never return anything

- see https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#returnnevertyperector

* Updated phpstan.dist.baseline.neon

* Added ddev rector command shortcut

* Change array_push() to direct variable assign

- see https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#changearraypushtoarrayassignrector

* Replace array_keys() and in_array() to array_key_exists()

- see https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#arraykeysandinarraytoarraykeyexistsrector

* Use common != instead of less known <> with same meaning

- see https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#commonnotequalrector

* Issue in rector found ...reverted

- see rectorphp/rector#7699

* Updated phpstan.dist.baseline.neon

* Change compact() call to own array

- see https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#compacttovariablesrector

* Change OR, AND to ||, && with more common understanding

- see https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#logicaltobooleanrector

* Replace the Double not operator (!!) by type-casting to boolean

- see https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#replacemultiplebooleannotrector

* Simplify array_search to in_array

- see https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#simplifyarraysearchrector

* Changes compared to value and return of expr to direct return

- see https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#simplifyifexactvaluereturnvaluerector

* Updated phpstan.dist.baseline.neon

This error goes and comes ...

* Changes strlen comparison to 0 to direct empty string compare

- see https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#strlenzerotoidenticalemptystringrector

* Simplify conditions

- see https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#simplifyconditionsrector

* Simplify tautology ternary to value

- see https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#simplifytautologyternaryrector

* Splits [$a, $b] = [5, 10] scalar assign to standalone lines

- see https://github.com/rectorphp/rector/blob/main/docs/rector_rules_overview.md#splitlistassigntoseparatelinerector
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Adminhtml Relates to Mage_Adminhtml Component: Api PageRelates to Mage_Api Component: Catalog Relates to Mage_Catalog Component: CatalogInventory Relates to Mage_CatalogInventory Component: Core Relates to Mage_Core Component: Dataflow Relates to Mage_Dataflow Component: Downloadable Relates to Mage_Downloadable Component: Eav Relates to Mage_Eav Component: ImportExport Relates to Mage_ImportExport Component: lib/Mage Relates to lib/Mage Component: lib/Varien Relates to lib/Varien Component: lib/* Relates to lib/* Component: Oauth Relates to Mage_Oauth Component: PayPal Relates to Mage_Paypal Component: Sales Relates to Mage_Sales phpstan
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants