jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/403940 )
Change subject: Detect variadic arguments in function comments
......................................................................
Detect variadic arguments in function comments
Avoid false positive detection of
"Doc comment for parameter "$params" missing"
in combination with
"Doc comment for parameter $params,... does not match actual variable
name $params"
when $params is part of the argument list.
The other option, when $params is not part of the argument list,
already works.
Bug: T175504
Change-Id: I051150bdb281ae22b8a3a2638dc09e60cadf46c9
---
M MediaWiki/Sniffs/Commenting/FunctionCommentSniff.php
M MediaWiki/Tests/files/Commenting/commenting_function.php
M MediaWiki/Tests/files/Commenting/commenting_function.php.expect
M MediaWiki/Tests/files/Commenting/commenting_function.php.fixed
4 files changed, 55 insertions(+), 5 deletions(-)
Approvals:
Legoktm: Looks good to me, approved
jenkins-bot: Verified
Thiemo Kreuz (WMDE): Looks good to me, but someone else must approve
diff --git a/MediaWiki/Sniffs/Commenting/FunctionCommentSniff.php
b/MediaWiki/Sniffs/Commenting/FunctionCommentSniff.php
index bb53b01..006fe4c 100644
--- a/MediaWiki/Sniffs/Commenting/FunctionCommentSniff.php
+++ b/MediaWiki/Sniffs/Commenting/FunctionCommentSniff.php
@@ -530,10 +530,21 @@
$error = 'Missing parameter type';
$phpcsFile->addError( $error, $tag,
'MissingParamType' );
}
+ $isVariadicArg = substr_compare( $var, ',...', -4, 4 )
=== 0;
+ if ( $isVariadicArg ) {
+ // Variadic args sometimes part of the argument
list,
+ // sometimes not. Remove the variadic indicator
from the doc name to
+ // compare it against the real name, when it is
part of the argument list.
+ // If it is not part of the argument list,
+ // the name of the extra paremter will not be
checked.
+ // This does not take care for the php5.6
...$var feature
+ $var = substr( $var, 0, -4 );
+ }
$params[] = [
'tag' => $tag,
'type' => $type,
'var' => $var,
+ 'variadic_arg' => $isVariadicArg,
'comment' => $comment,
'comment_first' => $commentFirst,
'param_space' => $paramSpace,
@@ -612,7 +623,7 @@
$var = $param['var'];
// Check for unneeded punctation
$matches = [];
- if ( preg_match(
'/^(.*?)((?:(?![\[\]_{}])\p{P})+)(?<!,\.\.\.)$/', $var, $matches ) ) {
+ if ( preg_match( '/^(.*?)((?:(?![\[\]_{}])\p{P})+)$/',
$var, $matches ) ) {
$fix = $phpcsFile->addFixableError(
'Param name should not end with
punctuation "%s"',
$param['tag'],
@@ -645,7 +656,7 @@
$error .= 'actual variable name %s';
$phpcsFile->addError( $error,
$param['tag'], $code, $data );
}
- } elseif ( substr( $var, -4 ) !== ',...' ) {
+ } elseif ( !$param['variadic_arg'] ) {
// We must have an extra parameter comment.
$error = 'Superfluous parameter comment';
$phpcsFile->addError( $error, $param['tag'],
'ExtraParamComment' );
@@ -732,6 +743,9 @@
$content = $fixParam['type'];
$content .= str_repeat( ' ', $fixParam['type_space'] );
$content .= $fixParam['var'];
+ if ( $fixParam['variadic_arg'] ) {
+ $content .= ',...';
+ }
$content .= str_repeat( ' ', $fixParam['var_space'] );
$content .= $fixParam['comment_first'];
$phpcsFile->fixer->replaceToken( ( $fixParam['tag'] + 2 ),
$content );
diff --git a/MediaWiki/Tests/files/Commenting/commenting_function.php
b/MediaWiki/Tests/files/Commenting/commenting_function.php
index fc48879..6d9e69e 100644
--- a/MediaWiki/Tests/files/Commenting/commenting_function.php
+++ b/MediaWiki/Tests/files/Commenting/commenting_function.php
@@ -185,6 +185,22 @@
$out = $in * $inOut;
$inOut = $in / $out;
}
+
+ /**
+ * Test with variadic argument in the list
+ * @param string $key A comment
+ * @param string $params,... A comment
+ */
+ public function testVariadicArgInArgList( $key, $params /* ... */ ) {
+ }
+
+ /**
+ * Test with variadic argument not in the list
+ * @param string $key A comment
+ * @param string $params,... A comment
+ */
+ public function testVariadicArgNotInArgList( $key /* ... */ ) {
+ }
}
class TestPassedExamples {
diff --git a/MediaWiki/Tests/files/Commenting/commenting_function.php.expect
b/MediaWiki/Tests/files/Commenting/commenting_function.php.expect
index eff5ead..400f2e7 100644
--- a/MediaWiki/Tests/files/Commenting/commenting_function.php.expect
+++ b/MediaWiki/Tests/files/Commenting/commenting_function.php.expect
@@ -199,10 +199,14 @@
182 | ERROR | [x] Use @param tag in function comment instead of
| | @param[in,out]
| | (MediaWiki.Commenting.FunctionComment.DirectionParam)
- 190 | ERROR | [ ] Only one object structure is allowed in a file
+ 194 | WARNING | [ ] Comments should start on new line.
+ | |
(MediaWiki.WhiteSpace.SpaceBeforeSingleLineComment.NewLineComment)
+ 202 | WARNING | [ ] Comments should start on new line.
+ | |
(MediaWiki.WhiteSpace.SpaceBeforeSingleLineComment.NewLineComment)
+ 206 | ERROR | [ ] Only one object structure is allowed in a file
| | (Generic.Files.OneObjectStructurePerFile.MultipleFound)
- 257 | ERROR | [ ] Only one object structure is allowed in a file
+ 273 | ERROR | [ ] Only one object structure is allowed in a file
| | (Generic.Files.OneObjectStructurePerFile.MultipleFound)
- 263 | ERROR | [ ] Only one object structure is allowed in a file
+ 279 | ERROR | [ ] Only one object structure is allowed in a file
| | (Generic.Files.OneObjectStructurePerFile.MultipleFound)
PHPCBF CAN FIX THE 75 MARKED SNIFF VIOLATIONS AUTOMATICALLY
diff --git a/MediaWiki/Tests/files/Commenting/commenting_function.php.fixed
b/MediaWiki/Tests/files/Commenting/commenting_function.php.fixed
index d7ac85f..8ea0b33 100644
--- a/MediaWiki/Tests/files/Commenting/commenting_function.php.fixed
+++ b/MediaWiki/Tests/files/Commenting/commenting_function.php.fixed
@@ -186,6 +186,22 @@
$out = $in * $inOut;
$inOut = $in / $out;
}
+
+ /**
+ * Test with variadic argument in the list
+ * @param string $key A comment
+ * @param string $params,... A comment
+ */
+ public function testVariadicArgInArgList( $key, $params /* ... */ ) {
+ }
+
+ /**
+ * Test with variadic argument not in the list
+ * @param string $key A comment
+ * @param string $params,... A comment
+ */
+ public function testVariadicArgNotInArgList( $key /* ... */ ) {
+ }
}
class TestPassedExamples {
--
To view, visit https://gerrit.wikimedia.org/r/403940
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I051150bdb281ae22b8a3a2638dc09e60cadf46c9
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/tools/codesniffer
Gerrit-Branch: master
Gerrit-Owner: Umherirrender <umherirrender_de...@web.de>
Gerrit-Reviewer: Addshore <addshorew...@gmail.com>
Gerrit-Reviewer: Legoktm <lego...@member.fsf.org>
Gerrit-Reviewer: Thiemo Kreuz (WMDE) <thiemo.kr...@wikimedia.de>
Gerrit-Reviewer: Umherirrender <umherirrender_de...@web.de>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
