Bleeding edge - added absent type checks to AssertRuleHelper

phpstan · Oct 4, 2024 · ca0a7e9 · ca0a7e9
1 parent dbe8b74
commit ca0a7e9
Show file tree

Hide file tree

Showing 7 changed files with 376 additions and 22 deletions.
diff --git a/conf/config.neon b/conf/config.neon
@@ -1083,6 +1083,10 @@ services:
 
 	-
 		class: PHPStan\Rules\PhpDoc\AssertRuleHelper
+		arguments:
+			checkMissingTypehints: %checkMissingTypehints%
+			checkClassCaseSensitivity: %checkClassCaseSensitivity%
+			absentTypeChecks: %featureToggles.absentTypeChecks%
 
 	-
 		class: PHPStan\Rules\PhpDoc\UnresolvableTypeHelper

diff --git a/src/Rules/PhpDoc/AssertRuleHelper.php b/src/Rules/PhpDoc/AssertRuleHelper.php
@@ -2,39 +2,63 @@
 
 namespace PHPStan\Rules\PhpDoc;
 
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Function_;
 use PHPStan\Node\Expr\TypeExpr;
+use PHPStan\PhpDoc\Tag\AssertTag;
 use PHPStan\Reflection\ExtendedMethodReflection;
 use PHPStan\Reflection\FunctionReflection;
 use PHPStan\Reflection\InitializerExprContext;
 use PHPStan\Reflection\InitializerExprTypeResolver;
 use PHPStan\Reflection\ParametersAcceptor;
+use PHPStan\Reflection\ReflectionProvider;
+use PHPStan\Rules\ClassNameCheck;
+use PHPStan\Rules\ClassNameNodePair;
+use PHPStan\Rules\Generics\GenericObjectTypeCheck;
 use PHPStan\Rules\IdentifierRuleError;
+use PHPStan\Rules\MissingTypehintCheck;
 use PHPStan\Rules\RuleErrorBuilder;
 use PHPStan\Type\ErrorType;
 use PHPStan\Type\ObjectType;
 use PHPStan\Type\VerbosityLevel;
 use function array_key_exists;
