Bleeding edge - check var tag type against native type

ondrejmirtes · ondrejmirtes · commit a69e3bc2f1e8 · 2023-01-13T13:55:37.000+01:00
diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon
@@ -27,3 +27,4 @@ parameters:
 		invarianceComposition: true
 		alwaysTrueAlwaysReported: true
 		disableUnreachableBranchesRules: true
+		varTagType: true
diff --git a/conf/config.level2.neon b/conf/config.level2.neon
@@ -42,7 +42,6 @@ rules:
 	- PHPStan\Rules\PhpDoc\InvalidPhpDocTagValueRule
 	- PHPStan\Rules\PhpDoc\InvalidPHPStanDocTagRule
 	- PHPStan\Rules\PhpDoc\InvalidThrowsPhpDocValueRule
-	- PHPStan\Rules\PhpDoc\WrongVariableNameInVarTagRule
 	- PHPStan\Rules\Properties\AccessPrivatePropertyThroughStaticRule
 
 conditionalTags:
@@ -75,3 +74,9 @@ services:
 			checkMissingVarTagTypehint: %checkMissingVarTagTypehint%
 		tags:
 			- phpstan.rules.rule
+	-
+		class: PHPStan\Rules\PhpDoc\WrongVariableNameInVarTagRule
+		arguments:
+			checkTypeAgainstNativeType: %featureToggles.varTagType%
+		tags:
+			- phpstan.rules.rule
diff --git a/conf/config.neon b/conf/config.neon
@@ -57,6 +57,7 @@ parameters:
 		invarianceComposition: false
 		alwaysTrueAlwaysReported: false
 		disableUnreachableBranchesRules: false
+		varTagType: false
 	fileExtensions:
 		- php
 	checkAdvancedIsset: false
@@ -280,6 +281,7 @@ parametersSchema:
 		invarianceComposition: bool()
 		alwaysTrueAlwaysReported: bool()
 		disableUnreachableBranchesRules: bool()
+		varTagType: bool()
 	])
 	fileExtensions: listOf(string())
 	checkAdvancedIsset: bool()
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
@@ -76,6 +76,11 @@ parameters:
 			count: 1
 			path: src/Command/CommandHelper.php
 
+		-
+			message: "#^PHPDoc tag @var with type array\\<callable\\>\\|false is not subtype of native type array\\.$#"
+			count: 2
+			path: src/Command/CommandHelper.php
+
 		-
 			message: "#^Parameter \\#1 \\$path of function dirname expects string, string\\|false given\\.$#"
 			count: 1
@@ -335,6 +340,11 @@ parameters:
 			count: 1
 			path: src/Reflection/InitializerExprTypeResolver.php
 
+		-
+			message: "#^PHPDoc tag @var with type float\\|int is not subtype of native type int\\.$#"
+			count: 1
+			path: src/Reflection/InitializerExprTypeResolver.php
+
 		-
 			message: "#^Creating new PHPStan\\\\Php8StubsMap is not covered by backward compatibility promise\\. The class might change in a minor PHPStan version\\.$#"
 			count: 1
@@ -403,6 +413,16 @@ parameters:
 			count: 1
 			path: src/Testing/PHPStanTestCase.php
 
