Skip to content

GetClassToClassKeywordFixer - introduction#5953

Merged
SpacePossum merged 1 commit intoPHP-CS-Fixer:masterfrom
paulbalandan:get-class-to-class-keyword
Dec 14, 2021
Merged

GetClassToClassKeywordFixer - introduction#5953
SpacePossum merged 1 commit intoPHP-CS-Fixer:masterfrom
paulbalandan:get-class-to-class-keyword

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 -----------

@paulbalandan paulbalandan force-pushed the get-class-to-class-keyword branch from 2a7fa99 to 30d7386 Compare September 4, 2021 16:06
@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?

@paulbalandan paulbalandan force-pushed the get-class-to-class-keyword branch from 752a326 to 3be174a Compare September 5, 2021 11:51
@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

@paulbalandan paulbalandan force-pushed the get-class-to-class-keyword branch from 3be174a to 6b808cc Compare September 13, 2021 12:56
@paulbalandan paulbalandan force-pushed the get-class-to-class-keyword branch from 6b808cc to c5a3dc0 Compare October 1, 2021 05:11
@keradus keradus force-pushed the get-class-to-class-keyword branch from c91a4dd to 0337605 Compare October 5, 2021 07:12
@keradus keradus added the RTM Ready To Merge label Oct 5, 2021
@paulbalandan paulbalandan force-pushed the get-class-to-class-keyword branch from 0337605 to c87dbde Compare October 5, 2021 09:03
@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 SpacePossum force-pushed the get-class-to-class-keyword branch from c7980d4 to ee541a6 Compare December 14, 2021 08:17
@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.

6 participants