[PropertyInfo] Get short description from promoted properties in PhpDocExtractor #59540

wuchen90 · 2025-01-17T16:45:35Z

Q	A
Branch?	6.4
Bug fix?	yes
License	MIT

Currently this kind of phpdoc doesn't work to get description:

class Foo {
  /**
   * @param mixed $foo This description can't be get by PhpDocExtractor
   */
  public function __construct(
    public $foo,
  ) {}
}

Whereas this one works:

class Foo {
  public function __construct(
    /** @var mixed $foo This description can be get by PhpDocExtractor */
    public $foo,
  ) {}
}

OskarStark · 2025-01-17T17:05:39Z

This is a feature and should target 7.3

wuchen90 · 2025-01-17T17:19:30Z

@OskarStark

This is a feature and should target 7.3

I don't know if this is really a feature because it works well with @var annotation.
Only, when we document a method, we usually use @param and not @var.

Beside, this code below points out that we support promoted properties:

symfony/src/Symfony/Component/PropertyInfo/Extractor/PhpDocExtractor.php

Line 239 in 02db9c2

    
           case $reflectionProperty?->isPromoted() && $docBlock = $this->getDocBlockFromConstructor($class, $property):

mtarld

Can you update the changelog as well, as it's a feature?

mtarld · 2025-01-17T17:19:58Z

src/Symfony/Component/PropertyInfo/Extractor/PhpDocExtractor.php

@@ -95,6 +95,18 @@ public function getShortDescription(string $class, string $property, array $cont
            }
        }

