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

Duplicated magic property #3716

Open
adriendupuis opened this issue May 17, 2024 · 2 comments
Open

Duplicated magic property #3716

adriendupuis opened this issue May 17, 2024 · 2 comments

Comments

@adriendupuis
Copy link
Contributor

A magic property can be declared several time without error.
With inheritance, a magic property can't be overridden, it's just duplicated.

Expected behavior

If a magic property is defined twice, there should be a warning.
If a magic property is overridden, the override should replace instead of just being added.

Actual behavior

A magic property can be documented several times.

Steps to reproduce the problem

  1. In an empty folder create the four following classes in respectively BaseClass.php, BaseValueClass.php, HeirClass.php, and HeirValueClass.php.
<?php

/**
 * @property-read BaseValueClass $value
 * @property-read string $value
 */
class BaseClass
{
    public function __construct(array $properties = [])
    {
        foreach ($properties as $property => $value) {
            $this->$property = $value;
        }
    }

    public function __get($property)
    {
        if (property_exists($this, $property)) {
            return $this->$property;
        }
    }
}
<?php

class BaseValueClass
{
}
<?php

/**
 * @property-read HeirValueClass $value
 */
class HeirClass extends BaseClass
{
}
<?php

class HeirValueClass extends BaseValueClass
{
}
  1. Generate the documentation from this folder.
curl -LO "https://github.com/phpDocumentor/phpDocumentor/releases/download/v3.4.3/phpDocumentor.phar";
php phpDocumentor.phar;
  1. Open HeirClass documentation (./.phpdoc/build/classes/HeirClass.html) and see the issues:

There is 3 $value in the Properties section:

  • $value: HeirValueClass
  • $value: BaseValueClass
  • $value: string

Two problems:

  • The double declaration of $value in BaseClass is just replicated in the documentation. No related warning was thrown during the built.
  • The redefinition of $value in HeirClass is added to the existing properties instead of overriding them.

Your environment

  • Version used: 3.4.3
  • phpDocumentor.phar downloaded and used as-is
  • PHP 8.3.6 (cli) (built: Apr 10 2024 14:21:20) (NTS)
  • macOS Sonoma 14.4.1 (23E224)
@jaapio
Copy link
Member

jaapio commented May 17, 2024

Thanks for your report, phpDocumentor doesn't do any validation right now so errors or warnings will not be generated.

I think you are right about the duplicated declarations, they should overwrite properties. This is a bug that we need to address

@adriendupuis
Copy link
Contributor Author

@jaapio thank you for your reply. I understand that validation is outside phpDocumentor scope/purpose. In fact, having a duplicated element in the doc block rendered as a duplicate element in the HTML doc is kind of logical, like a writer's choice.

I found another way to duplicate a declaration:

<?php

/**
 * @property-read BaseValueClass $value
 */
class BaseClass
{
    protected mixed $value;

    public function __construct(array $properties = [])
    {
        foreach ($properties as $property => $value) {
            $this->$property = $value;
        }
    }

    public function __get($property)
    {
        if (property_exists($this, $property)) {
            return $this->$property;
        }
    }
}

The code seems genuine to me. The property $value is protected against writing, while the magic __get make the property readable to public.

In this example, the $value property is documented twice in phpDocumentor output.

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

No branches or pull requests

2 participants