Problem/Motivation

On the new version: 8.3.10 we got warnings over array declarations should go in multiple lines due to 80 char. limit is reached.
The line in subject looks like this:
phpcs
The red line represents the 80 char limit. Such array declaration with 1 single element should be allowed to exceed the line limit. On the other hand, if I wouldn't have arrays defined in this line, I wouldn't get any warning for exceeding the 80 char limit even though it exceeded. It seems that if an array is declared in the end of the line that exceeds the char limit we will get such warning regardless of the array size and placement in the given line.

CommentFileSizeAuthor
#3 TablesTest.txt1.1 KBjoevagyok
phpcs.png8.84 KBjoevagyok

Comments

joevagyok created an issue. See original summary.

jonathan1055’s picture

jonathan1055 commented

Hi joevagyok,

I can't replicate this, using your graphic as an example. Can you prvide a short text file with the actual problem, and also show here the actual warning you get. Use flag -s to show the full sniff name in the report.

joevagyok’s picture

joevagyok commented
StatusFileSize
new TablesTest.txt1.1 KB

Hi, @jonathan1055,

I have attached a php test file as txt that produced the following error for us:

FILE: /var/www/html/tests/Kernel/TablesTest.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 29 | ERROR | The array declaration line has 91 characters (the limit
    |       | is 80). The array content should be split up over
    |       | multiple lines
----------------------------------------------------------------------
jonathan1055’s picture

jonathan1055 commented

Thanks @joevagyok, yes this test does produce the warning, but it was not the example you gave in the issue summary (which does not fail).

Anyway, yes it seems there could be a issue here - your line is reported as too long because overall it does exceed the limit, however the array portion of the line stops before the 80 char position.

jonathan1055’s picture

jonathan1055 commented
Status: Active » Needs review

I have created https://github.com/pfrenssen/coder/pull/119 to work on the correction

  • jonathan1055 authored dcdf657 on 8.x-3.x 
    fix(Array): Use $parenthesisCloser column instead of full line length (#...
arkener’s picture

arkener commented
Version: 8.3.10 » 8.x-3.x-dev
Status: Needs review » Fixed

Thanks for working on this @jonathan1055 and thank you @joevagyok for reporting this issue and providing the test code. This change has now been merged.

jonathan1055’s picture

jonathan1055 commented

Thanks for the commit Arkener.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

ayalon’s picture

ayalon commented

@jonathan1055: Thanks for the fix. Can you push a new version to drupal.org. On a large code base tons of files are affected due to this bug.

jonathan1055’s picture

jonathan1055 commented

Hi ayalon,
I can't push the version, I only fixed the code. @Arkener or @Klausi will have to do that. Version 8.3.10 was released 5 September, only just over one month ago, but I don't know the timescale for the next tagged release.

If you are running locally you can always use the -dev version of Coder. Or you can temporarily ignore this sniff by the adding the following to your phpcs.xml ruleset file:

  <rule ref="Drupal.Arrays.Array">
    <exclude name="Drupal.Arrays.Array.LongLineDeclaration"/>
  </rule>

Hope that helps.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

Coder 8.3.11 has been released with this fix!

jonathan1055’s picture

jonathan1055 commented
Title: Line character limit is incorrectly checked when arrays are present » Use $parenthesisCloser column to correctly determine inline array length

Changing title to better reflect the solution. The original title "Line character limit is incorrectly checked when arrays are present" is repeated here in this comment to allow for searching.