Add missing type hinting to User module docblocks

Patch no longer applies. It needs reroll.

Here is the updated patch

+++ b/core/modules/user/user.api.php
@@ -111,10 +111,10 @@ function hook_user_cancel_methods_alter(&$methods) {
- * @param $account
+ * @param object $account

$account is now typehinted by its interface, for example see http://cgit.drupalcode.org/drupal/tree/core/modules/user/src/Permissions...

  /**
   * Generates a hash that uniquely identifies a user's permissions.
   *
   * @param \Drupal\Core\Session\AccountInterface $account
   *   The user account for which to get the permissions hash.
   *
   * @return string
   *   A permissions hash.
   */
  public function generate(AccountInterface $account);

More work. I think I got most.

Ended up filing this issue, because there was a big @todo about refactoring with no associated issue: #2403729: Convert user_cancel_confirm() to a new-style Form object

1. +++ b/core/modules/user/src/Controller/UserController.php
   @@ -164,6 +164,8 @@ public function logout() {
   +   * Menu callback; Cancel a user account via email confirmation link.
   

   Is this really a menu callback? Isn't that a controller?

2. +++ b/core/modules/user/user.module
   @@ -508,14 +509,14 @@ function user_template_preprocess_default_variables_alter(&$variables) {
   + *   - account: The user account (Drupal\user\Plugin\Core\Entity\User).
   

   Its Drupal\user\Entity\User now.

3. +++ b/core/modules/user/user.module
   @@ -939,15 +940,15 @@ function user_delete_multiple(array $uids) {
   - * @param $account
   + * @param \Drupal\Core\Entity\EntityInterface $account
   
   @@ -957,15 +958,15 @@ function user_view($account, $view_mode = 'full', $langcode = NULL) {
   - * @param $accounts
   + * @param \Drupal\Core\Entity\EntityInterface[] $accounts
   

   Why can't we typehint for \Drupal\user\UserInterface here?

Yes, $account should be type hinted by its interface, I commented on it earlier in #7.

Thanks for the review.

#9.1: I just copied the docblock over from user_cancel_confirm(), and I guess I should remove that change from this patch.

#9.2: Well spotted, but I'll go you one better. The first thing this function does is this:

function template_preprocess_username(&$variables) {
  $account = $variables['account'] ?: new AnonymousUserSession();

Which means it has to be AccountInterface, although there is that weirdness with elseif (!empty($account->homepage)) {.

#9.3: user_delete_multiple() just calls entity_delete_multiple(), and user_view() just calls entity_view(). Both take an EntityInterface. There's really no difference between the user_ and entity_ methods in this regard, since entities are so well encapsulated.

In fact, there should be a static EntityInterface::deleteMultiple($entities[]); but that's a whole other scope.

Regardless.. I'll change it all.

I added actual type hinting in user.module. if that explodes, then a) there's a new issue to file, and b), I'll take it out and restrict my changes to only docblocks.

Thank you for taking the review into account.

Needs reroll

Reroll.

actual tag has no dash.

Doesn't need a reroll, needs review. :-)

Actually it did need a reroll :)

Changes look good! Let's get this in.

Testbot came back green, settings to RTBC.

took out some unrelated changes that were out of scope of the issue.

added the leading \ to namespaces https://www.drupal.org/node/1353118

added some types to function definition to match the types added to the @param's

1. +++ b/core/modules/user/user.module
   @@ -934,33 +942,33 @@ function user_delete_multiple(array $uids) {
   + * @param string|NULL $langcode
   

   lowercase null should be used for types in @param and @return
   https://www.drupal.org/node/1354#types

2. +++ b/core/modules/user/user.module
   @@ -1020,17 +1029,17 @@ function user_mail_tokens(&$replacements, $data, $options) {
   - * @return
   - *   An associative array with the role id as the key and the role name as
   - *   value.
   + * @return array
   + *   An array of the names of the roles.
   

   why this change?
   It is false that it is an associative array with the role id as the key?

3. +++ b/core/modules/user/user.module
   @@ -1096,14 +1105,15 @@ function user_user_role_delete(RoleInterface $role) {
   - * @return
   - *   An associative array with the role id as the key and the role object as
   + * @return array
   + *   An associative array with the role id as the key and the role name as
   

   why the change in the description? looks like it is returning an array of objects, not strings.

   Can we do
   \Drupal\user\EntityOwnerInterface[]
   ?

   relevant...
   $roles = Role::loadMultiple();

   maybe it should be
   \Drupal\user\RoleInterface[]

Needed a reroll.

#19.1: You got the nulls...

#19.2 and #19.3: Setting the @return comments to be the same as api.drupal.org.

Also in #19.3: Let's just say RoleInterface[]. The entity system doesn't enforce it, but it just so happens that Role implements RoleInterface, and the interface is where we want people to go for documentation.

Here it is again without changing the function signatures. This way it can be evaluated as a documentation change.

The patch in #21 removes the type hinting from the function signatures, so the feedback has been addressed or no longer applies since the patch updates the documentation only.

Um, really?

+++ b/core/modules/user/user.api.php
- * @param $account
+ * @param object $account
  *   The user object on which the operation is being performed.

User object is a stdObject object, not a particular class? (occurs multiple times in this patch). Shouldn't it be:

+ * @param \Drupal\Core\Session\AccountInterface $account

like in other spots in the patch?

Also, if you're fixing a function first line, like here:

 /**
- * Retrieve an array of roles matching specified conditions.
+ * Retrieve the names of roles matching specified conditions.

Might as well also change Retrieve to Retrieves?

Also this is missing the \ in user.module:

+ * @return Drupal\user\RoleInterface[]

I stopped reading the patch here but it's not really ready.

Also, PLEASE do not set this or any of the other related issues to RTBC until the patch has been ***carefully*** reviewed and verified to be correct.

My bad, I'll leave this for the documentation team :)

My bad, I'll try to get the documentation right next time instead of checking if the patch applies.

There is no "documentation team".

What we have is hopefully a community of Drupal developers who try to get the code documentation right. All I'm asking is that before marking a patch that is supposed to be fixing the documentation as "reviewed and tested", that the documentation actually be reviewed for accuracy, not just that the patch applies.

@idebr: Thanks for working on it, at least.

@jhodgdon: Thanks for the mini-review. This patch addresses everything you mention, and also some stale docs in user_user_logout().

Still applies, still addresses all missing hinting in docblocks.

Rerolling the patch as its not applied.

Looking Fine

Looks like all previous feedback has been addressed.

Committed and pushed to 8.0.x. Thanks!