NelmioApiDocBundle
NelmioApiDocBundle copied to clipboard
Entity helper recognized as accessor
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.
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.
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.
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).
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 https://github.com/nelmio/NelmioApiDocBundle/issues/1595), would that be acceptable?
Somewhat Related https://github.com/nelmio/NelmioApiDocBundle/issues/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.