Skip to content

Commit

Permalink
Merge branch 'feature/squiz-inlinecomment-incorrect-no-blank-line-aft…
Browse files Browse the repository at this point in the history
…er-comment-at-end-of-file' of https://github.com/jrfnl/PHP_CodeSniffer
  • Loading branch information
gsherwood committed Oct 11, 2017
2 parents c7e73b4 + 97bbb19 commit eafab36
Show file tree
Hide file tree
Showing 7 changed files with 184 additions and 5 deletions.
1 change: 1 addition & 0 deletions package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1177,6 +1177,7 @@ http://pear.php.net/dtd/package-2.0.xsd">
<file baseinstalldir="PHP/CodeSniffer" name="InlineCommentUnitTest.inc" role="test" />
<file baseinstalldir="PHP/CodeSniffer" name="InlineCommentUnitTest.inc.fixed" role="test" />
<file baseinstalldir="PHP/CodeSniffer" name="InlineCommentUnitTest.js" role="test" />
<file baseinstalldir="PHP/CodeSniffer" name="InlineCommentUnitTest.js.fixed" role="test" />
<file baseinstalldir="PHP/CodeSniffer" name="InlineCommentUnitTest.php" role="test" />
<file baseinstalldir="PHP/CodeSniffer" name="LongConditionClosingCommentUnitTest.inc" role="test" />
<file baseinstalldir="PHP/CodeSniffer" name="LongConditionClosingCommentUnitTest.inc.fixed" role="test" />
Expand Down
23 changes: 18 additions & 5 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 Expand Up @@ -278,6 +286,12 @@ public function process(File $phpcsFile, $stackPtr)
// Finally, the line below the last comment cannot be empty if this inline
// comment is on a line by itself.
if ($tokens[$previousContent]['line'] < $tokens[$stackPtr]['line']) {
$next = $phpcsFile->findNext(T_WHITESPACE, ($stackPtr + 1), null, true);
if ($next === false) {
// Ignore if the comment is the last non-whitespace token in a file.
return;
}

$start = false;
for ($i = ($stackPtr + 1); $i < $phpcsFile->numTokens; $i++) {
if ($tokens[$i]['line'] === ($tokens[$stackPtr]['line'] + 1)) {
Expand All @@ -292,7 +306,6 @@ public function process(File $phpcsFile, $stackPtr)
$error = 'There must be no blank line following an inline comment';
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpacingAfter');
if ($fix === true) {
$next = $phpcsFile->findNext(T_WHITESPACE, ($stackPtr + 1), null, true);
$phpcsFile->fixer->beginChangeset();
for ($i = ($stackPtr + 1); $i < $next; $i++) {
if ($tokens[$i]['line'] === $tokens[$next]['line']) {
Expand Down
15 changes: 15 additions & 0 deletions src/Standards/Squiz/Tests/Commenting/InlineCommentUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -122,3 +122,18 @@ echo $foo;
* Comments about the include
*/
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
* token in a file, does *not* throw an error as this would conflict with the common
* "new line required at end of file" rule.
*/

// For this test line having an empty line below it, is fine.
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,18 @@ echo $foo;
* Comments about the include
*/
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
* token in a file, does *not* throw an error as this would conflict with the common
* "new line required at end of file" rule.
*/

// For this test line having an empty line below it, is fine.
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
125 changes: 125 additions & 0 deletions src/Standards/Squiz/Tests/Commenting/InlineCommentUnitTest.js.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
// Some content here.
var code = 'hello';

// This comment contains # multiple
// hash signs (#).
code = 'hello';

/**
* This is the first line of a function comment.
* This is the second line.
*/
function testFunction()
{
// Callback methods which are added by external objects.
this.callbacks = {};

}//end testFunction()

/**
* This is the first line of a class comment.
* This is the second line.
*/
myClass.prototype = {

/**
* This is the first line of a method comment.
* This is the second line.
*/
load: function(url, callback)
{
// Some code here.
}
};

// some code goes here!
/*
A longer comment goes here.
It spans multiple lines!!
Or does it?
*/

// 0This is a simple multi-line
// comment!
code = 'hello';

// This is not valid.
code = 'hello';

// Neither is this!
code = 'hello';

code = 'hello';

/** Neither is this! **/
code = 'hello';

/**
* This is the first line of a function comment.
* This is the second line.
*/
var myFunction = function() {
}

/**
* This is the first line of a function comment.
* This is the second line.
*/
myFunction = function() {
}

/**
* This is the first line of a function comment.
* This is the second line.
*/
myClass.myFunction = function() {
}

dfx.getIframeDocument = function(iframe)
{
return doc;

};//end dfx.getIframeDocument()

mig.Gallery.prototype = {

init: function(cb)
{

},//end init()

imageClicked: function(id)
{

}//end imageClicked()

};

// Here is some inline example code:
// -> One
// -> One.One
// -> Two
/*
Here is some inline example code:
-> One
-> One.One
-> Two
*/


var foo = 'foo'; // Var set to foo.

console.info(foo);

// Comment here.
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 eafab36

Please sign in to comment.