Add PhpdocFullyQualifiedTypesFixer #3644

Slamdunk · 2018-03-28T13:59:38Z

Reference: #3276
What about fixing PHPDoc too?

 use Foo\Bar;

 /**
- * @param \Foo\Bar $foo
+ * @param Bar $foo
  */
 function foo($foo) {}

Ping @veewee

keradus · 2018-03-28T14:03:35Z

3276 was about changing FQCN to short CN, why doing the reverse ?

Slamdunk · 2018-03-28T14:04:34Z

3276 was about changing FQCN to short CN, why doing the reverse ?

The topic reports the Expected VS Actual diff of the test, nothing to reverse here.

veewee · 2018-03-28T14:18:30Z

@Slamdunk : that could indeed be a new feature. Currently I did not touch any docblocks. Personally, I am removing these kind of hard to maintain @param docblocks since they do not make any sense when using strict types anymore.

Slamdunk · 2018-03-28T14:35:30Z

Personally, I am removing these kind of hard to maintain @param docblocks since they do not make any sense when using strict types anymore.

Heh, if only we could...

that could indeed be a new feature. Currently I did not touch any docblocks.

I pinged you because #3276 seems a complex work and maybe you faces some issue I need to know before building this new feature.

veewee · 2018-03-28T14:50:16Z

Didn't face any issues regarding to the doblocks.
The fixer itself isn't that complex, the hard part was abstracting the logic into generic analyzers.
It should be fairly simple to detect the function docblock and change the param / return tags.

You could add a fixFunctionDocblocks() method underneeth this line:
https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/2.11/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php#L97

The way I think it should work:

Search for previous docblock token and make sure it is the doblock of current function.
Parse param and return tags. (There is a class for that)
Split up the method detectAndReplaceTypeWithShortType() in detectShortType() and replaceFqcnWithShortType() or something like that
Reuse the detectShortType to replace the param / return tags
replace old docblocks with new docblocks
party like it's 1999

keradus · 2018-03-28T15:47:41Z

The topic reports the Expected VS Actual diff of the test, nothing to reverse here.

I treated your opening post as feature request definition, not as output of failing test :D

little 👍 for the feature, but indeed, it's better to move it to prototype if possible, but that would be harder, so probably it's worth to do easier feature first.

Slamdunk · 2018-03-29T06:56:27Z

I treated your opening post as feature request definition, not as output of failing test :D

You are right, it was misleading. I do so for bug reports, but it is wrong for new feature .

it's better to move it to prototype if possible

I don't understand what you mean

ostrolucky · 2018-03-30T19:01:46Z

This should be different fixer, in Phpdoc namespace - to have better ability to deal with priority conflicts. FullyQualifiedStrictTypesFixer is wrong name for such a thing.

julienfalque · 2018-04-03T19:33:24Z

👍 for this. I'm fine if the current fixer also deals with PHPDocs rather than creating a new fixer.

ostrolucky · 2018-05-19T19:23:52Z

For others looking for this, there is Symplify\CodingStandard\Fixer\Import\ImportNamespacedNameFixer, which does this and additionally imports the statement if it wasn't imported yet

SpacePossum · 2020-01-29T16:19:03Z

src/Fixer/Phpdoc/PhpdocFullyQualifiedTypesFixer.php

