Skip to content

Commit

Permalink
Squiz/InlineComment: Fix two more issues where errors are incorrectly…
Browse files Browse the repository at this point in the history
… not being thrown

If two subsequent lines contained comments with code between them, they would be seen as part of the same comment block, while in reality there are not.

This meant that errors for `NotCapital` and `InvalidEndChar` where not always being thrown when they should be.

Includes additional unit tests demonstrating the issue.
  • Loading branch information
jrfnl committed Sep 25, 2017
1 parent d24981a commit f3b069c
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 4 deletions.
16 changes: 12 additions & 4 deletions src/Standards/Squiz/Sniffs/Commenting/InlineCommentSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -211,11 +211,14 @@ public function process(File $phpcsFile, $stackPtr)
// The below section determines if a comment block is correctly capitalised,
// and ends in a full-stop. It will find the last comment in a block, and
// work its way up.
$nextComment = $phpcsFile->findNext(array(T_COMMENT), ($stackPtr + 1), null, false);
if (($nextComment !== false)
&& (($tokens[$nextComment]['line']) === ($tokens[$stackPtr]['line'] + 1))
$nextComment = $phpcsFile->findNext(T_COMMENT, ($stackPtr + 1), null, false);
if ($nextComment !== false
&& $tokens[$nextComment]['line'] === ($tokens[$stackPtr]['line'] + 1)
) {
return;
$nextNonWhitespace = $phpcsFile->findNext(T_WHITESPACE, ($stackPtr + 1), $nextComment, true);
if ($nextNonWhitespace === false) {
return;
}
}

$topComment = $stackPtr;
Expand All @@ -225,6 +228,11 @@ public function process(File $phpcsFile, $stackPtr)
break;
}

$nextNonWhitespace = $phpcsFile->findNext(T_WHITESPACE, ($topComment + 1), $lastComment, true);
if ($nextNonWhitespace !== false) {
break;
}

$lastComment = $topComment;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,12 @@ echo $foo;
*/
include_once($blah);

// some comment without capital or full stop
echo $foo; // An unrelated comment.

// An unrelated comment.
echo $foo; // some comment without capital or full stop

/*
* N.B.: The below test line must be the last test in the file.
* Testing that a new line after an inline comment when it's the last non-whitespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,12 @@ echo $foo;
*/
include_once($blah);

// some comment without capital or full stop
echo $foo; // An unrelated comment.

// An unrelated comment.
echo $foo; // some comment without capital or full stop

/*
* N.B.: The below test line must be the last test in the file.
* Testing that a new line after an inline comment when it's the last non-whitespace
Expand Down
6 changes: 6 additions & 0 deletions src/Standards/Squiz/Tests/Commenting/InlineCommentUnitTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,9 @@ console.info(foo);
//**
* invalid comment
*/

// some comment without capital or full stop
console.log(foo); // An unrelated comment.

// An unrelated comment.
console.log(foo); // some comment without capital or full stop
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,9 @@ console.info(foo);
// **
* invalid comment
*/

// some comment without capital or full stop
console.log(foo); // An unrelated comment.

// An unrelated comment.
console.log(foo); // some comment without capital or full stop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ public function getErrorList($testFile='InlineCommentUnitTest.inc')
96 => 1,
97 => 3,
118 => 1,
126 => 2,
130 => 2,
);

return $errors;
Expand All @@ -60,6 +62,8 @@ public function getErrorList($testFile='InlineCommentUnitTest.inc')
104 => 3,
118 => 1,
121 => 1,
125 => 2,
129 => 2,
);
default:
return array();
Expand Down

0 comments on commit f3b069c

Please sign in to comment.