+use function array_merge;
+use function implode;
 use function sprintf;
 use function substr;
 
 final class AssertRuleHelper
 {
 
-	public function __construct(private InitializerExprTypeResolver $initializerExprTypeResolver)
+	public function __construct(
+		private InitializerExprTypeResolver $initializerExprTypeResolver,
+		private ReflectionProvider $reflectionProvider,
+		private UnresolvableTypeHelper $unresolvableTypeHelper,
+		private ClassNameCheck $classCheck,
+		private MissingTypehintCheck $missingTypehintCheck,
+		private GenericObjectTypeCheck $genericObjectTypeCheck,
+		private bool $absentTypeChecks,
+		private bool $checkClassCaseSensitivity,
+		private bool $checkMissingTypehints,
+	)
 	{
 	}
 
 	/**
 	 * @return list<IdentifierRuleError>
 	 */
-	public function check(ExtendedMethodReflection|FunctionReflection $reflection, ParametersAcceptor $acceptor): array
+	public function check(
+		Function_|ClassMethod $node,
+		ExtendedMethodReflection|FunctionReflection $reflection,
+		ParametersAcceptor $acceptor,
+	): array
 	{
 		$parametersByName = [];
 		foreach ($acceptor->getParameters() as $parameter) {
 			$parametersByName[$parameter->getName()] = $parameter->getType();
 		}
 
-		if ($reflection instanceof ExtendedMethodReflection) {
+		if ($reflection instanceof ExtendedMethodReflection && !$reflection->isStatic()) {
 			$class = $reflection->getDeclaringClass();
 			$parametersByName['this'] = new ObjectType($class->getName(), null, $class);
 		}
@@ -57,38 +81,138 @@ public function check(ExtendedMethodReflection|FunctionReflection $reflection, P
 
 			$assertedExpr = $assert->getParameter()->getExpr(new TypeExpr($parametersByName[$parameterName]));
 			$assertedExprType = $this->initializerExprTypeResolver->getType($assertedExpr, $context);
+			$assertedExprString = $assert->getParameter()->describe();
 			if ($assertedExprType instanceof ErrorType) {
+				if ($this->absentTypeChecks) {
+					$errors[] = RuleErrorBuilder::message(sprintf('Assert references unknown %s.', $assertedExprString))
+						->identifier('assert.unknownExpr')
+						->build();
+				}
 				continue;
 			}
 
 			$assertedType = $assert->getType();
 
+			$tagName = [
+				AssertTag::NULL => '@phpstan-assert',
+				AssertTag::IF_TRUE => '@phpstan-assert-if-true',
+				AssertTag::IF_FALSE => '@phpstan-assert-if-false',
+			][$assert->getIf()];
+
+			if ($this->absentTypeChecks) {
+				if ($this->unresolvableTypeHelper->containsUnresolvableType($assertedType)) {
+					$errors[] = RuleErrorBuilder::message(sprintf(
+						'PHPDoc tag %s for %s contains unresolvable type.',
+						$tagName,
+						$assertedExprString,
+					))->identifier('assert.unresolvableType')->build();
+					continue;
+				}
+			}
+
 			$isSuperType = $assertedType->isSuperTypeOf($assertedExprType);
-			if ($isSuperType->maybe()) {
+			if (!$isSuperType->maybe()) {
+				if ($assert->isNegated() ? $isSuperType->yes() : $isSuperType->no()) {
+					$errors[] = RuleErrorBuilder::message(sprintf(
+						'Asserted %stype %s for %s with type %s can never happen.',
+						$assert->isNegated() ? 'negated ' : '',
+						$assertedType->describe(VerbosityLevel::precise()),
+						$assertedExprString,
+						$assertedExprType->describe(VerbosityLevel::precise()),
+					))->identifier('assert.impossibleType')->build();
+				} elseif ($assert->isNegated() ? $isSuperType->no() : $isSuperType->yes()) {
+					$errors[] = RuleErrorBuilder::message(sprintf(
+						'Asserted %stype %s for %s with type %s does not narrow down the type.',
+						$assert->isNegated() ? 'negated ' : '',
+						$assertedType->describe(VerbosityLevel::precise()),
+						$assertedExprString,
+						$assertedExprType->describe(VerbosityLevel::precise()),
+					))->identifier('assert.alreadyNarrowedType')->build();
+				}
+			}
+
+			if (!$this->absentTypeChecks) {
 				continue;
 			}
 
-			$assertedExprString = $assert->getParameter()->describe();
+			foreach ($assertedType->getReferencedClasses() as $class) {
+				if (!$this->reflectionProvider->hasClass($class)) {
+					$errors[] = RuleErrorBuilder::message(sprintf(
+						'PHPDoc tag %s for %s contains unknown class %s.',
+						$tagName,
+						$assertedExprString,
+						$class,
+					))->identifier('class.notFound')->build();
+					continue;
+				}
+
+				$classReflection = $this->reflectionProvider->getClass($class);
+				if ($classReflection->isTrait()) {
+					$errors[] = RuleErrorBuilder::message(sprintf(
+						'PHPDoc tag %s for %s contains invalid type %s.',
+						$tagName,
+						$assertedExprString,
+						$class,
+					))->identifier('assert.trait')->build();
+					continue;
+				}
+
+				$errors = array_merge(
+					$errors,
+					$this->classCheck->checkClassNames([
+						new ClassNameNodePair($class, $node),
+					], $this->checkClassCaseSensitivity),
+				);
+			}
+
+			$errors = array_merge($errors, $this->genericObjectTypeCheck->check(
+				$assertedType,
+				sprintf('PHPDoc tag %s for %s contains generic type %%s but %%s %%s is not generic.', $tagName, $assertedExprString),
+				sprintf('Generic type %%s in PHPDoc tag %s for %s does not specify all template types of %%s %%s: %%s', $tagName, $assertedExprString),
+				sprintf('Generic type %%s in PHPDoc tag %s for %s specifies %%d template types, but %%s %%s supports only %%d: %%s', $tagName, $assertedExprString),
+				sprintf('Type %%s in generic type %%s in PHPDoc tag %s for %s is not subtype of template type %%s of %%s %%s.', $tagName, $assertedExprString),
+				sprintf('Call-site variance of %%s in generic type %%s in PHPDoc tag %s for %s is in conflict with %%s template type %%s of %%s %%s.', $tagName, $assertedExprString),
+				sprintf('Call-site variance of %%s in generic type %%s in PHPDoc tag %s for %s is redundant, template type %%s of %%s %%s has the same variance.', $tagName, $assertedExprString),
+			));
+
+			if (!$this->checkMissingTypehints) {
+				continue;
+			}
 
-			if ($assert->isNegated() ? $isSuperType->yes() : $isSuperType->no()) {
+			foreach ($this->missingTypehintCheck->getIterableTypesWithMissingValueTypehint($assertedType) as $iterableType) {
+				$iterableTypeDescription = $iterableType->describe(VerbosityLevel::typeOnly());
 				$errors[] = RuleErrorBuilder::message(sprintf(
-					'Asserted %stype %s for %s with type %s can never happen.',
-					$assert->isNegated() ? 'negated ' : '',
-					$assertedType->describe(VerbosityLevel::precise()),
+					'PHPDoc tag %s for %s has no value type specified in iterable type %s.',
+					$tagName,
 					$assertedExprString,
-					$assertedExprType->describe(VerbosityLevel::precise()),
-				))->identifier('assert.impossibleType')->build();
-			} elseif ($assert->isNegated() ? $isSuperType->no() : $isSuperType->yes()) {
+					$iterableTypeDescription,
+				))
+					->tip(MissingTypehintCheck::MISSING_ITERABLE_VALUE_TYPE_TIP)
+					->identifier('missingType.iterableValue')
+					->build();
+			}
+
+			foreach ($this->missingTypehintCheck->getNonGenericObjectTypesWithGenericClass($assertedType) as [$innerName, $genericTypeNames]) {
 				$errors[] = RuleErrorBuilder::message(sprintf(
-					'Asserted %stype %s for %s with type %s does not narrow down the type.',
-					$assert->isNegated() ? 'negated ' : '',
-					$assertedType->describe(VerbosityLevel::precise()),
+					'PHPDoc tag %s for %s contains generic %s but does not specify its types: %s',
+					$tagName,
 					$assertedExprString,
-					$assertedExprType->describe(VerbosityLevel::precise()),
-				))->identifier('assert.alreadyNarrowedType')->build();
+					$innerName,
+					implode(', ', $genericTypeNames),
+				))
+					->identifier('missingType.generics')
+					->build();
 			}
-		}
 
+			foreach ($this->missingTypehintCheck->getCallablesWithMissingSignature($assertedType) as $callableType) {
+				$errors[] = RuleErrorBuilder::message(sprintf(
+					'PHPDoc tag %s for %s has no signature specified for %s.',
+					$tagName,
+					$assertedExprString,
+					$callableType->describe(VerbosityLevel::typeOnly()),
+				))->identifier('missingType.callable')->build();
+			}
+		}
 		return $errors;
 	}
 

