Skip to content

Commit

Permalink
Add 15 PHPStan rules from Handyman, mostly Symfony, Doctrine and PHPU…
Browse files Browse the repository at this point in the history
…nit related (#153)

* Add 15 phpstan rules from handyman, mostly Symfony, Doctrine and PHPUnit related

* moving symfony and doctrine rules

* improve symfony rules location

* remove abstract symplify rule

* use rule identifiers

* add fixture stubs to make fixture repository rule work
  • Loading branch information
TomasVotruba authored Dec 18, 2024
1 parent b691662 commit 025b86d
Show file tree
Hide file tree
Showing 85 changed files with 2,075 additions and 205 deletions.
28 changes: 27 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@ includes:
- vendor/symplify/phpstan-rules/config/naming-rules.neon
- vendor/symplify/phpstan-rules/config/regex-rules.neon
- vendor/symplify/phpstan-rules/config/static-rules.neon

# project specific
- vendor/symplify/phpstan-rules/config/rector-rules.neon
- vendor/symplify/phpstan-rules/config/doctrine-rules.neon
- vendor/symplify/phpstan-rules/config/symfony-rules.neon
```
<br>
Expand Down Expand Up @@ -876,7 +880,7 @@ Use invokable controller with `__invoke()` method instead of named action method

```yaml
rules:
- Symplify\PHPStanRules\Symfony\Rules\RequireInvokableControllerRule
- Symplify\PHPStanRules\Rules\Symfony\RequireInvokableControllerRule
```
```php
Expand Down Expand Up @@ -1024,6 +1028,28 @@ final class SomeClass

<br>

## Doctrine-specific Rules

### NoGetRepositoryOutsideServiceRule

Instead of getting repository from EntityManager, use constructor injection and service pattern to keep code clean

```yaml
rules:
- Symplify\PHPStanRules\Rules\Doctrine\NoGetRepositoryOutsideServiceRule
```
```php
class SomeClass
{
public function run(EntityManagerInterface $entityManager)
{
return $entityManager->getRepository(SomeEntity::class);
}
}
```


<!-- ruledoc-end -->

<br>
Expand Down
1 change: 0 additions & 1 deletion composer-dependency-analyser.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
return (new Configuration())->addPathToScan(__DIR__ . '/src', false)
->addPathToExclude(__DIR__ . '/tests/Rules/Rector/NoInstanceOfStaticReflectionRule/Fixture')
->addPathToExclude(__DIR__ . '/tests/Rules/Enum/RequireUniqueEnumConstantRule/Fixture')
->addPathToExclude(__DIR__ . '/tests/Rules/NoReturnArrayVariableListRule/Fixture')
->addPathToExclude(__DIR__ . '/tests/Rules/ForbiddenExtendOfNonAbstractClassRule/Fixture')
->addPathToExclude(__DIR__ . '/tests/Rules/PHPUnit/NoTestMocksRule/Fixture')

Expand Down
5 changes: 4 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@
"autoload": {
"psr-4": {
"Symplify\\PHPStanRules\\": "src"
}
},
"files": [
"src/functions/fast-functions.php"
]
},
"autoload-dev": {
"psr-4": {
Expand Down
4 changes: 4 additions & 0 deletions config/doctrine-rules.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
rules:
- Symplify\PHPStanRules\PHPStan\Rules\Doctrine\NoGetRepositoryOutsideServiceRule
- Symplify\PHPStanRules\PHPStan\Rules\Doctrine\NoParentRepositoryRule
- Symplify\PHPStanRules\PHPStan\Rules\Doctrine\NoRepositoryCallInDataFixtureRule
3 changes: 3 additions & 0 deletions config/services/services.neon
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,6 @@ services:
- Symplify\PHPStanRules\PhpDoc\BarePhpDocParser
- Symplify\PHPStanRules\PhpDoc\PhpDocResolver
- Symplify\PHPStanRules\TypeAnalyzer\CallableTypeAnalyzer

# doctrine
- Symplify\PHPStanRules\Doctrine\DoctrineEntityDocumentAnalyser
7 changes: 7 additions & 0 deletions config/symfony-rules.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
rules:
- Symplify\PHPStanRules\Rules\Symfony\NoAbstractControllerConstructorRule
- Symplify\PHPStanRules\Rules\Symfony\NoRequiredOutsideClassRule
- Symplify\PHPStanRules\Rules\Symfony\SingleArgEventDispatchRule
- Symplify\PHPStanRules\Rules\Symfony\NoListenerWithoutContractRule
- Symplify\PHPStanRules\Rules\Symfony\NoStringInGetSubscribedEventsRule
- Symplify\PHPStanRules\Rules\Symfony\RequireInvokableControllerRule
27 changes: 4 additions & 23 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -22,48 +22,29 @@ parameters:
- */stubs/*
- */Fixture/*

# https://github.com/TomasVotruba/unused-public
unused_public:
methods: true
properties: true
constants: true

type_coverage:
return: 99
param: 99
property: 99

ignoreErrors:
# needless generics
- '#Class Symplify\\PHPStanRules\\(.*?)Rule implements generic interface PHPStan\\Rules\\Rule but does not specify its types\: TNodeType#'

- '#Method Symplify\\PHPStanRules\\Reflection\\ReflectionParser\:\:parseNativeClassReflection\(\) has parameter \$reflectionClass with generic class ReflectionClass but does not specify its types\: T#'

# overly detailed
- '#Class Symplify\\PHPStanRules\\(.*?) extends generic class PHPStan\\Testing\\RuleTestCase but does not specify its types\: TRule#'
- '#Method Symplify\\PHPStanRules\\(.*?)\:\:getRule\(\) return type with generic interface PHPStan\\Rules\\Rule does not specify its types\: TNodeType#'
- '#Parameter \#2 \$expectedErrors of method PHPStan\\Testing\\RuleTestCase<PHPStan\\Rules\\Rule>\:\:analyse\(\) expects list<array\{0\: string, 1\: int, 2\?\: string\|null\}>, (.*?) given#'

# part of public contract
- '#Method Symplify\\PHPStanRules\\Tests\\Rules\\PHPUnit\\(.*?)\\(.*?)Test\:\:testRule\(\) has parameter \$expectedErrorMessagesWithLines with no value type specified in iterable type array#'
- '#Method Symplify\\PHPStanRules\\Tests\\Rules\\(.*?)\\(.*?)Test\:\:testRule\(\) has parameter \$(expectedError(.*?)|expectedErrors) 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#'

# useful to have IDE know the types
- identifier: phpstanApi.instanceofType

# overly detailed
-
message: '#Parameter \#2 \$expectedErrors of method PHPStan\\Testing\\RuleTestCase<(.*?)>::analyse\(\) expects list<array\{0: string, 1: int, 2\?: string\|null\}>, (.*?) given#'
path: tests

# fast effective check
-
message: '#Function is_a\(\) is a runtime reflection concept that might not work in PHPStan because it uses fully static reflection engine#'
path: src/Rules/SeeAnnotationToTestRule.php

# handle next
- '#Method Symplify\\PHPStanRules\\Rules\\AbstractSymplifyRule\:\:processNode\(\) should return list<PHPStan\\Rules\\IdentifierRuleError> but returns array<PHPStan\\Rules\\RuleError\|string>#'

# used in tests
- '#Public constant "Symplify\\PHPStanRules\\(.*?)Rule\:\:ERROR_MESSAGE" is never used#'

- '#Although PHPStan\\Node\\InClassNode is covered by backward compatibility promise, this instanceof assumption might break because (.*?) not guaranteed to always stay the same#'
1 change: 1 addition & 0 deletions phpunit.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,6 @@
<testsuite name="all">
<directory>tests</directory>
<exclude>tests/Rules/ClassNameRespectsParentSuffixRule/Fixture/</exclude>
<exclude>tests/Rules/PHPUnit/PublicStaticDataProviderRule</exclude>
</testsuite>
</phpunit>
25 changes: 0 additions & 25 deletions src/Contract/ManyNodeRuleInterface.php

This file was deleted.

32 changes: 32 additions & 0 deletions src/Doctrine/DoctrineEntityDocumentAnalyser.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?php

declare(strict_types=1);

namespace Symplify\PHPStanRules\Doctrine;

use PHPStan\PhpDoc\ResolvedPhpDocBlock;
use PHPStan\Reflection\ClassReflection;

final readonly class DoctrineEntityDocumentAnalyser
{
/**
* @var string[]
*/
private const ENTITY_DOCBLOCK_MARKERS = ['@Document', '@ORM\\Document', '@Entity', '@ORM\\Entity'];

public static function isEntityClass(ClassReflection $classReflection): bool
{
$resolvedPhpDocBlock = $classReflection->getResolvedPhpDoc();
if (! $resolvedPhpDocBlock instanceof ResolvedPhpDocBlock) {
return false;
}

foreach (self::ENTITY_DOCBLOCK_MARKERS as $entityDocBlockMarkers) {
if (str_contains($resolvedPhpDocBlock->getPhpDocString(), $entityDocBlockMarkers)) {
return true;
}
}

return false;
}
}
22 changes: 21 additions & 1 deletion src/Enum/ClassName.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ final class ClassName
/**
* @var string
*/
public const EVENT_DISPATCHER_INTERFACE = 'Symfony\Component\EventDispatcher\EventSubscriberInterface';
public const EVENT_DISPATCHER_INTERFACE = 'Symfony\Component\EventDispatcher\EventDispatcherInterface';

/**
* @var string
*/
public const EVENT_SUBSCRIBER_INTERFACE = 'Symfony\Component\EventDispatcher\EventSubscriberInterface';

/**
* @var string
Expand All @@ -50,4 +55,19 @@ final class ClassName
* @var string
*/
public const RECTOR_ATTRIBUTE_KEY = 'Rector\NodeTypeResolver\Node\AttributeKey';

/**
* @var string
*/
public const FORM_EVENTS = 'Symfony\Component\Form\FormEvents';

/**
* @var string
*/
public const SYMFONY_CONTROLLER = 'Symfony\Bundle\FrameworkBundle\Controller\Controller';

/**
* @var string
*/
public const DOCTRINE_FIXTURE_INTERFACE = 'Doctrine\Common\DataFixtures\FixtureInterface';
}
39 changes: 39 additions & 0 deletions src/Enum/RuleIdentifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -160,4 +160,43 @@ final class RuleIdentifier
* @var string
*/
public const NO_VALUE_OBJECT_IN_SERVICE_CONSTRUCTOR = 'symplify.noValueObjectInServiceConstructor';

/**
* @var string
*/
public const DOCTRINE_NO_REPOSITORY_CALL_IN_DATA_FIXTURES = 'doctrine.noRepositoryCallInDataFixtures';

/**
* @var string
*/
public const PHPUNIT_NO_DOCUMENT_MOCKING = 'phpunit.noDocumentMocking';

/**
* @var string
*/
public const NO_DYNAMIC_NAME = 'symplify.noDynamicName';

public const NO_REFERENCE = 'symplify.noReference';

public const PHPUNIT_NO_MOCK_ONLY = 'phpunit.noMockOnly';

public const SINGLE_ARG_EVENT_DISPATCH = 'symfony.singleArgEventDispatch';

public const NO_ENTITY_MOCKING = 'doctrine.noEntityMocking';

public const NO_STRING_IN_GET_SUBSCRIBED_EVENTS = 'symfony.noStringInGetSubscribedEvents';

public const NO_LISTENER_WITHOUT_CONTRACT = 'symfony.noListenerWithoutContract';

public const DOCTRINE_NO_PARENT_REPOSITORY = 'doctrine.noParentRepository';

public const DOCTRINE_NO_GET_REPOSITORY_OUTSIDE_SERVICE = 'doctrine.noGetRepositoryOutsideService';

public const SYMFONY_NO_REQUIRED_OUTSIDE_CLASS = 'symfony.noRequiredOutsideClass';

