Fix 'Drupal.Commenting.InlineComment.InvalidEndChar' coding standard

I will fix this.

I am on it

done

Patch does not apply:

$ git apply Drupal.Commenting.InlineComment.InvalidEndChar-2719663-4.patch 
error: patch failed: core/modules/user/src/Plugin/Validation/Constraint/UserNameConstraintValidator.php:31
error: core/modules/user/src/Plugin/Validation/Constraint/UserNameConstraintValidator.php: patch does not apply
error: patch failed: core/modules/user/tests/src/Kernel/UserValidationTest.php:40
error: core/modules/user/tests/src/Kernel/UserValidationTest.php: patch does not apply

rerolled

There are changes in the patch which remove empty lines, which are not related?

+++ b/core/phpcs.xml.dist
@@ -63,6 +63,13 @@
   </rule>
+  <rule ref="Drupal.Commenting.InlineComment">
+    <exclude name="Drupal.Commenting.InlineComment.DocBlock"/>
+    <exclude name="Drupal.Commenting.InlineComment.NoSpaceBefore"/>
+    <exclude name="Drupal.Commenting.InlineComment.NotCapital"/>
+    <exclude name="Drupal.Commenting.InlineComment.SpacingBefore"/>
+    <exclude name="Drupal.Commenting.InlineComment.WrongStyle"/>
+  </rule>
   <rule ref="Drupal.ControlStructures.ElseIf"/>

Let's add the rule we're applying here, rather than excluding all the others. That way we are sure the patch only touches the one rule.

Also, the patch does not apply.

Please: Modify phpcs.xml.dist to use the rule Drupal.Commenting.InlineComment.InvalidEndChar (instead of excluding the others), re-run phpcbf, run phpcs to make sure all the errors are dealt with, read the changes to make sure the changes are sane, and then re-run phpcs again to make sure your sanity changes didn't break something else.

Thanks.

Rerolled, fixed some bugs manually.

Patch no longer applies

Re-rolled

Patch needs a reroll, sorry for not reviewing it early enough.

I applied the patch, then ran composer phpcs, there are no remaining errors.

error: patch failed: core/modules/field/tests/src/Kernel/FieldAttachStorageTest.php:326
error: core/modules/field/tests/src/Kernel/FieldAttachStorageTest.php: patch does not apply
error: patch failed: core/modules/search/search.module:261
error: core/modules/search/search.module: patch does not apply

Patch needs a reroll but also there's quite a bit more to fix.

FILE: ...upal8alt.dev/core/modules/comment/src/Tests/CommentPagerTest.php
----------------------------------------------------------------------
FOUND 10 ERRORS AFFECTING 10 LINES
----------------------------------------------------------------------
 269 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 271 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 273 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 275 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 277 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 293 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 295 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 297 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 299 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 301 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 10 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...v/core/modules/field/tests/src/Kernel/FieldAttachStorageTest.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 328 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...e/modules/field/tests/src/Kernel/Migrate/d7/MigrateFieldTest.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 124 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...ore/modules/image/tests/src/Functional/ImageFieldDisplayTest.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 79 | ERROR | [x] Inline comments must end in full-stops, exclamation
    |       |     marks, colons, question marks, or closing parentheses
    |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...t.dev/core/modules/link/src/Plugin/migrate/process/FieldLink.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 93 | ERROR | [x] Inline comments must end in full-stops, exclamation
    |       |     marks, colons, question marks, or closing parentheses
    |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...alt.dev/core/modules/menu_ui/tests/src/Functional/MenuUiTest.php
----------------------------------------------------------------------
FOUND 3 ERRORS AFFECTING 3 LINES
----------------------------------------------------------------------
 323 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 363 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 403 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: .../drupal8alt.dev/core/modules/rest/src/Routing/ResourceRoutes.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 97 | ERROR | [x] Inline comments must end in full-stops, exclamation
    |       |     marks, colons, question marks, or closing parentheses
    |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ....dev/core/modules/rest/tests/src/Functional/ResourceTestBase.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 387 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...vdisk/dev/sites/drupal8alt.dev/core/modules/search/search.module
