[meta] Fix 'Drupal.Files.LineLength' coding standard

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.

all is ok there after phpcbf work

output:
----------------------------------------------------
No fixable errors were found
Time: 4 mins, 10.39 secs; Memory: 125.25Mb

@vprocessor this is because phpcbf can't fix these errors. If you run phpcs you will see a lot of warnings about lines being too long.

Looks the sniffer does not work cos "phpcs" returns nothing

1197 Drupal.Files.LineLength.TooLong

Given #17 this is needs work

Patch in #10 no longer applies. Not marking as 'needs reroll' because whoever works on this should just re-do the work.

I'd wager that the rule in question here can't fix the errors it finds through phpcfb, which is why no errors were found in #10.

Updating instructions.

Here's a patch which only covers core/lib/Drupal/Component.

Pretty tedious work, also tedious to review. Should we split it up by directory? By module?

Also: The sniff doesn't count multibyte chars properly, so you end up with an error in core/lib/Drupal/Component/Utility/Html.php comments.

The sniff doesn't count multibyte chars properly, so you end up with an error in core/lib/Drupal/Component/Utility/Html.php comments.

We need to fix the sniff then.

Filed this issue: #2761027: False positive for Drupal.Files.LineLength for multibyte characters

From #2761027: False positive for Drupal.Files.LineLength for multibyte characters it turns out we need to do this:

@klausi: Make sure to set <arg name="encoding" value="utf-8"/> in your phpcs.xml file.

So here's a patch against 8.2.x since it's docs-only changes, for the Component namespace.

I experimentally removed the LineLength rule and left the UTF-8 encoding. phpcs didn't burp on anything, so it looks safe to add it.

Is it on purpose that we just fix the component bits?

Because there's no automated fix, and we're changing the line length of comments by hand. And then we have to review the changes. See #21.

I tested the patch in #2572709-25: [meta] Fix 'Drupal.Files.LineLength' coding standard there are no warnings for core/lib/Drupal/Component .

Is the plan to have one large patch, or to break this in to sections?

I find it interesting that some of the first warnings in core are permissible in the coding standards, but not caught by the sniff. Perhaps it is worth identifying these so that the sniff can be updated prior to attempting to correct all of the files?

E.g.

FILE: .../core/core.api.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
----------------------------------------------------------------------
68 | WARNING | Line exceeds 80 characters; contains 93 characters
...

* - @link https://www.drupal.org/project/examples Examples project (sample modules) @endlink

The sniff appears to account for @link exceeding 80 chars, but not when it is in a list.

@Everett Zufelt Could you please file an issue about that in the coder project?

@Everett Zufelt the plan with these issue is to get the coder sniff fixed and then apply it in one go to core and the enforce the rule on all following patches to prevent regression.

Added #2791183: LineLengthSniff warns when @link is > 80 chars if in a list.

It looks like #2791183: LineLengthSniff warns when @link is > 80 chars if in a list. is fixed in Coder 8.x-2.x.

Added corrections for

/core/core.api.php
/core/includes

Based on Coder 8.x-2.x, the 8.x-2.9 release does not include the fix mentioned in comment #33.

In /core/lib/Drupal/Core/Cache/Cache.php line 99 @deprocated is used. followed by a Use comment.

   * @deprecated
   *   Use assert('\Drupal\Component\Assertion\Inspector::assertAllStrings($tags)');

There are no examples of @deprocated in the comment standards. Is there a better way for this to be presented (perhaps using @code) or should this be allowed to exceed 80 chars?

