Skip to content

Set limits to Generic.Files.LineLength phpcs rule#132

Closed
glensc wants to merge 1 commit intolaminas:2.14.xfrom
glensc:override-cs-length
Closed

Set limits to Generic.Files.LineLength phpcs rule#132
glensc wants to merge 1 commit intolaminas:2.14.xfrom
glensc:override-cs-length

Conversation

@glensc
Copy link
Copy Markdown
Contributor

@glensc glensc commented Jan 26, 2021

Q A
Documentation yes/no
Bugfix yes/no
BC Break yes/no
New Feature yes/no
RFC yes/no
QA yes

Description

Update lineLimit phpcs rule to be able to merge #126

https://github.com/squizlabs/PHP_CodeSniffer/wiki/Customisable-Sniff-Properties#genericfileslinelength

Property Name Type Default Available Since
lineLimit int 80 -
absoluteLineLimit int 100 -
ignoreComments bool false 3.1.0

@glensc
Copy link
Copy Markdown
Contributor Author

glensc commented Jan 26, 2021

  1. This project loads: ./vendor/laminas/laminas-coding-standard/ruleset.xml: https://github.com/laminas/laminas-mail/blob/2.13.0/phpcs.xml#L3
  2. Which loads PSR2 <rule ref="PSR2"/>: https://github.com/laminas/laminas-coding-standard/blob/1.0.0/ruleset.xml#L10
  3. Which defines <property name="lineLimit" value="120"/>, <property name="absoluteLineLimit" value="0"/>: https://github.com/squizlabs/PHP_CodeSniffer/blob/2.9.2/CodeSniffer/Standards/PSR2/ruleset.xml#L30-L36
  4. so, according to PSR-2 soft limit should be 120 and no hardlimit: https://www.php-fig.org/psr/psr-2/#1-overview

and this is exactly how phpcs behaves:

[~/scm/php/zend-mail (override-cs-length)★] ➔ c cs-check
> phpcs
............................................................  60 / 152 (39%)
.............................................W.............. 120 / 152 (79%)
................................


FILE: /Users/glen/scm/php/zend-mail/test/Transport/SendmailTest.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------
  97 | WARNING | Line exceeds 120 characters; contains 130 characters
 283 | WARNING | Line exceeds 120 characters; contains 122 characters
----------------------------------------------------------------------

Time: 3.61 secs; Memory: 24Mb

Script phpcs handling the cs-check event returned with error code 1

but the problem is that WARNING is treated as fatal, perhaps turn that off? I currently can't find how is that behaviour controlled.

@froschdesign
Copy link
Copy Markdown
Member

This is the wrong direction because:

  1. It opens the door for more errors of this kind in the future and for entire code instead of fixing them or to use existing features.
  2. At the moment the old version of the coding standard (1.0) is still in use here, so modifications are not expedient.

@froschdesign froschdesign added the Invalid This doesn't seem right label Jan 27, 2021
@glensc
Copy link
Copy Markdown
Contributor Author

glensc commented Jan 27, 2021

@froschdesign such request came from @Ocramius:

perhaps you can jump in there and propose (show), how to set the exclude, as indicated there, previous attempts have failed (nothing works). the PR itself was extracted from larger PR as was unable to solve the cs exclude rule there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Invalid This doesn't seem right

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants