Skip to content

Commit 47a8a32

Browse files
Majkl578kukulich
Majkl578
authored and
kukulich
committed
TypeHintDeclarationSniff: Implemented inspection of useless parameter & return annotations
1 parent d45ad1e commit 47a8a32

7 files changed

+573
-74
lines changed

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ Sniff provides the following settings:
4646
* `enableVoidTypeHint`: enforces to transform `@return void` into native `void` return typehint. It's on by default if you're on PHP 7.1.
4747
* `traversableTypeHints`: enforces which typehints must have specified contained type. E. g. if you set this to `\Doctrine\Common\Collections\Collection`, then `\Doctrine\Common\Collections\Collection` must always be supplied with the contained type: `\Doctrine\Common\Collections\Collection|Foo[]`.
4848
* `usefulAnnotations`: prevents reporting and removing useless phpDocs if they contain an additional configured annotation like `@dataProvider`.
49+
* `enableEachParameterAndReturnInspection`: enables inspection and fixing of `@param` and `@return` annotations separately. Useful when you only want to document parameters or return values that could not be expressed natively (i.e. member types of `array` or `Traversable`).
4950

5051
#### SlevomatCodingStandard.Exceptions.ReferenceThrowableOnly 🔧
5152

‎SlevomatCodingStandard/Sniffs/TypeHints/TypeHintDeclarationSniff.php‎

Lines changed: 225 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22

33
namespace SlevomatCodingStandard\Sniffs\TypeHints;
44

5+
use SlevomatCodingStandard\Helpers\Annotation;
56
use SlevomatCodingStandard\Helpers\AnnotationHelper;
67
use SlevomatCodingStandard\Helpers\DocCommentHelper;
78
use SlevomatCodingStandard\Helpers\FunctionHelper;
89
use SlevomatCodingStandard\Helpers\NamespaceHelper;
910
use SlevomatCodingStandard\Helpers\PropertyHelper;
11+
use SlevomatCodingStandard\Helpers\ReturnTypeHint;
1012
use SlevomatCodingStandard\Helpers\SniffSettingsHelper;
1113
use SlevomatCodingStandard\Helpers\SuppressHelper;
1214
use SlevomatCodingStandard\Helpers\TokenHelper;
@@ -29,6 +31,10 @@ class TypeHintDeclarationSniff implements \PHP_CodeSniffer_Sniff
2931

3032
const CODE_MISSING_TRAVERSABLE_RETURN_TYPE_HINT_SPECIFICATION = 'MissingTraversableReturnTypeHintSpecification';
3133

34+
const CODE_USELESS_PARAMETER_ANNOTATION = 'UselessParameterAnnotation';
35+
36+
const CODE_USELESS_RETURN_ANNOTATION = 'UselessReturnAnnotation';
37+
3238
const CODE_USELESS_DOC_COMMENT = 'UselessDocComment';
3339

3440
/** @var bool */
@@ -37,6 +43,9 @@ class TypeHintDeclarationSniff implements \PHP_CodeSniffer_Sniff
3743
/** @var bool */
3844
public $enableVoidTypeHint = PHP_VERSION_ID >= 70100;
3945

46+
/** @var bool */
47+
public $enableEachParameterAndReturnInspection = false;
48+
4049
/** @var string[] */
4150
public $traversableTypeHints = [];
4251