+++ b/core/includes/bootstrap.inc
@@ -726,7 +726,7 @@ function drupal_installation_attempted() {
  *
- * @return string|null $profile
+ * @return string|null

unrelated change

@dawehner, I agree it is not directly related. It is however not permitted in the commenting standards, and is identified as an error by Coder 8.x-2.x.

Thanks, but the scope of this issue is for Drupal.Files.LineLength only. There are other issues for other rules: #2571965: [meta] Fix PHP coding standards in core, stage 1 That way the process of fixing these things is more manageable and consistent.

Also, we're normalizing on coder 8.2.8 for now: #2744463: Add phpcs, coder 8.2.8 as --dev requirements for drupal/core

@Mile23
Thank you for making that clear!

I hope one day we establish a culture of not considering to even think about out of scope changes.

It needs new patch after #2923670: The one-line summary of ImageEffectInterface::getDerivativeExtension() is too long

working on this

Hi,
this one is an eyes killer, so i'll stop it for today
the final patch promesses to be huge
here i attach a patch for my progress on this but the work stills in progress
will upload WIP patches as i progress on this

Patch #47 no longer applies
here is a reroll of it
Thanks

progressed on this but it stills WIP

more progress

And here it is ready for review

yay, it's green :)

@izus thank you for picking this up and working on it. Here is the start of a review. Unfortunately it is a really big job to get core to comply with our own standards :(

1. +++ b/core/modules/block_content/block_content.install
   @@ -42,7 +42,8 @@ function block_content_update_8002() {
    /**
   - * Add 'revision_created' and 'revision_user' fields to 'block_content' entities.
   + * Add 'revision_created' and 'revision_user' fields to 'block_content'
   + * entities.
     */
    function block_content_update_8003() {
   

   This displayed to a user and we have a rule that the first line must be on one line for a method.

2. +++ b/core/modules/block_content/src/BlockContentInterface.php
   @@ -20,7 +20,8 @@
   +   *   \Drupal\Core\Entity\RevisionLogInterface::getRevisionLogMessage()
   +   * instead.
   
   @@ -45,7 +46,8 @@ public function setInfo($info);
   +   *   \Drupal\Core\Entity\RevisionLogInterface::setRevisionLogMessage()
   +   * instead.
   

   The instead should be indented.

3. +++ b/core/modules/block_content/tests/src/Functional/Views/RevisionRelationshipsTest.php
   @@ -9,7 +9,8 @@
   - * Tests the integration of block_content_revision table of block_content module.
   + * Tests the integration of block_content_revision table of block_content
   + * module.
   

   The one line method rule

4. +++ b/core/modules/book/src/BookManagerInterface.php
   @@ -275,7 +275,8 @@ public function bookTreeCheckAccess(&$tree, $node_links = []);
   +   *   A subtree of book links in an array, in the order they should be
   +   * rendered.
   

   Should be indented

5. +++ b/core/modules/ckeditor/tests/modules/src/Plugin/CKEditorPlugin/LlamaButton.php
   @@ -5,7 +5,8 @@
   - * Defines a "LlamaButton" plugin, with a toolbar builder-enabled "llama" feature.
   + * Defines a "LlamaButton" plugin, with a toolbar builder-enabled "llama"
   + * feature.
     *
   

   One line rule.

6. +++ b/core/modules/ckeditor/tests/src/Functional/CKEditorLoadingTest.php
   @@ -77,7 +77,8 @@ protected function setUp() {
   +    // - doesn't have access to the filtered_html text format, so: no text
   +    // editor.
   

   The second line of a list should be indented.

7. +++ b/core/modules/comment/comment.tokens.inc
   @@ -151,8 +151,8 @@ function comment_tokens($type, $tokens, array $data, array $options, BubbleableM
   +          // Add the user cacheability metadata in case the author of the
   +          //  comment is not the anonymous user.
   

   Indentation

8. +++ b/core/modules/config_translation/tests/src/Functional/ConfigTranslationUiTest.php
   @@ -630,7 +630,8 @@ public function testViewsTranslationUI() {
   -   * Test the number of source elements for plural strings in config translation forms.
   +   * Test the number of source elements for plural strings in config translation
   +   * forms.
   

   One line rule.

9. +++ b/core/modules/config_translation/tests/src/Functional/ConfigTranslationUiTest.php
   @@ -1167,7 +1169,8 @@ protected function assertDisabledTextarea($id) {
   -   * Helper function that returns a .po file with a given number of plural forms.
   +   * Helper function that returns a .po file with a given number of plural
   +   * forms.
   
   +++ b/core/modules/config_translation/tests/src/Kernel/Migrate/d6/MigrateSystemMaintenanceTranslationTest.php
   @@ -28,7 +28,8 @@ protected function setUp() {
   -   * Tests migration of system (maintenance) variables to system.maintenance.yml.
   +   * Tests migration of system (maintenance) variables to
   +   * system.maintenance.yml.
   

   One line rule

10. +++ b/core/modules/content_translation/content_translation.module
    @@ -531,7 +531,8 @@ function content_translation_language_configuration_element_process(array $eleme
    - * Form validation handler for element added with content_translation_language_configuration_element_process().
    + * Form validation handler for element added with
    + * content_translation_language_configuration_element_process().
    
    @@ -554,7 +555,8 @@ function content_translation_language_configuration_element_validate($element, F
    - * Form submission handler for element added with content_translation_language_configuration_element_process().
    + * Form submission handler for element added with
    + * content_translation_language_configuration_element_process().
    

    More one line rule.

It looks like there is plenty to do because we don't have all the other rules implemented yet. I think before we tackle this one we should try to fix Drupal.Commenting.DocComment.ShortSingleLine first. It should be a sub-issue of #2572635: Fix 'Drupal.Commenting.DocComment.LongFullStop' coding standard as doing that will help us not introduce more instances of that here.

The remarks of @alexpott from comment #53 still need to be addressed.

Re-created the patch using comment #51 patch. Also made changes as per comment #53.
There are some files which are either not available in d9 or they have already fix applied for line exceeding 80 chars issue.
Here i am mentioning those files:

core/modules/block_content/block_content.install
core/modules/block_content/src/BlockContentInterface.php
core/modules/block_content/tests/src/Kernel/Views/RevisionRelationshipsTest.php
core/modules/comment/tests/src/Kernel/CommentUninstallTest.php
core/modules/content_translation/tests/src/Functional/ContentTranslationTestBase.php
core/modules/content_translation/tests/src/Functional/ContentTranslationUITestBase.php
core/modules/file/tests/src/Functional/FileFieldWidgetTest.php
core/modules/rest/src/Tests/RESTTestBase.php
core/modules/rest/tests/fixtures/update/rest-export-with-authentication-correction.php
core/modules/rest/tests/src/Functional/EntityResource/BlockContent/BlockContentResourceTestBase.php
core/modules/rest/tests/src/Functional/EntityResource/BlockContentType/BlockContentTypeResourceTestBase.php
core/modules/rest/tests/src/Functional/EntityResource/ConfigTest/ConfigTestResourceTestBase.php
core/modules/rest/tests/src/Functional/EntityResource/ContentLanguageSettings/ContentLanguageSettingsResourceTestBase.php
core/modules/rest/tests/src/Functional/EntityResource/EntityTest/EntityTestResourceTestBase.php
core/modules/rest/tests/src/Functional/EntityResource/EntityTestBundle/EntityTestBundleResourceTestBase.php
core/modules/rest/tests/src/Functional/EntityResource/EntityTestLabel/EntityTestLabelResourceTestBase.php
core/modules/rest/tests/src/Functional/EntityResource/RestResourceConfig/RestResourceConfigResourceTestBase.php
core/modules/taxonomy/src/TermForm.php
core/modules/tracker/tracker.module

So it will be great if someone can review and share feedback, if this needs more work.

The testbot returns 900 coding standard violations for the current patch. See: https://www.drupal.org/pift-ci-job/1710079.

Probably pick this again once get the ample time to fix this. Meanwhile Please feel free to fix as i am unassigning this.

I have create a new issue for coding standard issues here https://www.drupal.org/project/drupal/issues/3156059.

#3173782: Increase line length limit to 120

Working on the reroll, thanks!

Re-rolled the patch in #59, thank you!

Far too many style guide violations to review.

Shouldn't this work be halted until #3173782: Increase line length limit to 120 is decided? If the length is increased, even to 100 chars, then a huge number of these current failings will no longer be a problem.

@rifas-ali-pbi, credit for creating a fork has been removed per How is credit granted for Drupal core issues.

Yes, let's postpone.

I've been taking a closer look at this over the holiday period. I am un-postponing this because most new code is wrapped at 80 columns and while there are a lot of files to change, it is a small percentage. And even increasing the line length will not fix wrapping.

Should this meta add the phpcs rule, or we leave that to a child issue?