----------------------------------------------------------------------
FOUND 6 ERRORS AFFECTING 6 LINES
----------------------------------------------------------------------
 264 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 267 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 474 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 507 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 511 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 513 | ERROR | [x] Inline comments must end in full-stops, exclamation
     |       |     marks, colons, question marks, or closing parentheses
     |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 6 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...drupal8alt.dev/core/modules/views/src/Plugin/views/query/Sql.php
----------------------------------------------------------------------
FOUND 8 ERRORS AFFECTING 8 LINES
----------------------------------------------------------------------
  202 | ERROR | [x] Inline comments must end in full-stops, exclamation
      |       |     marks, colons, question marks, or closing parentheses
      |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
  582 | ERROR | [x] Inline comments must end in full-stops, exclamation
      |       |     marks, colons, question marks, or closing parentheses
      |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
  736 | ERROR | [x] Inline comments must end in full-stops, exclamation
      |       |     marks, colons, question marks, or closing parentheses
      |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
  834 | ERROR | [x] Inline comments must end in full-stops, exclamation
      |       |     marks, colons, question marks, or closing parentheses
      |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
  841 | ERROR | [x] Inline comments must end in full-stops, exclamation
      |       |     marks, colons, question marks, or closing parentheses
      |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 1288 | ERROR | [x] Inline comments must end in full-stops, exclamation
      |       |     marks, colons, question marks, or closing parentheses
      |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 1293 | ERROR | [x] Inline comments must end in full-stops, exclamation
      |       |     marks, colons, question marks, or closing parentheses
      |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
 1501 | ERROR | [x] Inline comments must end in full-stops, exclamation
      |       |     marks, colons, question marks, or closing parentheses
      |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 8 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...andard/tests/src/FunctionalJavascript/StandardJavascriptTest.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 43 | ERROR | [x] Inline comments must end in full-stops, exclamation
    |       |     marks, colons, question marks, or closing parentheses
    |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------


FILE: ...ts/Drupal/KernelTests/Core/Extension/ModuleExtensionListTest.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 27 | ERROR | [x] Inline comments must end in full-stops, exclamation
    |       |     marks, colons, question marks, or closing parentheses
    |       |     (Drupal.Commenting.InlineComment.InvalidEndChar)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------

