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

GetClassToClassKeywordFixer - introduction #5953

Merged

Conversation

paulbalandan
Copy link
Contributor

$ php php-cs-fixer describe get_class_to_class_keyword`

Description of get_class_to_class_keyword rule.
Replace `get_class` calls on object variables with class keyword syntax.

Fixer applying this rule is risky.
Risky if the `get_class` function is overridden.

Fixing examples:
 * Example #1.
   ---------- begin diff ----------
   --- Original
   +++ New
   @@ -1,2 +1,2 @@
    <?php
   -get_class($a);
   +$a::class;

   ----------- end diff -----------

 * Example #2.
   ---------- begin diff ----------
   --- Original
   +++ New
   @@ -1,4 +1,4 @@
    <?php

    $date = new \DateTimeImmutable();
   -$class = get_class($date);
   +$class = $date::class;

   ----------- end diff -----------

@coveralls
Copy link

coveralls commented Sep 4, 2021

Coverage Status

Coverage increased (+0.01%) to 93.039% when pulling ee541a6 on paulbalandan:get-class-to-class-keyword into ea5a185 on FriendsOfPHP:master.

@paulbalandan paulbalandan force-pushed the get-class-to-class-keyword branch 2 times, most recently from a60de76 to 752a326 Compare September 4, 2021 16:40
@julienfalque
Copy link
Member

I always considered accessing class constants via object instances a hacky/bad practice, is it not?

@keradus
Copy link
Member

keradus commented Sep 5, 2021

I would agree if we talk about class constant defined as part of class.
I would rather treat ::class as language keyword in this context.

  • you cannot create class constant inside class (Fatal error: A class constant must not be called 'class'; it is reserved for class)
  • calling ::class is also one extra opCode tan calling regular class-level constant (showing it's slightly different concept under the hood) ref

@keradus keradus added the RTM Ready To Merge label Oct 5, 2021
@SpacePossum
Copy link
Contributor

looking good! 👍
currently blocked by #6058

SpacePossum added a commit that referenced this pull request Dec 14, 2021
…okens (paulbalandan, SpacePossum)

This PR was squashed before being merged into the master branch (closes #6058).

Discussion
----------

Fix `Tokens::insertSlices` not moving around all affected tokens

While working on #5953 in refactoring to call `insertSlices` once, I stumbled into a problem where parse error would appear after fixing. After digging into it more, I found out that the culprit is in the `insertSlices` method itself. If there are multiple slices to be inserted in an index and this is repeated in other indexes, there exists a "hole" left in the moving around of tokens. This causes the unmoved tokens to be inadvertently replaced by other else possibly resulting to parse errors after.

Commits
-------

fbac555 Fix `Tokens::insertSlices` not moving around all affected tokens
@SpacePossum SpacePossum force-pushed the get-class-to-class-keyword branch 2 times, most recently from de85002 to c7980d4 Compare December 14, 2021 06:55
@SpacePossum
Copy link
Contributor

Thank you @paulbalandan.

@SpacePossum SpacePossum merged commit 00096f0 into PHP-CS-Fixer:master Dec 14, 2021
@SpacePossum SpacePossum removed the RTM Ready To Merge label Dec 14, 2021
@paulbalandan paulbalandan deleted the get-class-to-class-keyword branch December 14, 2021 08:41
@paulbalandan
Copy link
Contributor Author

Thanks, @SpacePossum

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants