From 753627d107fdfeadef078fcc003ac222b69401c1 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 20 May 2024 02:05:38 +0900 Subject: [PATCH] Remove RequireEnumDocBlockOnConstantListPassRule as reports many false positive, better shift to native enums (#119) * Remove RequireEnumDocBlockOnConstantListPassRule as reports many false positive, better shift to native enums * remove related services --- config/services/services.neon | 2 - docs/rules_overview.md | 63 +----- ...hodCallClassConstFetchPositionResolver.php | 57 ------ src/Reflection/MethodCallNodeAnalyzer.php | 57 ------ ...uireEnumDocBlockOnConstantListPassRule.php | 193 ------------------ .../Fixture/ClassWithoutParamEnumType.php | 19 -- .../Fixture/ExternalNoType.php | 17 -- .../Fixture/OnePositionCovered.php | 22 -- .../Fixture/SkipClassConstReference.php | 16 -- .../Fixture/SkipExternalCarWithType.php | 16 -- .../Fixture/SkipMoreStrings.php | 23 --- .../Fixture/SkipParameterProvider.php | 15 -- .../Fixture/SkipSelfReference.php | 17 -- .../Fixture/SkipWithEnumLikeType.php | 22 -- ...EnumDocBlockOnConstantListPassRuleTest.php | 61 ------ .../Source/Direction.php | 12 -- .../Source/ExternalCarWithType.php | 14 -- .../Source/ExternalCarWithoutType.php | 11 - .../Source/ParameterName.php | 10 - .../config/configured_rule.neon | 5 - 20 files changed, 1 insertion(+), 651 deletions(-) delete mode 100644 src/NodeAnalyzer/MethodCall/MethodCallClassConstFetchPositionResolver.php delete mode 100644 src/Reflection/MethodCallNodeAnalyzer.php delete mode 100644 src/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule.php delete mode 100644 tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/ClassWithoutParamEnumType.php delete mode 100644 tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/ExternalNoType.php delete mode 100644 tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/OnePositionCovered.php delete mode 100644 tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipClassConstReference.php delete mode 100644 tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipExternalCarWithType.php delete mode 100644 tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipMoreStrings.php delete mode 100644 tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipParameterProvider.php delete mode 100644 tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipSelfReference.php delete mode 100644 tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipWithEnumLikeType.php delete mode 100644 tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/RequireEnumDocBlockOnConstantListPassRuleTest.php delete mode 100644 tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Source/Direction.php delete mode 100644 tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Source/ExternalCarWithType.php delete mode 100644 tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Source/ExternalCarWithoutType.php delete mode 100644 tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Source/ParameterName.php delete mode 100644 tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/config/configured_rule.neon diff --git a/config/services/services.neon b/config/services/services.neon index e27f5439..15bee9ba 100644 --- a/config/services/services.neon +++ b/config/services/services.neon @@ -15,7 +15,6 @@ services: - Symplify\PHPStanRules\Naming\ClassToSuffixResolver - Symplify\PHPStanRules\NodeAnalyzer\AttributeFinder - Symplify\PHPStanRules\NodeAnalyzer\EnumAnalyzer - - Symplify\PHPStanRules\NodeAnalyzer\MethodCall\MethodCallClassConstFetchPositionResolver - Symplify\PHPStanRules\NodeAnalyzer\RegexFuncCallAnalyzer - Symplify\PHPStanRules\NodeAnalyzer\RegexStaticCallAnalyzer - Symplify\PHPStanRules\NodeFinder\ClassMethodNodeFinder @@ -26,7 +25,6 @@ services: - Symplify\PHPStanRules\PhpDoc\BarePhpDocParser - Symplify\PHPStanRules\PhpDoc\PhpDocResolver - Symplify\PHPStanRules\Printer\NodeComparator - - Symplify\PHPStanRules\Reflection\MethodCallNodeAnalyzer - Symplify\PHPStanRules\Reflection\MethodNodeAnalyser - Symplify\PHPStanRules\TypeAnalyzer\CallableTypeAnalyzer - Symplify\PHPStanRules\Matcher\Collector\PublicClassMethodMatcher diff --git a/docs/rules_overview.md b/docs/rules_overview.md index ca3b208c..a6705d3e 100644 --- a/docs/rules_overview.md +++ b/docs/rules_overview.md @@ -1,4 +1,4 @@ -# 44 Rules Overview +# 43 Rules Overview ## AnnotateRegexClassConstWithRegexLinkRule @@ -1398,67 +1398,6 @@ final class SomeAttribute
-## RequireEnumDocBlockOnConstantListPassRule - -On passing a constant, the method should have an enum type. See https://phpstan.org/writing-php-code/phpdoc-types#literals-and-constants - -- class: [`Symplify\PHPStanRules\Rules\Enum\RequireEnumDocBlockOnConstantListPassRule`](../src/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule.php) - -```php -final class Direction -{ - public const LEFT = 'left'; - - public const RIGHT = 'right'; -} - -final class Driver -{ - public function goToWork() - { - $this->turn(Direction::LEFT); - } - - private function turn(string $direction) - { - // ... - } -} -``` - -:x: - -
- -```php -final class Direction -{ - public const LEFT = 'left'; - - public const RIGHT = 'right'; -} - -final class Driver -{ - public function goToWork() - { - $this->turn(Direction::LEFT); - } - - /** - * @param Direction::* - */ - private function turn(string $direction) - { - // ... - } -} -``` - -:+1: - -
- ## RequireExceptionNamespaceRule `Exception` must be located in "Exception" namespace diff --git a/src/NodeAnalyzer/MethodCall/MethodCallClassConstFetchPositionResolver.php b/src/NodeAnalyzer/MethodCall/MethodCallClassConstFetchPositionResolver.php deleted file mode 100644 index ad2756c0..00000000 --- a/src/NodeAnalyzer/MethodCall/MethodCallClassConstFetchPositionResolver.php +++ /dev/null @@ -1,57 +0,0 @@ -getArgs() as $position => $arg) { - if ($arg->value instanceof ClassConstFetch) { - $classConstFetch = $arg->value; - if (! $this->isEnumLikeClassConstFetch($classConstFetch)) { - continue; - } - - if (! is_int($position)) { - throw new ShouldNotHappenException(); - } - - $argPositions[] = $position; - } - } - - return $argPositions; - } - - private function isEnumLikeClassConstFetch(ClassConstFetch $classConstFetch): bool - { - if (! $classConstFetch->name instanceof Identifier) { - return false; - } - - $classConstName = $classConstFetch->name; - if ($classConstFetch->class instanceof Expr) { - return false; - } - - if ($classConstFetch->class->toString() === 'self') { - return false; - } - - return $classConstName->toString() !== 'class'; - } -} diff --git a/src/Reflection/MethodCallNodeAnalyzer.php b/src/Reflection/MethodCallNodeAnalyzer.php deleted file mode 100644 index 053c77d9..00000000 --- a/src/Reflection/MethodCallNodeAnalyzer.php +++ /dev/null @@ -1,57 +0,0 @@ -resolveCallerClassReflection($scope, $methodCall); - - if (! $callerClassReflection instanceof ClassReflection) { - return null; - } - - if (! $methodCall->name instanceof Identifier) { - return null; - } - - $methodName = $methodCall->name->toString(); - - $extendedMethodReflection = $callerClassReflection->getMethod($methodName, $scope); - if (! $extendedMethodReflection instanceof PhpMethodReflection) { - return null; - } - - return $extendedMethodReflection; - } - - private function resolveCallerClassReflection(Scope $scope, MethodCall $methodCall): ?ClassReflection - { - $callerType = $scope->getType($methodCall->var); - if (! $callerType instanceof TypeWithClassName) { - return null; - } - - if ($callerType instanceof StaticType) { - $callerType = $callerType->getStaticObjectType(); - } - - if (! $callerType instanceof ObjectType) { - return null; - } - - return $callerType->getClassReflection(); - } -} diff --git a/src/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule.php b/src/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule.php deleted file mode 100644 index 061e696c..00000000 --- a/src/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule.php +++ /dev/null @@ -1,193 +0,0 @@ - - */ -final class RequireEnumDocBlockOnConstantListPassRule implements Rule, DocumentedRuleInterface -{ - /** - * @var string - */ - public const ERROR_MESSAGE = 'On passing a constant, the method should have an enum type. See https://phpstan.org/writing-php-code/phpdoc-types#literals-and-constants'; - - /** - * These types expect constant lists that are not enums - * - * @var string[] - */ - private const SKIPPED_CLASS_TYPES = [ - 'Symplify\PackageBuilder\Parameter\ParameterProvider', - AbstractConfigurator::class, - ParameterBagInterface::class, - ]; - - public function __construct( - private readonly MethodCallNodeAnalyzer $methodCallNodeAnalyzer, - private readonly MethodCallClassConstFetchPositionResolver $methodCallClassConstFetchPositionResolver, - ) { - } - - /** - * @return class-string - */ - public function getNodeType(): string - { - return MethodCall::class; - } - - /** - * @param MethodCall $node - * @return string[] - */ - public function processNode(Node $node, Scope $scope): array - { - // has argument of constant class reference - $classConstFetchArgPositions = $this->methodCallClassConstFetchPositionResolver->resolve($node); - if ($classConstFetchArgPositions === []) { - return []; - } - - $parameterReflections = $this->resolveParameterReflections($node, $scope); - - foreach ($parameterReflections as $position => $parameterReflection) { - // is desired arg position? - if (! in_array($position, $classConstFetchArgPositions, true)) { - continue; - } - - // this must be some more strict type - if ($parameterReflection->getType() instanceof StringType) { - return [self::ERROR_MESSAGE]; - } - } - - return []; - } - - public function getRuleDefinition(): RuleDefinition - { - return new RuleDefinition(self::ERROR_MESSAGE, [ - new CodeSample( - <<<'CODE_SAMPLE' -final class Direction -{ - public const LEFT = 'left'; - - public const RIGHT = 'right'; -} - -final class Driver -{ - public function goToWork() - { - $this->turn(Direction::LEFT); - } - - private function turn(string $direction) - { - // ... - } -} -CODE_SAMPLE - , - <<<'CODE_SAMPLE' -final class Direction -{ - public const LEFT = 'left'; - - public const RIGHT = 'right'; -} - -final class Driver -{ - public function goToWork() - { - $this->turn(Direction::LEFT); - } - - /** - * @param Direction::* - */ - private function turn(string $direction) - { - // ... - } -} -CODE_SAMPLE - ), - ]); - } - - /** - * @return array - */ - private function resolveParameterReflections(MethodCall $methodCall, Scope $scope): array - { - $phpMethodReflection = $this->methodCallNodeAnalyzer->resolveMethodCallReflection($methodCall, $scope); - if (! $phpMethodReflection instanceof PhpMethodReflection) { - return []; - } - - // is skipped type? - if ($this->shouldSkipClass($phpMethodReflection->getDeclaringClass())) { - return []; - } - - $parametersAcceptorWithPhpDocs = ParametersAcceptorSelector::selectSingle($phpMethodReflection->getVariants()); - return $parametersAcceptorWithPhpDocs->getParameters(); - } - - private function shouldSkipClass(ClassReflection $classReflection): bool - { - if ($classReflection->isInternal()) { - return true; - } - - $fileName = $classReflection->getFileName(); - if (! is_string($fileName)) { - return true; - } - - // skip vendor classes, as we cannot change them - if (str_contains($fileName, '/vendor/')) { - return true; - } - - foreach (self::SKIPPED_CLASS_TYPES as $skippedClassType) { - // skip vendor class, as we can't reach it - if ($classReflection->isSubclassOf($skippedClassType)) { - return true; - } - - if ($skippedClassType === $classReflection->getName()) { - return true; - } - } - - return false; - } -} diff --git a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/ClassWithoutParamEnumType.php b/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/ClassWithoutParamEnumType.php deleted file mode 100644 index a789fbc0..00000000 --- a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/ClassWithoutParamEnumType.php +++ /dev/null @@ -1,19 +0,0 @@ -turn(Direction::LEFT); - } - - private function turn(string $direction) - { - } -} diff --git a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/ExternalNoType.php b/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/ExternalNoType.php deleted file mode 100644 index 363511bc..00000000 --- a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/ExternalNoType.php +++ /dev/null @@ -1,17 +0,0 @@ -turn(Direction::LEFT); - } -} diff --git a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/OnePositionCovered.php b/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/OnePositionCovered.php deleted file mode 100644 index fbc11d3a..00000000 --- a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/OnePositionCovered.php +++ /dev/null @@ -1,22 +0,0 @@ -turn(Direction::LEFT, Direction::RIGHT); - } - - /** - * @param Direction::* $direction - */ - private function turn(string $direction, string $otherDirection) - { - } -} diff --git a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipClassConstReference.php b/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipClassConstReference.php deleted file mode 100644 index d04b38f1..00000000 --- a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipClassConstReference.php +++ /dev/null @@ -1,16 +0,0 @@ -turn(Direction::class); - } -} diff --git a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipExternalCarWithType.php b/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipExternalCarWithType.php deleted file mode 100644 index ca942c56..00000000 --- a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipExternalCarWithType.php +++ /dev/null @@ -1,16 +0,0 @@ -turn(Direction::LEFT); - } -} diff --git a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipMoreStrings.php b/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipMoreStrings.php deleted file mode 100644 index 405849cd..00000000 --- a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipMoreStrings.php +++ /dev/null @@ -1,23 +0,0 @@ -turn(Direction::LEFT, 'three'); - } - - /** - * @param Direction::* $direction - */ - private function turn(string $direction, string $gear) - { - } -} diff --git a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipParameterProvider.php b/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipParameterProvider.php deleted file mode 100644 index 6f91eaf7..00000000 --- a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipParameterProvider.php +++ /dev/null @@ -1,15 +0,0 @@ -provideParameter(ParameterName::SOURCE); - } -} diff --git a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipSelfReference.php b/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipSelfReference.php deleted file mode 100644 index c0ac2ef3..00000000 --- a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipSelfReference.php +++ /dev/null @@ -1,17 +0,0 @@ -turn(self::DIRECTION_LEFT); - } -} diff --git a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipWithEnumLikeType.php b/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipWithEnumLikeType.php deleted file mode 100644 index 218985f4..00000000 --- a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Fixture/SkipWithEnumLikeType.php +++ /dev/null @@ -1,22 +0,0 @@ -turn(Direction::LEFT); - } - - /** - * @param Direction::* $direction - */ - private function turn(string $direction) - { - } -} diff --git a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/RequireEnumDocBlockOnConstantListPassRuleTest.php b/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/RequireEnumDocBlockOnConstantListPassRuleTest.php deleted file mode 100644 index a45e670b..00000000 --- a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/RequireEnumDocBlockOnConstantListPassRuleTest.php +++ /dev/null @@ -1,61 +0,0 @@ -analyse([$filePath], $expectedErrorsWithLines); - } - - public static function provideData(): Iterator - { - yield [__DIR__ . '/Fixture/SkipWithEnumLikeType.php', []]; - yield [__DIR__ . '/Fixture/SkipMoreStrings.php', []]; - yield [__DIR__ . '/Fixture/SkipExternalCarWithType.php', []]; - yield [__DIR__ . '/Fixture/SkipClassConstReference.php', []]; - yield [__DIR__ . '/Fixture/SkipSelfReference.php', []]; - yield [__DIR__ . '/Fixture/SkipParameterProvider.php', []]; - - yield [ - __DIR__ . '/Fixture/ClassWithoutParamEnumType.php', - [[RequireEnumDocBlockOnConstantListPassRule::ERROR_MESSAGE, 13]], - ]; - - yield [ - __DIR__ . '/Fixture/OnePositionCovered.php', - [[RequireEnumDocBlockOnConstantListPassRule::ERROR_MESSAGE, 13]], - ]; - - yield [ - __DIR__ . '/Fixture/ExternalNoType.php', - [[RequireEnumDocBlockOnConstantListPassRule::ERROR_MESSAGE, 15]], - ]; - } - - /** - * @return string[] - */ - public static function getAdditionalConfigFiles(): array - { - return [__DIR__ . '/config/configured_rule.neon']; - } - - protected function getRule(): Rule - { - return self::getContainer()->getByType(RequireEnumDocBlockOnConstantListPassRule::class); - } -} diff --git a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Source/Direction.php b/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Source/Direction.php deleted file mode 100644 index 73c7f7ea..00000000 --- a/tests/Rules/Enum/RequireEnumDocBlockOnConstantListPassRule/Source/Direction.php +++ /dev/null @@ -1,12 +0,0 @@ -