+++ b/core/lib/Drupal/Core/Extension/ExtensionDiscovery.php
@@ -352,7 +352,7 @@ protected function sort(array $all_files, array $weights) {
     // 1 1 core/profiles/testing/modules/compatible_test/compatible_test.module
     // 2   sites/all/modules/common/common.module
     // 3   modules/devel/devel.module
-    // 4   sites/default/modules/custom/custom.module
+    // 4   sites/default/modules/custom/custom.module.

Changes like this need a bit thought just sticking a full stop here doesn't seem quite right. There are other lists where this also does not look right.

Re-roll for D9 as the patch #18 is failing on D9.1.x

Hi @sharma.amitt16, I went through the patch. It seems to fix most of the issues

diff --git a/core/lib/Drupal/Component/ProxyBuilder/ProxyBuilder.php b/core/lib/Drupal/Component/ProxyBuilder/ProxyBuilder.php
index 98354ba9fb..5a21ba3da2 100644
--- a/core/lib/Drupal/Component/ProxyBuilder/ProxyBuilder.php
+++ b/core/lib/Drupal/Component/ProxyBuilder/ProxyBuilder.php
@@ -106,7 +106,7 @@ public function build($class_name, $proxy_class_name = '') {
 
     $output .= $this->buildUseStatements();
 
-    // The actual class;
+   // The actual class;.
     $properties = <<<'EOS'
 /**
  * The id of the original proxied service.
@@ -307,7 +307,7 @@ protected function buildMethodBody(\ReflectionMethod $reflection_method) {
       $output .= "    \\$class_name::$function_name(";
     }
 
-    // Add parameters;
+    // Add parameters;.

I see a full stop after the semicolon and there are 24 such instances in a patch. The semicolon needs to removed from here.

@himanshu_sindhwani, thanks for the review of such a long patch. I have fixed your findings in this patch. Please review.

Seems good to me now. But I am not sure if #22 is addressed.

@himanshu thanks for review. #22 is not addressed in this patch as I am not sure whether to do this in scope of this issue or not as there are multiple such instances available in multiple files.

Also it is done as per sniff rule. IMHO, if it needs to fix then it should be done in separate issue.

While probably 90% of these are correct to change, there are lots of problems. I didn't scan the full patch but this is what I found on a partial review:

1. +++ b/core/lib/Drupal/Core/Asset/CssOptimizer.php
   @@ -96,7 +96,7 @@ public function loadFile($file, $optimize = NULL, $reset_basepath = TRUE) {
   -    // Store the value of $optimize for preg_replace_callback with nested
   +    // Store the value of $optimize for preg_replace_callback with nested.
        // @import loops.
   

   This doesn't make sense with the full stop.

2. +++ b/core/lib/Drupal/Core/Config/ConfigBase.php
   @@ -102,7 +102,7 @@ public static function validateName($name) {
        // The name must not contain any of the following characters:
   -    // : ? * < > " ' / \
   +    // : ? * < > " ' / \.
   

   This is misleading as the dot is allowed.

3. +++ b/core/lib/Drupal/Core/Config/TypedConfigManager.php
   @@ -246,13 +246,13 @@ protected function getFallbackName($name) {
            // breakpoint.breakpoint.*.* becomes
   -        // breakpoint.breakpoint.*
   +        // breakpoint.breakpoint.*.
   ...
            // Check for next level. For example, if breakpoint.breakpoint.* has
   -        // been checked and no match found then check breakpoint.*.*
   +        // been checked and no match found then check breakpoint.*.*.
   

   This is confusing now. Maybe the examples can be wrapped in double quotes.

4. +++ b/core/lib/Drupal/Core/Database/Query/Condition.php
   @@ -210,14 +210,14 @@ public function compile(Connection $connection, PlaceholderInterface $queryPlace
   -          // If the operator and value were not passed in to the
   +          // If the operator and value were not passed in to the.
              // @see ConditionInterface::condition() method (and thus have the
              // default value as defined over there) it is assumed to be a valid
   

   This doesn't make sense, we don't need a full stop here.

5. +++ b/core/lib/Drupal/Core/Database/Query/Condition.php
   @@ -210,14 +210,14 @@ public function compile(Connection $connection, PlaceholderInterface $queryPlace
   -          // Left hand part is a literal string added with the
   +          // Left hand part is a literal string added with the.
              // @see ConditionInterface::where() method. Put brackets around
   

   This doesn't make sense with the full stop either.

6. +++ b/core/lib/Drupal/Core/Extension/module.api.php
   @@ -636,7 +636,7 @@ function hook_install_tasks_alter(&$tasks, $install_state) {
      // For non-batch updates, the signature can simply be:
   -  // function hook_update_N() {
   +  // function hook_update_N() {.
   

   This is misleading now, as you don't put a full stop in the code.

7. +++ b/core/modules/rdf/tests/src/Functional/StandardProfileTest.php
   @@ -212,7 +212,7 @@ protected function doFrontPageRdfaTests() {
   -    // $this->assertEqual(2, count($this->getRdfGraph(Url::fromRoute('<front>'), $this->baseUri)->allOfType('http://schema.org/Article')), 'Two articles found on front page.');
   +    // $this->assertEqual(2, count($this->getRdfGraph(Url::fromRoute('<front>'), $this->baseUri)->allOfType('http://schema.org/Article')), 'Two articles found on front page.').
   

   This is commented out code, it should end with a ; not a .

8. +++ b/core/modules/search/tests/src/Kernel/SearchExcerptTest.php
   @@ -61,7 +61,7 @@ public function testSearchExcerpt() {
   -    // 123456789 HTMLTest +123456789+‘  +‘  +‘  +‘  +12345678      +‘  +‘  +‘   ‘
   +    // 123456789 HTMLTest +123456789+‘  +‘  +‘  +‘  +12345678      +‘  +‘  +‘   &lsquo.
   

   This is sample HTML, ending in a semicolon is correct.

Updated patch as per the changes suggested in #34, please review.

pretty long patcch for review :(. Did one full pass and below are some of my observations.

1. diff --git a/core/lib/Drupal/Component/Utility/Unicode.php b/core/lib/Drupal/Component/Utility/Unicode.php
   index 3729c7d9ee..54bfe1c179 100644
   --- a/core/lib/Drupal/Component/Utility/Unicode.php
   +++ b/core/lib/Drupal/Component/Utility/Unicode.php
   @@ -396,7 +396,7 @@ public static function strcasecmp($str1, $str2) {
       */
      public static function mimeHeaderEncode($string, $shorten = FALSE) {
        if (preg_match('/[^\x20-\x7E]/', $string)) {
   -      // floor((75 - strlen("=?UTF-8?B??=")) * 0.75);
   +      // floor((75 - strlen("=?UTF-8?B??=")) * 0.75).
          $chunk_size = 47;

   Can we change it to "chunk size calculation: floor((75 - strlen("=?UTF-8?B??=")) * 0.75)."

2. #34.3
   

   diff --git a/core/lib/Drupal/Core/Config/TypedConfigManager.php b/core/lib/Drupal/Core/Config/TypedConfigManager.php
   index 8585f8399e..f62b9faca3 100644
   --- a/core/lib/Drupal/Core/Config/TypedConfigManager.php
   +++ b/core/lib/Drupal/Core/Config/TypedConfigManager.php
   @@ -246,13 +246,13 @@ protected function getFallbackName($name) {
            // No definition for this level. Collapse multiple wildcards to a single
            // wildcard to see if there is a greedy match. For example,
            // breakpoint.breakpoint.*.* becomes
   -        // breakpoint.breakpoint.*
   +        // breakpoint.breakpoint.*.

   Still not addressed

3. diff --git a/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php b/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php
   index c2aef48a0f..7b1eb5bf06 100644
   --- a/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php
   +++ b/core/lib/Drupal/Core/Database/Driver/pgsql/Connection.php
   @@ -92,7 +92,7 @@ public static function open(array &$connection_options = []) {
        // so backslashes in the password need to be doubled up.
        // The bug was reported against pdo_pgsql 1.0.2, backslashes in passwords
        // will break on this doubling up when the bug is fixed, so check the version
   -    // elseif (phpversion('pdo_pgsql') < 'version_this_was_fixed_in') {
   +    // elseif (phpversion('pdo_pgsql') < 'version_this_was_fixed_in') {.

   We can remove this as Drupal 9 requires PHP 7.3 or higher and PostgreSQL 10.

4. --- a/core/lib/Drupal/Core/Database/Query/Select.php
   +++ b/core/lib/Drupal/Core/Database/Query/Select.php
   @@ -797,13 +797,13 @@ public function __toString() {
        // Create a sanitized comment string to prepend to the query.
        $comments = $this->connection->makeComment($this->comments);
    
   -    // SELECT
   +    // SELECT.
        $query = $comments . 'SELECT ';
        if ($this->distinct) {
          $query .= 'DISTINCT ';
        }
    
   -    // FIELDS and EXPRESSIONS
   +    // FIELDS and EXPRESSIONS.
        $fields = [];
        foreach ($this->tables as $alias => $table) {
          if (!empty($table['all_fields'])) {
   @@ -853,18 +853,18 @@ public function __toString() {
          }
        }
    
   -    // WHERE
   +    // WHERE.
        if (count($this->condition)) {
          // There is an implicit string cast on $this->condition.
          $query .= "\nWHERE " . $this->condition;
        }
    
   -    // GROUP BY
   +    // GROUP BY.
        if ($this->group) {
          $query .= "\nGROUP BY " . implode(', ', $this->group);
        }
    
   -    // HAVING
   +    // HAVING.
        if (count($this->having)) {
          // There is an implicit string cast on $this->having.
          $query .= "\nHAVING " . $this->having;
   @@ -878,7 +878,7 @@ public function __toString() {
          }
        }
    
   -    // ORDER BY
   +    // ORDER BY.
        if ($this->order) {
          $query .= "\nORDER BY ";
          $fields = [];<code>
   I would prefer SELECT: FIELDS and EXPRESSIONS: WHERE: GROUP BY: HAVING: ORDER BY: instead of SELECT. FIELDS and EXPRESSIONS. WHERE. GROUP BY. HAVING: ORDER BY. </li>
   <li><code>diff --git a/core/lib/Drupal/Core/Database/StatementInterface.php b/core/lib/Drupal/Core/Database/StatementInterface.php
   index 4f4248df04..d8cfe8ce71 100644
   --- a/core/lib/Drupal/Core/Database/StatementInterface.php
   +++ b/core/lib/Drupal/Core/Database/StatementInterface.php
   @@ -37,7 +37,7 @@ interface StatementInterface extends \Traversable {
       * "the access type must be omitted" if it is protected; i.e., conflicting
       * statements). The access type has to be protected.
       */
   -  // protected function __construct(Connection $dbh);
   +  // protected function __construct(Connection $dbh).

   Commented code. Leave as is.

5. diff --git a/core/lib/Drupal/Core/Form/FormBuilder.php b/core/lib/Drupal/Core/Form/FormBuilder.php
   index ae2851ed22..6bf2b0cc2f 100644
   --- a/core/lib/Drupal/Core/Form/FormBuilder.php
   +++ b/core/lib/Drupal/Core/Form/FormBuilder.php
   @@ -691,7 +691,7 @@ public function prepareForm($form_id, &$form, FormStateInterface &$form_state) {
          // dynamically generated action URLs don't have poor cacheability.
          // Use the proper API to generate the placeholder, when we have one. See
          // https://www.drupal.org/node/2562341. The placeholder uses a fixed string
   -      // that is Crypt::hashBase64('Drupal\Core\Form\FormBuilder::prepareForm');
   +      // that is Crypt::hashBase64('Drupal\Core\Form\FormBuilder::prepareForm').
          $placeholder = 'form_action_p_pvdeGsVG5zNF_XLGPTvYSKCf43t8qZYSwcfZl2uzM';

   diff --git a/core/lib/Drupal/Core/Utility/Token.php b/core/lib/Drupal/Core/Utility/Token.php
   index a250d5892b..73218811de 100644
   --- a/core/lib/Drupal/Core/Utility/Token.php
   +++ b/core/lib/Drupal/Core/Utility/Token.php
   @@ -250,7 +250,7 @@ public function scan($text) {
    
        // Iterate through the matches, building an associative array containing
        // $tokens grouped by $types, pointing to the version of the token found in
   -    // the source text. For example, $results['node']['title'] = '[node:title]';
   +    // the source text. For example, $results['node']['title'] = '[node:title]'.
        $results = [];
        for ($i = 0; $i < count($tokens); $i++) {
          $results[$types[$i]][$tokens[$i]] = $matches[0][$i];
   diff --git a/core/modules/ban/tests/src/Functional/IpAddressBlockingTest.php b/core/modules/ban/tests/src/Functional/IpAddressBlockingTest.php
   index 0647b16cb2..5aa0b92427 100644
   --- a/core/modules/ban/tests/src/Functional/IpAddressBlockingTest.php
   +++ b/core/modules/ban/tests/src/Functional/IpAddressBlockingTest.php
   @@ -80,7 +80,7 @@ public function testIPAddressValidation() {
        // $edit = array();
        // $edit['ip'] = \Drupal::request()->getClientIP();
        // $this->drupalPostForm('admin/config/people/ban', $edit, t('Save'));
   -    // $this->assertText(t('You may not ban your own IP address.'));
   +    // $this->assertText(t('You may not ban your own IP address.')).

   diff --git a/core/modules/comment/tests/src/Functional/CommentThreadingTest.php b/core/modules/comment/tests/src/Functional/CommentThreadingTest.php
   index 50217d38c1..a7816df865 100644
   --- a/core/modules/comment/tests/src/Functional/CommentThreadingTest.php
   +++ b/core/modules/comment/tests/src/Functional/CommentThreadingTest.php
   @@ -140,7 +140,7 @@ protected function assertParentLink($cid, $pid) {
        //   <p class="parent">
        //     <a href="...comment-1"></a>
        //   </p>
   -    //  </article>
   +    //  </article>.
        $pattern = "//article[@id='comment-$cid']//p[contains(@class, 'parent')]//a[contains(@href, 'comment-$pid')]";
    
        $this->assertFieldByXpath($pattern, NULL, new FormattableMarkup(
   @@ -163,7 +163,7 @@ protected function assertNoParentLink($cid) {
        // <a id="comment-2"></a>
        // <article>
        //   <p class="parent"></p>
   -    //  </article>
   +    //  </article>.

   diff --git a/core/modules/field/tests/src/Kernel/EntityReference/Views/EntityReferenceRelationshipTest.php b/core/modules/field/tests/src/Kernel/EntityReference/Views/EntityReferenceRelationshipTest.php
   index 8688441167..dc1f57af93 100644
   --- a/core/modules/field/tests/src/Kernel/EntityReference/Views/EntityReferenceRelationshipTest.php
   +++ b/core/modules/field/tests/src/Kernel/EntityReference/Views/EntityReferenceRelationshipTest.php
   @@ -278,7 +278,7 @@ public function testDataTableRelationshipWithLongFieldName() {
          $this->assertEqual($row->_entity->id(), $this->entities[$index]->id());
    
          // Test the forward relationship.
   -      // $this->assertEqual($row->entity_test_entity_test_mul__field_data_test_id, 1);
   +      // $this->assertEqual($row->entity_test_entity_test_mul__field_data_test_id, 1).

   diff --git a/core/modules/field/tests/src/Kernel/FieldImportDeleteTest.php b/core/modules/field/tests/src/Kernel/FieldImportDeleteTest.php
   index 5282923131..9d5f795f44 100644
   --- a/core/modules/field/tests/src/Kernel/FieldImportDeleteTest.php
   +++ b/core/modules/field/tests/src/Kernel/FieldImportDeleteTest.php
   @@ -36,7 +36,7 @@ public function testImportDelete() {
        // - field.storage.entity_test.field_test_import_2
        // - field.field.entity_test.entity_test.field_test_import
        // - field.field.entity_test.entity_test.field_test_import_2
   -    // - field.field.entity_test.test_bundle.field_test_import_2
   +    // - field.field.entity_test.test_bundle.field_test_import_2.

   diff --git a/core/modules/node/tests/src/Functional/Views/NodeFieldTokensTest.php b/core/modules/node/tests/src/Functional/Views/NodeFieldTokensTest.php
   index 4e32b723ab..d70333d7da 100644
   --- a/core/modules/node/tests/src/Functional/Views/NodeFieldTokensTest.php
   +++ b/core/modules/node/tests/src/Functional/Views/NodeFieldTokensTest.php
   @@ -52,16 +52,16 @@ public function testViewsTokenReplacement() {
    
        $this->drupalGet('test_node_tokens');
    
   -    // Body: {{ body }}<br />
   +    // Body: {{ body }}<br />.
        $this->assertRaw("Body: <p>$body</p>");
    
   -    // Raw value: {{ body__value }}<br />
   +    // Raw value: {{ body__value }}<br />.
        $this->assertRaw("Raw value: $body");
    
   -    // Raw summary: {{ body__summary }}<br />
   +    // Raw summary: {{ body__summary }}<br />.
        $this->assertRaw("Raw summary: $summary");
    
   -    // Raw format: {{ body__format }}<br />
   +    // Raw format: {{ body__format }}<br />.
        $this->assertRaw("Raw format: plain_text");
      }
    

   diff --git a/core/modules/rest/tests/src/Functional/ResourceTestBase.php b/core/modules/rest/tests/src/Functional/ResourceTestBase.php
   index 531226dc7c..f9803b82ca 100644
   --- a/core/modules/rest/tests/src/Functional/ResourceTestBase.php
   +++ b/core/modules/rest/tests/src/Functional/ResourceTestBase.php
   @@ -383,7 +383,7 @@ protected function assertResourceResponse($expected_status_code, $expected_body,
          // sets it to 'text/html' by default. We also cannot detect the presence
          // of Apache either here in the CLI. For now having this documented here
          // is all we can do.
   -      // $this->assertFalse($response->hasHeader('Content-Type'));
   +      // $this->assertFalse($response->hasHeader('Content-Type')).
          $this->assertSame('', (string) $response->getBody());
        }
        else {

   Many places code or sample code is in comments. leave such comment as as is(without punctuation)

Updated patch as per the changes suggested in #37, please review.

Re-roll of #39 in Merge Request.

Postponed on #3207576: Problem with Drupal.Commenting.InlineComment.InvalidEndChar: //cspel

No longer postponed. I rebased onto 11.x

Was a large MR to review but was easy. If anyone curious what the extra deletion is from

core/modules/pgsql/src/Driver/Database/pgsql/Connection.php deleted a commented out elseif line

Some feedback where the change makes the comment misleading - generally where we are listing punctuation in some way - plus some nitpicks and some weird cases where only some comments appear to be taken into account (especially in TaxonomyFieldAllTermsTest and EntityQueryTest).

I set this to NR for initial feedback because all I did was create and MR for the latest patch. I didn't expect it to become RTBC. Now that I look more closely the first thing I see is that the sniff is not enabled!

I will assign to myself and clean this up.

When autofixing a full stop is added to cspell ignore lines

// cSpell:ignore TÉXT äöüåøhello aouaohello aeoeueaohello Pácê.

which results in spelling errors.
core/modules/file/tests/src/Unit/SanitizeNameTest.php:102:13 - Unknown word (Pácê)

Found #3424914: Improve the cspell exception in Drupal.Commenting.InlineComment.InvalidEndChar which explains why only some cspell lines are failing. The sniff ignores lines where cspell is all lower case.

But there are other problems with the sniff. Therefor postponing on the Coder issue.

No longer postponed.

I rebased and fixed the remaining.

Reviewing this.

Been over 5 days so going to pick

Changes appear to be pretty 1 to 1. The extra deletions coming from core/lib/Drupal/Core/DrupalKernel.php.

Previous threads still appear to be open, 1-2 seem relevant

If you are another contributor eager to jump in, please allow the previous poster at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!

Responded to longwave's feedback.

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.

Rebase

I think this looks good. It does what it is supposed to do.

There are eight unresolved threads in the MR. So I don't think this should be set to RTBC until those are addressed and closed. In contrib MRs you can set push rules to prevent merging when there are open threads, but I'm not sure if that is done in the Core workflow. But in anycase those comments should be checked, and closed if done. But I don't they all can be closed yet.

I think on every open comment there was a change already, but the comment was still left open.
Only your comment is still open, so I will put it back to needs work

Thanks but many of the comments were not addressed, even if there was a later change. I've been through the entire list of chnages, and opened some more threads. It would be great if those who opened the earlier ones could check their own and close them when solved. Then we can really use the open threads as the driver to finally get this issue fixed.

@jonathan1055, thanks for the detailed review!

I agree it would help if reviewers would resolve comments. Thanks.

Tried to resolve the threads but some wasn't 100% sure on.

Excellent, you've reduced it from 24 unresolved threads to just 9.

One odd thing I commented on is why it appears that the 'Docblock' sniff is being excluded here?
https://git.drupalcode.org/project/drupal/-/merge_requests/505#note_477884

There are different views here about how to format lists in in-line comments. So, I am going to make a child issue to fix the easy ones. That child won't be able to enable the sniff but it will reduce the noise on this MR.

#3513596: Fix 'Drupal.Commenting.InlineComment.InvalidEndChar' coding standard, pt 1 has been merged, so unpostponing this now.