Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 15 PHPStan rules from Handyman, mostly Symfony, Doctrine and PHPUnit related #153

Merged
merged 6 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading