Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.I am not certain if it is allowed by coding standards but it looks a bit weird and inconsistent with other files.
use Drupal\Component\Utility\Crypt;
use Drupal\Component\Render\PlainTextOutput;
use Drupal\Component\Utility\Unicode;
use Drupal\Core\Asset\AttachedAssetsInterface;
use Drupal\Core\Cache\Cache;
use Drupal\Core\Entity\Display\EntityViewDisplayInterface;
use Drupal\Core\Field\BaseFieldDefinition;
use Drupal\Core\Render\Element;
use Drupal\Core\Routing\RouteMatchInterface;
use Drupal\Core\Session\AccountInterface;
use Drupal\Core\Session\AnonymousUserSession;
use Drupal\Core\Site\Settings;
use Drupal\Core\Url;
use Drupal\user\Entity\Role;
use Drupal\user\Entity\User;
use Drupal\user\RoleInterface;
use Drupal\user\UserInterface;
/**
* @file
* Enables the user registration and login system.
*/
https://api.drupal.org/api/drupal/core%21modules%21user%21user.module/8
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | user_module_coding-2624822-17.patch | 3.27 KB | chgasparoto |
| #17 | interdiff-2624822-12-17.txt | 703 bytes | chgasparoto |
| #12 | user_module_coding-2624822-12.patch | 3.95 KB | markdorison |
| #11 | user_module_coding-2624822-11.patch | 4.72 KB | anish.a |
Comments
Comment #2
joshi.rohit100Comment #3
Lars Toomre CreditAttribution: Lars Toomre as a volunteer commentedWhen this file user.module is run through PHP_CodeSniffer using the Drupal standard, approximately thirty additional issues with this one file are flagged as errors or warnings. Does it make sense to repurpose this issue to clean up the additional issues in this one file? I think it does.
Comment #4
Lars Toomre CreditAttribution: Lars Toomre as a volunteer commentedAttached are the sniffs that result from checking user.module with code_sniffer:
Edit: Added
tags to clean up spacing.Comment #5
cilefen CreditAttribution: cilefen commentedYes, why not do it all?
Comment #6
Lars Toomre CreditAttribution: Lars Toomre as a volunteer commentedAttached is a WIP patch that addresses many of the sniffs from #4. I did not touch any of the 'Implements hook_foo() warnings. There are also a couple of the sniffs that are giving what I think are false positives.
This file then also needs a careful read through to catch the active verb tense at the start of docblocks and that all of the (optional) variables have been documented as such (along with their default values).
Here are the coder_sniffer summary after applying this patch:
Comment #7
Lars Toomre CreditAttribution: Lars Toomre as a volunteer commentedForgot to update the status.
Comment #8
anish.a CreditAttribution: anish.a as a volunteer commentedComment #9
anish.a CreditAttribution: anish.a as a volunteer commentedComment #11
anish.a CreditAttribution: anish.a as a volunteer commentedMuch of the issues mentioned in the diff are already fixed according to the coder module check I have made.
Adding the patch for issues I could fix. Checked out from 8.2.x branch.
Comment #12
markdorisonRe-rolled against 8.2.x.
Comment #13
markdorisonComment #14
markdorisonComment #15
Mile23Heads-up that we're fixing coding standards by sniff over here: #2571965: [meta] Fix PHP coding standards in core
$options seems to be unused in this method, so +1.
$name and $mail are never used, so +1.
$form_object is not used so +1.
Comment #16
alexpottThank you for your work on cleaning up Drupal core's code style!
This is two different things. The removal of the blank line is a coding standard that should be fixed in a maintainable way, all our coding standards issues should be done on a per-rule basis across all of core, rather than fixing standards in individual modules or files. We should also separate fixes where we need to write new documentation from fixes where we need to correct existing standards. This all should be done as part of #2571965: [meta] Fix PHP coding standards in core. A good place to start is the child issues of #2572645: [Meta] Fix 'Drupal.Commenting.FunctionComment' coding standard.
For background information on why we usually will not commit coding standards fixes that aren't scoped in that way, see the core issue scope guidelines, especially the note about coding standards cleanups. That document also includes numerous suggestions for scoping issues including documentation coding standards cleanups.
Contributing to the overall plan above will help ensure that your fixes for core's coding standards remain in core the long term.
This issue can focus on the second task of removing unused variables because whilst this is automatable we have no current plans to try to do this with coder and also the review needs to check the code code to ensure that it truly is unused so simpler patch contexts are better.
Comment #17
chgasparoto CreditAttribution: chgasparoto as a volunteer and at CI&T commentedApplied changes suggested by #16.
Please review it.
Comment #18
Mile23See review in #15, which still applies. The out-of-scope CS change is gone.
Looks good.
There are still unused variables in user module, but they are for constructs like
foreach ($array as $key=>$unused).Comment #19
catchCommitted/pushed to 8.2.x, thanks!