Entity helper recognized as accessor

[fmarchalemisys]

My entities have some helper methods looking like this:

public function setPosition($latitude, $longitude)
{
    $this->setLatitude($latitude);
    $this->setLongitude($longitude);
    return $this;
}

It seems Symfony 4 as an improved PropertyInfo class that has become too smart for its own good. It returns the nonexistent property $position because of the method looking like a setter.

Looking at ReflectionExtractor in property-info bundle, any method beginning with 'add', 'remove', 'set', 'is', 'can', 'get', 'has' will generate such a ghost property.

The solution here is to declare a fake, unused, $position with a dummy type (for instance @var string) just to avoid the error saying the type of $position can't be guessed. It's a dirty hack. Is there any cleaner solution?

PS: I like the idea of creating fake setters/getters in cases like this:

/**
 * @var boolean
 */
private $result;

public function getResult()
{
    return $this->result;
}

/**
 * @return string
 */
public function getResultLabel()
{
    return $this->result ? "allowed" : "denied";
}

Skipping any "property" returned by property-info that doesn't have a real property in the class would prevent the above alternate getter from working.

[haroldiedema]

We're having the same issues as OP describes.

The "bug" appears to be in the ObjectModelDescriber.
A list of properties is returned by the ReflectionExtractor (part of the Symfony PropertyInfo component) that contains properties that don't exist, due to accessor methods being there that serve as utility/helper functions.

The ObjectModelDescriber iterates over these properties, it even checks if the property actually exists using property_exists (line 61) on the given class. However, if the property doesn't exist, $property = $properties->get($propertyName); is called, which will just add the property to the schema without any type information since it doesn't exist.

Replacing this line with a simple continue; statement solves the problem. However, I am unaware of any side-effects this might cause.

Here is the original code:

        foreach ($propertyInfoProperties as $propertyName) {
            // read property options from Swagger Property annotation if it exists
            if (property_exists($class, $propertyName)) {
                $reflectionProperty = new \ReflectionProperty($class, $propertyName);
                $property = $properties->get($annotationsReader->getPropertyName($reflectionProperty, $propertyName));

                $groups = $model->getGroups();
                if (isset($groups[$propertyName]) && is_array($groups[$propertyName])) {
                    $groups = $model->getGroups()[$propertyName];
                }

                $annotationsReader->updateProperty($reflectionProperty, $property, $groups);
            } else {
                $property = $properties->get($propertyName);
            }

            // ...

The last line ($property = $properties->get($propertyName)) actually modifies the schema by adding the property to it because it doesn't exist. I don't think this should happen.

Replacing this line with continue; so it skips the rest of the iteration solves the issue.

[redheadedstep]

I have the same issue with accessors and have been beating my head against the wall trying to figure out how to ignore the property.

protected $boosters;

/**
 * @var string[]
 * @return string[]
 */
function getBoosters() {
    if (is_null($this->boosters)) {
        $this->boosters = [];
    }
    return $this->boosters;
}

/**
 * @param string[]
 * @var string[]
 * @return string[]
 */
function setBoosters($boosters) {
    $this->boosters = $boosters;
    return $this->boosters;
}

/**
 * @param string $key
 * @param string $value
 * @return string[]
 */
function setBooster($key, $value) {
    $boosters = $this->getBoosters();
    $boosters[$key] = $value;
    return $this->setBoosters($boosters);
}

And then I receive an error on NelmioApiBundle about how it can't find the property $booster.

As a fix, should the ObjectModelDescriber even be parsing out "Setter" methods? Sure, they might return something, but for the most part, they are used to set data, not retrieve data. When modeling an object, I want to know what data the object will return (i.e. "Getters" or public properties).

At the very least, there should be a check if a $property even exists on the object. If not, then skip it, otherwise it is just adding "ghost" properties to an object, which causes more headache. I had another property with the same error called $mpKeyValue and did a search throughout my code and could not find the property anywhere! It ended up coming from a function called setMpKeyValue($key, $value)...definitely not very intuitive.

[redheadedstep]

To piggy back on my previous comment, since it is parsing the "setter", ObjectModelDescriber is parsing the api documentation for return, var, and param. By parsing the param comment, it is adding that value to the list of types. In this case, the parser is storing that the $booster property returns string and string[].

I mention this because once we solve the error for The PropertyInfo component was not able to guess the type of %s::$%s, another error will occur which is: Property %s::$%s defines more than one type.

These are on lines 84 and 87 in ObjectModelDescriber. This would all be mitigated if the setters were not parsed. The error comes from the getTypes() function in PhpDocExtractor on line 115-154.

I haven't found a way around the second error except by removing comments (which is a very anti-pattern).

[GuilhemN]

I don't think we should ignore all setters, they will be normalized by the Symfony serializer if you do nothing and hence should be documented imo.
However, a solution could be to use the new @Ignore annotation from the Symfony serializer (see this similar issue #1595), would that be acceptable?

[chrisguitarguy]

Somewhat Related #1324

I've seen this issue a few times with relation to class hierarchies and inherited properties, but also stuff like this. I think the @Ignore annotation is a good solution for code folks own, but I suspect we need some solution for parent classes and traits that end users don't own.