Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Callable prototypes #430

Closed
wants to merge 10 commits into from
3 changes: 3 additions & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ parameters:
checkGenericClassInNonGenericObjectType: false
checkInternalClassCaseSensitivity: false
checkMissingIterableValueType: false
checkMissingCallablePrototype: false
checkMissingVarTagTypehint: false
checkArgumentsPassedByReference: false
checkMaybeUndefinedVariables: false
Expand Down Expand Up @@ -186,6 +187,7 @@ parametersSchema:
checkGenericClassInNonGenericObjectType: bool()
checkInternalClassCaseSensitivity: bool()
checkMissingIterableValueType: bool()
checkMissingCallablePrototype: bool()
checkMissingVarTagTypehint: bool()
checkArgumentsPassedByReference: bool()
checkMaybeUndefinedVariables: bool()
Expand Down Expand Up @@ -727,6 +729,7 @@ services:
arguments:
checkMissingIterableValueType: %checkMissingIterableValueType%
checkGenericClassInNonGenericObjectType: %checkGenericClassInNonGenericObjectType%
checkMissingCallablePrototype: %checkMissingCallablePrototype%

-
class: PHPStan\Rules\NullsafeCheck
Expand Down
4 changes: 2 additions & 2 deletions src/Command/AnalyserRunner.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ public function __construct(
/**
* @param string[] $files
* @param string[] $allAnalysedFiles
* @param \Closure|null $preFileCallback
* @param \Closure|null $postFileCallback
* @param (\Closure(string $file): void)|null $preFileCallback
* @param (\Closure(int): void)|null $postFileCallback
* @param bool $debug
* @param bool $allowParallel
* @param string|null $projectConfigFile
Expand Down
9 changes: 9 additions & 0 deletions src/Rules/Functions/MissingFunctionParameterTypehintRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,15 @@ private function checkFunctionParameter(FunctionReflection $functionReflection,
))->tip(MissingTypehintCheck::TURN_OFF_NON_GENERIC_CHECK_TIP)->build();
}

foreach ($this->missingTypehintCheck->getCallablesWithMissingPrototype($parameterType) as $callableType) {
$messages[] = RuleErrorBuilder::message(sprintf(
'Function %s() has parameter $%s with no prototype specified for callable type %s.',
$functionReflection->getName(),
$parameterReflection->getName(),
$callableType->describe(VerbosityLevel::typeOnly())
))->build();
}

return $messages;
}

Expand Down
8 changes: 8 additions & 0 deletions src/Rules/Functions/MissingFunctionReturnTypehintRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,14 @@ public function processNode(Node $node, Scope $scope): array
))->tip(MissingTypehintCheck::TURN_OFF_NON_GENERIC_CHECK_TIP)->build();
}

foreach ($this->missingTypehintCheck->getCallablesWithMissingPrototype($returnType) as $callableType) {
$messages[] = RuleErrorBuilder::message(sprintf(
'Function %s() return type has no prototype specified for callable type %s.',
$functionReflection->getName(),
$callableType->describe(VerbosityLevel::typeOnly())
))->build();
}

return $messages;
}

Expand Down
10 changes: 10 additions & 0 deletions src/Rules/Methods/MissingMethodParameterTypehintRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,16 @@ private function checkMethodParameter(MethodReflection $methodReflection, Parame
))->tip(MissingTypehintCheck::TURN_OFF_NON_GENERIC_CHECK_TIP)->build();
}

foreach ($this->missingTypehintCheck->getCallablesWithMissingPrototype($parameterType) as $callableType) {
$messages[] = RuleErrorBuilder::message(sprintf(
'Method %s::%s() has parameter $%s with no prototype specified for callable type %s.',
$methodReflection->getDeclaringClass()->getDisplayName(),
$methodReflection->getName(),
$parameterReflection->getName(),
$callableType->describe(VerbosityLevel::typeOnly())
))->build();
}

return $messages;
}

Expand Down
9 changes: 9 additions & 0 deletions src/Rules/Methods/MissingMethodReturnTypehintRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,15 @@ public function processNode(Node $node, Scope $scope): array
))->tip(MissingTypehintCheck::TURN_OFF_NON_GENERIC_CHECK_TIP)->build();
}

foreach ($this->missingTypehintCheck->getCallablesWithMissingPrototype($returnType) as $callableType) {
$messages[] = RuleErrorBuilder::message(sprintf(
'Method %s::%s() return type has no prototype specified for callable type %s.',
$methodReflection->getDeclaringClass()->getDisplayName(),
$methodReflection->getName(),
$callableType->describe(VerbosityLevel::typeOnly())
))->build();
}

return $messages;
}

Expand Down
31 changes: 30 additions & 1 deletion src/Rules/MissingTypehintCheck.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace PHPStan\Rules;

use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Type\CallableType;
use PHPStan\Type\Generic\GenericObjectType;
use PHPStan\Type\Generic\TemplateType;
use PHPStan\Type\Generic\TemplateTypeHelper;
Expand Down Expand Up @@ -32,15 +33,20 @@ class MissingTypehintCheck

private bool $checkGenericClassInNonGenericObjectType;

/** @var bool */
private $checkMissingCallablePrototype;

public function __construct(
ReflectionProvider $reflectionProvider,
bool $checkMissingIterableValueType,
bool $checkGenericClassInNonGenericObjectType
bool $checkGenericClassInNonGenericObjectType,
bool $checkMissingCallablePrototype
)
{
$this->reflectionProvider = $reflectionProvider;
$this->checkMissingIterableValueType = $checkMissingIterableValueType;
$this->checkGenericClassInNonGenericObjectType = $checkGenericClassInNonGenericObjectType;
$this->checkMissingCallablePrototype = $checkMissingCallablePrototype;
}

/**
Expand Down Expand Up @@ -133,4 +139,27 @@ public function getNonGenericObjectTypesWithGenericClass(Type $type): array
return $objectTypes;
}

/**
* @param \PHPStan\Type\Type $type
* @return \PHPStan\Type\Type[]
*/
public function getCallablesWithMissingPrototype(Type $type): array
{
if (!$this->checkMissingCallablePrototype) {
return [];
}

$result = [];
TypeTraverser::map($type, static function (Type $type, callable $traverse) use (&$result): Type {
if (
($type instanceof CallableType && $type->isCommonCallable()) ||
($type instanceof ObjectType && $type->getClassName() === \Closure::class)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only thing I'm not happy with. I think there should be a better, more generalized way to do this, but if there is one it's not apparent to me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably fine... There's Type::isCallable() and Type::getCallableParametersAcceptors() but there's no way to extract whether it's general callable or already specified callable from that.

So as long as this place is synchronized with TypeNodeResolver::resolveCallableTypeNode(), we're fine.

$result[] = $type;
}
return $traverse($type);
});

return $result;
}

}
9 changes: 9 additions & 0 deletions src/Rules/Properties/MissingPropertyTypehintRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,15 @@ public function processNode(Node $node, Scope $scope): array
))->tip(MissingTypehintCheck::TURN_OFF_NON_GENERIC_CHECK_TIP)->build();
}