public const NO_CONSTRUCTOR_OVERRIDE = 'symplify.noConstructorOverride';

public const SYMFONY_NO_ABSTRACT_CONTROLLER_CONSTRUCTOR = 'symfony.noAbstractControllerConstructor';

public const PHPUNIT_PUBLIC_STATIC_DATA_PROVIDER = 'phpunit.publicStaticDataProvider';
}
65 changes: 65 additions & 0 deletions src/PHPStan/Rules/Doctrine/NoGetRepositoryOutsideServiceRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
<?php

declare(strict_types=1);

namespace Symplify\PHPStanRules\PHPStan\Rules\Doctrine;

use PhpParser\Node;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Identifier;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use Symplify\PHPStanRules\Enum\RuleIdentifier;

/**
* @see \Symplify\PHPStanRules\Tests\Rules\Doctrine\NoGetRepositoryOutsideServiceRule\NoGetRepositoryOutsideServiceRuleTest
*
* @implements Rule<MethodCall>
*/
final class NoGetRepositoryOutsideServiceRule implements Rule
{
/**
* @var string
*/
public const ERROR_MESSAGE = 'Instead of getting repository from EntityManager, use constructor injection and service pattern to keep code clean';

public function getNodeType(): string
{
return MethodCall::class;
}

/**
* @param MethodCall $node
*/
public function processNode(Node $node, Scope $scope): array
{
if (! $node->name instanceof Identifier) {
return [];
}

if ($node->name->toString() !== 'getRepository') {
return [];
}

if (! $scope->isInClass()) {
$ruleError = RuleErrorBuilder::message(self::ERROR_MESSAGE)
->identifier(RuleIdentifier::DOCTRINE_NO_GET_REPOSITORY_OUTSIDE_SERVICE)
->build();

return [$ruleError];
}

// dummy check
$classReflection = $scope->getClassReflection();
if (str_ends_with($classReflection->getName(), 'Repository')) {
return [];
}

$ruleError = RuleErrorBuilder::message(self::ERROR_MESSAGE)
->identifier(RuleIdentifier::DOCTRINE_NO_GET_REPOSITORY_OUTSIDE_SERVICE)
->build();

return [$ruleError];
}
}
Loading

0 comments on commit 025b86d

Please sign in to comment.