Skip to content

Commit

Permalink
Bleeding edge - check var tag type against native type
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Jan 13, 2023
1 parent ce24c83 commit a69e3bc
Show file tree
Hide file tree
Showing 18 changed files with 160 additions and 20 deletions.
1 change: 1 addition & 0 deletions conf/bleedingEdge.neon
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ parameters:
invarianceComposition: true
alwaysTrueAlwaysReported: true
disableUnreachableBranchesRules: true
varTagType: true
7 changes: 6 additions & 1 deletion conf/config.level2.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -75,3 +74,9 @@ services:
checkMissingVarTagTypehint: %checkMissingVarTagTypehint%
tags:
- phpstan.rules.rule
-
class: PHPStan\Rules\PhpDoc\WrongVariableNameInVarTagRule
arguments:
checkTypeAgainstNativeType: %featureToggles.varTagType%
tags:
- phpstan.rules.rule
2 changes: 2 additions & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ parameters:
invarianceComposition: false
alwaysTrueAlwaysReported: false
disableUnreachableBranchesRules: false
varTagType: false
fileExtensions:
- php
checkAdvancedIsset: false
Expand Down Expand Up @@ -280,6 +281,7 @@ parametersSchema:
invarianceComposition: bool()
alwaysTrueAlwaysReported: bool()
disableUnreachableBranchesRules: bool()
varTagType: bool()
])
fileExtensions: listOf(string())
checkAdvancedIsset: bool()
Expand Down
20 changes: 20 additions & 0 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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\\:
Expand Down
3 changes: 0 additions & 3 deletions src/Command/CommandHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace PHPStan\Command;

use Closure;
use Composer\XdebugHandler\XdebugHandler;
use Nette\DI\Helpers;
use Nette\DI\InvalidConfigurationException;
Expand Down Expand Up @@ -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)));
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/Command/FixerApplication.php
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 0 additions & 2 deletions src/File/FuzzyRelativePathHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/Parallel/ParallelAnalyser.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/Reflection/FunctionVariantWithPhpDocs.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public function __construct(
*/
public function getParameters(): array
{
/** @var ParameterReflectionWithPhpDocs[] $parameters */
/** @var array<int, ParameterReflectionWithPhpDocs> $parameters */
$parameters = parent::getParameters();

return $parameters;
Expand Down
35 changes: 32 additions & 3 deletions src/Rules/PhpDoc/WrongVariableNameInVarTagRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -34,6 +38,7 @@ class WrongVariableNameInVarTagRule implements Rule

public function __construct(
private FileTypeMapper $fileTypeMapper,
private bool $checkTypeAgainstNativeType,
)
{
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/Type/ClosureTypeFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
Expand Down
1 change: 0 additions & 1 deletion src/Type/Constant/ConstantStringType.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
16 changes: 12 additions & 4 deletions tests/PHPStan/Analyser/AnalyserIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/LegacyNodeScopeResolverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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_)) {
Expand All @@ -75,6 +74,7 @@ private function getFileScope(string $filename): Scope
$testScope = $scope;
});

/** @var Scope */
return $testScope;
}

Expand Down
26 changes: 26 additions & 0 deletions tests/PHPStan/Analyser/data/bug-6842.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down
22 changes: 22 additions & 0 deletions tests/PHPStan/Rules/PhpDoc/WrongVariableNameInVarTagRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
}

Expand Down Expand Up @@ -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,
],
]);
}

}
34 changes: 34 additions & 0 deletions tests/PHPStan/Rules/PhpDoc/data/wrong-var-native-type.php
Original file line number Diff line number Diff line change
@@ -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
{

}

}

0 comments on commit a69e3bc

Please sign in to comment.