From a69e3bc2f1e87f6da1e65d7935f1cc36bd5c42fe Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 13 Jan 2023 12:59:36 +0100 Subject: [PATCH] Bleeding edge - check var tag type against native type --- conf/bleedingEdge.neon | 1 + conf/config.level2.neon | 7 +++- conf/config.neon | 2 ++ phpstan-baseline.neon | 20 +++++++++++ src/Command/CommandHelper.php | 3 -- src/Command/FixerApplication.php | 2 +- src/File/FuzzyRelativePathHelper.php | 2 -- src/Parallel/ParallelAnalyser.php | 2 +- .../SourceLocator/AutoloadSourceLocator.php | 1 - src/Reflection/FunctionVariantWithPhpDocs.php | 2 +- .../PhpDoc/WrongVariableNameInVarTagRule.php | 35 +++++++++++++++++-- src/Type/ClosureTypeFactory.php | 2 +- src/Type/Constant/ConstantStringType.php | 1 - .../Analyser/AnalyserIntegrationTest.php | 16 ++++++--- .../Analyser/LegacyNodeScopeResolverTest.php | 2 +- tests/PHPStan/Analyser/data/bug-6842.php | 26 ++++++++++++++ .../WrongVariableNameInVarTagRuleTest.php | 22 ++++++++++++ .../PhpDoc/data/wrong-var-native-type.php | 34 ++++++++++++++++++ 18 files changed, 160 insertions(+), 20 deletions(-) create mode 100644 tests/PHPStan/Rules/PhpDoc/data/wrong-var-native-type.php diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index b7f85db78c..a45580e3e7 100644 --- 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 index c30d85a244..b15754dac3 100644 --- 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 index d820e422f2..8fb05a1a26 100644 --- 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 index ac4ac07798..f18affc6e1 100644 --- 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\\\\|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 index ce4ac9f90d..81d0b8092e 100644 --- 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 index d96f93d4bb..6b23b5931c 100644 --- 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 index d380fb7f08..d8bb21ba88 100644 --- 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 index 872e22d5b5..501d1683fc 100644 --- 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 index fd213829f4..fd022aaff8 100644 --- 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 index aae15cb864..f26654b917 100644 --- 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 $parameters */ $parameters = parent::getParameters(); return $parameters; diff --git a/src/Rules/PhpDoc/WrongVariableNameInVarTagRule.php b/src/Rules/PhpDoc/WrongVariableNameInVarTagRule.php index c1f3dac121..668c656ac6 100644 --- 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 index 52d43410de..07a395a5fe 100644 --- 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 index 82ca4cd69b..bb77bfbdd3 100644 --- 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 index 3f9e5ad2c2..3a5fa5bf25 100644 --- 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 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 index d2ecfb4f3f..433acb53e6 100644 --- 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 index ee3b1232c4..7565f71c85 100644 --- 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 + */ + public function getScheduledEvents2( + \DateTimeInterface $startDate, + \DateTimeInterface $endDate + ): \Iterator { + $interval = \DateInterval::createFromDateString('1 day'); + + /** @var \DatePeriod<\DateTimeInterface, \DateTimeInterface, null>&iterable $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 index 63164b9bda..b940bae5c2 100644 --- 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 new file mode 100644 index 0000000000..f2ef0a829f --- /dev/null +++ b/tests/PHPStan/Rules/PhpDoc/data/wrong-var-native-type.php @@ -0,0 +1,34 @@ +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 + { + + } + +}