Skip to content

Commit

Permalink
Add pure tests & subsequent fix
Browse files Browse the repository at this point in the history
- ExtendedProduct wasn't resolved and needed extra TLC

TODO: Tests for mixed occurrences of Interface and Attributes

Add attributes from spike - yet without tests
  • Loading branch information
ebln committed Jun 2, 2024
1 parent 5f428a2 commit 2496167
Show file tree
Hide file tree
Showing 18 changed files with 533 additions and 12 deletions.
5 changes: 2 additions & 3 deletions .github/workflows/buildTest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,10 @@ jobs:
./vendor/bin/phpstan --no-interaction --no-ansi analyse
- name: Mess Detector Sources
run: |
./vendor/bin/phpmd src text codesize,controversial,design,naming,unusedcode,design
./vendor/bin/phpmd src text codesize,controversial,naming,unusedcode
- name: Mess Detector Tests
run: |
./vendor/bin/phpmd tests text codesize,controversial,design
./vendor/bin/phpmd src text codesize,controversial,naming,unusedcode
/vendor/bin/phpmd tests text codesize,controversial,design
- name: php-cs-fixer
run: |
./tools/php-cs-fixer/vendor/bin/php-cs-fixer fix -v --config=.php-cs-fixer.dist.php --using-cache=no --dry-run
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
"psalm --find-unused-psalm-suppress",
"phpstan analyse",
"@style-check",
"phpmd src ansi codesize,controversial,design,naming,unusedcode,design",
"phpmd src ansi codesize,controversial,naming,unusedcode",
"phpmd tests ansi codesize,controversial,design"
],
"style-check": "php-cs-fixer fix -v --config=.php-cs-fixer.dist.php --using-cache=no --dry-run",
Expand Down
8 changes: 8 additions & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,11 @@ parameters:
paths:
- src
- tests

excludePaths:
analyseAndScan:
- tests/data/*
- tests/dataAttrib/*

rules:
- \Ebln\PHPStan\EnforceFactory\ForceFactoryRule
36 changes: 36 additions & 0 deletions src/ForceFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php

declare(strict_types=1);

namespace Ebln\PHPStan\EnforceFactory;

/**
* Marks classes to be instanciated by certain factories
*
* If used together with ForceFactoryInterface
* the configured factories must be congruent!
* This is enforced for PHP 8 and later.
*/
#[\Attribute(\Attribute::TARGET_CLASS)]
class ForceFactory
{
/** @var array<int, class-string> */
private array $allowedFactories;

/** @param class-string ...$factories */
public function __construct(string ...$factories)
{
$allowedFactories = [];
foreach ($factories as $factory) {
$allowedFactories[$factory] = $factory;
}

$this->allowedFactories = array_values($allowedFactories);
}

/** @return array<int, class-string> */
public function getAllowedFactories(): array
{
return $this->allowedFactories;
}
}
82 changes: 76 additions & 6 deletions src/ForceFactoryRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\ReflectionProvider;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;