foreach ($this->missingTypehintCheck->getCallablesWithMissingPrototype($propertyType) as $callableType) {
$messages[] = RuleErrorBuilder::message(sprintf(
'Property %s::$%s type has no prototype specified for callable type %s.',
$propertyReflection->getDeclaringClass()->getDisplayName(),
$node->getName(),
$callableType->describe(VerbosityLevel::typeOnly())
))->build();
}

return $messages;
}

Expand Down
5 changes: 5 additions & 0 deletions src/Type/CallableType.php
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,11 @@ public function isNumericString(): TrinaryLogic
return TrinaryLogic::createNo();
}

public function isCommonCallable(): bool
{
return $this->isCommonCallable;
}

/**
* @param mixed[] $properties
* @return Type
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/data/Foo-callable.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
class Foo
{
/**
* @param Foo|callable $xxx
* @param Foo|(callable(): mixed) $xxx
*/
public function abc($xxx): void
{
Expand Down
20 changes: 20 additions & 0 deletions tests/PHPStan/Levels/data/acceptTypes-6.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@
"line": 148,
"ignorable": true
},
{
"message": "Method Levels\\AcceptTypes\\Foo::expectCallable() has parameter $callable with no prototype specified for callable type callable.",
"line": 148,
"ignorable": true
},
{
"message": "Method Levels\\AcceptTypes\\Foo::iterableCountable() has no return typehint specified.",
"line": 160,
Expand Down Expand Up @@ -144,9 +149,24 @@
"line": 570,
"ignorable": true
},
{
"message": "Method Levels\\AcceptTypes\\ArrayShapes::doFoo() has parameter $callables with no prototype specified for callable type callable.",
"line": 570,
"ignorable": true
},
{
"message": "Method Levels\\AcceptTypes\\ArrayShapes::doFoo() has parameter $iterable with no prototype specified for callable type callable.",
"line": 570,
"ignorable": true
},
{
"message": "Method Levels\\AcceptTypes\\ArrayShapes::doBar() has no return typehint specified.",
"line": 603,
"ignorable": true
},
{
"message": "Method Levels\\AcceptTypes\\ArrayShapes::doBar() has parameter $one with no prototype specified for callable type callable.",
"line": 603,
"ignorable": true
}
]
2 changes: 1 addition & 1 deletion tests/PHPStan/Levels/data/casts.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class Foo

