Add return types to UserCreationTrait

I forgot ✨ Enforce return types in all new methods and functions RTBC was not committed to 10.4.x so probably don't need to fix this there. But leaving the MR up in case we want to do the same approach for 11.x

Nice chunk taken out of the phpstan file!

Thought about closing the 10.4 file as it has test failures but maybe 11.x can be merged and if committers want this can be sent back to be backported.

alexpott → changed the visibility of the branch 3480442-usercreationttrait-10.4.x to hidden.

I agree that we should just do this in 11.x - hidden the 10.4.x branch.

So any tests in contrib / custom that wrap one of these methods will fail - also we're not adding return type hints to the rest of the trait and if we did then we'd find that we'd have to update \Drupal\Tests\system\Functional\Entity\EntityOperationsTest::createRole() too.

As much as I'd like to do this I feel we should be doing this in 12.x.

Regardless of when we do this the MR needs to be updated as I had to add the same ignores in another MR that has been merged.

Discussed a bit with catch as we're unsure if we should make an exception for tests.

Anyhow this issue revealed that the addition of \Drupal\Tests\system\Functional\Entity\EntityOperationsTest::createRole() was unnecessary - it was fixed in a better way by a follow-up to #2004408: Allow modules to alter the result of EntityListController::getOperations → .

Not sure about #8, it caused test failures. I've rebased and reverted that commit and it seems to pass now. Haven't investigated further, but it seems out of scope to remove it.

@mstrelan it seems that linkExists() and the underlying code deals with the escape for us. Change the assert to $this->assertSession()->linkExists('Test Operation: ' . $role->label()); means that we can remove the override of createRole.

#10 makes more sense, but I wasn't seeing that assertion change in the MR, maybe it wasn't pushed? I've fixed that up now.

Appears to have merge errors

Rebased

Rebase seems good, least the baseline file doesn't seem to have any conflicts. Moving back to RTBC unless it is decided this should wait till 12.x

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Re the 11.x vs 12.x discussion - https://www.drupal.org/about/core/policies/core-change-policies/bc-polic... → says:

Test traits and abstract base classes should generally use deprecation where possible rather than breaking backwards compatibility, but they are still considered an internal API and may change if necessary.

So I think we can do this in 11.x

Fixed conflicts in the baseline.

Needs another rebase.

Rebased once more

Committed/pushed to 11.x and cherry-picked to 11.1.x, thanks!

catch → closed merge request !9835

Automatically closed - issue fixed for 2 weeks with no activity.