+		-
+			message: "#^PHPDoc tag @var with type float\\|int is not subtype of native type int\\.$#"
+			count: 3
+			path: src/Type/Constant/ConstantArrayTypeBuilder.php
+
+		-
+			message: "#^PHPDoc tag @var with type int\\|string is not subtype of native type string\\.$#"
+			count: 1
+			path: src/Type/Constant/ConstantStringType.php
+
 		-
 			message: """
 				#^Call to deprecated method getInstance\\(\\) of class PHPStan\\\\Broker\\\\Broker\\:
diff --git a/src/Command/CommandHelper.php b/src/Command/CommandHelper.php
@@ -2,7 +2,6 @@
 
 namespace PHPStan\Command;
 
-use Closure;
 use Composer\XdebugHandler\XdebugHandler;
 use Nette\DI\Helpers;
 use Nette\DI\InvalidConfigurationException;
@@ -91,7 +90,6 @@ public static function begin(
 	{
 		$stdOutput = new SymfonyOutput($output, new SymfonyStyle(new ErrorsConsoleStyle($input, $output)));
 
-		/** @var Output $errorOutput */
 		$errorOutput = (static function () use ($input, $output): Output {
 			$symfonyErrorOutput = $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output;
 			return new SymfonyOutput($symfonyErrorOutput, new SymfonyStyle(new ErrorsConsoleStyle($input, $symfonyErrorOutput)));
@@ -474,7 +472,6 @@ public static function begin(
 		/** @var StubFilesProvider $stubFilesProvider */
 		$stubFilesProvider = $container->getByType(StubFilesProvider::class);
 
-		/** @var Closure(): array{string[], bool} $filesCallback */
 		$filesCallback = static function () use ($currentWorkingDirectoryFileHelper, $stubFilesProvider, $fileFinder, $pathRoutingParser, $paths): array {
 			$fileFinderResult = $fileFinder->findFiles($paths);
 			$files = $fileFinderResult->getFiles();
diff --git a/src/Command/FixerApplication.php b/src/Command/FixerApplication.php
@@ -120,7 +120,7 @@ public function run(
 		/** @var string $serverAddress */
 		$serverAddress = $server->getAddress();
 
-		/** @var int $serverPort */
+		/** @var int<0, 65535> $serverPort */
 		$serverPort = parse_url($serverAddress, PHP_URL_PORT);
 
 		$reanalyseProcessQueue = new RunnableQueue(
diff --git a/src/File/FuzzyRelativePathHelper.php b/src/File/FuzzyRelativePathHelper.php
@@ -61,13 +61,11 @@ public function __construct(
 		) {
 			[$pathBeginning, $currentWorkingDirectory] = $trimBeginning($currentWorkingDirectory);
 
-			/** @var string[] $pathToTrimArray */
 			$pathToTrimArray = explode($directorySeparator, $currentWorkingDirectory);
 		}
 		foreach ($analysedPaths as $pathNumber => $path) {
 			[$tempPathBeginning, $path] = $trimBeginning($path);
 
-			/** @var string[] $pathArray */
 			$pathArray = explode($directorySeparator, $path);
 			$pathTempParts = [];
 			$pathArraySize = count($pathArray);
diff --git a/src/Parallel/ParallelAnalyser.php b/src/Parallel/ParallelAnalyser.php
@@ -99,7 +99,7 @@ public function analyse(
 		/** @var string $serverAddress */
 		$serverAddress = $server->getAddress();
 
-		/** @var int $serverPort */
+		/** @var int<0, 65535> $serverPort */
 		$serverPort = parse_url($serverAddress, PHP_URL_PORT);
 
 		$internalErrorsCount = 0;
diff --git a/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php b/src/Reflection/BetterReflection/SourceLocator/AutoloadSourceLocator.php
@@ -334,7 +334,6 @@ private function locateClassByName(string $className): ?array
 		$this->silenceErrors();
 
 		try {
-			/** @var array{string[], string, null}|null */
 			$result = FileReadTrapStreamWrapper::withStreamWrapperOverride(
 				static function () use ($className): ?array {
 					$functions = spl_autoload_functions();
diff --git a/src/Reflection/FunctionVariantWithPhpDocs.php b/src/Reflection/FunctionVariantWithPhpDocs.php
@@ -37,7 +37,7 @@ public function __construct(
 	 */
 	public function getParameters(): array
 	{
-		/** @var ParameterReflectionWithPhpDocs[] $parameters */
+		/** @var array<int, ParameterReflectionWithPhpDocs> $parameters */
 		$parameters = parent::getParameters();
 
 		return $parameters;
diff --git a/src/Rules/PhpDoc/WrongVariableNameInVarTagRule.php b/src/Rules/PhpDoc/WrongVariableNameInVarTagRule.php
@@ -15,7 +15,11 @@
 use PHPStan\Rules\RuleError;
 use PHPStan\Rules\RuleErrorBuilder;
 use PHPStan\ShouldNotHappenException;
+use PHPStan\Type\ConstantType;
 use PHPStan\Type\FileTypeMapper;
+use PHPStan\Type\Generic\GenericObjectType;
+use PHPStan\Type\ObjectType;
+use PHPStan\Type\VerbosityLevel;
 use function array_keys;
 use function array_map;
 use function array_merge;
@@ -34,6 +38,7 @@ class WrongVariableNameInVarTagRule implements Rule
 
 	public function __construct(
 		private FileTypeMapper $fileTypeMapper,
+		private bool $checkTypeAgainstNativeType,
 	)
 	{
 	}
@@ -127,12 +132,36 @@ public function processNode(Node $node, Scope $scope): array
 	 * @param VarTag[] $varTags
 	 * @return RuleError[]
 	 */
-	private function processAssign(Scope $scope, Node\Expr $var, array $varTags): array
+	private function processAssign(Scope $scope, Node\Expr $var, Node\Expr $expr, array $varTags): array
 	{
 		$errors = [];
 		$hasMultipleMessage = false;
 		$assignedVariables = $this->getAssignedVariables($var);
-		foreach (array_keys($varTags) as $key) {
+		foreach ($varTags as $key => $varTag) {
+			if ($this->checkTypeAgainstNativeType) {
+				$exprNativeType = $scope->getNativeType($expr);
+				if ($expr instanceof Expr\New_) {
+					if ($exprNativeType instanceof GenericObjectType) {
+						$exprNativeType = new ObjectType($exprNativeType->getClassName());
+					}
+				}
+
+				if ($exprNativeType instanceof ConstantType) {
+					$report = $exprNativeType->isSuperTypeOf($varTag->getType())->no();
+				} else {
+					$report = !$exprNativeType->isSuperTypeOf($varTag->getType())->yes();
+				}
+
+				if ($report) {
+					$verbosity = VerbosityLevel::getRecommendedLevelByType($exprNativeType, $varTag->getType());
+					$errors[] = RuleErrorBuilder::message(sprintf(
+						'PHPDoc tag @var with type %s is not subtype of native type %s.',
+						$varTag->getType()->describe($verbosity),
+						$exprNativeType->describe($verbosity),
+					))->build();
+				}
+			}
+
 			if (is_int($key)) {
 				if (count($varTags) !== 1) {
 					if (!$hasMultipleMessage) {
@@ -289,7 +318,7 @@ private function processStatic(array $vars, array $varTags): array
 	private function processExpression(Scope $scope, Expr $expr, array $varTags): array
 	{
 		if ($expr instanceof Node\Expr\Assign || $expr instanceof Node\Expr\AssignRef) {
-			return $this->processAssign($scope, $expr->var, $varTags);
+			return $this->processAssign($scope, $expr->var, $expr->expr, $varTags);
 		}
 
 		return $this->processStmt($scope, $varTags, null);
diff --git a/src/Type/ClosureTypeFactory.php b/src/Type/ClosureTypeFactory.php
@@ -54,7 +54,7 @@ public function fromClosureObject(Closure $closure): ClosureType
 			throw new ShouldNotHappenException('Closure reflection not found.');
 		}
 
-		/** @var \PHPStan\BetterReflection\Reflection\ReflectionFunction[] $reflections */
+		/** @var list<\PHPStan\BetterReflection\Reflection\ReflectionFunction> $reflections */
 		$reflections = $find($this->reflector, $ast, new IdentifierType(IdentifierType::IDENTIFIER_FUNCTION), $locatedSource);
 		if (count($reflections) !== 1) {
 			throw new ShouldNotHappenException('Closure reflection not found.');
diff --git a/src/Type/Constant/ConstantStringType.php b/src/Type/Constant/ConstantStringType.php
@@ -245,7 +245,6 @@ public function getCallableParametersAcceptors(ClassMemberAccessAnswerer $scope)
 	public function toNumber(): Type
 	{
 		if (is_numeric($this->value)) {
-			/** @var mixed $value */
 			$value = $this->value;
 			$value = +$value;
 			if (is_float($value)) {
diff --git a/tests/PHPStan/Analyser/AnalyserIntegrationTest.php b/tests/PHPStan/Analyser/AnalyserIntegrationTest.php
@@ -545,7 +545,9 @@ public function testBug6375(): void
 	public function testBug6501(): void
 	{
 		$errors = $this->runAnalyse(__DIR__ . '/data/bug-6501.php');
-		$this->assertNoErrors($errors);
+		$this->assertCount(1, $errors);
+		$this->assertSame('PHPDoc tag @var with type R of Exception|stdClass is not subtype of native type stdClass.', $errors[0]->getMessage());
+		$this->assertSame(24, $errors[0]->getLine());
 	}
 
 	public function testBug6114(): void
@@ -591,9 +593,15 @@ public function testBug6649(): void
 	public function testBug6842(): void
 	{
 		$errors = $this->runAnalyse(__DIR__ . '/data/bug-6842.php');
-		$this->assertCount(1, $errors);
-		$this->assertSame('Generator expects value type T of DateTimeInterface, DateTime|DateTimeImmutable|T of DateTimeInterface given.', $errors[0]->getMessage());
-		$this->assertSame(28, $errors[0]->getLine());
+		$this->assertCount(3, $errors);
+		$this->assertSame('PHPDoc tag @var with type Iterator<mixed, T of DateTimeInterface> is not subtype of native type DatePeriod.', $errors[0]->getMessage());
+		$this->assertSame(22, $errors[0]->getLine());
+
+		$this->assertSame('Generator expects value type T of DateTimeInterface, DateTime|DateTimeImmutable|T of DateTimeInterface given.', $errors[1]->getMessage());
+		$this->assertSame(28, $errors[1]->getLine());
+
+		$this->assertSame('Generator expects value type T of DateTimeInterface, DateTime|DateTimeImmutable|T of DateTimeInterface given.', $errors[2]->getMessage());
+		$this->assertSame(54, $errors[2]->getLine());
 	}
 
 	public function testBug6896(): void
diff --git a/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php b/tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php
@@ -65,7 +65,6 @@ public function testClassMethodScope(): void
 
 	private function getFileScope(string $filename): Scope
 	{
-		/** @var Scope $testScope */
 		$testScope = null;
 		$this->processFile($filename, static function (Node $node, Scope $scope) use (&$testScope): void {
 			if (!($node instanceof Exit_)) {
@@ -75,6 +74,7 @@ private function getFileScope(string $filename): Scope
 			$testScope = $scope;
 		});
 
+		/** @var Scope */
 		return $testScope;
 	}
 
diff --git a/tests/PHPStan/Analyser/data/bug-6842.php b/tests/PHPStan/Analyser/data/bug-6842.php
@@ -30,6 +30,32 @@ public function getScheduledEvents(
 		}
 	}
 
+	/**
+	 * @template T of \DateTimeInterface|\DateTime|\DateTimeImmutable
+	 *
+	 * @param T $startDate
+	 * @param T $endDate
+	 *
+	 * @return \Iterator<T>
+	 */
+	public function getScheduledEvents2(
+		\DateTimeInterface $startDate,
+		\DateTimeInterface $endDate
+	): \Iterator {
+		$interval = \DateInterval::createFromDateString('1 day');
+
+		/** @var \DatePeriod<\DateTimeInterface, \DateTimeInterface, null>&iterable<T> $datePeriod */
+		$datePeriod = new \DatePeriod($startDate, $interval, $endDate);
+
+		foreach ($datePeriod as $dateTime) {
+			$scheduledEvent = $this->createScheduledEventFromSchedule($dateTime);
+
+			if ($scheduledEvent >= $startDate) {
+				yield $scheduledEvent;
+			}
+		}
+	}
+
 	/**
 	 * @template T of \DateTimeInterface
 	 *
diff --git a/tests/PHPStan/Rules/PhpDoc/WrongVariableNameInVarTagRuleTest.php b/tests/PHPStan/Rules/PhpDoc/WrongVariableNameInVarTagRuleTest.php
@@ -13,10 +13,13 @@
 class WrongVariableNameInVarTagRuleTest extends RuleTestCase
 {
 
+	private bool $checkTypeAgainstNativeType = false;
+
 	protected function getRule(): Rule
 	{
 		return new WrongVariableNameInVarTagRule(
 			self::getContainer()->getByType(FileTypeMapper::class),
+			$this->checkTypeAgainstNativeType,
 		);
 	}
 
@@ -188,4 +191,23 @@ public function testEnums(): void
 		]);
 	}
 
+	public function testReportWrongType(): void
+	{
+		$this->checkTypeAgainstNativeType = true;
+		$this->analyse([__DIR__ . '/data/wrong-var-native-type.php'], [
+			[
+				'PHPDoc tag @var with type string|null is not subtype of native type string.',
+				14,
+			],
+			[
+				'PHPDoc tag @var with type stdClass is not subtype of native type SplObjectStorage.',
+				23,
+			],
+			[
+				'PHPDoc tag @var with type int is not subtype of native type \'foo\'.',
+				26,
+			],
+		]);
+	}
+
 }
diff --git a/tests/PHPStan/Rules/PhpDoc/data/wrong-var-native-type.php b/tests/PHPStan/Rules/PhpDoc/data/wrong-var-native-type.php
@@ -0,0 +1,34 @@
+<?php
+
+namespace WrongVarNativeType;
+
+class Foo
+{
+
+	public function doFoo(): void
+	{
+		/** @var 'a' $a */
+		$a = $this->doBar();
+
+		/** @var string|null $stringOrNull */
+		$stringOrNull = $this->doBar();
+
+		/** @var string|null $null */
+		$null = null;
+
+		/** @var \SplObjectStorage<\stdClass, array{int, string}> $running */
+		$running = new \SplObjectStorage();
+
+		/** @var \stdClass $running2 */
+		$running2 = new \SplObjectStorage();
+
+		/** @var int $int */
+		$int = 'foo';
+	}
+
+	public function doBar(): string
+	{
+
+	}
+
+}

Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,7 @@ public function __construct(`
`37`	`37`	`*/`
`38`	`38`	`public function getParameters(): array`
`39`	`39`	`{`
`40`		`- /** @var ParameterReflectionWithPhpDocs[] $parameters */`
	`40`	`+ /** @var array<int, ParameterReflectionWithPhpDocs> $parameters */`
`41`	`41`	`$parameters = parent::getParameters();`
`42`	`42`
`43`	`43`	`return $parameters;`