PropertyTypeHintSniff: Improved support for union types

slevomat · Jan 31, 2020 · fd919f0 · fd919f0
1 parent b313242
commit fd919f0
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 77 deletions.
diff --git a/SlevomatCodingStandard/Sniffs/TypeHints/PropertyTypeHintSniff.php b/SlevomatCodingStandard/Sniffs/TypeHints/PropertyTypeHintSniff.php
@@ -29,6 +29,7 @@
 use function count;
 use function in_array;
 use function sprintf;
+use function strtolower;
 use const PHP_VERSION_ID;
 use const T_COMMA;
 use const T_DOC_COMMENT_CLOSE_TAG;
@@ -139,77 +140,55 @@ private function checkTypeHint(
 
 		$typeNode = $propertyAnnotation->getType();
 
+		$typeHints = [];
+
 		if (AnnotationTypeHelper::containsOneType($typeNode)) {
-			/** @var ArrayTypeNode|ArrayShapeNode|GenericTypeNode|IdentifierTypeNode|ThisTypeNode $typeNode */
+			/** @var ArrayTypeNode|ArrayShapeNode|IdentifierTypeNode|ThisTypeNode|GenericTypeNode|CallableTypeNode $typeNode */
 			$typeNode = $typeNode;
-			$possibleTypeHint = $typeNode instanceof ArrayTypeNode || $typeNode instanceof ArrayShapeNode
-				? 'array'
-				: AnnotationTypeHelper::getTypeHintFromOneType($typeNode);
-			$nullableTypeHint = false;
-
-		} else {
-			$possibleTypeHint = null;
-			$nullableTypeHint = false;
+			$typeHints[] = AnnotationTypeHelper::getTypeHintFromOneType($typeNode);
 
-			if ($typeNode instanceof UnionTypeNode && !AnnotationTypeHelper::containsJustTwoTypes($typeNode)) {
-				$typeHints = [];
-				foreach ($typeNode->types as $innerTypeNode) {
-					if (!($innerTypeNode instanceof CallableTypeNode
-						|| $innerTypeNode instanceof GenericTypeNode
-						|| $innerTypeNode instanceof IdentifierTypeNode
-						|| $innerTypeNode instanceof ThisTypeNode)
-					) {
-						return;
-					}
-
-					$typeHints[] = AnnotationTypeHelper::getTypeHintFromOneType($innerTypeNode);
+		} elseif ($typeNode instanceof UnionTypeNode || $typeNode instanceof IntersectionTypeNode) {
+			foreach ($typeNode->types as $innerTypeNode) {
+				if (!AnnotationTypeHelper::containsOneType($innerTypeNode)) {
+					return;
 				}
 
-				$typeHints = array_values(array_unique($typeHints));
+				/** @var ArrayTypeNode|ArrayShapeNode|IdentifierTypeNode|ThisTypeNode|GenericTypeNode|CallableTypeNode $innerTypeNode */
+				$innerTypeNode = $innerTypeNode;
 
-				if (count($typeHints) === 1) {
-					$possibleTypeHint = $typeHints[0];
-					$nullableTypeHint = false;
-				} elseif (count($typeHints) === 2 && ($typeHints[0] === 'null' || $typeHints[1] === 'null')) {
-					$possibleTypeHint = $typeHints[0] === 'null' ? $typeHints[1] : $typeHints[0];
-					$nullableTypeHint = true;
-				} else {
-					return;
-				}
+				$typeHints[] = AnnotationTypeHelper::getTypeHintFromOneType($innerTypeNode);
 			}
+		} else {
+			return;
+		}
 
-			if ($possibleTypeHint === null) {
+		$typeHints = array_values(array_unique($typeHints));
+
+		if (count($typeHints) === 1) {
+			$possibleTypeHint = $typeHints[0];
+			$nullableTypeHint = false;
+		} elseif (count($typeHints) === 2) {
+			if (strtolower($typeHints[0]) === 'null' || strtolower($typeHints[1]) === 'null') {
+				$possibleTypeHint = strtolower($typeHints[0]) === 'null' ? $typeHints[1] : $typeHints[0];
+				$nullableTypeHint = true;
+			} else {
 				/** @var UnionTypeNode|IntersectionTypeNode $typeNode */
 				$typeNode = $typeNode;
 
-				if (
-					!AnnotationTypeHelper::containsNullType($typeNode)
-					&& !AnnotationTypeHelper::containsTraversableType($typeNode, $phpcsFile, $propertyPointer, $this->getTraversableTypeHints())
-				) {
+				$itemsSpecificationTypeHint = AnnotationTypeHelper::getItemsSpecificationTypeFromType($typeNode, $this->getTraversableTypeHints());
+				if (!$itemsSpecificationTypeHint instanceof ArrayTypeNode) {
 					return;
 				}
 
-				if (AnnotationTypeHelper::containsNullType($typeNode)) {
-					/** @var ArrayTypeNode|ArrayShapeNode|IdentifierTypeNode|ThisTypeNode|GenericTypeNode $notNullTypeHintNode */
-					$notNullTypeHintNode = AnnotationTypeHelper::getTypeFromNullableType($typeNode);
-					$possibleTypeHint = $notNullTypeHintNode instanceof ArrayTypeNode || $notNullTypeHintNode instanceof ArrayShapeNode
-						? 'array'
-						: AnnotationTypeHelper::getTypeHintFromOneType($notNullTypeHintNode);
-					$nullableTypeHint = true;
-				} else {
-					$itemsSpecificationTypeHint = AnnotationTypeHelper::getItemsSpecificationTypeFromType($typeNode, $this->getTraversableTypeHints());
-					if (!$itemsSpecificationTypeHint instanceof ArrayTypeNode) {
-						return;
-					}
-
-					$possibleTypeHint = AnnotationTypeHelper::getTraversableTypeHintFromType($typeNode, $this->getTraversableTypeHints());
-					$nullableTypeHint = false;
-
-					if (!TypeHintHelper::isTraversableType(TypeHintHelper::getFullyQualifiedTypeHint($phpcsFile, $propertyPointer, $possibleTypeHint), $this->getTraversableTypeHints())) {
-						return;
-					}
+				$possibleTypeHint = AnnotationTypeHelper::getTraversableTypeHintFromType($typeNode, $this->getTraversableTypeHints());
+				$nullableTypeHint = false;
+
+				if (!TypeHintHelper::isTraversableType(TypeHintHelper::getFullyQualifiedTypeHint($phpcsFile, $propertyPointer, $possibleTypeHint), $this->getTraversableTypeHints())) {
+					return;
 				}
 			}
+		} else {
+			return;
 		}
 
 		if ($possibleTypeHint === 'callable') {

diff --git a/tests/Sniffs/TypeHints/PropertyTypeHintSniffTest.php b/tests/Sniffs/TypeHints/PropertyTypeHintSniffTest.php
@@ -49,7 +49,7 @@ public function testEnabledNativeErrors(): void
 			'traversableTypeHints' => ['Traversable'],
 		]);
 
-		self::assertSame(28, $report->getErrorCount());
+		self::assertSame(29, $report->getErrorCount());
 
 		self::assertSniffError($report, 6, PropertyTypeHintSniff::CODE_MISSING_ANY_TYPE_HINT);
 		self::assertSniffError($report, 11, PropertyTypeHintSniff::CODE_MISSING_NATIVE_TYPE_HINT);
@@ -76,9 +76,10 @@ public function testEnabledNativeErrors(): void
 		self::assertSniffError($report, 94, PropertyTypeHintSniff::CODE_USELESS_ANNOTATION);
 		self::assertSniffError($report, 99, PropertyTypeHintSniff::CODE_MISSING_NATIVE_TYPE_HINT);
 		self::assertSniffError($report, 102, PropertyTypeHintSniff::CODE_MISSING_NATIVE_TYPE_HINT);
-		self::assertSniffError($report, 107, PropertyTypeHintSniff::CODE_MISSING_NATIVE_TYPE_HINT);
-		self::assertSniffError($report, 112, PropertyTypeHintSniff::CODE_MISSING_NATIVE_TYPE_HINT);
-		self::assertSniffError($report, 117, PropertyTypeHintSniff::CODE_MISSING_NATIVE_TYPE_HINT);
+		self::assertSniffError($report, 105, PropertyTypeHintSniff::CODE_MISSING_NATIVE_TYPE_HINT);
+		self::assertSniffError($report, 108, PropertyTypeHintSniff::CODE_MISSING_NATIVE_TYPE_HINT);
+		self::assertSniffError($report, 111, PropertyTypeHintSniff::CODE_MISSING_NATIVE_TYPE_HINT);
+		self::assertSniffError($report, 114, PropertyTypeHintSniff::CODE_MISSING_NATIVE_TYPE_HINT);
 
 		self::assertAllFixedInFile($report);
 	}

diff --git a/tests/Sniffs/TypeHints/data/propertyTypeHintEnabledNativeErrors.fixed.php b/tests/Sniffs/TypeHints/data/propertyTypeHintEnabledNativeErrors.fixed.php
@@ -88,19 +88,16 @@ class Whatever
 
 	public static int $staticSecond;
 
-	/**
-	 * @var array<string>|array<int>|array<bool>
-	 */
+	/** @var array<string>|array<int> */
 	public array $unionWithSameBase;
 
-	/**
-	 * @var array<int>|bool[]
-	 */
+	/** @var array<string>|array<int>|array<bool> */
+	public array $unionWithSameBaseAndMoreTypes;
+
+	/** @var array<int>|bool[] */
 	public array $unionWithSameBaseToo;
 
-	/**
-	 * @var array<string>|array<int>|array<bool>|null
-	 */
+	/** @var array<string>|array<int>|array<bool>|null */
 	public ?array $unionWithSameNullableBase = null;
 
 }
diff --git a/tests/Sniffs/TypeHints/data/propertyTypeHintEnabledNativeErrors.php b/tests/Sniffs/TypeHints/data/propertyTypeHintEnabledNativeErrors.php
@@ -101,19 +101,16 @@ class Whatever
 	/** @var int */
 	public static $staticSecond;
 
-	/**
-	 * @var array<string>|array<int>|array<bool>
-	 */
+	/** @var array<string>|array<int> */
 	public $unionWithSameBase;
 
-	/**
-	 * @var array<int>|bool[]
-	 */
+	/** @var array<string>|array<int>|array<bool> */
+	public $unionWithSameBaseAndMoreTypes;
+
+	/** @var array<int>|bool[] */
 	public $unionWithSameBaseToo;
 
-	/**
-	 * @var array<string>|array<int>|array<bool>|null
-	 */
+	/** @var array<string>|array<int>|array<bool>|null */
 	public $unionWithSameNullableBase;
 
 }
diff --git a/tests/Sniffs/TypeHints/data/propertyTypeHintEnabledNativeNoErrors.php b/tests/Sniffs/TypeHints/data/propertyTypeHintEnabledNativeNoErrors.php
@@ -118,4 +118,9 @@ class Whatever
 	 */
 	public $unionWithDifferentNullableBase;
 
+	/**
+	 * @var ?int
+	 */
+	public $nullableNotSupported;
+
 }