Update the user.api.php documentation and add the correct type hints in the user's hooks implementations

This looks like a solid documentation improvement, I checked and it is indeed the account that is passed trough the hooks.

Since this is just hook documentation and not like an interface where adding a type hint breaks existing implementations, we could actually also add the explicit type hint. Leaving the decision to core maintainers to add that here as well or not.

Moreover, other example hooks from user.api.php have the right PHPDoc.

+++ b/core/modules/user/user.api.php
@@ -130,7 +130,7 @@ function hook_user_format_name_alter(&$name, $account) {
 function hook_user_login($account) {

@@ -144,7 +144,7 @@ function hook_user_login($account) {
+ * @param \Drupal\Core\Session\AccountInterface $account

We can add the type hint to the function too.

The correct typehint for login is UserInterface - look at user_login_finalize(). For logout it is AccountInterface.

We already have function system_user_login(UserInterface $account) {

Also we should update user_user_login() and user_user_logout() to use the type hint.

Adding Type hints to hook_user_login(), hook_user_logout and hook_user_cancel().

Used the FQN instead of only the class name and added a space before the accountInterface argument. Interdiff is to #7.

The correct typehint for login is UserInterface - look at user_login_finalize(). For logout it is AccountInterface.

Also as we're adding typehints lets add them to the implementations like comment_user_cancel() for example.

Plus hook_user_cancel is UserInterface - see user_cancel()

1. +++ b/core/modules/system/tests/modules/session_test/session_test.module
   @@ -8,7 +8,7 @@
   -function session_test_user_login($account) {
   +function session_test_user_login(\Drupal\user\UserInterface $account) {
   

   Should have a use statement

2. +++ b/core/modules/user/user.api.php
   @@ -144,10 +144,10 @@ function hook_user_login($account) {
   -function hook_user_logout($account) {
   +function hook_user_logout(\Drupal\user\UserInterface $account) {
   

   This is not passed the user it is passed the account proxy - so needs to be the AccountInterface. See user_logout().

Something went wrong on the testbot, PHP 7 & MySQL 5.5 114 pass 114 tests doesn't seem like that is the correct number of tests.

That should be my bad.... I have deleted the interdiff , which I uploaded earlier with .patch extenssion....
I am uploading the patch ones again for runnning the test.

@rakesh.gectcr I have applied your patch it applies cleanly but I have point that function user_user_login(UserInterface $account) should be function user_user_login(AccountInterface $account)?

-function hook_user_login($account) {
+function hook_user_login(\Drupal\user\UserInterface $account) {
   $config = \Drupal::config('system.date');
-function hook_user_login($account) {
+function hook_user_login(\Drupal\user\AccountInterface $account) {
   $config = \Drupal::config('system.date');

Adding tag for the sprint weekend

as part of Nwdug_may18, i will update the patch

Attached is an updated patch and interdiff as per comments

Missed off changes from the previous patch in #19 (In other words please ignore comment #23)

See updated patch file

Could do with an issue summary update to list all the changes that the patch should make.

I just updated the summary.

I reviewed the patch and I found a problem here:

+++ b/core/modules/user/user.api.php
@@ -130,10 +133,10 @@ function hook_user_format_name_alter(&$name, $account) {
-function hook_user_login($account) {
+function hook_user_login(AccountInterface $account) {

Here must be UserInterface not AccountInterface

I'm going to reroll the patch and give it a more detailed review.

Ok.

New patch attached, I grep the core looking if all the hooks have the correct type hint:

hook_user_cancel

grep -EHrin "function [a-z_]+_user_cancel\(.*\)" .
./modules/comment/comment.module:522:function comment_user_cancel($edit, UserInterface $account, $method) {
./modules/user/user.api.php:42:function hook_user_cancel($edit, UserInterface $account, $method) {
./modules/history/history.module:163:function history_user_cancel($edit, UserInterface $account, $method) {
./modules/node/node.module:717:function node_user_cancel($edit, UserInterface $account, $method) {

hook_user_login

 grep -EHrin "function [a-z_]+_user_login\(.*\)" .
./modules/user/user.module:572:function user_user_login(UserInterface $account) {
./modules/user/user.api.php:139:function hook_user_login(UserInterface $account) {
./modules/system/tests/modules/session_test/session_test.module:13:function session_test_user_login(UserInterface $account) {
./modules/system/system.module:834:function system_user_login(UserInterface $account) {

hook_user_format_name_alter

grep -EHrin "function [a-z_]+_user_format_name_alter\(.*\)" .
./modules/user/tests/modules/user_hooks_test/user_hooks_test.module:14:function user_hooks_test_user_format_name_alter(&$name, AccountInterface $account) {
./modules/user/user.api.php:126:function hook_user_format_name_alter(&$name, AccountInterface $account) {

hook_user_login

grep -EHrin "function [a-z_]+_user_login\(.*\)" .
./modules/user/user.module:572:function user_user_login(UserInterface $account) {
./modules/user/user.api.php:139:function hook_user_login(UserInterface $account) {
./modules/system/tests/modules/session_test/session_test.module:13:function session_test_user_login(UserInterface $account) {
./modules/system/system.module:834:function system_user_login(UserInterface $account) {

hook_user_logout

grep -EHrin "function [a-z_]+_user_logout\(.*\)" .
./modules/user/user.module:581:function user_user_logout(AccountInterface $account) {
./modules/user/user.api.php:160:function hook_user_logout(AccountInterface $account) {

And I rerolled and fixed a couple things.

Patch and interdiff attached.

> Here must be UserInterface not AccountInterface

The patch doesn't have this change.

hook_user_format_name_alter() is invoked here in the User entity, so $this is a UserInterface:

  public function getDisplayName() {
    $name = $this->getAccountName() ?: \Drupal::config('user.settings')->get('anonymous');
    \Drupal::moduleHandler()->alter('user_format_name', $name, $this);
    return $name;
  }

It is also called in \Drupal\Core\Session\UserSession::getDisplayName(), where it is *not*.

Thanks for the clarification!

In that case this is RTBC.

But I will file a follow-up for cleaning up the docs for hook_user_format_name_alter() in light of #34.

-- done: #3000240: hook_user_format_name_alter() should document that it's invoked for both user entities and session objects.

Unfortunately needs a reroll.

Done.

Thanks!

I've check the re-roll by @gnuget in comment 37.

All of the functions in comment 29 have been changed and no new functions have been added.

Setting back to RTBC.

Committed 400413f and pushed to 8.7.x. Thanks!
Committed ac623bf and pushed to 8.6.x. Thanks!