Skip to content

fix no multiline comments after declarations, fixes #96 #104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 22, 2018
Merged

fix no multiline comments after declarations, fixes #96 #104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 17 additions & 21 deletions MO4/Sniffs/Commenting/PropertyCommentSniff.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ protected function processTokenWithinScope(
File $phpcsFile,
$stackPtr,
$currScope
) {
): void {
$find = [
T_COMMENT,
T_DOC_COMMENT_CLOSE_TAG,
Expand All @@ -95,7 +95,10 @@ protected function processTokenWithinScope(
// check if we have a single line comment after it on the same line,
// and if that one is OK.
$postComment = $phpcsFile->findNext(
[T_DOC_COMMENT_OPEN_TAG],
[
T_DOC_COMMENT_OPEN_TAG,
T_COMMENT,
],
$stackPtr
);
if ($postComment !== false
Expand All @@ -104,34 +107,27 @@ protected function processTokenWithinScope(
if ($tokens[$postComment]['content'] === '/**') {
// That's an error already.
$phpcsFile->addError(
'no doc blocks are allowed after declaration',
'no doc blocks are allowed directly after declaration',
$stackPtr,
'NoDocBlockAllowed'
);
} else {
$postCommentEnd = $tokens[$postComment]['comment_closer'];
$postCommentLine = $tokens[$postCommentEnd]['line'];
if ($tokens[$postComment]['line'] !== $postCommentLine) {
$phpcsFile->addError(
'no multiline comments after declarations allowed',
$stackPtr,
'MustBeOneLine'
);
}
} else if (0 !== \strpos($tokens[$postComment]['content'], '//')
&& \substr($tokens[$postComment]['content'], -2) !== '*/'
) {
$phpcsFile->addError(
'no multiline comments after declarations allowed',
$stackPtr,
'MustBeOneLine'
);
}
}
}//end if

// Don't do constants for now.
if ($tokens[$stackPtr]['code'] === T_CONST) {
return;
}

$commentEnd = $phpcsFile->findPrevious($find, ($stackPtr - 1));
if ($commentEnd === false) {
return;
}

$commentEnd = (int) $commentEnd;
$commentEnd = (int) $phpcsFile->findPrevious($find, ($stackPtr - 1));

$conditions = $tokens[$commentEnd]['conditions'];
$lastCondition = array_pop($conditions);
Expand Down Expand Up @@ -170,7 +166,7 @@ protected function processTokenWithinScope(

$vars = PregLibrary::mo4_preg_split('/\s+@var\s+/', $tokensAsString);

$varCount = (count($vars) - 1);
$varCount = (\count($vars) - 1);
if (($varCount === 0) || ($varCount > 1)) {
$phpcsFile->addError(
'property doc comment must have exactly one @var annotation',
Expand Down
8 changes: 6 additions & 2 deletions MO4/Tests/Commenting/PropertyCommentUnitTest.fail.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,14 @@ class X
// no doc block
private $_otherPriv;

private $z = null; /* invalid
private $z = null; /** invalid
multiline
comment */

private $y = null; /** invalid doc block y is null by default */
private $q = null; /* invalid doc block y is null by default */
private $x = null;
}
private $u = null; /* invalid
multiline
comment */
}
8 changes: 6 additions & 2 deletions MO4/Tests/Commenting/PropertyCommentUnitTest.fail.inc.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,14 @@ class X
// no doc block
private $_otherPriv;

private $z = null; /* invalid
private $z = null; /** invalid
multiline
comment */

private $y = null; /** invalid doc block y is null by default */
private $q = null; /* invalid doc block y is null by default */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/* invalid doc block after declaration */ -- please :), same above

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. This should be /* valid single line comment */

private $x = null;
}
private $u = null; /* invalid
multiline
comment */
}
4 changes: 3 additions & 1 deletion MO4/Tests/Commenting/PropertyCommentUnitTest.pass.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ class X
/** @var int $bla */
$bla = 1;
/** @var int $bla */

return ($bla === $foo); /* yeah! */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why /* yeah! */? it reads awkward if in the same changeset with /* invalid doc block y is null by default */.
As the later suggests any comment after declarations/code is unwanted.

P.S.
I see, only after declarations. Maybe state this in the faulty comment above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@calbrecht absolutely right. /* yeah! */ is misleading. I'll replace it with /* allowed single line comment */. That should settle this.

Thank for this insightful comment 👍

}

private $_endThing;
Expand All @@ -52,4 +54,4 @@ class X
const STUFF = 1;

public $stuff = [self::STUFF];
}
}
3 changes: 2 additions & 1 deletion MO4/Tests/Commenting/PropertyCommentUnitTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,9 @@ protected function getErrorList(string $testFile=''): array
26 => 2,
29 => 1,
34 => 1,
37 => 1,
37 => 2,
41 => 1,
44 => 1,
];
}

Expand Down