Fix 'Drupal.Commenting.DocComment.LongFullStop' coding standard

There are a lot of undesirable changes that you have made that make it too much of a task to review at the moment. Could you perhaps advise on the script or process that you used to apply these changes and we could help you to amend the process to achieve the desired output. It is probably best that you manually check each change before submitting the next patch.

These are a few examples of the undesirable changes but there are many good changes amongst them:

1. +++ b/core/core.api.php
   @@ -1867,7 +1867,7 @@ function hook_queue_info_alter(&$queues) {
   - * hook_mail_alter() allows modification of email messages created and sent
   + * Hook_mail_alter() allows modification of email messages created and sent
   

   This is an undesirable change

2. +++ b/core/core.api.php
   @@ -1923,7 +1923,7 @@ function hook_mail_alter(&$message) {
   + * Prepares a message based on parameters;.
   

   This should be: Prepares a message based on parameters.

3. +++ b/core/core.api.php
   @@ -2098,7 +2098,8 @@ function hook_rebuild() {
   - *   @link batch Batch operations @endlink documentation.
   + *
   +   * @link batch Batch operations @endlink documentation.
   

   This indentation is wrong, as is the new line.

If you see this file what i've attached there is a lot of non-fixed issues, this topic needs deeper analysis and manual fixes.

As agreed between the mentors at Drupalcon, according to issues to avoid for novices, I am untagging this issue as "Beginner". This issue contains changes across a very wide range of files and might create too many other patches to need to be rerolled at this particular time. This patch has an automated way to be rerolled later so better to implement it after Drupalcon.

+++ b/core/authorize.php
@@ -17,6 +17,7 @@
  * system in modules/system/system.module. For more information, see:
+ *
  * @link authorize Authorized operation helper functions @endlink

+++ b/core/core.api.php
@@ -1867,7 +1867,7 @@ function hook_queue_info_alter(&$queues) {
- * hook_mail_alter() allows modification of email messages created and sent
+ * Hook_mail_alter() allows modification of email messages created and sent

@@ -1923,7 +1923,7 @@ function hook_mail_alter(&$message) {
- * Prepares a message based on parameters;
+ * Prepares a message based on parameters;.

@@ -2098,7 +2098,8 @@ function hook_rebuild() {
  *   For more information on creating batches, see the
- *   @link batch Batch operations @endlink documentation.
+ *
+   * @link batch Batch operations @endlink documentation.

+++ b/core/includes/bootstrap.inc
@@ -298,7 +298,8 @@ function drupal_get_path($type, $name) {
- * a user is passed through the t() function, or a related function. See the
+ * a user is passed through the t() function, or a related function. See the.
+ *

@link doesn't need a blank line before it.

Adding a period to the end of a line doesn't make it correct.

function names aren't capitalized.

---

How is this issue/patch supposed to be useful as automated?

EDIT: Hah, my observations are almost identical to #4. @nlisgo++

Would it not be more appropriate to open test cases around the coder module and then move this issue to that project. Once it is ready then you can unleash it on core?

I would suggest this issue is not the best way forward. You have had a couple of people offer feedback and you've just invited us to go somewhere else to file the bug but you have opened an issue on core?

It's not required that all coding standards can be fixed automatically, but it is of course a huge bonus if they can. What *is* required is that the suggested patch passes the coding standards check from the Coder module. So if we have any false positives they will need to be fixed in Coder first and this issue should be postponed on that.

So a massive patch like this that fixes numerous different kinds of standards is really difficult to review and get in. I understand that this is a single rule in drupalcs, but it is many different specific errors.

It would be better to split this into single-scope patches, e.g. one that adds all the missing periods, one that adds missing blank lines, etc. etc. Especially since there are a few errors in the patch at least.

Started #2620856: Extend FunctionCommentSniff for incorrect casing of data types such as 'NULL' to address the incorrect use of PHP type 'NULL in docblock directives. That was a follow up to #2608444: Some docblock fixes for PHP type 'NULL'. As the patch in comment #4 of #2620856: Extend FunctionCommentSniff for incorrect casing of data types such as 'NULL' enhances FunctionCommentSniff.php, the already very large patch in this issue will increase in size yet further.

I am wondering if more consideration has been given as to how the documentation fixes will be applied as patches to core once we again enter the RC phase. Perhaps it makes sense to break up FunctionCommentSniff.php into several discrete sniffs that will result in more manageable patch files? If so, the work in progress mentioned above probably can be broken out into a discrete sniff. Opinions are welcome.

this could be done with phpcbf --sniffs=Drupal.Commenting.DocComment core

I went through maybe 1/20th of the patch. There are a lot of bad fixes here, but it seems to boil down to 1 or 2 closely related bugs. Maybe it would be better to report these and fix them in Coder, and postpone this issue.

There will also be a lot of hand work involved here, most notable with URLs that are not prefixed with @see. This can maybe be split off into a different issue.

1. +++ b/core/core.api.php
   @@ -1945,7 +1945,7 @@ function hook_queue_info_alter(&$queues) {
   - * hook_mail_alter() allows modification of email messages created and sent
   + * Hook_mail_alter() allows modification of email messages created and sent
   

   This is the name of a function, it shouldn't be capitalized. Maybe we can rewrite it slightly to not make it start with the function name.

2. +++ b/core/core.api.php
   @@ -2002,7 +2002,7 @@ function hook_mail_alter(&$message) {
   - * Prepares a message based on parameters;
   + * Prepares a message based on parameters;.
   

   The semicolon should be removed.

3. +++ b/core/includes/common.inc
   @@ -403,7 +402,7 @@ function drupal_set_time_limit($time_limit) {
   - * base_path() adds a "/" to the beginning and end of the returned path if the
   + * Base_path() adds a "/" to the beginning and end of the returned path if the
   

   Starting with a function name again. This seems to be a recurring thing. Maybe we can support this in Coder? It's pretty easy to detect a function name.

4. +++ b/core/includes/errors.inc
   @@ -16,7 +16,7 @@
     * The error constants are documented at
   - * http://php.net/manual/errorfunc.constants.php
   + * http://php.net/manual/errorfunc.constants.php.
   

   We should use @see instead.

5. +++ b/core/includes/form.inc
   @@ -681,7 +679,8 @@ function template_preprocess_form_element_label(&$variables) {
   - *     @elapsed. Defaults to t('Completed @current of @total.').
   + *
   +     * @elapsed. Defaults to t('Completed @current of @total.').
   

   This seems a bug in Coder.

6. +++ b/core/includes/install.inc
   @@ -363,7 +363,8 @@ function drupal_rewrite_settings($settings = array(), $settings_file = NULL) {
   - *   @see token_name().
   + *
   +   * @see token_name().
   

   Here's this bug again. We should report it.

7. +++ b/core/includes/install.inc
   @@ -387,7 +388,8 @@ function _drupal_rewrite_settings_is_simple($type, $value) {
   - *   @see token_name().
   + *
   +   * @see token_name().
   

   Another instance.

8. +++ b/core/lib/Drupal.php
   @@ -326,7 +326,7 @@ public static function lock() {
   -   * This is the main entry point to the configuration API. Calling
   +   * This is the main entry point to the configuration API. Calling.
       * @code \Drupal::config('book.admin') @endcode will return a configuration
       * object in which the book module can store its administrative settings.
   

   This is probably related to the same logic that makes the wrong indentation to lines starting with an @ above.

9. +++ b/core/lib/Drupal/Component/Datetime/DateTimePlus.php
   @@ -32,7 +32,7 @@ class DateTimePlus {
   -   * http://tools.ietf.org/html/rfc7231#section-7.1.1.1
   +   * Http://tools.ietf.org/html/rfc7231#section-7.1.1.1
   

   Prefix with @see.

10. +++ b/core/lib/Drupal/Component/Datetime/DateTimePlus.php
    @@ -103,7 +103,8 @@ class DateTimePlus {
    -   *   @see __construct()
    +   *
    +     * @see __construct()
    

    Wrongly indented.

11. +++ b/core/lib/Drupal/Component/Datetime/DateTimePlus.php
    @@ -152,9 +153,11 @@ public static function createFromArray(array $date_parts, $timezone = NULL, $set
    -   *   @see __construct()
    +   *
    +     * @see __construct()
        * @param array $settings
    -   *   @see __construct()
    +   *
    +     * @see __construct()
    

    Wrongly indented.

12. +++ b/core/lib/Drupal/Component/Diff/Engine/DiffEngine.php
    @@ -9,7 +9,7 @@
      * The algorithm used here is mostly lifted from the perl module
      * Algorithm::Diff (version 1.06) by Ned Konz, which is available at:
    - *   http://www.perl.com/CPAN/authors/id/N/NE/NEDKONZ/Algorithm-Diff-1.06.zip
    + *   http://www.perl.com/CPAN/authors/id/N/NE/NEDKONZ/Algorithm-Diff-1.06.zip.
    

    We can rewrite this to use @see.

13. +++ b/core/lib/Drupal/Component/Diff/MappedDiff.php
    @@ -4,8 +4,11 @@
     
     /**
      * FIXME: bad name.
    + *
      * @todo document
    + *
      * @private
    + *
      * @subpackage DifferenceEngine
      */
     class MappedDiff extends Diff {
    

    This is a docblock we can be proud of :D

14. +++ b/core/lib/Drupal/Component/Gettext/PoStreamReader.php
    @@ -8,7 +8,7 @@
      * The PO file format parsing is implemented according to the documentation at
    - * http://www.gnu.org/software/gettext/manual/gettext.html#PO-Files
    + * http://www.gnu.org/software/gettext/manual/gettext.html#PO-Files.
    

    Use @see here.

Adding related issue to ignore the DiffEngine class from coding standings checking because it's an external library we use.

So we have one less doc block to fix - we don't need 12 from the suggestions in #19 :)

What's the real scope of this task? Does it fix all issues under Drupal.Commenting.DocComment? I didn't see any changes to phpcs.xml.dist. At least Drupal.Commenting.DocComment.SpacingBeforeTags is fixed in #2842949: Fix Drupal.Commenting.DocComment.SpacingBeforeTags coding standard.

This sniff needs split because of many places that needs paraphrase

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
----------------------------------------------------------------------
SOURCE                                                           COUNT
----------------------------------------------------------------------
Drupal.Commenting.DocComment.MissingShort                        871
Drupal.Commenting.DocComment.ShortSingleLine                     542
Drupal.Commenting.DocComment.TagsNotGrouped                      369
Drupal.Commenting.DocComment.ShortNotCapital                     98
Drupal.Commenting.DocComment.ParamNotFirst                       97
Drupal.Commenting.DocComment.ParamGroup                          49
Internal.Tokenizer.Exception                                     15
----------------------------------------------------------------------
A TOTAL OF 2041 SNIFF VIOLATIONS WERE FOUND IN 7 SOURCES
----------------------------------------------------------------------

Filed follow-up #2914709: Fix 'Drupal.Commenting.DocComment.LongNotCapital' coding standard

Indeed, I think this issue should be transformed into a meta with child issues for each sniff. The results I got for latest 8.5.x are:

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
----------------------------------------------------------------------
    SOURCE                                                       COUNT
----------------------------------------------------------------------
[ ] Drupal.Commenting.DocComment.MissingShort                    864
[ ] Drupal.Commenting.DocComment.ShortSingleLine                 539
[ ] Drupal.Commenting.DocComment.TagsNotGrouped                  367
[x] Drupal.Commenting.DocComment.ShortFullStop                   251
[x] Drupal.Commenting.DocComment.TagGroupSpacing                 209
[ ] Drupal.Commenting.DocComment.ShortNotCapital                 102
[ ] Drupal.Commenting.DocComment.ParamNotFirst                   97
[x] Drupal.Commenting.DocComment.LongFullStop                    89
[x] Drupal.Commenting.DocComment.SpacingBeforeTags               73
[x] Drupal.Commenting.DocComment.LongNotCapital                  29
[ ] Drupal.Commenting.DocComment.ParamGroup                      22
----------------------------------------------------------------------
A TOTAL OF 2642 SNIFF VIOLATIONS WERE FOUND IN 11 SOURCES
----------------------------------------------------------------------
PHPCBF CAN FIX THE 5 MARKED SOURCES AUTOMATICALLY (655 VIOLATIONS IN TOTAL)
----------------------------------------------------------------------

If this is transformed into a meta, #2903027: Fix 'Drupal.Commenting.DocComment.ShortNotCapital' coding standard and #2842949: Fix Drupal.Commenting.DocComment.SpacingBeforeTags coding standard should have this issue as parent.

Re #32

There is a meta issue already #2571965: [meta] Fix PHP coding standards in core. I do not think it's necessary to have another one just for comment, but it's ok to link each other as related issue. And agree on splitting this into small ones. So I'm rescoping this one to fix one of them.

Thank all for your efforts.

+++ b/core/lib/Drupal/Core/Asset/AssetResolverInterface.php
@@ -26,7 +26,7 @@ interface AssetResolverInterface {
-   * - CSS_THEME
+   * - CSS_THEME.

+++ b/core/lib/Drupal/Core/Entity/EntityReferenceSelection/SelectionPluginBase.php
@@ -115,7 +115,7 @@ public function entityQueryAlter(SelectInterface $query) {}
-   *   - setting_N
+   *   - setting_N.

+++ b/core/lib/Drupal/Core/Entity/Query/ConditionFundamentals.php
@@ -18,7 +18,7 @@ abstract class ConditionFundamentals {
    * - AND (default)
-   * - OR
+   * - OR.

It's weird adding . to the last item of a list. Any suggestions?

Instead of creating a new meta issue for fixing coding standards in the comment, I've just added a subsection Comments under the Remaining tasks of to the parent issue #2571965, and each one links to a child issue.

Wrapping list-ish into @code @endcode can get CI passed, but those are not true code, So would it be possible to introduce a new tag @list @endlist or using phpcs:disable/phpcs:enable to ignore them on purpose?

The patch needs a reroll for 9.1.

Rerolled patch for 9.1.x, please review.

@jungle: For me there is something wrong with "Drupal.Commenting.DocComment.LongFullStop".

At the start of this issue the patches were attempting to fix numerous DocComment coding standards, and the auto-fixes were not always producing good changes.

After @jungle's comments in #34 now that this issue is solely for Commenting.DocComment.LongFullStop it meant that DocComment.ShortFullStop did not have its own issue. So I have started #3183673: Fix 'Drupal.Commenting.DocComment.ShortFullStop' coding standard - part 1 auto-fixes.

To assist that work I have also made some improvements to the Coder sniff in #3184314: Improve the auto-fixing for DocComment.ShortFullStop coding standard to avoid the unwanted auto-fixes and make review and correction easier.

Patch #4 in [#3183673-4: Fix 'Drupal.Commenting.DocComment.ShortFullStop' coding standard - part 1 auto-fixes fixes 211 of the 251 ShortFullStop errors and is now ready for final review, if anyone here would like to help. I can then finish the 40 remaining manual changes, and we can discuss similar improvements to the LongFullStop fixer.

Commited #2623718: Fix 'Drupal.Commenting.HookComment' coding standard

Re-Rolled
Reverted changes for these two cases, which I think is not required.

--- b/core/lib/Drupal/Core/Template/Loader/FilesystemLoader.php
+++ a/core/lib/Drupal/Core/Template/Loader/FilesystemLoader.php
@@ -10,7 +10,7 @@
  * Loads templates from the filesystem.
  *
  * This loader adds module and theme template paths as namespaces to the Twig
+ * filesystem loader so that templates can be referenced by namespace, like
- * filesystem loader so that templates can be referenced by namespace, like.
  * @block/block.html.twig or @mytheme/page.html.twig.
  */

--- b/core/lib/Drupal/Core/Entity/Query/ConditionFundamentals.php
+++ a/core/lib/Drupal/Core/Entity/Query/ConditionFundamentals.php
@@ -18,7 +18,7 @@
    * The conjunction of this condition group. The value is one of the following:
    *
    * - AND (default)
+   * - OR
-   * - OR.
    *
    * @var string
    */

When I install the patch (9.2.x) on my local machine and I run PHPCS command I get the following errors:

FILE: ...l/web/core/modules/system/tests/src/Functional/System/TrustedHostsTest.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 35 | ERROR | [x] Doc comment long description must end with a full stop
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: /var/www/html/web/core/modules/views/views.module
--------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------
 507 | ERROR | [x] Doc comment long description must end with a full stop
--------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------


FILE: ...r/www/html/web/core/modules/views/src/Plugin/views/sort/SortPluginBase.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 187 | ERROR | [x] Doc comment long description must end with a full stop
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: ...r/www/html/web/core/modules/rest/tests/src/Functional/ResourceTestBase.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 165 | ERROR | [x] Doc comment long description must end with a full stop
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: ...var/www/html/web/core/lib/Drupal/Core/Entity/Sql/TableMappingInterface.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 133 | ERROR | [x] Doc comment long description must end with a full stop
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: ...r/www/html/web/core/lib/Drupal/Core/Entity/Query/ConditionFundamentals.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 22 | ERROR | [x] Doc comment long description must end with a full stop
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: ...var/www/html/web/core/lib/Drupal/Core/Template/Loader/FilesystemLoader.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 13 | ERROR | [x] Doc comment long description must end with a full stop
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: /var/www/html/web/core/tests/Drupal/Tests/DrupalTestBrowser.php
-------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------
 18 | ERROR | [x] Doc comment long description must end with a full stop
-------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------

Updated the points mentioned in #50

Thanks @anmolgoyal74 and @daffie. However, some of the changes introduced in #51 were the ones you removed from #49. We need to be very careful that we do not simply follow the coding standard but make the grammar worse.

--- a/core/lib/Drupal/Core/Entity/Query/ConditionFundamentals.php
+++ b/core/lib/Drupal/Core/Entity/Query/ConditionFundamentals.php
@@ -19,7 +19,7 @@ abstract class ConditionFundamentals {
    *
    * The value is one of the following:
    * - AND (default)
-   * - OR
+   * - OR.
    *
    * @var string
    */

and

--- a/core/lib/Drupal/Core/Template/Loader/FilesystemLoader.php
+++ b/core/lib/Drupal/Core/Template/Loader/FilesystemLoader.php
@@ -10,7 +10,7 @@
  * Loads templates from the filesystem.
  *
  * This loader adds module and theme template paths as namespaces to the Twig
- * filesystem loader so that templates can be referenced by namespace, like
+ * filesystem loader so that templates can be referenced by namespace, like.
  * @block/block.html.twig or @my_theme/page.html.twig.
  */

are not good fixes.

--- a/core/tests/Drupal/Tests/DrupalTestBrowser.php
+++ b/core/tests/Drupal/Tests/DrupalTestBrowser.php
@@ -15,7 +15,7 @@
  *
  * This code is heavily based on the following projects:
  * - https://github.com/FriendsOfPHP/Goutte
- * - https://github.com/minkphp/MinkGoutteDriver
+ * - https://github.com/minkphp/MinkGoutteDriver.
  */
 class DrupalTestBrowser extends AbstractBrowser {

This alters the url and would make it unclickable if rendered as a link in an IDE.

In these cases we need to improve the coder sniff. See my comments in #47. I've not checked to see if these were automated fixes or manual. Can you tell us please?

Postponing this issue until #3199280: Problems with Drupal.Commenting.DocComment.LongFullStop is fixed.

I requeued the patch in #51 just for interest, as the test failure looked un-related, and sure enough it passed second time.

Thanks for #3199280: Problems with Drupal.Commenting.DocComment.LongFullStop I will make some comments on that issue and start working on the improved sniff sometime soon.