Fix method comments in non tests for Drupal.Commenting.DocComment.ShortSingleLine

The patch in #2 applied successfully to 10.0.x branch.

Added reroll of patch #2 on Drupal 10.0.x. and removed needs reroll tag.

Patch or MR doesn't apply anymore
The last patch or MR doesn't apply to the target branch, please reroll the code so that it can be reviewed by the automated testbot.

Rerolled patch for 10.1

+++ b/core/modules/views/src/Plugin/views/BrokenHandlerTrait.php
@@ -29,8 +29,9 @@ public function defineOptions() {
+   * Ensure the main table for this handler is in the query.
+   *
+   * This is used a lot.

I'm not sure we need to keep this "this is used a lot.", does add any value imho

That is the only nitpick I could find. It was already in there so I think we can keep it like this. This is a good improvement over the current state and brings us closer to being able to enable the CS rule.

Thanks for working on this! It will be great to finally have this rule enabled.

I only got halfway through this patch because there are a lot of things in it that are against our documentation standards. Please extrapolate from the below points and also review the function documentation standards to fix similar errors in the second half of the patch. I know cleanups like these are a lot of work because we often have to read the whole method.

1. +++ b/core/lib/Drupal/Component/Gettext/PoItem.php
   @@ -103,8 +103,7 @@ public function setContext($context) {
   -   * Gets the source string or the array of strings if the translation has
   -   * plurals.
   +   * Gets source string or array of strings if the translation has plurals.
   
   @@ -113,8 +112,7 @@ public function getSource() {
   -   * Set the source string or the array of strings if the translation has
   -   * plurals.
   +   * Set source string or array of strings if the translation has plurals.
   
   @@ -124,8 +122,7 @@ public function setSource($source) {
   -   * Gets the translation string or the array of strings if the translation has
   -   * plurals.
   +   * Gets translation string or array of strings if the translation has plurals.
   
   @@ -134,8 +131,7 @@ public function getTranslation() {
   -   * Set the translation string or the array of strings if the translation has
   -   * plurals.
   +   * Set translation string or array of strings if the translation has plurals.
   
   +++ b/core/lib/Drupal/Component/PhpStorage/MTimeProtectedFastFileStorage.php
   @@ -185,8 +185,7 @@ public function garbageCollection() {
   -   * Gets the full path of the containing directory where the file is or should
   -   * be stored.
   +   * Gets full path of containing directory where file is or should be stored.
   

   We should not make docs fit the 80-character rule by removing articles ("the") or other small words. It's supposed to be a fully grammatical English predicate of 80 characters or less that begins with a third-person indicative verb. Say that five times fast! :)

   One option for these is (e.g.):

   Gets the source string(s) if the translation has plural.

2. +++ b/core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EntityReferenceItem.php
   @@ -635,9 +635,9 @@ public function getSettableOptions(AccountInterface $account = NULL) {
   -   * Render API callback: Processes the field settings form.
   +   * Render API callback.
   
   @@ -667,9 +668,10 @@ public static function fieldSettingsAjaxProcessElement(&$element, $main_form) {
   -   * Render API callback: Moves entity_reference specific Form API elements
   -   * (i.e. 'handler_settings') up a level for easier processing by the
   -   * validation and submission handlers.
   +   * Render API callback.
   +   *
   +   * Moves entity_reference specific Form API elements (i.e. 'handler_settings')
   +   * up a level for easier processing by the validation and submission handlers.
   

   The Foo callback: Does blah format is also a standard that we should not break. "Render API callback" is way too generic to be one-line documentation.

   Also, the first one isn't over 80 characters; why are we even changing it?

3. +++ b/core/lib/Drupal/Core/File/FileSystemInterface.php
   @@ -173,8 +173,9 @@ public function dirname($uri);
   +   * Optionally creating missing components in the path to the directory.
   

   Any subsequent paragraphs in the documentation should use full sentences.

4. +++ b/core/lib/Drupal/Core/Form/FormBuilder.php
   @@ -108,8 +108,10 @@ class FormBuilder implements FormBuilderInterface, FormValidatorInterface, FormS
   -   * Defines element value callables which are safe to run even when the form
   -   * state has an invalid CSRF token.
   +   * Defines element value callables which are safe to run.
   +   *
   +   * These Element value callables are safe to run even when the form state has
   +   * an invalid CSRF token.
   

   Hm. "Safe to run" is too vague by itself. Maybe:

   Defines element value callables that are safe to run with invalid CSRF tokens.

5. +++ b/core/lib/Drupal/Core/Form/FormState.php
   @@ -82,7 +82,7 @@ class FormState implements FormStateInterface {
   -   * Determines whether the form is rebuilt.
   +   * Indicates if new copy of the form is immediately built and sent to browser.
   
   @@ -107,7 +107,7 @@ class FormState implements FormStateInterface {
   -   * Determines if only safe element value callbacks are called.
   +   * Indicates if the form will skip calling form element value callbacks.
   

   Again, how are these in scope?

6. +++ b/core/lib/Drupal/Core/Form/FormState.php
   @@ -122,7 +122,7 @@ class FormState implements FormStateInterface {
   -   * The response object.
   +   * Response to return.
   

   This isn't even method documentation; why is it included?

7. +++ b/core/lib/Drupal/Core/ImageToolkit/ImageToolkitOperationBase.php
   @@ -86,8 +86,9 @@ protected function getToolkit() {
   -   * Checks if required arguments are passed in and adds defaults for non passed
   -   * in optional arguments.
   +   * Checks if required arguments are passed in.
   +   *
   +   * Also adds defaults for non passed in optional arguments.
   

   This should be something like:

   Checks for required arguments and adds optional argument defaults.

8. +++ b/core/lib/Drupal/Core/Render/theme.api.php
   @@ -821,7 +821,9 @@ function hook_element_plugin_alter(array &$definitions) {
   - * Perform necessary alterations to the JavaScript before it is presented on
   + * Alter JavaScript before it is presented on the page.
   + *
   + * Performs necessary alterations to the JavaScript before it is presented on
     * the page.
   

   The second paragraph here is completely redundant and can be removed.

9. +++ b/core/lib/Drupal/Core/Template/TwigEnvironment.php
   @@ -50,8 +50,9 @@ class TwigEnvironment extends Environment {
   +   * Constructs a TwigEnvironment object.
   +   *
   +   * Stores cache and storage internally.
   

   The second paragraph should again use complete sentences; the subject is missing.

10. +++ b/core/modules/content_translation/src/ContentTranslationHandlerInterface.php
    @@ -21,8 +21,7 @@ interface ContentTranslationHandlerInterface {
    -   * Checks if the user can perform the given operation on translations of the
    -   * wrapped entity.
    +   * Tests that user can perform an operation on translation of wrapped entity.
    

    Checks whether the user can perform the operation on the entity translation.

11. +++ b/core/modules/content_translation/src/ContentTranslationHandlerInterface.php
    --- a/core/modules/file/src/Plugin/Field/FieldWidget/FileWidget.php
    +++ b/core/modules/file/src/Plugin/Field/FieldWidget/FileWidget.php
    
    +++ b/core/modules/file/src/Plugin/Field/FieldWidget/FileWidget.php
    @@ -339,8 +339,9 @@ public static function value($element, $input, FormStateInterface $form_state) {
    -   * Form element validation callback for upload element on file widget. Checks
    -   * if user has uploaded more files than allowed.
    +   * Form element validation callback for upload element on file widget.
    +   *
    +   * Checks if user has uploaded more files than allowed.
    

    I believe these have a special standard: https://www.drupal.org/docs/develop/standards/php/api-documentation-and-...

    It's a bit archaic and based on D7 and earlier when we needed a clear way to group the form builders, validation handlers, and submission handlers together, back before these things were grouped together in, you know, classes. So I'm not sure it applies anymore. Indeed, even with all the forms in core, there are only 13 of these anymore:

    [ayrton:maintainer | Mon 19:47:22] $ grep -r "Form validation handler" * | wc -l
          13
    

    So I think this can be something much simpler like:

    Validates the number of uploaded files.

    (We don't need to say it's for the file widget anymore, because it's now in a class called FileWidget.)

12. +++ b/core/modules/migrate/src/Plugin/Migration.php
    @@ -205,6 +205,8 @@ class Migration extends PluginBase implements MigrationInterface, RequirementsIn
    +   * The source row status.
    +   *
        * Specify value of source_row_status for current map row. Usually set by
        * MigrateFieldHandler implementations.
    

    Not a method so outside the current scope.

13. +++ b/core/modules/migrate/src/Plugin/MigrationInterface.php
    @@ -195,8 +195,7 @@ public function getInterruptionResult();
    -   * Signal that the migration should be interrupted with the specified result
    -   * code.
    +   * Signal that migration should be interrupted with specified result code.
    

    Sets the migration status as interrupted with a given result code.

14. +++ b/core/modules/migrate/src/Plugin/MigrationInterface.php
    @@ -204,8 +203,7 @@ public function clearInterruptionResult();
    -   * Get the normalized process pipeline configuration describing the process
    -   * plugins.
    +   * Get normalized process pipeline configuration describing process plugins.
    

    Wow is that a mouthful. How much wood could a woodchuck chuck, if a woodchuck could chuck wood?

    How about:

    Gets the normalized process plugin configuration.

    (Still a mouthful, but not as much.)

15. +++ b/core/modules/node/src/Plugin/views/argument/Type.php
    @@ -52,16 +52,18 @@ public static function create(ContainerInterface $container, array $configuratio
    -   * Override the behavior of summaryName(). Get the user friendly version
    -   * of the node type.
    +   * Override the behavior of summaryName().
    +   *
    +   * Get the user friendly version of the node type.
    ...
    -   * Override the behavior of title(). Get the user friendly version of the
    -   * node type.
    +   * Override the behavior of title().
    +   *
    +   * Get the user friendly version of the node type.
    

    This should probably just be {@inheritdoc}.

16. +++ b/core/modules/pgsql/src/Driver/Database/pgsql/Schema.php
    @@ -321,8 +321,7 @@ protected function createTableSql($name, $table) {
    -   * Create an SQL string for a field to be used in table creation or
    -   * alteration.
    +   * Create SQL string for a field to be used in table creation or alteration.
    

    Creates an SQL string for table creation or alteration.

17. +++ b/core/modules/pgsql/src/Driver/Database/pgsql/Schema.php
    @@ -1029,8 +1028,9 @@ public function getComment($table, $column = NULL) {
    +   * Per PostgreSQL documentation: 4.1. Lexical Structure.
    

    Should probably be "This is per...." since it's the second paragraph and therefore needs to be a complete sentence.

18. +++ b/core/modules/system/src/Controller/TimezoneController.php
    @@ -10,8 +10,9 @@
    -   * Retrieve a JSON object containing a time zone name given a timezone
    -   * abbreviation.
    +   * Retrieve a JSON object.
    +   *
    +   * JSON object contains a time zone name given a timezone abbreviation.
    

    Retrives a JSON object containing the timezone name.

    (The "given" is implied by the parameter documentation.)

19. +++ b/core/modules/views/src/ManyToOneHelper.php
    @@ -128,8 +128,10 @@ public function getJoin() {
    -   * Provide the proper join for summary queries. This is important in part because
    -   * it will cooperate with other arguments if possible.
    +   * Provide the proper join for summary queries.
    +   *
    +   * This is important in part because it will cooperate with other arguments if
    +   * possible.
    

    This should start with "Provides".

    Also I have no idea what "it will cooperate with other arguments if possible" means.

20. +++ b/core/modules/views/src/Plugin/views/BrokenHandlerTrait.php
    @@ -29,8 +29,9 @@ public function defineOptions() {
    -   * Ensure the main table for this handler is in the query. This is used
    -   * a lot.
    +   * Ensure the main table for this handler is in the query.
    +   *
    +   * This is used a lot.
    

    This should start with "Ensures". I also agree that "This is used a lot" is not needed.

21. +++ b/core/modules/views/src/Plugin/views/BrokenHandlerTrait.php
    --- a/core/modules/views/src/Plugin/views/HandlerBase.php
    +++ b/core/modules/views/src/Plugin/views/HandlerBase.php
    
    +++ b/core/modules/views/src/Plugin/views/HandlerBase.php
    @@ -365,6 +365,8 @@ public function submitGroupByForm(&$form, FormStateInterface $form_state) {
    +   * Has extra options.
    +   *
    

    What?

Also, I suggest converting this issue to a merge request. Then I could use the "suggestion" feature to rewrite the bad docs myself and save a lot of review churn. The workflow is that anyone commenting on emerge request can make a suggestion for how to change it, and then the original author can accept the suggestion, or another reviewer can comment saying +1 for the suggestion and I can apply it with a click of a button. It is one to GitLab feature that is definitely superior to the patch workflow. It is much better for this type of issue than a patch and should save us a lot of time spent on extra long bulleted lists etc.

Tested this out by commenting out the phpcs line and running against changes in the MR.

They all passed locally with that rule disabled.

Got further this time, all the way into views. Boy is a lot of this change set in Views...

Again, if you could, please extrapolate and make note of where other docs standards (like the sentence structure and grammar) are not being followed elsewhere in the MR.

Changes look good.

Do we need a follow up for EntityFormInterface?

Opened up #3339760: Update archaic standard in comments. for the follow up. Wasn't entirely sure what the grep command was though.

Some of the views plugin descriptions are still not readable to me at all, similar to feedback in #9.

These are not my best will fully admit.

But I took a look at MR 3275 and the changes make sense to me.

Committed and pushed to 10.1.x, 10.0.x and 9.5.x. Thanks!

The MR did not apply to 9.5.x in core/lib/Drupal/Core/Routing/Router.php but I skipped that change there because in 9.5.x the docblock was already just @inheritdoc.