Skip to content

Commit

Permalink
Generic.Files.LineLength ignoreComments prop now ignores trailing com…
Browse files Browse the repository at this point in the history
…ments + sniff only checks for comment wrapping for comment-only lines (ref #2533)
  • Loading branch information
gsherwood committed Aug 8, 2019
1 parent c3f7eed commit 9400761
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 22 deletions.
5 changes: 4 additions & 1 deletion package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,11 @@ http://pear.php.net/dtd/package-2.0.xsd">
-- Enforce the use of a strict types declaration in PHP files
- Added PSR12.Files.DeclareStatement sniff
-- Enforce the formatting of declare statements within a file
- Generic.Files.LineLength ignoreComments property now only ignores comments that are on a line by themselves
- Generic.Files.LineLength ignoreComments property now ignores comments at the end of a line
-- Previously, this property was incorrectly causing the sniff to ignore any line that ended with a comment
-- Now, the trailing comment is not included in the line length, but the rest of the line is still checked
- Generic.Files.LineLength now only ignores unwrappable comments when the comment is on a line by itself
-- Previously, a short unwrappable comment at the end of the line would have the sniff ignore the entire line
- Generic.Functions.FunctionCallArgumentSpacing no longer checks spacing around assignment operators inside function calls
-- Use the Squiz.WhiteSpace.OperatorSpacing sniff to enforce spacing around assignment operators
--- Note that this sniff checks spacing around all assignment operators, not just inside function calls
Expand Down
48 changes: 29 additions & 19 deletions src/Standards/Generic/Sniffs/Files/LineLengthSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ class LineLengthSniff implements Sniff
public $absoluteLineLimit = 100;

/**
* Whether or not to ignore comment lines.
* Whether or not to ignore trailing comments.
*
* This has the effect of also ignoring all lines
* that only contain comments.
*
* @var boolean
*/
Expand Down Expand Up @@ -109,16 +112,35 @@ protected function checkLineLength($phpcsFile, $tokens, $stackPtr)
$stackPtr--;
}

if (isset(Tokens::$phpcsCommentTokens[$tokens[$stackPtr]['code']]) === true) {
$prevNonWhiteSpace = $phpcsFile->findPrevious(T_WHITESPACE, ($stackPtr - 1), null, true);
$onlyComment = false;
if (isset(Tokens::$commentTokens[$tokens[$stackPtr]['code']]) === true) {
$prevNonWhiteSpace = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($stackPtr - 1), null, true);
if ($tokens[$stackPtr]['line'] !== $tokens[$prevNonWhiteSpace]['line']) {
// Ignore PHPCS annotation comments if they are on a line by themselves.
return;
$onlyComment = true;
}
}

if ($onlyComment === true
&& isset(Tokens::$phpcsCommentTokens[$tokens[$stackPtr]['code']]) === true
) {
// Ignore PHPCS annotation comments that are on a line by themselves.
return;
}

$lineLength = ($tokens[$stackPtr]['column'] + $tokens[$stackPtr]['length'] - 1);

if ($this->ignoreComments === true
&& isset(Tokens::$commentTokens[$tokens[$stackPtr]['code']]) === true
) {
// Trailing comments are being ignored in line length calculations.
if ($onlyComment === true) {
// The comment is the only thing on the line, so no need to check length.
return;
}

$lineLength -= $tokens[$stackPtr]['length'];
}

// Record metrics for common line length groupings.
if ($lineLength <= 80) {
$phpcsFile->recordMetric($stackPtr, 'Line length', '80 or less');
Expand All @@ -130,25 +152,13 @@ protected function checkLineLength($phpcsFile, $tokens, $stackPtr)
$phpcsFile->recordMetric($stackPtr, 'Line length', '151 or more');
}

if ($tokens[$stackPtr]['code'] === T_COMMENT
|| $tokens[$stackPtr]['code'] === T_DOC_COMMENT_STRING
) {
// The current line ends with a comment.
// If the comment is the only thing on the line and we are ignoring comments,
// we can stop processing the line here.
if ($this->ignoreComments === true) {
$prevNonWhiteSpace = $phpcsFile->findPrevious(T_WHITESPACE, ($stackPtr - 1), null, true);
if ($tokens[$stackPtr]['line'] !== $tokens[$prevNonWhiteSpace]['line']) {
return;
}
}

if ($onlyComment === true) {
// If this is a long comment, check if it can be broken up onto multiple lines.
// Some comments contain unbreakable strings like URLs and so it makes sense
// to ignore the line length in these cases if the URL would be longer than the max
// line length once you indent it to the correct level.
if ($lineLength > $this->lineLimit) {
$oldLength = strlen($tokens[$stackPtr]['content']);
$oldLength = $tokens[$stackPtr]['length'];
$newLength = strlen(ltrim($tokens[$stackPtr]['content'], "/#\t "));
$indent = (($tokens[$stackPtr]['column'] - 1) + ($oldLength - $newLength));

Expand Down
2 changes: 2 additions & 0 deletions src/Standards/Generic/Tests/Files/LineLengthUnitTest.1.inc
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,5 @@ $ab = $ab = $ab = $ab = $ab = $ab = $ab = $ab = $ab = $ab;

// ... but not when combined with statements.
$a = $b; // phpcs:ignore Standard.Category.Sniff.ErrorCode1,Standard.Category.Sniff.ErrorCode2 -- for reasons ...

if (($anotherReallyLongVarName === true) {} // This comment makes the line too long.
6 changes: 5 additions & 1 deletion src/Standards/Generic/Tests/Files/LineLengthUnitTest.4.inc
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,8 @@ if($thisIsOk === true) {}
/* This line is too long but will be ignored. This line is too long but will be ignored. */
if (($anotherReallyLongVarName === true) || (is_array($anotherReallyLongVarName) === false)) {}

if (($anotherReallyLongVarName === true) {} // This comment makes the line too long.
if (($anotherReallyLongVarName === true) {} // This comment makes the line too long, but should be ignored.

if (($anotherReallyLongVarName === true) || (is_array($anotherReallyLongVarName) === false)) {} // Comment goes here.

if (($anotherReallyLongVarName === true) {} // phpcs:ignore Standard.Category.Sniff.ErrorCode1 -- for reasons
3 changes: 2 additions & 1 deletion src/Standards/Generic/Tests/Files/LineLengthUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ public function getWarningList($testFile='')
63 => 1,
73 => 1,
75 => 1,
84 => 1,
];
break;
case 'LineLengthUnitTest.2.inc':
Expand All @@ -96,7 +97,7 @@ public function getWarningList($testFile='')
case 'LineLengthUnitTest.4.inc':
return [
10 => 1,
12 => 1,
14 => 1,
];
break;
default:
Expand Down

0 comments on commit 9400761

Please sign in to comment.