Skip to content

Commit

Permalink
feat(refactor): get rid of most instanceof ConstantStringType checks
Browse files Browse the repository at this point in the history
  • Loading branch information
@canvural
canvural committed Nov 28, 2022
1 parent 50ae8d6 commit 7b0cce7
Show file tree
Hide file tree
Showing 16 changed files with 148 additions and 140 deletions.
2 changes: 1 addition & 1 deletion composer.json
View file
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
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
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
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
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
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
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

0 comments on commit 7b0cce7

Please sign in to comment.