Fixed Class @property phpdocs for magic methods/properties
- Thread starter Xon
- Start date
DragonByte Tech
Well-known member
https://docs.phpdoc.org/references/phpdoc/tags/property.html
Simple example;
PHP:
/**
* @property string $myProperty
*/
class class
{
public function __get()
{
<...>
}
}In phpstorm at least, you don't need the $ as part of the property name.
DragonByte Tech
Well-known member
Sorry, that isn't very clear to me, being unfamiliar with this sort of thing 
Do you mean to say that if I have an entity with a
Fillip
Do you mean to say that if I have an entity with a
getTitle() function, I should add @property string $title to the top docblock?Fillip
Yup, exactly.
A good example to look at is the
Honestly, if it's a manual job, I can't see us doing it as it isn't a great use of time right now. Also, I wouldn't go as far as to call this a bug either; pretty much the exact definition of a feature request.
It's possible we could write something to generate these automatically and for that reason we'll leave this open for now, but it likely won't be considered a priority and don't necessarily be surprised if it eventually gets moved to Suggestions.
XF\Widget\Entity class where we've type hinted a known column (options), a getter (handler) and a relation (WidgetDefinition).Honestly, if it's a manual job, I can't see us doing it as it isn't a great use of time right now. Also, I wouldn't go as far as to call this a bug either; pretty much the exact definition of a feature request.
It's possible we could write something to generate these automatically and for that reason we'll leave this open for now, but it likely won't be considered a priority and don't necessarily be surprised if it eventually gets moved to Suggestions.
Liam W
in memoriam 1998-2020
@Chris D @Kier
I was bored, so I made a Cli command that auto-generates them. Seems to work.
Created this for the User entity:
Feel free to use it for core development (and maybe stick it in the core).
Liam
I was bored, so I made a Cli command that auto-generates them. Seems to work.
Created this for the User entity:
PHP:
/**
* @property int $user_id
* @property string $username
* @property string $email
* @property int $style_id
* @property int $language_id
* @property string $timezone
* @property bool $visible
* @property bool $activity_visible
* @property int $user_group_id
* @property array $secondary_group_ids
* @property int $display_style_group_id
* @property int $permission_combination_id
* @property int $message_count
* @property int $alerts_unread
* @property int $conversations_unread
* @property int $register_date
* @property int $last_activity
* @property int $trophy_points
* @property int $avatar_date
* @property int $avatar_width
* @property int $avatar_height
* @property bool $avatar_highdpi
* @property string $gravatar
* @property string $user_state
* @property bool $is_moderator
* @property bool $is_admin
* @property bool $is_staff
* @property bool $is_banned
* @property int $like_count
* @property string $custom_title
* @property int $warning_points
* @property string $secret_key
* @property int $xfrm_resource_count
* @property int $xfmg_album_count
* @property int $xfmg_media_count
* @property int $xfmg_media_quota
* @property string $vanity_name
* @property int $liamw_threadsolutions_solution_count
* @property mixed PermissionSet
* @property mixed is_super_admin
* @property mixed email_confirm_key
* @property mixed warning_count
* @property \XF\Entity\Admin Admin
* @property \XF\Entity\UserAuth Auth
* @property \XF\Entity\UserConnectedAccount ConnectedAccounts
* @property \XF\Entity\UserOption Option
* @property \XF\Entity\PermissionCombination PermissionCombination
* @property \XF\Entity\UserProfile Profile
* @property \XF\Entity\UserPrivacy Privacy
* @property \XF\Entity\UserBan Ban
* @property \XF\Entity\UserReject Reject
* @property \XF\Entity\SessionActivity Activity
* @property \XF\Entity\ApprovalQueue ApprovalQueue
* @property \XF\Entity\UserFollow Following
*/Feel free to use it for core development (and maybe stick it in the core).
Liam
I was bored last night too...
Obviously this was an open bug report so until we say otherwise there's every danger that we'd have implemented it (which we have for the next release).
You may still want to use your own version, though. Ours, by design, only adds class properties for the structure of the original class (so it bypasses structure extensions). There's a few other improvements in that it attempts to type hint getter function types and maintains the original column type if a getter overlaps it.
PHP:
/**
* COLUMNS
* @property int|null $user_id
* @property string $username
* @property string $email
* @property int $style_id
* @property int $language_id
* @property string $timezone
* @property bool $visible
* @property bool $activity_visible
* @property int $user_group_id
* @property array $secondary_group_ids
* @property int $display_style_group_id
* @property int $permission_combination_id
* @property int $message_count
* @property int $alerts_unread
* @property int $conversations_unread
* @property int $register_date
* @property int $last_activity
* @property int $trophy_points
* @property int $avatar_date
* @property int $avatar_width
* @property int $avatar_height
* @property bool $avatar_highdpi
* @property string $gravatar
* @property string $user_state
* @property bool $is_moderator
* @property bool $is_admin
* @property bool $is_staff
* @property bool $is_banned
* @property int $like_count
* @property string $custom_title
* @property int $warning_points
* @property string $secret_key
* @property int $permission_combination_id_
* @property int $last_activity_
*
* GETTERS
* @property \XF\PermissionSet $PermissionSet
* @property bool $is_super_admin
* @property string $email_confirm_key
* @property int $warning_count
*
* RELATIONS
* @property \XF\Entity\Admin $Admin
* @property \XF\Entity\UserAuth $Auth
* @property \XF\Entity\UserConnectedAccount[] $ConnectedAccounts
* @property \XF\Entity\UserOption $Option
* @property \XF\Entity\PermissionCombination $PermissionCombination
* @property \XF\Entity\UserProfile $Profile
* @property \XF\Entity\UserPrivacy $Privacy
* @property \XF\Entity\UserBan $Ban
* @property \XF\Entity\UserReject $Reject
* @property \XF\Entity\SessionActivity $Activity
* @property \XF\Entity\ApprovalQueue $ApprovalQueue
* @property \XF\Entity\UserFollow[] $Following
*/You may still want to use your own version, though. Ours, by design, only adds class properties for the structure of the original class (so it bypasses structure extensions). There's a few other improvements in that it attempts to type hint getter function types and maintains the original column type if a getter overlaps it.
Liam W
in memoriam 1998-2020
Typical. No matter, yours is better. I was just bored and wanted to see if I could do it
Liam
Steffen
Well-known member
Sorry for digging up this old thread but it provides the necessary context.
I've noticed that PHPStan expects property names to be prefixed with a dollar sign which is how @property is documented on the PHPDoc website, too. XenForo's PHPDoc syntax seems to be wrong at first glance (I'm not an expert on this subject).
This seems wrong and doesn't work for PHPStan:
This is the right syntax and would work for PHPStan:
What's the reason for the missing dollar sign in XenForo's PHPDoc? Would it be possible to fix this?
(Strangely, the code posted above by @Chris D does have the dollar signs but the actual XenForo 2.2 source doesn't doesn't. Was this changed later on?)
I've noticed that PHPStan expects property names to be prefixed with a dollar sign which is how @property is documented on the PHPDoc website, too. XenForo's PHPDoc syntax seems to be wrong at first glance (I'm not an expert on this subject).
This seems wrong and doesn't work for PHPStan:
Code:
@property \XF\Entity\UserProfile ProfileThis is the right syntax and would work for PHPStan:
Code:
@property \XF\Entity\UserProfile $ProfileWhat's the reason for the missing dollar sign in XenForo's PHPDoc? Would it be possible to fix this?
(Strangely, the code posted above by @Chris D does have the dollar signs but the actual XenForo 2.2 source doesn't doesn't. Was this changed later on?)
Last edited:
Frankly this is PHPStan being pedantic. plsam doesn't require it.
The given syntax given;
The [name] is never actually described of what it's composition should be.
The documentation doesn't actually describe it as required. Even the eventually linked to php-fig document doesn't either; https://github.com/php-fig/fig-standards/blob/master/proposed/phpdoc-tags.md#510-property
The given syntax given;
Code:
@property[<-read|-write>] ["Type"] [name] [<description>]The PHPDoc stuff primarily exists to aid in development. In other words, if it works in PhpStorm then it does its job. Interestingly if you get PhpStorm to add a missing property it does so without the $ prefix.
I agree with Xon's interpretation of the documentation; it doesn't appear to be a clear requirement. We may have decided to forego the prefix simply because it looks a bit cleaner or simply because it works fine in PhpStorm so there wasn't really a clear signal that it wasn't correct. It appears to work in other development environments too including Visual Studio Code.
That said, this isn't a huge change as the properties are written automatically. I don't see a particular reason not to do it if it helps other software parse the properties correctly.
I agree with Xon's interpretation of the documentation; it doesn't appear to be a clear requirement. We may have decided to forego the prefix simply because it looks a bit cleaner or simply because it works fine in PhpStorm so there wasn't really a clear signal that it wasn't correct. It appears to work in other development environments too including Visual Studio Code.
That said, this isn't a huge change as the properties are written automatically. I don't see a particular reason not to do it if it helps other software parse the properties correctly.
Steffen
Well-known member
While trying to find out why tools disagree on the syntax, I've stumbled upon another static analyzer having issues with this: https://github.com/phan/phan/issues/3584
There is a PHPStorm bug report about the missing dollar sign: https://youtrack.jetbrains.com/issue/WI-34398
It seems like the PHPDoc syntax will be changed to explicitly require the dollar sign: https://github.com/php-fig/fig-standards/pull/1234
But I'd understand it if you want to wait for a final decision by the PHPDoc team and/or a bugfix in PHPStorm.
There is a PHPStorm bug report about the missing dollar sign: https://youtrack.jetbrains.com/issue/WI-34398
It seems like the PHPDoc syntax will be changed to explicitly require the dollar sign: https://github.com/php-fig/fig-standards/pull/1234
But I'd understand it if you want to wait for a final decision by the PHPDoc team and/or a bugfix in PHPStorm.
It was already done as of this message yesterday
