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.

Sign up for GitHub

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

Jump to bottom

feat(refactor): get rid of most instanceof ConstantStringType checks #1459

Merged
merged 1 commit into from Nov 28, 2022
Merged

feat(refactor): get rid of most instanceof ConstantStringType checks #1459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
View file
Open in desktop
Expand Up @@ -22,7 +22,7 @@
"illuminate/support": "^9",
"mockery/mockery": "^1.4.4",
"phpmyadmin/sql-parser": "^5.5",
"phpstan/phpstan": "^1.9.0"
"phpstan/phpstan": "^1.9.2"
},
"require-dev": {
"nikic/php-parser": "^4.13.2",
Expand Down
38 changes: 24 additions & 14 deletions src/ReturnTypes/ContainerArrayAccessDynamicMethodReturnTypeExtension.php
View file
Open in desktop
Expand Up @@ -8,12 +8,12 @@
use PhpParser\Node\Expr\MethodCall;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\MethodReflection;
use PHPStan\Reflection\ParametersAcceptorSelector;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\DynamicMethodReturnTypeExtension;
use PHPStan\Type\ErrorType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\TypeUtils;

class ContainerArrayAccessDynamicMethodReturnTypeExtension implements DynamicMethodReturnTypeExtension
{
Expand Down Expand Up @@ -43,31 +43,41 @@ public function getTypeFromMethodCall(
MethodReflection $methodReflection,
MethodCall $methodCall,
Scope $scope
): Type {
): ?Type {
$args = $methodCall->getArgs();

if (count($args) === 0) {
return ParametersAcceptorSelector::selectSingle($methodReflection->getVariants())->getReturnType();
return null;
}

$argType = $scope->getType($args[0]->value);

if (! $argType instanceof ConstantStringType) {
return ParametersAcceptorSelector::selectSingle($methodReflection->getVariants())->getReturnType();
$argStrings = TypeUtils::getConstantStrings($argType);

if ($argStrings === []) {
return null;
}

$resolvedValue = $this->resolve($argType->getValue());
$argTypes = [];

if ($resolvedValue === null) {
return new ErrorType();
}
foreach ($argStrings as $argString) {
$resolvedValue = $this->resolve($argString->getValue());

if ($resolvedValue === null) {
$argTypes[] = new ErrorType();
continue;
}

if (is_object($resolvedValue)) {
$class = get_class($resolvedValue);

if (is_object($resolvedValue)) {
$class = get_class($resolvedValue);
$argTypes[] = new ObjectType($class);
continue;
}

return new ObjectType($class);
$argTypes[] = $scope->getTypeFromValue($resolvedValue);
}

return $scope->getTypeFromValue($resolvedValue);
return count($argTypes) > 1 ? TypeCombinator::union(...$argTypes) : $argTypes[0];
}
}
23 changes: 10 additions & 13 deletions src/ReturnTypes/GuardDynamicStaticMethodReturnTypeExtension.php
View file
Open in desktop
Expand Up @@ -11,11 +11,11 @@
use PhpParser\Node\Expr\StaticCall;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\MethodReflection;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\DynamicStaticMethodReturnTypeExtension;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\TypeUtils;

class GuardDynamicStaticMethodReturnTypeExtension implements DynamicStaticMethodReturnTypeExtension
{
Expand Down Expand Up @@ -58,25 +58,22 @@ public function getTypeFromStaticMethodCall(
}

$argType = $scope->getType($methodCall->getArgs()[0]->value);
$argStrings = TypeUtils::getConstantStrings($argType);

if (! $argType instanceof ConstantStringType) {
if (count($argStrings) !== 1) {
return $defaultReturnType;
}

return $this->findTypeFromGuardDriver($argType->getValue()) ?? $defaultReturnType;
return $this->findTypeFromGuardDriver($argStrings[0]->getValue()) ?? $defaultReturnType;
}

private function findTypeFromGuardDriver(string $driver): ?Type
{
switch ($driver) {
case 'session':
return new ObjectType(\Illuminate\Auth\SessionGuard::class);
case 'token':
return new ObjectType(\Illuminate\Auth\TokenGuard::class);
case 'passport':
return new ObjectType(\Illuminate\Auth\RequestGuard::class);
default:
return null;
}
return match ($driver) {
'session' => new ObjectType(\Illuminate\Auth\SessionGuard::class),
'token' => new ObjectType(\Illuminate\Auth\TokenGuard::class),
'passport' => new ObjectType(\Illuminate\Auth\RequestGuard::class),
default => null,
};
}
}
12 changes: 6 additions & 6 deletions src/ReturnTypes/GuardExtension.php
View file
Open in desktop
Expand Up @@ -11,12 +11,11 @@
use PhpParser\Node\Expr\StaticCall;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\MethodReflection;
use PHPStan\Reflection\ParametersAcceptorSelector;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\DynamicMethodReturnTypeExtension;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\TypeUtils;

final class GuardExtension implements DynamicMethodReturnTypeExtension
{
Expand All @@ -40,7 +39,7 @@ public function getTypeFromMethodCall(
MethodReflection $methodReflection,
MethodCall $methodCall,
Scope $scope
): Type {
): ?Type {
$config = $this->getContainer()->get('config');
$authModel = null;

Expand All @@ -50,7 +49,7 @@ public function getTypeFromMethodCall(
}

if ($authModel === null) {
return ParametersAcceptorSelector::selectSingle($methodReflection->getVariants())->getReturnType();
return null;
}

return TypeCombinator::addNull(new ObjectType($authModel));
Expand All @@ -71,11 +70,12 @@ private function getGuardFromMethodCall(Scope $scope, MethodCall $methodCall): ?
}

$guardType = $scope->getType($methodCall->var->getArgs()[0]->value);
$constantStrings = TypeUtils::getConstantStrings($guardType);

if (! $guardType instanceof ConstantStringType) {
if (count($constantStrings) !== 1) {
return null;
}

return $guardType->getValue();
return $constantStrings[0]->getValue();
}
}
22 changes: 18 additions & 4 deletions src/ReturnTypes/TestCaseExtension.php
View file
Open in desktop
Expand Up @@ -8,11 +8,12 @@
use PhpParser\Node\Expr\MethodCall;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\MethodReflection;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\DynamicMethodReturnTypeExtension;
use PHPStan\Type\ErrorType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\TypeUtils;

/**
* @internal
Expand Down Expand Up @@ -40,14 +41,27 @@ public function getTypeFromMethodCall(
): Type {
$defaultReturnType = new ObjectType('Mockery\\MockInterface');

if (count($methodCall->args) === 0) {
return new ErrorType();
}

$classType = $scope->getType($methodCall->getArgs()[0]->value);
$constantStrings = TypeUtils::getConstantStrings($classType);

if (! $classType instanceof ConstantStringType) {
if ($constantStrings === []) {
return $defaultReturnType;
}

$objectType = new ObjectType($classType->getValue());
$returnTypes = [];

foreach ($constantStrings as $constantString) {
$objectType = new ObjectType($constantString->getValue());

$returnTypes[] = TypeCombinator::intersect($defaultReturnType, $objectType);
}

return TypeCombinator::intersect($defaultReturnType, $objectType);
return count($returnTypes) === 1
? $returnTypes[0]
: TypeCombinator::union(...$returnTypes);
}
}
61 changes: 33 additions & 28 deletions src/Rules/ModelProperties/ModelPropertiesRuleHelper.php
View file
Open in desktop
Expand Up @@ -16,11 +16,8 @@
use PHPStan\ShouldNotHappenException;
use PHPStan\Type\ArrayType;
use PHPStan\Type\Constant\ConstantArrayType;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\GeneralizePrecision;
use PHPStan\Type\IntegerType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeUtils;
use PHPStan\Type\UnionType;
Expand Down Expand Up @@ -68,59 +65,67 @@ public function check(MethodReflection $methodReflection, Scope $scope, array $a
if ($argType instanceof ConstantArrayType) {
$errors = [];

$keyType = TypeUtils::generalizeType($argType->getKeyType(), GeneralizePrecision::lessSpecific());
$keyType = $argType->getKeyType()->generalize(GeneralizePrecision::lessSpecific());

if ($keyType instanceof IntegerType) {
if ($keyType->isInteger()->yes()) {
$valueTypes = $argType->getValuesArray()->getValueTypes();
} elseif ($keyType instanceof StringType) {
} elseif ($keyType->isString()->yes()) {
$valueTypes = $argType->getKeysArray()->getValueTypes();
} else {
$valueTypes = [];
}

foreach ($valueTypes as $valueType) {
$strings = TypeUtils::getConstantStrings($valueType);

// It could be something like `DB::raw`
// We only want to analyze strings
if (! $valueType instanceof ConstantStringType) {
if ($strings === []) {
continue;
}

// TODO: maybe check table names and columns here. And for JSON access maybe just the column name
if (mb_strpos($valueType->getValue(), '.') !== false || mb_strpos($valueType->getValue(), '->') !== false) {
continue;
}
foreach ($strings as $string) {
// TODO: maybe check table names and columns here. And for JSON access maybe just the column name
if (mb_strpos($string->getValue(), '.') !== false || mb_strpos($string->getValue(), '->') !== false) {
continue;
}

if (! $modelType->hasProperty($valueType->getValue())->yes()) {
$error = sprintf('Property \'%s\' does not exist in %s model.', $valueType->getValue(), $modelType->describe(VerbosityLevel::typeOnly()));
if (! $modelType->hasProperty($string->getValue())->yes()) {
$error = sprintf('Property \'%s\' does not exist in %s model.', $string->getValue(), $modelType->describe(VerbosityLevel::typeOnly()));

if ($methodReflection->getDeclaringClass()->getName() === BelongsToMany::class) {
$error .= sprintf(" If '%s' exists as a column on the pivot table, consider using 'wherePivot' or prefix the column with table name instead.", $valueType->getValue());
}
if ($methodReflection->getDeclaringClass()->getName() === BelongsToMany::class) {
$error .= sprintf(" If '%s' exists as a column on the pivot table, consider using 'wherePivot' or prefix the column with table name instead.", $string->getValue());
}

$errors[] = $error;
$errors[] = $error;
}
}
}

return $errors;
}

if (! $argType instanceof ConstantStringType) {
return [];
}
$argStrings = TypeUtils::getConstantStrings($argType);

// TODO: maybe check table names and columns here. And for JSON access maybe just the column name
if (mb_strpos($argType->getValue(), '.') !== false || mb_strpos($argType->getValue(), '->') !== false) {
if ($argStrings === []) {
return [];
}

if (! $modelType->hasProperty($argType->getValue())->yes()) {
$error = sprintf('Property \'%s\' does not exist in %s model.', $argType->getValue(), $modelType->describe(VerbosityLevel::typeOnly()));

if ((new ObjectType(BelongsToMany::class))->isSuperTypeOf(ParametersAcceptorSelector::selectSingle($methodReflection->getVariants())->getReturnType())->yes()) {
$error .= sprintf(" If '%s' exists as a column on the pivot table, consider using 'wherePivot' or prefix the column with table name instead.", $argType->getValue());
foreach ($argStrings as $argString) {
// TODO: maybe check table names and columns here. And for JSON access maybe just the column name
if (mb_strpos($argString->getValue(), '.') !== false || mb_strpos($argString->getValue(), '->') !== false) {
return [];
}

return [$error];
if (! $modelType->hasProperty($argString->getValue())->yes()) {
$error = sprintf('Property \'%s\' does not exist in %s model.', $argString->getValue(), $modelType->describe(VerbosityLevel::typeOnly()));

if ((new ObjectType(BelongsToMany::class))->isSuperTypeOf(ParametersAcceptorSelector::selectSingle($methodReflection->getVariants())->getReturnType())->yes()) {
$error .= sprintf(" If '%s' exists as a column on the pivot table, consider using 'wherePivot' or prefix the column with table name instead.", $argString->getValue());
}

return [$error];
}
}

return [];
Expand Down
14 changes: 4 additions & 10 deletions src/Rules/NoModelMakeRule.php
View file
Open in desktop
Expand Up @@ -16,8 +16,8 @@
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\TypeUtils;

/**
* Catches inefficient instantiation of models using Model::make().
Expand Down Expand Up @@ -96,17 +96,11 @@ protected function isCalledOnModel(StaticCall $call, Scope $scope): bool
if ($class instanceof FullyQualified) {
$type = new ObjectType($class->toString());
} elseif ($class instanceof Expr) {
$exprType = $scope->getType($class);
$type = $scope->getType($class);

if (! $exprType instanceof ConstantStringType) {
return false;
if ($type->isClassStringType()->yes() && TypeUtils::getConstantStrings($type) !== []) {
$type = new ObjectType($type->getValue()); // @phpstan-ignore-line
}

if (! $exprType->isClassString()) {
return false;
}

$type = new ObjectType($exprType->getValue());
} else {
// TODO can we handle relative names, do they even occur here?
return false;
Expand Down