diff --git a/config/code-complexity-rules.neon b/config/code-complexity-rules.neon index 74417803..c006ee0d 100644 --- a/config/code-complexity-rules.neon +++ b/config/code-complexity-rules.neon @@ -1,3 +1,15 @@ rules: - Symplify\PHPStanRules\Rules\NoDynamicNameRule - Symplify\PHPStanRules\Rules\NoReturnArrayVariableListRule + - Symplify\PHPStanRules\Rules\NoSingleInterfaceImplementerRule + +services: + - + class: Symplify\PHPStanRules\Collector\InterfaceCollector + tags: + - phpstan.collector + + - + class: Symplify\PHPStanRules\Collector\ImplementedInterfaceCollector + tags: + - phpstan.collector diff --git a/config/static-rules.neon b/config/static-rules.neon index 7feca6b2..a6c8f4eb 100644 --- a/config/static-rules.neon +++ b/config/static-rules.neon @@ -1,5 +1,6 @@ rules: - Symplify\PHPStanRules\Rules\ForbiddenExtendOfNonAbstractClassRule + - Symplify\PHPStanRules\Rules\NoGlobalConstRule # domain - Symplify\PHPStanRules\Rules\Domain\RequireExceptionNamespaceRule diff --git a/docs/rules_overview.md b/docs/rules_overview.md index a0fdd202..63613e55 100644 --- a/docs/rules_overview.md +++ b/docs/rules_overview.md @@ -1,4 +1,4 @@ -# 28 Rules Overview +# 32 Rules Overview ## AnnotateRegexClassConstWithRegexLinkRule @@ -381,6 +381,40 @@ return strlen('...');
+## ForbiddenStaticClassConstFetchRule + +Avoid static access of constants, as they can change value. Use interface and contract method instead + +- class: [`Symplify\PHPStanRules\Rules\ForbiddenStaticClassConstFetchRule`](../src/Rules/ForbiddenStaticClassConstFetchRule.php) + +```php +class SomeClass +{ + public function run() + { + return static::SOME_CONST; + } +} +``` + +:x: + +
+ +```php +class SomeClass +{ + public function run() + { + return self::SOME_CONST; + } +} +``` + +:+1: + +
+ ## NoDuplicatedShortClassNameRule Class with base "%s" name is already used in "%s". Use unique name to make classes easy to recognize @@ -470,6 +504,70 @@ class SomeClass
+## NoEntityOutsideEntityNamespaceRule + +Class with #[Entity] attribute must be located in "Entity" namespace to be loaded by Doctrine + +- class: [`Symplify\PHPStanRules\Rules\NoEntityOutsideEntityNamespaceRule`](../src/Rules/NoEntityOutsideEntityNamespaceRule.php) + +```php +namespace App\ValueObject; + +use Doctrine\ORM\Mapping as ORM; + +#[ORM\Entity] +class Product +{ +} +``` + +:x: + +
+ +```php +namespace App\Entity; + +use Doctrine\ORM\Mapping as ORM; + +#[ORM\Entity] +class Product +{ +} +``` + +:+1: + +
+ +## NoGlobalConstRule + +Global constants are forbidden. Use enum-like class list instead + +- class: [`Symplify\PHPStanRules\Rules\NoGlobalConstRule`](../src/Rules/NoGlobalConstRule.php) + +```php +const SOME_GLOBAL_CONST = 'value'; +``` + +:x: + +
+ +```php +class SomeClass +{ + public function run() + { + return self::SOME_CONST; + } +} +``` + +:+1: + +
+ ## NoInlineStringRegexRule Use local named constant instead of inline string for regex to explain meaning by constant name @@ -642,6 +740,44 @@ final class SomeClass
+## NoSingleInterfaceImplementerRule + +Interface "%s" has only single implementer. Consider using the class directly as there is no point in using the interface. + +- class: [`Symplify\PHPStanRules\Rules\NoSingleInterfaceImplementerRule`](../src/Rules/NoSingleInterfaceImplementerRule.php) + +```php +class SomeClass implements SomeInterface +{ +} + +interface SomeInterface +{ +} +``` + +:x: + +
+ +```php +class SomeClass implements SomeInterface +{ +} + +class AnotherClass implements SomeInterface +{ +} + +interface SomeInterface +{ +} +``` + +:+1: + +
+ ## NoTestMocksRule Mocking "%s" class is forbidden. Use direct/anonymous class instead for better static analysis diff --git a/phpstan.neon b/phpstan.neon index 208b2d1e..d18bc2ca 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -10,8 +10,6 @@ parameters: - config - tests - # reportUnmatchedIgnoredErrors: false - excludePaths: # parallel - packages/*-phpstan-printer/tests/*ToPhpCompiler/Fixture* @@ -23,20 +21,13 @@ parameters: ignoreErrors: - - message: '#Generator expects value type array\|string>, array>\|string> given#' - paths: - - tests/Rules - - - - message: '#Generator expects value type array\|string>, array>\|string> given#' + message: '#Generator expects value type (.*?) given#' paths: - tests/Rules # needless generics - '#Class Symplify\\PHPStanRules\\(.*?)Rule implements generic interface PHPStan\\Rules\\Rule but does not specify its types\: TNodeType#' - - '#Parameter \#1 \$values of method Symplify\\PHPStanRules\\Rules\\Enum\\RequireUniqueEnumConstantRule\:\:filterDuplicatedValues\(\) expects array, array given#' - - '#Class PHP_CodeSniffer\\Sniffs\\Sniff not found#' - '#Method Symplify\\PHPStanRules\\Reflection\\ReflectionParser\:\:parseNativeClassReflection\(\) has parameter \$reflectionClass with generic class ReflectionClass but does not specify its types\: T#' @@ -49,3 +40,6 @@ parameters: # part of public contract - '#Method Symplify\\PHPStanRules\\Tests\\Rules\\PHPUnit\\(.*?)\\(.*?)Test\:\:testRule\(\) has parameter \$expectedErrorMessagesWithLines with no value type specified in iterable type array#' + + # overly detailed + - '#Class Symplify\\PHPStanRules\\Collector\\(.*?) implements generic interface PHPStan\\Collectors\\Collector but does not specify its types\: TNodeType, TValue#' diff --git a/src/Collector/ImplementedInterfaceCollector.php b/src/Collector/ImplementedInterfaceCollector.php new file mode 100644 index 00000000..5b97a12e --- /dev/null +++ b/src/Collector/ImplementedInterfaceCollector.php @@ -0,0 +1,33 @@ +implements as $implement) { + $implementedInterfaceNames[] = $implement->toString(); + } + + return $implementedInterfaceNames; + } +} diff --git a/src/Collector/InterfaceCollector.php b/src/Collector/InterfaceCollector.php new file mode 100644 index 00000000..04fc1aa8 --- /dev/null +++ b/src/Collector/InterfaceCollector.php @@ -0,0 +1,31 @@ +namespacedName instanceof Name) { + return null; + } + + return $node->namespacedName->toString(); + } +} diff --git a/src/Reflection/ReflectionParser.php b/src/Reflection/ReflectionParser.php index 6aa8b217..668ccbca 100644 --- a/src/Reflection/ReflectionParser.php +++ b/src/Reflection/ReflectionParser.php @@ -7,7 +7,6 @@ use Nette\Utils\FileSystem; use PhpParser\Node\Stmt\ClassLike; use PhpParser\Node\Stmt\ClassMethod; -use PhpParser\Node\Stmt\Property; use PhpParser\NodeTraverser; use PhpParser\NodeVisitor\NameResolver; use PhpParser\Parser; @@ -16,13 +15,9 @@ use PHPStan\Reflection\MethodReflection; use ReflectionClass; use ReflectionMethod; -use ReflectionProperty; use Symplify\PHPStanRules\NodeFinder\TypeAwareNodeFinder; use Throwable; -/** - * @api - */ final class ReflectionParser { /** @@ -49,26 +44,6 @@ public function parseMethodReflection(ReflectionMethod|MethodReflection $reflect return $classLike->getMethod($reflectionMethod->getName()); } - public function parsePropertyReflection(ReflectionProperty $reflectionProperty): ?Property - { - $classLike = $this->parseNativeClassReflection($reflectionProperty->getDeclaringClass()); - if (! $classLike instanceof ClassLike) { - return null; - } - - return $classLike->getProperty($reflectionProperty->getName()); - } - - public function parseClassReflection(ClassReflection $classReflection): ?ClassLike - { - $fileName = $classReflection->getFileName(); - if ($fileName === null) { - return null; - } - - return $this->parseFilenameToClass($fileName); - } - private function parseNativeClassReflection(ReflectionClass|ClassReflection $reflectionClass): ?ClassLike { $fileName = $reflectionClass->getFileName(); diff --git a/src/Rules/Enum/RequireUniqueEnumConstantRule.php b/src/Rules/Enum/RequireUniqueEnumConstantRule.php index d57dbca5..49d722b0 100644 --- a/src/Rules/Enum/RequireUniqueEnumConstantRule.php +++ b/src/Rules/Enum/RequireUniqueEnumConstantRule.php @@ -93,8 +93,8 @@ class SomeClass extends Enum } /** - * @param array $values - * @return array + * @param array $values + * @return array */ private function filterDuplicatedValues(array $values): array { diff --git a/src/Rules/ForbiddenStaticClassConstFetchRule.php b/src/Rules/ForbiddenStaticClassConstFetchRule.php index 06a75e14..aae53e98 100644 --- a/src/Rules/ForbiddenStaticClassConstFetchRule.php +++ b/src/Rules/ForbiddenStaticClassConstFetchRule.php @@ -9,7 +9,6 @@ use PhpParser\Node\Name; use PHPStan\Analyser\Scope; use PHPStan\Rules\Rule; -use Symplify\RuleDocGenerator\Contract\ConfigurableRuleInterface; use Symplify\RuleDocGenerator\Contract\DocumentedRuleInterface; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -17,7 +16,7 @@ /** * @see \Symplify\PHPStanRules\Tests\Rules\ForbiddenStaticClassConstFetchRule\ForbiddenStaticClassConstFetchRuleTest */ -final class ForbiddenStaticClassConstFetchRule implements Rule, DocumentedRuleInterface, ConfigurableRuleInterface +final class ForbiddenStaticClassConstFetchRule implements Rule, DocumentedRuleInterface { /** * @var string diff --git a/src/Rules/NoEntityOutsideEntityNamespaceRule.php b/src/Rules/NoEntityOutsideEntityNamespaceRule.php new file mode 100644 index 00000000..44983726 --- /dev/null +++ b/src/Rules/NoEntityOutsideEntityNamespaceRule.php @@ -0,0 +1,101 @@ + + */ +final class NoEntityOutsideEntityNamespaceRule implements Rule, DocumentedRuleInterface +{ + /** + * @var string + */ + public const ERROR_MESSAGE = 'Class with #[Entity] attribute must be located in "Entity" namespace to be loaded by Doctrine'; + + public function getNodeType(): string + { + return Class_::class; + } + + /** + * @param Class_ $node + */ + public function processNode(Node $node, Scope $scope): array + { + if (! $this->hasEntityAttribute($node)) { + return []; + } + + // we need a namespace to check + if (! $node->namespacedName instanceof Name) { + return []; + } + + $namespaceParts = $node->namespacedName->getParts(); + + if (in_array('Entity', $namespaceParts, true)) { + return []; + } + + return [self::ERROR_MESSAGE]; + } + + public function getRuleDefinition(): RuleDefinition + { + return new RuleDefinition( + self::ERROR_MESSAGE, + [ + new CodeSample( + <<<'CODE_SAMPLE' +namespace App\ValueObject; + +use Doctrine\ORM\Mapping as ORM; + +#[ORM\Entity] +class Product +{ +} +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +namespace App\Entity; + +use Doctrine\ORM\Mapping as ORM; + +#[ORM\Entity] +class Product +{ +} +CODE_SAMPLE + )] + ); + } + + private function hasEntityAttribute(Class_ $class): bool + { + foreach ($class->attrGroups as $attrGroup) { + foreach ($attrGroup->attrs as $attr) { + if ($attr->name->toString() === 'Doctrine\ORM\Mapping\Entity') { + return true; + } + + if ($attr->name->toString() === 'Doctrine\ORM\Mapping\Embeddable') { + return true; + } + } + } + + return false; + } +} diff --git a/src/Rules/NoGlobalConstRule.php b/src/Rules/NoGlobalConstRule.php new file mode 100644 index 00000000..043f56d9 --- /dev/null +++ b/src/Rules/NoGlobalConstRule.php @@ -0,0 +1,59 @@ + + * @see \Symplify\PHPStanRules\Tests\Rules\NoGlobalConstRule\NoGlobalConstRuleTest + */ +final class NoGlobalConstRule implements Rule, DocumentedRuleInterface +{ + /** + * @var string + */ + public const ERROR_MESSAGE = 'Global constants are forbidden. Use enum-like class list instead'; + + public function getNodeType(): string + { + return Const_::class; + } + + /** + * @param Const_ $node + */ + public function processNode(Node $node, Scope $scope): array + { + return [self::ERROR_MESSAGE]; + } + + public function getRuleDefinition(): RuleDefinition + { + return new RuleDefinition(self::ERROR_MESSAGE, [ + new CodeSample( + <<<'CODE_SAMPLE' +const SOME_GLOBAL_CONST = 'value'; +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +class SomeClass +{ + public function run() + { + return self::SOME_CONST; + } +} +CODE_SAMPLE + ), + ]); + } +} diff --git a/src/Rules/NoSingleInterfaceImplementerRule.php b/src/Rules/NoSingleInterfaceImplementerRule.php new file mode 100644 index 00000000..d1518720 --- /dev/null +++ b/src/Rules/NoSingleInterfaceImplementerRule.php @@ -0,0 +1,123 @@ +get(ImplementedInterfaceCollector::class)); + $interfaces = Arrays::flatten($node->get(InterfaceCollector::class)); + + $onceUsedInterfaces = $this->resolveOnceUsedInterfaces($implementedInterfaces); + + $onceImplementedInterfaces = array_intersect($onceUsedInterfaces, $interfaces); + if ($onceImplementedInterfaces === []) { + return []; + } + + $errorMessages = []; + foreach ($onceImplementedInterfaces as $onceImplementedInterface) { + $interfaceReflection = $this->reflectionProvider->getClass($onceImplementedInterface); + + // most likely internal + if ($interfaceReflection->getFileName() === null) { + continue; + } + + $errorMessages[] = RuleErrorBuilder::message(sprintf(self::ERROR_MESSAGE, $onceImplementedInterface)) + ->file($interfaceReflection->getFileName()) + ->build(); + } + + return $errorMessages; + } + + public function getRuleDefinition(): RuleDefinition + { + return new RuleDefinition(self::ERROR_MESSAGE, [ + new CodeSample( + <<<'CODE_SAMPLE' +class SomeClass implements SomeInterface +{ +} + +interface SomeInterface +{ +} +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +class SomeClass implements SomeInterface +{ +} + +class AnotherClass implements SomeInterface +{ +} + +interface SomeInterface +{ +} +CODE_SAMPLE + ), + ]); + } + + /** + * @param string[] $implementedInterfaces + * @return string[] + */ + private function resolveOnceUsedInterfaces(array $implementedInterfaces): array + { + $onceUsedInterfaces = []; + + $implementedInterfacesToCount = array_count_values($implementedInterfaces); + foreach ($implementedInterfacesToCount as $interfaceName => $countUsed) { + if ($countUsed !== 1) { + continue; + } + + $onceUsedInterfaces[] = $interfaceName; + } + + return $onceUsedInterfaces; + } +} diff --git a/tests/Rules/NoGlobalConstRule/Fixture/SomeGlobalConst.php b/tests/Rules/NoGlobalConstRule/Fixture/SomeGlobalConst.php new file mode 100644 index 00000000..b5980352 --- /dev/null +++ b/tests/Rules/NoGlobalConstRule/Fixture/SomeGlobalConst.php @@ -0,0 +1,7 @@ +analyse([$filePath], $expectedErrorMessagesWithLines); + } + + public static function provideData(): Iterator + { + yield [__DIR__ . '/Fixture/SomeGlobalConst.php', [ + [NoGlobalConstRule::ERROR_MESSAGE, 7], + ]]; + } + + /** + * @return string[] + */ + public static function getAdditionalConfigFiles(): array + { + return [__DIR__ . '/config/configured_rule.neon']; + } + + protected function getRule(): Rule + { + return self::getContainer()->getByType(NoGlobalConstRule::class); + } +} diff --git a/tests/Rules/NoGlobalConstRule/config/configured_rule.neon b/tests/Rules/NoGlobalConstRule/config/configured_rule.neon new file mode 100644 index 00000000..4b1dd907 --- /dev/null +++ b/tests/Rules/NoGlobalConstRule/config/configured_rule.neon @@ -0,0 +1,5 @@ +includes: + - ../../../config/included_services.neon + +rules: + - Symplify\PHPStanRules\Rules\NoGlobalConstRule diff --git a/tests/Rules/NoSingleInterfaceImplementerRule/Fixture/ImplementsSimpleInterface.php b/tests/Rules/NoSingleInterfaceImplementerRule/Fixture/ImplementsSimpleInterface.php new file mode 100644 index 00000000..5126d954 --- /dev/null +++ b/tests/Rules/NoSingleInterfaceImplementerRule/Fixture/ImplementsSimpleInterface.php @@ -0,0 +1,9 @@ + + */ +final class NoSingleInterfaceImplementerRuleTest extends RuleTestCase +{ + /** + * @param string[] $filePaths + * @param mixed[] $expectedErrorMessagesWithLines + */ + #[DataProvider('provideData')] + public function testRule(array $filePaths, array $expectedErrorMessagesWithLines): void + { + $this->analyse($filePaths, $expectedErrorMessagesWithLines); + } + + public static function provideData(): Iterator + { + yield [[__DIR__ . '/Fixture/SimpleInterface.php'], []]; + + yield [ + [ + __DIR__ . '/Fixture/SimpleInterface.php', + __DIR__ . '/Fixture/ImplementsSimpleInterface.php', + ], [ + [ + sprintf(NoSingleInterfaceImplementerRule::ERROR_MESSAGE, SimpleInterface::class), + -1, + ], + ]]; + } + + /** + * @return string[] + */ + public static function getAdditionalConfigFiles(): array + { + return [__DIR__ . '/config/configured_rule.neon']; + } + + protected function getCollectors(): array + { + return [ + self::getContainer()->getByType(ImplementedInterfaceCollector::class), + self::getContainer()->getByType(InterfaceCollector::class), + ]; + } + + protected function getRule(): Rule + { + return self::getContainer()->getByType(NoSingleInterfaceImplementerRule::class); + } +} diff --git a/tests/Rules/NoSingleInterfaceImplementerRule/Source/SomeType.php b/tests/Rules/NoSingleInterfaceImplementerRule/Source/SomeType.php new file mode 100644 index 00000000..f0033281 --- /dev/null +++ b/tests/Rules/NoSingleInterfaceImplementerRule/Source/SomeType.php @@ -0,0 +1,7 @@ +