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
  • Loading branch information
ebln committed Jun 2, 2024
1 parent af4322f commit 35f29b0
Show file tree
Hide file tree
Showing 16 changed files with 440 additions and 8 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/buildTest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ jobs:
./vendor/bin/phpmd src text codesize,controversial,design,naming,unusedcode,design
- name: Mess Detector Tests
run: |
./vendor/bin/phpmd tests text codesize,controversial,design
./vendor/bin/phpmd tests text codesize,controversial,naming,unusedcode
- 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
1 change: 1 addition & 0 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ parameters:
excludePaths:
analyseAndScan:
- tests/data/*
- tests/dataAttrib/*

rules:
- \Ebln\PHPStan\EnforceFactory\ForceFactoryRule
35 changes: 29 additions & 6 deletions src/ForceFactoryRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ public function processNode(Node $node, Scope $scope): array
private function getAllowedFactories(string $className): ?array
{
$allowedFactories = $this->getFactoriesFromAttribute($className);

if (is_a($className, ForceFactoryInterface::class, true)) {
/* phpstan-var class-string<ForceFactoryInterface> $className */
$interfaceFactories = $className::getFactories();
Expand All @@ -100,21 +99,45 @@ private function getFactoriesFromAttribute(string $className): ?array
if (\PHP_VERSION_ID < 80000 && $this->reflectionProvider->hasClass($className)) {
return null;
}
/** @psalm-suppress UndefinedDocblockClass */
$reflection = $this->reflectionProvider->getClass($className)->getNativeReflection();

$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();
sort($allowedFactories);

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

return null;
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());
}
}
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();
}
}
109 changes: 109 additions & 0 deletions tests/dataAttrib/ForcedFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
<?php

declare(strict_types=1);

namespace Test\Ebln\PHPStan\EnforceFactory\dataAttrib;

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

class ForcedFactory
{
public function class(): ForcedFactoryProduct
{
return new ForcedFactoryProduct();
}

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

$new = new $class();
}

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

$new = new $class();
}

public function variableMixed(bool $toggle): void
{
if ($toggle) {
$class = '\Test\Ebln\PHPStan\EnforceFactory\dataAttrib\code\ForcedFactoryProduct';
} else {
$class = ForcedFactoryProduct::class;
}

$new = new $class();
}

public function variableMixedProducts(bool $toggle): void
{
if ($toggle) {
$class = '\Test\Ebln\PHPStan\EnforceFactory\dataAttrib\code\ForcedFactoryProduct';
} else {
$class = FreeProduct::class;
}

$new = new $class();
}

// TODO needs to fail!
public function variableUnpredictable(bool $toggle): void
{
if ($toggle) {
$class = '\Test\Ebln\PHPStan\EnforceFactory\dataAttrib\code\ForcedFactoryProduct';
} else {
$class = 'Hello world-' . random_int(10, 99);
}

$new = new $class();
}

public function anonymousExtending(): void
{
$x = new class() extends ForcedFactoryProduct
{
public function foo(): string
{
return 'bar';
}
};

$bar = $x->foo();
}

public function anonymousExtendingSquare(): void
{
$x = new class() extends ExtendedProduct
{
public function foo(): string
{
return 'bar';
}
};

$bar = $x->foo();
}

public function anonymousPassing(): void
{
$x = new class()
{
public function foo(): string
{
return 'bar';
}
};

$bar = $x->foo();
}

public static function staticClass(): ForcedFactoryProduct
{
return new ForcedFactoryProduct();
}
}
19 changes: 19 additions & 0 deletions tests/dataAttrib/LoopholeFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?php

declare(strict_types=1);

namespace Test\Ebln\PHPStan\EnforceFactory\dataAttrib;

class LoopholeFactory
{
public function variableUninferable(bool $toggle): void
{
if ($toggle) {
$class = '\Test\Ebln\PHPStan\EnforceFactory\dataAttrib\code\ForcedFactoryProduct';
} else {
$class = 'Hello world-' . random_int(10, 99);
}

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

0 comments on commit 35f29b0

Please sign in to comment.