@@ -451,112 +460,155 @@ private function checkReturnTypeHints(\PHP_CodeSniffer_File $phpcsFile, int $fun
451460

452461
private function checkUselessDocComment(\PHP_CodeSniffer_File $phpcsFile, int $functionPointer)
453462
{
454-
if (SuppressHelper::isSniffSuppressed($phpcsFile, $functionPointer, $this->getSniffName(self::CODE_USELESS_DOC_COMMENT))) {
463+
$docCommentSniffSuppressed = SuppressHelper::isSniffSuppressed($phpcsFile, $functionPointer, $this->getSniffName(self::CODE_USELESS_DOC_COMMENT));
464+
$returnSniffSuppressed = SuppressHelper::isSniffSuppressed($phpcsFile, $functionPointer, $this->getSniffName(self::CODE_USELESS_RETURN_ANNOTATION));
465+
$parameterSniffSuppressed = SuppressHelper::isSniffSuppressed($phpcsFile, $functionPointer, $this->getSniffName(self::CODE_USELESS_PARAMETER_ANNOTATION));
466+
467+
if ($docCommentSniffSuppressed && $returnSniffSuppressed && $parameterSniffSuppressed) {
455468
return;
456469
}
457470

458471
if (!DocCommentHelper::hasDocComment($phpcsFile, $functionPointer)) {
459472
return;
460473
}
461474

462-
if (DocCommentHelper::hasDocCommentDescription($phpcsFile, $functionPointer)) {
463-
return;
464-
}
475+
$containsUsefulInformation = DocCommentHelper::hasDocCommentDescription($phpcsFile, $functionPointer);
465476

466477
foreach (FunctionHelper::getParametersAnnotations($phpcsFile, $functionPointer) as $parameterAnnotation) {
467478
if ($parameterAnnotation->getContent() !== null && preg_match('~^\\S+\\s+(?:(?:\.{3}\\s*)?\$\\S+\\s+)?[^$]~', $parameterAnnotation->getContent())) {
468-
return;
479+
$containsUsefulInformation = true;
480+
break;
469481
}
470482
}
471483

472-
$isAbstract = FunctionHelper::isAbstract($phpcsFile, $functionPointer);
484+
$returnTypeHint = FunctionHelper::findReturnTypeHint($phpcsFile, $functionPointer);
485+
$returnAnnotation = FunctionHelper::findReturnAnnotation($phpcsFile, $functionPointer);
486+
$isReturnAnnotationUseless = $this->isReturnAnnotationUseless($phpcsFile, $functionPointer, $returnTypeHint, $returnAnnotation);
473487

474-
$typeHintEqualsAnnotation = function (string $typeHint, string $typeHintInAnnotation) use ($phpcsFile, $functionPointer): bool {
475-
return TypeHintHelper::isSimpleTypeHint($typeHint) || TypeHintHelper::getFullyQualifiedTypeHint($phpcsFile, $functionPointer, $typeHint) === TypeHintHelper::getFullyQualifiedTypeHint($phpcsFile, $functionPointer, $typeHintInAnnotation);
476-
};
488+
$parameterTypeHints = FunctionHelper::getParametersTypeHints($phpcsFile, $functionPointer);
489+
$parametersAnnotationTypeHints = $this->getFunctionParameterTypeHintsDefinitions($phpcsFile, $functionPointer);
490+
$uselessParameterNames = $this->getUselessParameterNames($phpcsFile, $functionPointer, $parameterTypeHints, $parametersAnnotationTypeHints);
477491

478-
if ($isAbstract || FunctionHelper::returnsValue($phpcsFile, $functionPointer)) {
479-
$returnTypeHint = FunctionHelper::findReturnTypeHint($phpcsFile, $functionPointer);
480-
$returnAnnotation = FunctionHelper::findReturnAnnotation($phpcsFile, $functionPointer);
492+
foreach (AnnotationHelper::getAnnotations($phpcsFile, $functionPointer) as list($annotation)) {
493+
if ($annotation->getName() === SuppressHelper::ANNOTATION) {
494+
$containsUsefulInformation = true;
495+
break;
496+
}
481497

482-
if ($returnAnnotation !== null) {
483-
if ($returnTypeHint === null) {
484-
return;
498+
foreach ($this->getNormalizedUsefulAnnotations() as $usefulAnnotation) {
499+
if ($annotation->getName() === $usefulAnnotation) {
500+
$containsUsefulInformation = true;
501+
break;
485502
}
486503

487-
if ($this->isTraversableTypeHint(TypeHintHelper::getFullyQualifiedTypeHint($phpcsFile, $functionPointer, $returnTypeHint->getTypeHint()))) {
488-
return;
504+
if (substr($usefulAnnotation, -1) === '\\' && strpos($annotation->getName(), $usefulAnnotation) === 0) {
505+
$containsUsefulInformation = true;
506+
break;
489507
}
508+
}
509+
}
490510

491-
if ($returnAnnotation !== null) {
492-
if (preg_match('~^\\S+\\s+\\S+~', $returnAnnotation->getContent())) {
493-
return;
494-
}
511+
$isWholeDocCommentUseless = !$containsUsefulInformation
512+
&& ($returnAnnotation === null || $isReturnAnnotationUseless)
513+
&& count($uselessParameterNames) === count($parametersAnnotationTypeHints);
495514

496-
$returnTypeHintsDefinition = preg_split('~\\s+~', $returnAnnotation->getContent())[0];
497-
if ($this->definitionContainsStaticOrThisTypeHint($returnTypeHintsDefinition)) {
498-
return;
499-
} elseif ($this->enableNullableTypeHints && $this->definitionContainsJustTwoTypeHints($returnTypeHintsDefinition) && $this->definitionContainsNullTypeHint($returnTypeHintsDefinition)) {
500-
$returnTypeHintDefinitionParts = explode('|', $returnTypeHintsDefinition);
501-
$returnTypeHintInAnnotation = strtolower($returnTypeHintDefinitionParts[0]) === 'null' ? $returnTypeHintDefinitionParts[1] : $returnTypeHintDefinitionParts[0];
502-
if (!$typeHintEqualsAnnotation($returnTypeHint->getTypeHint(), $returnTypeHintInAnnotation)) {
503-
return;
515+
if ($this->enableEachParameterAndReturnInspection && (!$isWholeDocCommentUseless || $docCommentSniffSuppressed)) {
516+
if ($returnAnnotation !== null && $isReturnAnnotationUseless && !$returnSniffSuppressed) {
517+
$fix = $phpcsFile->addFixableError(
518+
sprintf(
519+
'%s %s() has useless @return annotation.',
520+
$this->getFunctionTypeLabel($phpcsFile, $functionPointer),
521+
FunctionHelper::getFullyQualifiedName($phpcsFile, $functionPointer)
522+
),
523+
$functionPointer,
524+
self::CODE_USELESS_RETURN_ANNOTATION
525+
);
526+
if ($fix) {
527+
$tokens = $phpcsFile->getTokens();
528+
$docCommentOpenPointer = DocCommentHelper::findDocCommentOpenToken($phpcsFile, $functionPointer);
529+
$docCommentClosePointer = $tokens[$docCommentOpenPointer]['comment_closer'];
530+
531+
for ($i = $docCommentOpenPointer + 1; $i < $docCommentClosePointer; $i++) {
532+
if ($tokens[$i]['code'] !== T_DOC_COMMENT_TAG) {
533+
continue;
504534
}
505-
} elseif (!$this->definitionContainsOneTypeHint($returnTypeHintsDefinition)) {
506-
return;
507-
} elseif (!$typeHintEqualsAnnotation($returnTypeHint->getTypeHint(), $returnTypeHintsDefinition)) {
508-
return;
509-
}
510-
}
511-
}
512-
}
513535

514-
$parametersTypeHintsDefinitions = $this->getFunctionParameterTypeHintsDefinitions($phpcsFile, $functionPointer);
515-
foreach (FunctionHelper::getParametersTypeHints($phpcsFile, $functionPointer) as $parameterName => $parameterTypeHint) {
516-
if ($parameterTypeHint === null) {
517-
if (array_key_exists($parameterName, $parametersTypeHintsDefinitions)) {
518-
return;
519-
} else {
520-
continue;
521-
}
522-
}
536+
if ($tokens[$i]['content'] !== '@return') {
537+
continue;
538+
}
523539

524-
if ($this->isTraversableTypeHint(TypeHintHelper::getFullyQualifiedTypeHint($phpcsFile, $functionPointer, $parameterTypeHint->getTypeHint()))) {
525-
return;
526-
}
540+
$changeStart = $phpcsFile->findPrevious([T_DOC_COMMENT_STAR], $i - 1, $docCommentOpenPointer);
541+
$changeEnd = $phpcsFile->findNext([T_DOC_COMMENT_CLOSE_TAG, T_DOC_COMMENT_STAR], $i - 1, $docCommentClosePointer + 1) - 1;
542+
$phpcsFile->fixer->beginChangeset();
543+
for ($j = $changeStart; $j <= $changeEnd; $j++) {
544+
$phpcsFile->fixer->replaceToken($j, '');
545+
}
546+
$phpcsFile->fixer->endChangeset();
527547

528-
if (array_key_exists($parameterName, $parametersTypeHintsDefinitions)) {
529-
$parameterTypeHintDefinition = $parametersTypeHintsDefinitions[$parameterName];
530-
if ($this->definitionContainsStaticOrThisTypeHint($parameterTypeHintDefinition)) {
531-
return;
532-
} elseif ($this->definitionContainsJustTwoTypeHints($parameterTypeHintDefinition) && $this->definitionContainsNullTypeHint($parameterTypeHintDefinition)) {
533-
$parameterTypeHintDefinitionParts = explode('|', $parameterTypeHintDefinition);
534-
$parameterTypeHintInAnnotation = strtolower($parameterTypeHintDefinitionParts[0]) === 'null' ? $parameterTypeHintDefinitionParts[1] : $parameterTypeHintDefinitionParts[0];
535-
if (!$typeHintEqualsAnnotation($parameterTypeHint->getTypeHint(), $parameterTypeHintInAnnotation)) {
536-
return;
548+
break;
537549
}
538-
} elseif (!$this->definitionContainsOneTypeHint($parameterTypeHintDefinition)) {
539-
return;
540-
} elseif (!$typeHintEqualsAnnotation($parameterTypeHint->getTypeHint(), $parameterTypeHintDefinition)) {
541-
return;
542550
}
543551
}
544-
}
545552

546-
foreach (AnnotationHelper::getAnnotations($phpcsFile, $functionPointer) as list($annotation)) {
547-
if ($annotation->getName() === SuppressHelper::ANNOTATION) {
548-
return;
553+
if (!$parameterSniffSuppressed) {
554+
foreach ($uselessParameterNames as $uselessParameterName) {
555+
$fix = $phpcsFile->addFixableError(
556+
sprintf(
557+
'%s %s() has useless @param annotation for parameter %s.',
558+
$this->getFunctionTypeLabel($phpcsFile, $functionPointer),
559+
FunctionHelper::getFullyQualifiedName($phpcsFile, $functionPointer),
560+
$uselessParameterName
561+
),
562+
$functionPointer,
563+
self::CODE_USELESS_PARAMETER_ANNOTATION
564+
);
565+
if ($fix) {
566+
$tokens = $phpcsFile->getTokens();
567+
$docCommentOpenPointer = DocCommentHelper::findDocCommentOpenToken($phpcsFile, $functionPointer);
568+
$docCommentClosePointer = $tokens[$docCommentOpenPointer]['comment_closer'];
569+
570+
for ($i = $docCommentOpenPointer + 1; $i < $docCommentClosePointer; $i++) {
571+
if ($tokens[$i]['code'] !== T_DOC_COMMENT_TAG) {
572+
continue;
573+
}
574+
575+
if ($tokens[$i]['content'] !== '@param') {
576+
continue;
577+
}
578+
579+
$parameterInformationPointer = $phpcsFile->findNext([T_DOC_COMMENT_WHITESPACE], $i + 1, $docCommentClosePointer + 1, true);
580+
581+
if ($parameterInformationPointer === false || $tokens[$parameterInformationPointer]['code'] !== T_DOC_COMMENT_STRING) {
582+
continue;
583+
}
584+
585+
if (!preg_match('~\S+\s+(\$\S+)~', $tokens[$parameterInformationPointer]['content'], $match)) {
586+
continue;
587+
}
588+
589+
if (!in_array($match[1], $uselessParameterNames, true)) {
590+
continue;
591+
}
592+
593+
$changeStart = $phpcsFile->findPrevious([T_DOC_COMMENT_STAR], $i - 1);
594+
$changeEnd = $phpcsFile->findNext([T_DOC_COMMENT_CLOSE_TAG, T_DOC_COMMENT_STAR], $i - 1) - 1;
595+
$phpcsFile->fixer->beginChangeset();
596+
for ($j = $changeStart; $j <= $changeEnd; $j++) {
597+
$phpcsFile->fixer->replaceToken($j, '');
598+
}
599+
$phpcsFile->fixer->endChangeset();
600+
601+
break;
602+
}
603+
}
604+
}
549605
}
550606

551-
foreach ($this->getNormalizedUsefulAnnotations() as $usefulAnnotation) {
552-
if ($annotation->getName() === $usefulAnnotation) {
553-
return;
554-
}
607+
return;
608+
}
555609

556-
if (substr($usefulAnnotation, -1) === '\\' && strpos($annotation->getName(), $usefulAnnotation) === 0) {
557-
return;
558-
}
559-
}
610+
if (!$isWholeDocCommentUseless || $docCommentSniffSuppressed) {
611+
return;
560612
}
561613

562614
$fix = $phpcsFile->addFixableError(
@@ -741,4 +793,103 @@ private function getFunctionParameterTypeHintsDefinitions(\PHP_CodeSniffer_File
741793
return $parametersTypeHintsDefinitions;
742794
}
743795

796+
private function typeHintEqualsAnnotation(\PHP_CodeSniffer_File $phpcsFile, int $functionPointer, string $typeHint, string $typeHintInAnnotation): bool
797+
{
798+
return TypeHintHelper::isSimpleTypeHint($typeHint)
799+
|| TypeHintHelper::getFullyQualifiedTypeHint($phpcsFile, $functionPointer, $typeHint) === TypeHintHelper::getFullyQualifiedTypeHint($phpcsFile, $functionPointer, $typeHintInAnnotation);
800+
}
801+
802+
private function isReturnAnnotationUseless(\PHP_CodeSniffer_File $phpcsFile, int $functionPointer, ReturnTypeHint $returnTypeHint = null, Annotation $returnAnnotation = null): bool
803+
{
804+
if (!FunctionHelper::isAbstract($phpcsFile, $functionPointer) && !FunctionHelper::returnsValue($phpcsFile, $functionPointer) && $returnTypeHint === null) {
805+
return true;
806+
}
807+
808+
if ($returnTypeHint === null || $returnAnnotation === null || $returnAnnotation->getContent() === null) {
809+
return false;
810+
}
811+
812+
if (preg_match('~^\\S+\\s+\\S+~', $returnAnnotation->getContent())) {
813+
return false;
814+
}
815+
816+
$returnTypeHintDefinition = preg_split('~\\s+~', $returnAnnotation->getContent())[0];
817+
818+
if ($this->isTraversableTypeHint(TypeHintHelper::getFullyQualifiedTypeHint($phpcsFile, $functionPointer, $returnTypeHint->getTypeHint()))) {
819+
return false;
820+
}
821+
822+
if ($this->definitionContainsStaticOrThisTypeHint($returnTypeHintDefinition)) {
823+
return false;
824+
}
825+
826+
if ($this->enableNullableTypeHints && $this->isTypeHintDefinitionCompoundOfNull($returnTypeHintDefinition)) {
827+
return $this->typeHintEqualsAnnotation($phpcsFile, $functionPointer, $returnTypeHint->getTypeHint(), $this->getTypeFromNullableTypeHintDefinition($returnTypeHintDefinition));
828+
}
829+
830+
if (!$this->definitionContainsOneTypeHint($returnTypeHintDefinition)) {
831+
return false;
832+
}
833+
834+
if (!$this->typeHintEqualsAnnotation($phpcsFile, $functionPointer, $returnTypeHint->getTypeHint(), $returnTypeHintDefinition)) {
835+
return false;
836+
}
837+
838+
return true;
839+
}
840+
841+
private function isTypeHintDefinitionCompoundOfNull(string $definition): bool
842+
{
843+
return $this->definitionContainsJustTwoTypeHints($definition) && $this->definitionContainsNullTypeHint($definition);
844+
}
845+
846+
private function getTypeFromNullableTypeHintDefinition(string $definition): string
847+
{
848+
$defitionParts = explode('|', $definition);
849+
return strtolower($defitionParts[0]) === 'null' ? $defitionParts[1] : $defitionParts[0];
850+
}
851+
852+
/**
853+
* @param \PHP_CodeSniffer_File $phpcsFile
854+
* @param int $functionPointer
855+
* @param \SlevomatCodingStandard\Helpers\ParameterTypeHint[]|null[] $functionTypeHints
856+
* @param string[]|null[] $parametersTypeHintsDefinitions
857+
* @return string[] names of parameters with useless annotation hint
858+
*/
859+
private function getUselessParameterNames(\PHP_CodeSniffer_File $phpcsFile, int $functionPointer, array $functionTypeHints, array $parametersTypeHintsDefinitions): array
860+
{
861+
$uselessParameterNames = [];
862+
863+
foreach ($functionTypeHints as $parameterName => $parameterTypeHint) {
864+
if ($parameterTypeHint === null) {
865+
continue;
866+
}
867+
868+
if (!array_key_exists($parameterName, $parametersTypeHintsDefinitions)) {
869+
continue;
870+
}
871+
872+
if ($this->isTraversableTypeHint(TypeHintHelper::getFullyQualifiedTypeHint($phpcsFile, $functionPointer, $parameterTypeHint->getTypeHint()))) {
873+
continue;
874+
}
875+
876+
$parameterTypeHintDefinition = $parametersTypeHintsDefinitions[$parameterName];
877+
if ($this->definitionContainsStaticOrThisTypeHint($parameterTypeHintDefinition)) {
878+
continue;
879+
} elseif ($this->isTypeHintDefinitionCompoundOfNull($parameterTypeHintDefinition)) {
880+
if (!$this->typeHintEqualsAnnotation($phpcsFile, $functionPointer, $parameterTypeHint->getTypeHint(), $this->getTypeFromNullableTypeHintDefinition($parameterTypeHintDefinition))) {
881+
continue;
882+
}
883+
} elseif (!$this->definitionContainsOneTypeHint($parameterTypeHintDefinition)) {
884+
continue;
885+
} elseif (!$this->typeHintEqualsAnnotation($phpcsFile, $functionPointer, $parameterTypeHint->getTypeHint(), $parameterTypeHintDefinition)) {
886+
continue;
887+
}
888+
889+
$uselessParameterNames[] = $parameterName;
890+
}
891+
892+
return $uselessParameterNames;
893+
}
894+
744895
}

0 commit comments

Comments
 (0)