From 3666e16ad422a1e504d826478115017ee894dc78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=A0pa=C4=8Dek?= Date: Sun, 5 Mar 2023 16:50:27 +0100 Subject: [PATCH] Remove `instanceof *Type` checks And replace them according to this blog post: https://phpstan.org/blog/why-is-instanceof-type-wrong-and-getting-deprecated --- src/Calls/NewCalls.php | 47 +++++++++---------- ...wedCallParamValueCaseInsensitiveExcept.php | 22 ++++++--- src/Params/DisallowedCallParamValueExcept.php | 5 +- .../DisallowedCallParamValueSpecific.php | 5 +- src/RuleErrors/DisallowedMethodRuleErrors.php | 10 +++- src/Usages/ClassConstantUsages.php | 28 +++++++---- tests/Usages/ClassConstantUsagesTest.php | 2 +- 7 files changed, 70 insertions(+), 49 deletions(-) diff --git a/src/Calls/NewCalls.php b/src/Calls/NewCalls.php index 8d8adf6..815fde5 100644 --- a/src/Calls/NewCalls.php +++ b/src/Calls/NewCalls.php @@ -11,7 +11,6 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\RuleError; use PHPStan\ShouldNotHappenException; -use PHPStan\Type\Constant\ConstantStringType; use Spaze\PHPStan\Rules\Disallowed\DisallowedCall; use Spaze\PHPStan\Rules\Disallowed\DisallowedCallFactory; use Spaze\PHPStan\Rules\Disallowed\RuleErrors\DisallowedRuleErrors; @@ -62,39 +61,39 @@ public function getNodeType(): string */ public function processNode(Node $node, Scope $scope): array { + $classNames = $names = $errors = []; if ($node->class instanceof Name) { - $className = $node->class; + $classNames[] = $node->class; } elseif ($node->class instanceof Expr) { $type = $scope->getType($node->class); - if ($type instanceof ConstantStringType) { - $className = new Name($type->getValue()); + foreach ($type->getConstantStrings() as $constantString) { + $classNames[] = new Name($constantString->getValue()); } } - if (!isset($className)) { + if ($classNames === []) { return []; } - $type = $scope->resolveTypeByName($className); - $names = [ - $type->getClassName(), - ]; - $reflection = $type->getClassReflection(); - if ($reflection) { - foreach ($reflection->getParents() as $parent) { - $names[] = $parent->getName(); + foreach ($classNames as $className) { + $type = $scope->resolveTypeByName($className); + $names[] = $type->getClassName(); + $reflection = $type->getClassReflection(); + if ($reflection) { + foreach ($reflection->getParents() as $parent) { + $names[] = $parent->getName(); + } + foreach ($reflection->getInterfaces() as $interface) { + $names[] = $interface->getName(); + } } - foreach ($reflection->getInterfaces() as $interface) { - $names[] = $interface->getName(); - } - } - $errors = []; - foreach ($names as $name) { - $name .= self::CONSTRUCT; - $errors = array_merge( - $errors, - $this->disallowedRuleErrors->get($node, $scope, $name, $type->getClassName() . self::CONSTRUCT, $this->disallowedCalls) - ); + foreach ($names as $name) { + $name .= self::CONSTRUCT; + $errors = array_merge( + $errors, + $this->disallowedRuleErrors->get($node, $scope, $name, $type->getClassName() . self::CONSTRUCT, $this->disallowedCalls) + ); + } } return $errors; diff --git a/src/Params/DisallowedCallParamValueCaseInsensitiveExcept.php b/src/Params/DisallowedCallParamValueCaseInsensitiveExcept.php index c52eee5..ac52667 100644 --- a/src/Params/DisallowedCallParamValueCaseInsensitiveExcept.php +++ b/src/Params/DisallowedCallParamValueCaseInsensitiveExcept.php @@ -3,8 +3,6 @@ namespace Spaze\PHPStan\Rules\Disallowed\Params; -use PHPStan\Type\Constant\ConstantStringType; -use PHPStan\Type\ConstantScalarType; use PHPStan\Type\Type; use Spaze\PHPStan\Rules\Disallowed\Exceptions\UnsupportedParamTypeException; @@ -19,12 +17,24 @@ class DisallowedCallParamValueCaseInsensitiveExcept extends DisallowedCallParamV */ public function matches(Type $type): bool { - if (!$type instanceof ConstantScalarType) { + if (!$type->isConstantScalarValue()->yes()) { throw new UnsupportedParamTypeException(); } - $a = is_string($this->getValue()) ? strtolower($this->getValue()) : $this->getValue(); - $b = $type instanceof ConstantStringType ? strtolower($type->getValue()) : $type->getValue(); - return $a !== $b; + $values = []; + foreach ($type->getConstantScalarValues() as $value) { + $values[] = $this->getLowercaseValue($value); + } + return !in_array($this->getLowercaseValue($this->getValue()), $values, true); + } + + + /** + * @param mixed $value + * @return mixed + */ + private function getLowercaseValue($value) + { + return is_string($value) ? strtolower($value) : $value; } } diff --git a/src/Params/DisallowedCallParamValueExcept.php b/src/Params/DisallowedCallParamValueExcept.php index 5a2814a..cce22a7 100644 --- a/src/Params/DisallowedCallParamValueExcept.php +++ b/src/Params/DisallowedCallParamValueExcept.php @@ -3,7 +3,6 @@ namespace Spaze\PHPStan\Rules\Disallowed\Params; -use PHPStan\Type\ConstantScalarType; use PHPStan\Type\Type; use Spaze\PHPStan\Rules\Disallowed\Exceptions\UnsupportedParamTypeException; @@ -18,10 +17,10 @@ class DisallowedCallParamValueExcept extends DisallowedCallParamValue */ public function matches(Type $type): bool { - if (!$type instanceof ConstantScalarType) { + if (!$type->isConstantScalarValue()->yes()) { throw new UnsupportedParamTypeException(); } - return $this->getValue() !== $type->getValue(); + return !in_array($this->getValue(), $type->getConstantScalarValues(), true); } } diff --git a/src/Params/DisallowedCallParamValueSpecific.php b/src/Params/DisallowedCallParamValueSpecific.php index b824ad8..ebd2818 100644 --- a/src/Params/DisallowedCallParamValueSpecific.php +++ b/src/Params/DisallowedCallParamValueSpecific.php @@ -3,7 +3,6 @@ namespace Spaze\PHPStan\Rules\Disallowed\Params; -use PHPStan\Type\ConstantScalarType; use PHPStan\Type\Type; use Spaze\PHPStan\Rules\Disallowed\Exceptions\UnsupportedParamTypeException; @@ -18,10 +17,10 @@ class DisallowedCallParamValueSpecific extends DisallowedCallParamValue */ public function matches(Type $type): bool { - if (!$type instanceof ConstantScalarType) { + if (!$type->isConstantScalarValue()->yes()) { throw new UnsupportedParamTypeException(); } - return $this->getValue() === $type->getValue(); + return [$this->getValue()] === $type->getConstantScalarValues(); } } diff --git a/src/RuleErrors/DisallowedMethodRuleErrors.php b/src/RuleErrors/DisallowedMethodRuleErrors.php index 6a83ba8..f32b5ee 100644 --- a/src/RuleErrors/DisallowedMethodRuleErrors.php +++ b/src/RuleErrors/DisallowedMethodRuleErrors.php @@ -12,7 +12,6 @@ use PHPStan\Analyser\Scope; use PHPStan\Rules\RuleError; use PHPStan\ShouldNotHappenException; -use PHPStan\Type\TypeWithClassName; use Spaze\PHPStan\Rules\Disallowed\DisallowedCall; use Spaze\PHPStan\Rules\Disallowed\Type\TypeResolver; @@ -50,7 +49,14 @@ public function get($class, CallLike $node, Scope $scope, array $disallowedCalls $calledOnType = $this->typeResolver->getType($class, $scope); if ($calledOnType->canCallMethods()->yes() && $calledOnType->hasMethod($node->name->name)->yes()) { $method = $calledOnType->getMethod($node->name->name, $scope); - $calledAs = ($calledOnType instanceof TypeWithClassName ? $this->disallowedRuleErrors->getFullyQualified($calledOnType->getClassName(), $method) : null); + $classNames = $calledOnType->getObjectClassNames(); + if (count($classNames) === 0) { + $calledAs = null; + } elseif (count($classNames) === 1) { + $calledAs = $this->disallowedRuleErrors->getFullyQualified($classNames[0], $method); + } else { + $calledAs = $this->disallowedRuleErrors->getFullyQualified('{' . implode(',', $classNames) . '}', $method); + } foreach ($method->getDeclaringClass()->getTraits() as $trait) { if ($trait->hasMethod($method->getName())) { diff --git a/src/Usages/ClassConstantUsages.php b/src/Usages/ClassConstantUsages.php index 7e18c1c..b059cac 100644 --- a/src/Usages/ClassConstantUsages.php +++ b/src/Usages/ClassConstantUsages.php @@ -12,7 +12,6 @@ use PHPStan\Rules\RuleErrorBuilder; use PHPStan\ShouldNotHappenException; use PHPStan\Type\Constant\ConstantStringType; -use PHPStan\Type\TypeWithClassName; use PHPStan\Type\VerbosityLevel; use Spaze\PHPStan\Rules\Disallowed\DisallowedConstant; use Spaze\PHPStan\Rules\Disallowed\DisallowedConstantFactory; @@ -84,9 +83,14 @@ public function processNode(Node $node, Scope $scope): array return []; } - $displayName = ($usedOnType instanceof TypeWithClassName ? $this->getFullyQualified($usedOnType->getClassName(), $constant) : null); - if ($usedOnType instanceof ConstantStringType) { - $className = ltrim($usedOnType->getValue(), '\\'); + $displayName = $usedOnType->getObjectClassNames() ? $this->getFullyQualified($usedOnType->getObjectClassNames(), $constant) : null; + if ($usedOnType->getConstantStrings()) { + $classNames = array_map( + function (ConstantStringType $constantString): string { + return ltrim($constantString->getValue(), '\\'); + }, + $usedOnType->getConstantStrings() + ); } else { if ($usedOnType->hasConstant($constant)->no()) { return [ @@ -97,18 +101,22 @@ public function processNode(Node $node, Scope $scope): array ))->build(), ]; } else { - $className = $usedOnType->getConstant($constant)->getDeclaringClass()->getDisplayName(); + $classNames = [$usedOnType->getConstant($constant)->getDeclaringClass()->getDisplayName()]; } } - $constant = $this->getFullyQualified($className, $constant); - - return $this->disallowedConstantRuleErrors->get($constant, $scope, $displayName, $this->disallowedConstants); + return $this->disallowedConstantRuleErrors->get($this->getFullyQualified($classNames, $constant), $scope, $displayName, $this->disallowedConstants); } - private function getFullyQualified(string $class, string $constant): string + /** + * @param non-empty-list $classNames + * @param string $constant + * @return string + */ + private function getFullyQualified(array $classNames, string $constant): string { - return "{$class}::{$constant}"; + $className = count($classNames) === 1 ? $classNames[0] : '{' . implode(',', $classNames) . '}'; + return $className . '::' . $constant; } } diff --git a/tests/Usages/ClassConstantUsagesTest.php b/tests/Usages/ClassConstantUsagesTest.php index 361290a..06e8e73 100644 --- a/tests/Usages/ClassConstantUsagesTest.php +++ b/tests/Usages/ClassConstantUsagesTest.php @@ -132,7 +132,7 @@ public function testRule(): void 23, ], [ - 'Using PhpOption\Option::NAME is forbidden, no PhpOption', + 'Using PhpOption\Option::NAME (as PhpOption\None::NAME) is forbidden, no PhpOption', 37, ], ]);