diff --git a/src/Rules/PhpDoc/FunctionAssertRule.php b/src/Rules/PhpDoc/FunctionAssertRule.php
@@ -31,7 +31,7 @@ public function processNode(Node $node, Scope $scope): array
 			return [];
 		}
 
-		return $this->helper->check($function, $variants[0]);
+		return $this->helper->check($node->getOriginalNode(), $function, $variants[0]);
 	}
 
 }
diff --git a/src/Rules/PhpDoc/MethodAssertRule.php b/src/Rules/PhpDoc/MethodAssertRule.php
@@ -31,7 +31,7 @@ public function processNode(Node $node, Scope $scope): array
 			return [];
 		}
 
-		return $this->helper->check($method, $variants[0]);
+		return $this->helper->check($node->getOriginalNode(), $method, $variants[0]);
 	}
 
 }
diff --git a/tests/PHPStan/Rules/PhpDoc/FunctionAssertRuleTest.php b/tests/PHPStan/Rules/PhpDoc/FunctionAssertRuleTest.php
@@ -3,6 +3,11 @@
 namespace PHPStan\Rules\PhpDoc;
 
 use PHPStan\Reflection\InitializerExprTypeResolver;
+use PHPStan\Rules\ClassCaseSensitivityCheck;
+use PHPStan\Rules\ClassForbiddenNameCheck;
+use PHPStan\Rules\ClassNameCheck;
+use PHPStan\Rules\Generics\GenericObjectTypeCheck;
+use PHPStan\Rules\MissingTypehintCheck;
 use PHPStan\Rules\Rule;
 use PHPStan\Testing\RuleTestCase;
 
@@ -15,7 +20,18 @@ class FunctionAssertRuleTest extends RuleTestCase
 	protected function getRule(): Rule
 	{
 		$initializerExprTypeResolver = self::getContainer()->getByType(InitializerExprTypeResolver::class);
-		return new FunctionAssertRule(new AssertRuleHelper($initializerExprTypeResolver));
+		$reflectionProvider = $this->createReflectionProvider();
+		return new FunctionAssertRule(new AssertRuleHelper(
+			$initializerExprTypeResolver,
+			$reflectionProvider,
+			new UnresolvableTypeHelper(),
+			new ClassNameCheck(new ClassCaseSensitivityCheck($reflectionProvider, true), new ClassForbiddenNameCheck(self::getContainer())),
+			new MissingTypehintCheck(true, true, true, true, []),
+			new GenericObjectTypeCheck(),
+			true,
+			true,
+			true,
+		));
 	}
 
 	public function testRule(): void