Expand All @@ -14,6 +15,13 @@
*/
class ForceFactoryRule implements Rule
{
private ReflectionProvider $reflectionProvider;

public function __construct(ReflectionProvider $reflectionProvider)
{
$this->reflectionProvider = $reflectionProvider;
}

public function getNodeType(): string
{
return \PhpParser\Node\Expr\New_::class;
Expand All @@ -26,17 +34,18 @@ public function processNode(Node $node, Scope $scope): array
if (!$isName) {
continue; // newly instantiated class name couldn't be infered
}
/** @var class-string $class → sadly Psalm cannot be convinced to return class-string for getClassNames in reasonable amount of time */
$allowedFactories = $this->getAllowedFactories($class);
if (null === $allowedFactories) {
continue; // newly instantiated class dowsn't implement ForceFactory interface
continue; // newly instantiated class doesn't implement ForceFactory interface
}

if (empty($allowedFactories)) {
if ([] === $allowedFactories) {
$errors[] = RuleErrorBuilder::message(
ltrim($class, '\\') . ' cannot be instantiated by other classes; see ' . ForceFactoryInterface::class
ltrim($class, '\\') . ' has either no factories defined or a conflict between interface and attribute!'
)->build();

continue;
continue; // bogus configuration
}

/** @psalm-suppress PossiblyNullReference | sad that even phpstan cannot infer that from isInClass */
Expand All @@ -57,17 +66,78 @@ public function processNode(Node $node, Scope $scope): array
}

/**
* @phpstan-param class-string $className
*
* @return null|string[] List of FQCNs
*
* @phpstan-return null|class-string[]
*/
private function getAllowedFactories(string $className): ?array
{
if (!is_a($className, ForceFactoryInterface::class, true)) {
$allowedFactories = $this->getFactoriesFromAttribute($className);
if (is_a($className, ForceFactoryInterface::class, true)) {
/* phpstan-var class-string<ForceFactoryInterface> $className */
$interfaceFactories = $className::getFactories();
sort($interfaceFactories);
if (null === $allowedFactories) {
$allowedFactories = $interfaceFactories;
} elseif ($allowedFactories !== $interfaceFactories) {
$allowedFactories = []; // Will result in a bogus definition error
}
}

return $allowedFactories;
}

/**
* @phpstan-param class-string $className
*
* @return array<class-string>
*/
private function getFactoriesFromAttribute(string $className): ?array
{
if (\PHP_VERSION_ID < 80000 && $this->reflectionProvider->hasClass($className)) {
return null;
}

return $className::getFactories();
$reflection = $this->reflectionProvider->getClass($className);
/* psalm-suppress UndefinedClass */
$allowedFactories = [];
do {
/** @psalm-suppress UndefinedClass */
$allowedFactories = [...$allowedFactories, ...$this->getFactoriesFromAttributeByClass($reflection->getNativeReflection())];
} while ($reflection = $reflection->getParentClass());

if (empty($allowedFactories)) {
return null;
}
$allowedFactories = array_filter($allowedFactories);
sort($allowedFactories);

return $allowedFactories;
}

/**
* @psalm-suppress UndefinedDocblockClass,MismatchingDocblockParamType
*
* @psalm-param \PHPStan\BetterReflection\Reflection\Adapter\ReflectionClass|\PHPStan\BetterReflection\Reflection\Adapter\ReflectionEnum $reflection
*
* @return array<int, null|class-string>
*/
private function getFactoriesFromAttributeByClass(\ReflectionClass $reflection): array
{
/** @psalm-suppress UndefinedClass */
foreach ($reflection->getAttributes() as $attribute) {
if (ForceFactory::class === $attribute->getName()) {
/** @var ForceFactory $forceFactory */
$forceFactory = $attribute->newInstance();
$allowedFactories = $forceFactory->getAllowedFactories();

return empty($allowedFactories) ? [null] : $allowedFactories;
}
}

return [];
}

/**
Expand Down
73 changes: 73 additions & 0 deletions tests/AttribForceFactoryRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<?php

declare(strict_types=1);

namespace Test\Ebln\PHPStan\EnforceFactory;

use Ebln\PHPStan\EnforceFactory\ForceFactoryRule;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;

/**
* @requires PHP >= 8.0
* @extends RuleTestCase<ForceFactoryRule>
*/
class AttribForceFactoryRuleTest extends RuleTestCase
{
private const ERROR_MESSAGE = 'Test\Ebln\PHPStan\EnforceFactory\dataAttrib\code\ForcedFactoryProduct must be instantiated by Test\Ebln\PHPStan\EnforceFactory\dataAttrib\ForcedFactory or Test\Ebln\PHPStan\EnforceFactory\dataAttrib\TraitFactory!';

// Sadly this remains a vector, as phpstan fails to infer the created class name
public function testLoopholeFactory(): void
{
$this->analyse([__DIR__ . '/dataAttrib/LoopholeFactory.php'], []);
}

public function testEmptyAllowedClasses(): void
{
$this->analyse([__DIR__ . '/dataAttrib/EmptyFactory.php'], [
[
'Test\Ebln\PHPStan\EnforceFactory\dataAttrib\code\EmptyProduct has either no factories defined or a conflict between interface and attribute!',
13,
],
]);
}

public function testRogueFactory(): void
{
$this->analyse([__DIR__ . '/dataAttrib/RogueFactory.php'], [
[self::ERROR_MESSAGE, 15],
[self::ERROR_MESSAGE, 22],
[self::ERROR_MESSAGE, 29],
[self::ERROR_MESSAGE, 40],
[self::ERROR_MESSAGE, 40],
[self::ERROR_MESSAGE, 51],
[self::ERROR_MESSAGE, 56],
['Test\Ebln\PHPStan\EnforceFactory\dataAttrib\code\ExtendedProduct must be instantiated by Test\Ebln\PHPStan\EnforceFactory\dataAttrib\ForcedFactory or Test\Ebln\PHPStan\EnforceFactory\dataAttrib\TraitFactory!', 69],
[self::ERROR_MESSAGE, 95],
]);
}

public function testRogueFactoryAndTrait(): void
{
$this->analyse([__DIR__ . '/dataAttrib/RogueTraitFactory.php', __DIR__ . '/dataAttrib/FactoryTrait.php'], [
[self::ERROR_MESSAGE, 13],
[self::ERROR_MESSAGE, 20],
[self::ERROR_MESSAGE, 27],
]);
}

public function testTraitedFactory(): void
{
$this->analyse([__DIR__ . '/dataAttrib/TraitFactory.php', __DIR__ . '/dataAttrib/FactoryTrait.php'], []);
}

public function testAllowedFactory(): void
{
$this->analyse([__DIR__ . '/dataAttrib/ForcedFactory.php'], []);
}

protected function getRule(): Rule
{
return new ForceFactoryRule($this->createReflectionProvider());
}
}
4 changes: 2 additions & 2 deletions tests/ForceFactoryRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public function testEmptyAllowedClasses(): void
{
$this->analyse([__DIR__ . '/data/EmptyFactory.php'], [
[
'Test\Ebln\PHPStan\EnforceFactory\data\code\EmptyProduct cannot be instantiated by other classes; see Ebln\PHPStan\EnforceFactory\ForceFactoryInterface',
'Test\Ebln\PHPStan\EnforceFactory\data\code\EmptyProduct has either no factories defined or a conflict between interface and attribute!',
13,
],
]);
Expand Down Expand Up @@ -67,6 +67,6 @@ public function testAllowedFactory(): void

protected function getRule(): Rule
{
return new ForceFactoryRule();
return new ForceFactoryRule($this->createReflectionProvider());
}
}
15 changes: 15 additions & 0 deletions tests/dataAttrib/EmptyFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

declare(strict_types=1);

namespace Test\Ebln\PHPStan\EnforceFactory\dataAttrib;

use Test\Ebln\PHPStan\EnforceFactory\dataAttrib\code\EmptyProduct;

class EmptyFactory
{
public function class(): EmptyProduct
{
return new EmptyProduct();
}
}
29 changes: 29 additions & 0 deletions tests/dataAttrib/FactoryTrait.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

declare(strict_types=1);

namespace Test\Ebln\PHPStan\EnforceFactory\dataAttrib;

use Test\Ebln\PHPStan\EnforceFactory\dataAttrib\code\ForcedFactoryProduct;

trait FactoryTrait
{
public function traitedClass(): ForcedFactoryProduct
{
return new ForcedFactoryProduct();
}

public function traitedClassVariable(): void
{
$class = ForcedFactoryProduct::class;

$new = new $class();
}

public function traitedStringVariable(): void
{
$class = '\Test\Ebln\PHPStan\EnforceFactory\dataAttrib\code\ForcedFactoryProduct';

$new = new $class();
}
}
Loading

0 comments on commit 2496167

Please sign in to comment.