+        foreach ($docBlock->getTagsByName('param') as $param) {


Can you update PhpStanExtractor as well?

Ok, I've looked at it and it seems it needs more work for PhpStanExtractor than what I've done here.
So I propose to do it in another PR.

mtarld · 2025-01-17T17:20:56Z

src/Symfony/Component/PropertyInfo/Extractor/PhpDocExtractor.php

+
+            $paramDescription = $param->getDescription()?->render();
+
+            if (null !== $paramDescription && '' !== $paramDescription) {


Suggested change

if (null !== $paramDescription && '' !== $paramDescription) {

if ($paramDescription) {

Are you sure?

<?php $paramDescription = '0'; echo $paramDescription ? 'ok' : 'ko'; // 'ko' echo null !== $paramDescription && '' !== $paramDescription ? 'ok' : 'ko'; // 'ok'

not sure 0 a useful description either

Trimming 0 won't make it less predictable?
I mean in the way that, from the user point of view, we naturally expect that empty string or null can be skipped.
But other than these two cases, shouldn't we return what the user has input or is it what Symfony used to do for this kind of code?
If we skip 0 then I should add a test for it.

OskarStark · 2025-01-17T17:25:58Z

Looking at your code change this seems like it wasn't implemented before, there is no wrong behavior, the feature is just not existent, so a feature to me.

stof · 2025-01-17T17:27:49Z

I think the implementation would now read @param in docblock of any properties, not just for the case of reading the constructor docblock for a promoted property.

wuchen90

Target branch changed to 7.3.

wuchen90 · 2025-01-20T09:19:04Z

src/Symfony/Component/PropertyInfo/Extractor/PhpDocExtractor.php

@@ -95,6 +95,18 @@ public function getShortDescription(string $class, string $property, array $cont
            }
        }

+        foreach ($docBlock->getTagsByName('param') as $param) {


Ok, I've looked at it and it seems it needs more work for PhpStanExtractor than what I've done here.
So I propose to do it in another PR.

wuchen90 · 2025-01-20T09:24:50Z

src/Symfony/Component/PropertyInfo/Extractor/PhpDocExtractor.php

+
+            $paramDescription = $param->getDescription()?->render();
+
+            if (null !== $paramDescription && '' !== $paramDescription) {


Are you sure?

<?php $paramDescription = '0'; echo $paramDescription ? 'ok' : 'ko'; // 'ko' echo null !== $paramDescription && '' !== $paramDescription ? 'ok' : 'ko'; // 'ok'

mtarld · 2025-01-20T10:31:07Z

Indeed, forgot about it, you can keep null !== $paramDescription && '' !== $paramDescription then 🙂

src/Symfony/Component/PropertyInfo/Tests/Fixtures/Extractor/PromotedPropertiesWithDocBlock.php

src/Symfony/Component/PropertyInfo/Tests/Extractor/PhpDocExtractorTest.php

nicolas-grekas · 2025-01-28T16:41:50Z

src/Symfony/Component/PropertyInfo/Extractor/PhpDocExtractor.php

+
+            $paramDescription = $param->getDescription()?->render();
+
+            if (null !== $paramDescription && '' !== $paramDescription) {


not sure 0 a useful description either

…ocExtractor

wuchen90 requested a review from dunglas as a code owner January 17, 2025 16:45

carsonbot added Status: Needs Review Bug PropertyInfo labels Jan 17, 2025

carsonbot added this to the 6.4 milestone Jan 17, 2025

OskarStark modified the milestones: 6.4, 7.3 Jan 17, 2025

mtarld suggested changes Jan 17, 2025

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Needs Review labels Jan 17, 2025

wuchen90 force-pushed the fix-phpdoc-extractor branch from 8a99cd3 to f6c61c1 Compare January 20, 2025 08:25

wuchen90 requested review from lyrixx, yceruto, chalasr, OskarStark, jderusse and xabbuh as code owners January 20, 2025 08:25

carsonbot added Status: Needs Review and removed Status: Needs Work labels Jan 20, 2025

wuchen90 changed the base branch from 6.4 to 7.3 January 20, 2025 08:25

wuchen90 commented Jan 20, 2025

View reviewed changes

nicolas-grekas reviewed Jan 28, 2025

View reviewed changes

[PropertyInfo] Get short description from promoted properties in PhpD…

9141b68

…ocExtractor

wuchen90 force-pushed the fix-phpdoc-extractor branch from f6c61c1 to 9141b68 Compare January 31, 2025 08:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PropertyInfo] Get short description from promoted properties in PhpDocExtractor #59540

[PropertyInfo] Get short description from promoted properties in PhpDocExtractor #59540

wuchen90 commented Jan 17, 2025 •

edited

Loading

OskarStark commented Jan 17, 2025

wuchen90 commented Jan 17, 2025 •

edited

Loading

mtarld left a comment

mtarld Jan 17, 2025

wuchen90 Jan 20, 2025

mtarld Jan 17, 2025

wuchen90 Jan 20, 2025

nicolas-grekas Jan 28, 2025

wuchen90 Jan 31, 2025 •

edited

Loading

OskarStark commented Jan 17, 2025

stof commented Jan 17, 2025

wuchen90 left a comment

wuchen90 Jan 20, 2025

wuchen90 Jan 20, 2025

mtarld commented Jan 20, 2025

nicolas-grekas Jan 28, 2025

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!


		$paramDescription = $param->getDescription()?->render();

		if (null !== $paramDescription && '' !== $paramDescription) {

	if (null !== $paramDescription && '' !== $paramDescription) {
	if ($paramDescription) {

[PropertyInfo] Get short description from promoted properties in PhpDocExtractor #59540

Are you sure you want to change the base?

[PropertyInfo] Get short description from promoted properties in PhpDocExtractor #59540

Conversation

wuchen90 commented Jan 17, 2025 • edited Loading

OskarStark commented Jan 17, 2025

wuchen90 commented Jan 17, 2025 • edited Loading

mtarld left a 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

wuchen90 Jan 31, 2025 • edited Loading

Choose a reason for hiding this comment

OskarStark commented Jan 17, 2025

stof commented Jan 17, 2025

wuchen90 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtarld commented Jan 20, 2025

Choose a reason for hiding this comment

wuchen90 commented Jan 17, 2025 •

edited

Loading

wuchen90 commented Jan 17, 2025 •

edited

Loading

wuchen90 Jan 31, 2025 •

edited

Loading