+    public function getPriority()
+    {
+        /*
+         * Should be run before all other docblock fixers apart from the


is this a copy-paste-comment?

Yes , I've started from PhpdocScalarFixer and it seemed to me a good idea to retain part of it.

ok, than we should back up this comments claim with priority test fixtures so we no longer need this comment 👍

So, I've added all the priority tests that I've could imaging, and then had to delete most of them except two cause of a test allowing only meaningful priority tests 😃

That is my concern 😃 so a lot PHPDoc fixer claim that they must run before a whole bunch of other fixers, yet creating a priority test to prove it seems not possible.

src/AbstractPhpdocTypesFixer.php

src/Fixer/Phpdoc/PhpdocFullyQualifiedTypesFixer.php

SpacePossum · 2020-01-29T16:35:36Z

src/Fixer/Phpdoc/PhpdocFullyQualifiedTypesFixer.php

+            return $type;
+        }
+
+        return (new TypeShortNameResolver())->resolve($tokens, $type);


is there nice way to prevent creating a new instance of new TypeShortNameResolver() for each candidate? (maybe private property of the fixer)

TypeShortNameResolver is a stateless class, and I've read few times @julienfalque to prefer not saving anything privately due to performance reason.

Moreover, FullyQualifiedStrictTypesFixer do the same:

https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/c8afb599858876e95e8ebfcd97812d383fa23f02/src/Fixer/Import/FullyQualifiedStrictTypesFixer.php#L155

this is no longer true

Slamdunk · 2020-01-30T07:45:43Z

Could someone please fix the PR label to kind/feature?

mvorisek · 2023-07-30T10:32:03Z

tests/Fixer/Phpdoc/PhpdocFullyQualifiedTypesFixerTest.php

+use Foo\Bar;
+
+/**
+ * @return null|int|Bar


add some more complex phpdoc types like nested type, generic, array shape, conditional type

mvorisek · 2023-07-30T10:34:39Z

src/Fixer/Phpdoc/PhpdocFullyQualifiedTypesFixer.php

+        }
+
+        // Dummy start and end indexes, we only need isReservedType result
+        if ((new TypeAnalysis(substr($type, 1), 1, 2))->isReservedType()) {


since https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/pull/7188/files#diff-9eb8d1884736f293d0a3ea5127a6713fea7a744e293f41b48b263e910297a4bdR66 the 1, 2 dummy values are not needed

Wirone · 2024-01-09T16:25:28Z

Actually it was implemented in #7622 so we can close this 🙂. Thanks @Slamdunk for your work!

Slamdunk mentioned this pull request Mar 30, 2018

Apply fully_qualified_strict_types in annotations #3655

Open

Slamdunk mentioned this pull request Apr 3, 2018

Add PhpdocToReturnTypeFixer #3640

Merged

14 tasks

julienfalque added the kind/feature label Apr 3, 2018

keradus added kind/feature request and removed kind/feature labels May 17, 2018

Slamdunk force-pushed the fq_to_sq_phpdoc branch from 8f8055c to 1553a3f Compare January 29, 2020 11:54

Slamdunk changed the title ~~FullyQualifiedStrictTypesFixer: fix PHPDoc too~~ Add PhpdocFullyQualifiesTypesFixer Jan 29, 2020

Slamdunk requested a review from SpacePossum January 29, 2020 14:56

SpacePossum reviewed Jan 29, 2020

View reviewed changes

src/AbstractPhpdocTypesFixer.php Outdated Show resolved Hide resolved

SpacePossum reviewed Jan 29, 2020

View reviewed changes

src/Fixer/Phpdoc/PhpdocFullyQualifiedTypesFixer.php Show resolved Hide resolved

SpacePossum reviewed Jan 29, 2020

View reviewed changes

SpacePossum added this to the 2.17.0 milestone Jan 29, 2020

Slamdunk added 7 commits January 30, 2020 07:49

Test docblock too

a9f0ad9

First implementation

99c4865

FullyQualifiedStrictTypesFixerTest isn't needed anymore

52ffca5

Import test cases from FullyQualifiedStrictTypesFixerTest

d72e310

Handle reserved types and union types

5227508

Use separate Tokens for analysis

076f5bb

Redact correct priority

bd09a2c

Slamdunk added 3 commits January 30, 2020 07:49

Corrected fixer name in readme too

a2788c9

Better AbstractPhpdocTypesFixer::applyFix performance with reverse cycle

723e87a

Explain TypeAnalysis dummy parameters

1bddabf

julienfalque changed the title ~~Add PhpdocFullyQualifiesTypesFixer~~ Add PhpdocFullyQualifiedTypesFixer Jan 30, 2020

Add priority tests with comment_to_phpdoc and phpdoc_align

2f8ca5a

Slamdunk force-pushed the fq_to_sq_phpdoc branch from d82119e to 2f8ca5a Compare January 30, 2020 07:41

julienfalque added kind/feature and removed kind/feature request labels Jan 30, 2020

keradus modified the milestones: 2.17.0, 2.17.1, 2.18.0 Dec 7, 2020

keradus removed this from the 2.18.0 milestone Jan 18, 2021

Wirone added the status/to verify issue needs to be confirmed or analysed to continue label Jun 6, 2023

Wirone added topic/phpdoc PHPDoc tags, Doctrine Annotations etc. topic/fqcn Fully Qualified Class Name usage and conversions and removed status/to verify issue needs to be confirmed or analysed to continue labels Jun 17, 2023

Wirone added the status/rebase required PR is outdated and required synchronisation with main branch label Jul 2, 2023

mvorisek reviewed Jul 30, 2023

View reviewed changes

Wirone closed this Jan 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add PhpdocFullyQualifiedTypesFixer #3644

Add PhpdocFullyQualifiedTypesFixer #3644

Slamdunk commented Mar 28, 2018 •

edited

keradus commented Mar 28, 2018

Slamdunk commented Mar 28, 2018

veewee commented Mar 28, 2018

Slamdunk commented Mar 28, 2018

veewee commented Mar 28, 2018

keradus commented Mar 28, 2018

Slamdunk commented Mar 29, 2018 •

edited

ostrolucky commented Mar 30, 2018 •

edited

julienfalque commented Apr 3, 2018

ostrolucky commented May 19, 2018

SpacePossum Jan 29, 2020

Slamdunk Jan 30, 2020

SpacePossum Jan 30, 2020

Slamdunk Jan 30, 2020

SpacePossum Jan 30, 2020

SpacePossum Jan 29, 2020

Slamdunk Jan 30, 2020

mvorisek Jul 30, 2023

Slamdunk commented Jan 30, 2020

mvorisek Jul 30, 2023

mvorisek Jul 30, 2023

Wirone commented Jan 9, 2024

Add PhpdocFullyQualifiedTypesFixer #3644

Add PhpdocFullyQualifiedTypesFixer #3644

Conversation

Slamdunk commented Mar 28, 2018 • edited

keradus commented Mar 28, 2018

Slamdunk commented Mar 28, 2018

veewee commented Mar 28, 2018

Slamdunk commented Mar 28, 2018

veewee commented Mar 28, 2018

keradus commented Mar 28, 2018

Slamdunk commented Mar 29, 2018 • edited

ostrolucky commented Mar 30, 2018 • edited

julienfalque commented Apr 3, 2018

ostrolucky commented May 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Slamdunk commented Jan 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Wirone commented Jan 9, 2024

Slamdunk commented Mar 28, 2018 •

edited

Slamdunk commented Mar 29, 2018 •

edited

ostrolucky commented Mar 30, 2018 •

edited