From 3716fead7a2911f7a3cd44be74b724eee72fbdfb Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 29 May 2024 22:02:28 +0900 Subject: [PATCH] Remove NoMissingDirPathRule, as often not clear if target file or required file path --- config/static-rules.neon | 1 - docs/rules_overview.md | 396 +----------------- .../FileCheckingFuncCallAnalyzer.php | 33 -- .../FlatConcatFindingNodeVisitor.php | 63 --- src/Rules/NoMissingDirPathRule.php | 144 ------- .../Fixture/FileMissing.php | 13 - .../Fixture/SkipAssertMethod.php | 24 -- ...SkipBracketPathFromSymfonyConfigImport.php | 13 - .../Fixture/SkipConcat.php | 13 - .../Fixture/SkipExistingFile.php | 13 - .../Fixture/SkipFileExistsFuncCall.php | 15 - .../SkipFileExistsFuncCallOneLayerAbove.php | 15 - .../Fixture/SkipFnMatch.php | 13 - .../Fixture/SkipVendorAutoload.php | 13 - .../NoMissingDirPathRuleTest.php | 51 --- .../config/configured_rule.neon | 5 - 16 files changed, 1 insertion(+), 824 deletions(-) delete mode 100644 src/NodeAnalyzer/FileCheckingFuncCallAnalyzer.php delete mode 100644 src/NodeVisitor/FlatConcatFindingNodeVisitor.php delete mode 100644 src/Rules/NoMissingDirPathRule.php delete mode 100644 tests/Rules/NoMissingDirPathRule/Fixture/FileMissing.php delete mode 100644 tests/Rules/NoMissingDirPathRule/Fixture/SkipAssertMethod.php delete mode 100644 tests/Rules/NoMissingDirPathRule/Fixture/SkipBracketPathFromSymfonyConfigImport.php delete mode 100644 tests/Rules/NoMissingDirPathRule/Fixture/SkipConcat.php delete mode 100644 tests/Rules/NoMissingDirPathRule/Fixture/SkipExistingFile.php delete mode 100644 tests/Rules/NoMissingDirPathRule/Fixture/SkipFileExistsFuncCall.php delete mode 100644 tests/Rules/NoMissingDirPathRule/Fixture/SkipFileExistsFuncCallOneLayerAbove.php delete mode 100644 tests/Rules/NoMissingDirPathRule/Fixture/SkipFnMatch.php delete mode 100644 tests/Rules/NoMissingDirPathRule/Fixture/SkipVendorAutoload.php delete mode 100644 tests/Rules/NoMissingDirPathRule/NoMissingDirPathRuleTest.php delete mode 100644 tests/Rules/NoMissingDirPathRule/config/configured_rule.neon diff --git a/config/static-rules.neon b/config/static-rules.neon index 8f254f14..9618f7e1 100644 --- a/config/static-rules.neon +++ b/config/static-rules.neon @@ -8,7 +8,6 @@ rules: - Symplify\PHPStanRules\Rules\PreventParentMethodVisibilityOverrideRule # paths - - Symplify\PHPStanRules\Rules\NoMissingDirPathRule - Symplify\PHPStanRules\Rules\NoReferenceRule - Symplify\PHPStanRules\Rules\NoNullableArrayPropertyRule diff --git a/docs/rules_overview.md b/docs/rules_overview.md index 8922269f..a0fdd202 100644 --- a/docs/rules_overview.md +++ b/docs/rules_overview.md @@ -1,4 +1,4 @@ -# 38 Rules Overview +# 28 Rules Overview ## AnnotateRegexClassConstWithRegexLinkRule @@ -99,53 +99,6 @@ interface ProductRepositoryInterface
-## CheckTypehintCallerTypeRule - -Parameter %d should use "%s" type as the only type passed to this method - -- class: [`Symplify\PHPStanRules\Rules\CheckTypehintCallerTypeRule`](../src/Rules/CheckTypehintCallerTypeRule.php) - -```php -use PhpParser\Node; -use PhpParser\Node\Expr\MethodCall; - -class SomeClass -{ - public function run(MethodCall $node) - { - $this->isCheck($node); - } - - private function isCheck(Node $node) - { - } -} -``` - -:x: - -
- -```php -use PhpParser\Node\Expr\MethodCall; - -class SomeClass -{ - public function run(MethodCall $node) - { - $this->isCheck($node); - } - - private function isCheck(MethodCall $node) - { - } -} -``` - -:+1: - -
- ## ClassNameRespectsParentSuffixRule Class should have suffix "%s" to respect parent type @@ -428,134 +381,6 @@ return strlen('...');
-## ForbiddenParamTypeRemovalRule - -Removing parent param type is forbidden - -- class: [`Symplify\PHPStanRules\Rules\ForbiddenParamTypeRemovalRule`](../src/Rules/ForbiddenParamTypeRemovalRule.php) - -```php -interface RectorInterface -{ - public function refactor(Node $node); -} - -final class SomeRector implements RectorInterface -{ - public function refactor($node) - { - } -} -``` - -:x: - -
- -```php -interface RectorInterface -{ - public function refactor(Node $node); -} - -final class SomeRector implements RectorInterface -{ - public function refactor(Node $node) - { - } -} -``` - -:+1: - -
- -## NarrowPublicClassMethodParamTypeByCallerTypeRule - -Parameters should use "%s" types as the only types passed to this method - -- class: [`Symplify\PHPStanRules\Rules\NarrowType\NarrowPublicClassMethodParamTypeByCallerTypeRule`](../src/Rules/NarrowType/NarrowPublicClassMethodParamTypeByCallerTypeRule.php) - -```php -use PhpParser\Node\Expr\MethodCall; - -final class SomeClass -{ - public function run(SomeService $someService, MethodCall $methodCall) - { - $someService->isCheck($node); - } -} - -final class SomeService -{ - public function isCheck($methodCall) - { - } -} -``` - -:x: - -
- -```php -use PhpParser\Node\Expr\MethodCall; - -final class SomeClass -{ - public function run(SomeService $someService, MethodCall $methodCall) - { - $someService->isCheck($node); - } -} - -final class SomeService -{ - public function isCheck(MethodCall $methodCall) - { - } -} -``` - -:+1: - -
- -## NoArrayAccessOnObjectRule - -Use explicit methods over array access on object - -- class: [`Symplify\PHPStanRules\Rules\NoArrayAccessOnObjectRule`](../src/Rules/NoArrayAccessOnObjectRule.php) - -```php -class SomeClass -{ - public function run(MagicArrayObject $magicArrayObject) - { - return $magicArrayObject['more_magic']; - } -} -``` - -:x: - -
- -```php -class SomeClass -{ - public function run(MagicArrayObject $magicArrayObject) - { - return $magicArrayObject->getExplicitValue(); - } -} -``` - -:+1: - -
- ## NoDuplicatedShortClassNameRule Class with base "%s" name is already used in "%s". Use unique name to make classes easy to recognize @@ -684,131 +509,6 @@ class SomeClass
-## NoIssetOnObjectRule - -Use default null value and nullable compare instead of isset on object - -- class: [`Symplify\PHPStanRules\Rules\NoIssetOnObjectRule`](../src/Rules/NoIssetOnObjectRule.php) - -```php -class SomeClass -{ - public function run() - { - if (random_int(0, 1)) { - $object = new SomeClass(); - } - - if (isset($object)) { - return $object; - } - } -} -``` - -:x: - -
- -```php -class SomeClass -{ - public function run() - { - $object = null; - if (random_int(0, 1)) { - $object = new SomeClass(); - } - - if ($object !== null) { - return $object; - } - } -} -``` - -:+1: - -
- -## NoMissingDirPathRule - -The path "%s" was not found - -- class: [`Symplify\PHPStanRules\Rules\NoMissingDirPathRule`](../src/Rules/NoMissingDirPathRule.php) - -```php -$filePath = __DIR__ . '/missing_location.txt'; -``` - -:x: - -
- -```php -$filePath = __DIR__ . '/existing_location.txt'; -``` - -:+1: - -
- -## NoMixedMethodCallerRule - -Anonymous variable in a `%s->...()` method call can lead to false dead methods. Make sure the variable type is known - -- class: [`Symplify\PHPStanRules\Rules\Explicit\NoMixedMethodCallerRule`](../src/Rules/Explicit/NoMixedMethodCallerRule.php) - -```php -function run($unknownType) -{ - return $unknownType->call(); -} -``` - -:x: - -
- -```php -function run(KnownType $knownType) -{ - return $knownType->call(); -} -``` - -:+1: - -
- -## NoMixedPropertyFetcherRule - -Anonymous variables in a "%s->..." property fetch can lead to false dead property. Make sure the variable type is known - -- class: [`Symplify\PHPStanRules\Rules\Explicit\NoMixedPropertyFetcherRule`](../src/Rules/Explicit/NoMixedPropertyFetcherRule.php) - -```php -function run($unknownType) -{ - return $unknownType->name; -} -``` - -:x: - -
- -```php -function run(KnownType $knownType) -{ - return $knownType->name; -} -``` - -:+1: - -
- ## NoNullableArrayPropertyRule Use required typed property over of nullable array property @@ -904,58 +604,6 @@ final class ReturnVariables
-## NoReturnFalseInNonBoolClassMethodRule - -Returning false in non return bool class method. Use null instead - -- class: [`Symplify\PHPStanRules\Rules\NarrowType\NoReturnFalseInNonBoolClassMethodRule`](../src/Rules/NarrowType/NoReturnFalseInNonBoolClassMethodRule.php) - -```php -class SomeClass -{ - /** - * @var Item[] - */ - private $items = []; - - public function getItem($key) - { - if (isset($this->items[$key])) { - return $this->items[$key]; - } - - return false; - } -} -``` - -:x: - -
- -```php -class SomeClass -{ - /** - * @var Item[] - */ - private $items = []; - - public function getItem($key): ?Item - { - if (isset($this->items[$key])) { - return $this->items[$key]; - } - - return null; - } -} -``` - -:+1: - -
- ## NoReturnSetterMethodRule Setter method cannot return anything, only set value @@ -1309,48 +957,6 @@ final class SomeController extends AbstractController
-## RequireSpecificReturnTypeOverAbstractRule - -Provide more specific return type "%s" over abstract one - -- class: [`Symplify\PHPStanRules\Rules\Explicit\RequireSpecificReturnTypeOverAbstractRule`](../src/Rules/Explicit/RequireSpecificReturnTypeOverAbstractRule.php) - -```php -final class IssueControlFactory -{ - public function create(): Control - { - return new IssueControl(); - } -} - -final class IssueControl extends Control -{ -} -``` - -:x: - -
- -```php -final class IssueControlFactory -{ - public function create(): IssueControl - { - return new IssueControl(); - } -} - -final class IssueControl extends Control -{ -} -``` - -:+1: - -
- ## RequireUniqueEnumConstantRule Enum constants "%s" are duplicated. Make them unique instead diff --git a/src/NodeAnalyzer/FileCheckingFuncCallAnalyzer.php b/src/NodeAnalyzer/FileCheckingFuncCallAnalyzer.php deleted file mode 100644 index ed1b0011..00000000 --- a/src/NodeAnalyzer/FileCheckingFuncCallAnalyzer.php +++ /dev/null @@ -1,33 +0,0 @@ -cond instanceof FuncCall) { - return false; - } - - $funcCallCond = $node->cond; - if (! $funcCallCond->name instanceof Name) { - return false; - } - - $funcCallName = $funcCallCond->name->toString(); - - return in_array($funcCallName, ['is_file', 'file_exists', 'is_dir'], true); - } -} diff --git a/src/NodeVisitor/FlatConcatFindingNodeVisitor.php b/src/NodeVisitor/FlatConcatFindingNodeVisitor.php deleted file mode 100644 index 3271c703..00000000 --- a/src/NodeVisitor/FlatConcatFindingNodeVisitor.php +++ /dev/null @@ -1,63 +0,0 @@ -foundNodes = []; - return null; - } - - public function enterNode(Node $node): int|Node|null - { - if ($this->fileCheckingFuncCallAnalyzer->isFileExistCheck($node)) { - return NodeTraverser::DONT_TRAVERSE_CHILDREN; - } - - if (! $node instanceof Concat) { - return null; - } - - if ($node->left instanceof Concat) { - return NodeTraverser::DONT_TRAVERSE_CHILDREN; - } - - if ($node->right instanceof Concat) { - return NodeTraverser::DONT_TRAVERSE_CURRENT_AND_CHILDREN; - } - - $this->foundNodes[] = $node; - return null; - } - - /** - * @return Concat[] - */ - public function getFoundNodes(): array - { - return $this->foundNodes; - } -} diff --git a/src/Rules/NoMissingDirPathRule.php b/src/Rules/NoMissingDirPathRule.php deleted file mode 100644 index acf8ef63..00000000 --- a/src/Rules/NoMissingDirPathRule.php +++ /dev/null @@ -1,144 +0,0 @@ - - */ - public function getNodeType(): string - { - return InClassNode::class; - } - - /** - * @param InClassNode $node - * @return RuleError[] - */ - public function processNode(Node $node, Scope $scope): array - { - $classLike = $node->getOriginalNode(); - - $classReflection = $scope->getClassReflection(); - if (! $classReflection instanceof ClassReflection) { - return []; - } - - // test fixture can exist or not, better skip this case to avoid false positives - if ($classReflection->isSubclassOf(TestCase::class)) { - return []; - } - - // mimics node finding visitors of NodeFinder with ability to stop traversing deeper - $nodeTraverser = new NodeTraverser(); - $flatConcatFindingNodeVisitor = new FlatConcatFindingNodeVisitor(new FileCheckingFuncCallAnalyzer()); - - $nodeTraverser->addVisitor($flatConcatFindingNodeVisitor); - $nodeTraverser->traverse($classLike->stmts); - - $concats = $flatConcatFindingNodeVisitor->getFoundNodes(); - $errorMessages = []; - - foreach ($concats as $concat) { - if (! $concat->left instanceof Dir) { - return []; - } - - if (! $concat->right instanceof String_) { - return []; - } - - $string = $concat->right; - $relativeDirPath = $string->value; - - if ($this->shouldSkip($relativeDirPath)) { - continue; - } - - $realDirectory = dirname($scope->getFile()); - $fileRealPath = $realDirectory . $relativeDirPath; - - if (file_exists($fileRealPath)) { - continue; - } - - $errorMessages[] = RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $relativeDirPath)) - ->line($concat->getLine()) - ->build(); - } - - return $errorMessages; - } - - public function getRuleDefinition(): RuleDefinition - { - return new RuleDefinition(self::ERROR_MESSAGE, [ - new CodeSample( - <<<'CODE_SAMPLE' -$filePath = __DIR__ . '/missing_location.txt'; -CODE_SAMPLE - , - <<<'CODE_SAMPLE' -$filePath = __DIR__ . '/existing_location.txt'; -CODE_SAMPLE - ), - ]); - } - - private function shouldSkip(string $relativeDirPath): bool - { - // is vendor autolaod? it yet to be exist - if (Strings::match($relativeDirPath, self::VENDOR_REGEX)) { - return true; - } - - if (\str_contains($relativeDirPath, '*')) { - return true; - } - - $bracketMatches = Strings::match($relativeDirPath, self::BRACKET_PATH_REGEX); - return $bracketMatches !== null; - } -} diff --git a/tests/Rules/NoMissingDirPathRule/Fixture/FileMissing.php b/tests/Rules/NoMissingDirPathRule/Fixture/FileMissing.php deleted file mode 100644 index 6910d282..00000000 --- a/tests/Rules/NoMissingDirPathRule/Fixture/FileMissing.php +++ /dev/null @@ -1,13 +0,0 @@ -assertFileExists(__DIR__ . '/../PotentialFile.php'); - $this->assertFileNotExists(__DIR__ . '/../../PotentialFile.php'); - - if (method_exists($this, 'assertFileDoesNotExist')) { - $this->assertFileDoesNotExist(__DIR__ . '/temp/file.php'); - } else { - $this->assertFileNotExists(__DIR__ . '/temp/file.php'); - } - - $possibleFile = __DIR__ . '/Whatever'; - } -} diff --git a/tests/Rules/NoMissingDirPathRule/Fixture/SkipBracketPathFromSymfonyConfigImport.php b/tests/Rules/NoMissingDirPathRule/Fixture/SkipBracketPathFromSymfonyConfigImport.php deleted file mode 100644 index f3bede11..00000000 --- a/tests/Rules/NoMissingDirPathRule/Fixture/SkipBracketPathFromSymfonyConfigImport.php +++ /dev/null @@ -1,13 +0,0 @@ -analyse([$filePath], $expectedErrorMessagesWithLines); - } - - public static function provideData(): Iterator - { - $message = sprintf(NoMissingDirPathRule::ERROR_MESSAGE, '/not_here.php'); - yield [__DIR__ . '/Fixture/FileMissing.php', [[$message, 11]]]; - - yield [__DIR__ . '/Fixture/SkipBracketPathFromSymfonyConfigImport.php', []]; - yield [__DIR__ . '/Fixture/SkipConcat.php', []]; - yield [__DIR__ . '/Fixture/SkipVendorAutoload.php', []]; - yield [__DIR__ . '/Fixture/SkipVendorAutoload.php', []]; - yield [__DIR__ . '/Fixture/SkipAssertMethod.php', []]; - yield [__DIR__ . '/Fixture/SkipFnMatch.php', []]; - yield [__DIR__ . '/Fixture/SkipFileExistsFuncCall.php', []]; - yield [__DIR__ . '/Fixture/SkipFileExistsFuncCallOneLayerAbove.php', []]; - } - - /** - * @return string[] - */ - public static function getAdditionalConfigFiles(): array - { - return [__DIR__ . '/config/configured_rule.neon']; - } - - protected function getRule(): Rule - { - return self::getContainer()->getByType(NoMissingDirPathRule::class); - } -} diff --git a/tests/Rules/NoMissingDirPathRule/config/configured_rule.neon b/tests/Rules/NoMissingDirPathRule/config/configured_rule.neon deleted file mode 100644 index fa682f0c..00000000 --- a/tests/Rules/NoMissingDirPathRule/config/configured_rule.neon +++ /dev/null @@ -1,5 +0,0 @@ -includes: - - ../../../config/included_services.neon - -rules: - - Symplify\PHPStanRules\Rules\NoMissingDirPathRule