diff --git a/tests/PHPStan/Rules/PhpDoc/MethodAssertRuleTest.php b/tests/PHPStan/Rules/PhpDoc/MethodAssertRuleTest.php
@@ -3,6 +3,11 @@
 namespace PHPStan\Rules\PhpDoc;
 
 use PHPStan\Reflection\InitializerExprTypeResolver;
+use PHPStan\Rules\ClassCaseSensitivityCheck;
+use PHPStan\Rules\ClassForbiddenNameCheck;
+use PHPStan\Rules\ClassNameCheck;
+use PHPStan\Rules\Generics\GenericObjectTypeCheck;
+use PHPStan\Rules\MissingTypehintCheck;
 use PHPStan\Rules\Rule;
 use PHPStan\Testing\RuleTestCase;
 
@@ -15,7 +20,18 @@ class MethodAssertRuleTest extends RuleTestCase
 	protected function getRule(): Rule
 	{
 		$initializerExprTypeResolver = self::getContainer()->getByType(InitializerExprTypeResolver::class);
-		return new MethodAssertRule(new AssertRuleHelper($initializerExprTypeResolver));
+		$reflectionProvider = $this->createReflectionProvider();
+		return new MethodAssertRule(new AssertRuleHelper(
+			$initializerExprTypeResolver,
+			$reflectionProvider,
+			new UnresolvableTypeHelper(),
+			new ClassNameCheck(new ClassCaseSensitivityCheck($reflectionProvider, true), new ClassForbiddenNameCheck(self::getContainer())),
+			new MissingTypehintCheck(true, true, true, true, []),
+			new GenericObjectTypeCheck(),
+			true,
+			true,
+			true,
+		));
 	}
 
 	public function testRule(): void
@@ -54,6 +70,67 @@ public function testRule(): void
 				'Asserted negated type string for $i with type int does not narrow down the type.',
 				72,
 			],
+			[
+				'PHPDoc tag @phpstan-assert for $this->fooProp contains unresolvable type.',
+				94,
+			],
+			[
+				'PHPDoc tag @phpstan-assert-if-true for $a contains unresolvable type.',
+				94,
+			],
+			[
+				'PHPDoc tag @phpstan-assert for $a contains unknown class MethodAssert\Nonexistent.',
+				105,
+			],
+			[
+				'PHPDoc tag @phpstan-assert for $b contains invalid type MethodAssert\FooTrait.',
+				105,
+			],
+			[
+				'Class MethodAssert\Foo referenced with incorrect case: MethodAssert\fOO.',
+				105,
+			],
+			[
+				'Assert references unknown $this->barProp.',
+				105,
+			],
+			[
+				'Assert references unknown parameter $this.',
+				113,
+			],
+			[
+				'PHPDoc tag @phpstan-assert for $m contains generic type Exception<int, float> but class Exception is not generic.',
+				131,
+			],
+			[
+				'Generic type MethodAssert\FooBar<mixed> in PHPDoc tag @phpstan-assert for $m does not specify all template types of class MethodAssert\FooBar: T, TT',
+				138,
+			],
+			[
+				'Type mixed in generic type MethodAssert\FooBar<mixed> in PHPDoc tag @phpstan-assert for $m is not subtype of template type T of int of class MethodAssert\FooBar.',
+				138,
+			],
+			[
+				'Generic type MethodAssert\FooBar<int> in PHPDoc tag @phpstan-assert for $m does not specify all template types of class MethodAssert\FooBar: T, TT',
+				145,
+			],
+			[
+				'Generic type MethodAssert\FooBar<int, string, float> in PHPDoc tag @phpstan-assert for $m specifies 3 template types, but class MethodAssert\FooBar supports only 2: T, TT',
+				152,
+			],
+			[
+				'PHPDoc tag @phpstan-assert for $m has no value type specified in iterable type array.',
+				194,
+				'See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type',
+			],
+			[
+				'PHPDoc tag @phpstan-assert for $m contains generic class MethodAssert\FooBar but does not specify its types: T, TT',
+				202,
+			],
+			[
+				'PHPDoc tag @phpstan-assert for $m has no signature specified for callable.',
+				210,
+			],
 		]);
 	}