/**
* @param mixed[] $array
* @param mixed[]|callable $arrayOrCallable
* @param mixed[]|(callable(): mixed) $arrayOrCallable
* @param mixed[]|float|int $arrayOrFloatOrInt
*/
public function doFoo(
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Levels/data/echo_.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class Foo
{
/**
* @param mixed[] $array
* @param mixed[]|callable $arrayOrCallable
* @param mixed[]|(callable(): mixed) $arrayOrCallable
* @param mixed[]|float|int $arrayOrFloatOrInt
* @param mixed[]|string $arrayOrString
*/
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Levels/data/encapsedString.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class Foo

/**
* @param mixed[] $array
* @param mixed[]|callable $arrayOrCallable
* @param mixed[]|(callable(): mixed) $arrayOrCallable
* @param mixed[]|float|int $arrayOrFloatOrInt
* @param mixed[]|string $arrayOrString
*/
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Levels/data/print_.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class Foo
{
/**
* @param mixed[] $array
* @param mixed[]|callable $arrayOrCallable
* @param mixed[]|(callable(): mixed) $arrayOrCallable
* @param mixed[]|float|int $arrayOrFloatOrInt
* @param mixed[]|string $arrayOrString
*/
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Rules/Classes/MixinRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ protected function getRule(): Rule
$reflectionProvider,
new ClassCaseSensitivityCheck($reflectionProvider),
new GenericObjectTypeCheck(),
new MissingTypehintCheck($reflectionProvider, true, true),
new MissingTypehintCheck($reflectionProvider, true, true, true),
true
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class MissingFunctionParameterTypehintRuleTest extends \PHPStan\Testing\RuleTest
protected function getRule(): \PHPStan\Rules\Rule
{
$broker = $this->createReflectionProvider();
return new MissingFunctionParameterTypehintRule(new MissingTypehintCheck($broker, true, true));
return new MissingFunctionParameterTypehintRule(new MissingTypehintCheck($broker, true, true, true));
}

public function testRule(): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class MissingFunctionReturnTypehintRuleTest extends \PHPStan\Testing\RuleTestCas
protected function getRule(): \PHPStan\Rules\Rule
{
$broker = $this->createReflectionProvider();
return new MissingFunctionReturnTypehintRule(new MissingTypehintCheck($broker, true, true));
return new MissingFunctionReturnTypehintRule(new MissingTypehintCheck($broker, true, true, true));
}

public function testRule(): void
Expand Down Expand Up @@ -48,6 +48,18 @@ public function testRule(): void
105,
'You can turn this off by setting <fg=cyan>checkGenericClassInNonGenericObjectType: false</> in your <fg=cyan>%configurationFile%</>.',
],
[
'Function MissingFunctionReturnTypehint\closureWithNoPrototype() return type has no prototype specified for callable type Closure.',
113,
],
[
'Function MissingFunctionReturnTypehint\callableWithNoPrototype() return type has no prototype specified for callable type callable.',
127,
],
[
'Function MissingFunctionReturnTypehint\callableNestedNoPrototype() return type has no prototype specified for callable type callable.',
141,
],
]);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,46 @@ function genericGenericMissingTemplateArgs(): GenericClass
{

}

/**
* @return \Closure
*/
function closureWithNoPrototype() : \Closure{

}

/**
* @return \Closure(int) : void
*/
function closureWithPrototype() : \Closure{

}

/**
* @return callable
*/
function callableWithNoPrototype() : callable{

}

/**
* @return callable(int) : void
*/
function callableWithPrototype() : callable{

}

/**
* @return callable(callable) : void
*/
function callableNestedNoPrototype() : callable{

}

/**
* @return callable(callable(int) : void) : void
*/
function callableNestedWithPrototype() : callable{

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class MissingMethodParameterTypehintRuleTest extends \PHPStan\Testing\RuleTestCa
protected function getRule(): \PHPStan\Rules\Rule
{
$broker = $this->createReflectionProvider();
return new MissingMethodParameterTypehintRule(new MissingTypehintCheck($broker, true, true));
return new MissingMethodParameterTypehintRule(new MissingTypehintCheck($broker, true, true, true));
}

public function testRule(): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class MissingMethodReturnTypehintRuleTest extends \PHPStan\Testing\RuleTestCase
protected function getRule(): \PHPStan\Rules\Rule
{
$broker = $this->createReflectionProvider();
return new MissingMethodReturnTypehintRule(new MissingTypehintCheck($broker, true, true));
return new MissingMethodReturnTypehintRule(new MissingTypehintCheck($broker, true, true, true));
}

public function testRule(): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ protected function getRule(): Rule
$broker,
new ClassCaseSensitivityCheck($broker),
new GenericObjectTypeCheck(),
new MissingTypehintCheck($broker, true, true),
new MissingTypehintCheck($broker, true, true, true),
true,
true
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class MissingPropertyTypehintRuleTest extends \PHPStan\Testing\RuleTestCase
protected function getRule(): \PHPStan\Rules\Rule
{
$broker = $this->createReflectionProvider();
return new MissingPropertyTypehintRule(new MissingTypehintCheck($broker, true, true));
return new MissingPropertyTypehintRule(new MissingTypehintCheck($broker, true, true, true));
}

public function testRule(): void
Expand Down