Skip to content

Commit

Permalink
Merge pull request #5 from fabio-ivona/fix-builtin-functions-not-dete…
Browse files Browse the repository at this point in the history
…cted

[fix] core expressions not detected
  • Loading branch information
nunomaduro authored Aug 21, 2023
2 parents 527ff87 + 7c1a045 commit 042b351
Show file tree
Hide file tree
Showing 11 changed files with 234 additions and 50 deletions.
5 changes: 5 additions & 0 deletions pint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"notPath": [
"tests/Fixtures/Misc/HasNativeFunctions.php"
]
}
16 changes: 12 additions & 4 deletions src/Blueprint.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,12 @@
use Pest\Arch\Repositories\ObjectsRepository;
use Pest\Arch\Support\AssertLocker;
use Pest\Arch\Support\Composer;
use Pest\Arch\Support\PhpCoreExpressions;
use Pest\Arch\ValueObjects\Dependency;
use Pest\Arch\ValueObjects\Targets;
use Pest\Arch\ValueObjects\Violation;
use Pest\TestSuite;
use PhpParser\Node\Expr;
use PhpParser\Node\Name;
use PHPUnit\Architecture\ArchitectureAsserts;
use PHPUnit\Architecture\Elements\ObjectDescription;
Expand Down Expand Up @@ -227,14 +229,20 @@ private function getUsagePathAndLines(Layer $layer, string $objectName, string $
/** @var ObjectDescription $dependOnObject */
$dependOnObject = array_pop($dependOnObjects);

$names = ServiceContainer::$nodeFinder->findInstanceOf(
$class = PhpCoreExpressions::getClass($target) ?? Name::class;

$nodes = ServiceContainer::$nodeFinder->findInstanceOf(
$dependOnObject->stmts,
Name::class,
$class,
);

/** @var array<int, Name> $names */
/** @var array<int, Name|Expr> $nodes */
$names = array_values(array_filter(
$names, static fn (Name $name): bool => $name->toString() === $target, // @phpstan-ignore-line
$nodes, static function ($node) use ($target): bool {
$name = $node instanceof Name ? $node->toString() : PhpCoreExpressions::getName($node);

return $name === $target;
}
));

if ($names === []) {
Expand Down
9 changes: 5 additions & 4 deletions src/Factories/ObjectDescriptionFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

namespace Pest\Arch\Factories;

use Pest\Arch\Objects\ObjectDescription;
use Pest\Arch\Objects\VendorObjectDescription;
use PHPUnit\Architecture\Asserts\Dependencies\Elements\ObjectUses;
use PHPUnit\Architecture\Elements\ObjectDescription;
use PHPUnit\Architecture\Services\ServiceContainer;
use ReflectionClass;
use ReflectionFunction;
Expand All @@ -24,7 +24,7 @@ final class ObjectDescriptionFactory
/**
* Makes a new Object Description instance, is possible.
*/
public static function make(string $filename, bool $onlyUserDefinedUses = true): ?ObjectDescription
public static function make(string $filename, bool $onlyUserDefinedUses = true): ?\PHPUnit\Architecture\Elements\ObjectDescription
{
self::ensureServiceContainerIsInitialized();

Expand All @@ -36,7 +36,8 @@ public static function make(string $filename, bool $onlyUserDefinedUses = true):
try {
$object = $isFromVendor
? VendorObjectDescription::make($filename)
: ServiceContainer::$descriptionClass::make($filename);
: ObjectDescription::make($filename);

} finally {
error_reporting($originalErrorReportingLevel);
}
Expand Down Expand Up @@ -88,7 +89,7 @@ interface_exists($use) => (new ReflectionClass($use))->isUserDefined(),
/**
* Checks if the given use is in the same layer as the given object.
*/
private static function isSameLayer(ObjectDescription $object, string $use): bool
private static function isSameLayer(\PHPUnit\Architecture\Elements\ObjectDescription $object, string $use): bool
{
return $use === 'self'
|| $use === 'static'
Expand Down
60 changes: 60 additions & 0 deletions src/Objects/ObjectDescription.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<?php

declare(strict_types=1);

namespace Pest\Arch\Objects;

use Pest\Arch\Support\PhpCoreExpressions;
use PhpParser\Node\Expr;
use PHPUnit\Architecture\Asserts\Dependencies\Elements\ObjectUses;
use PHPUnit\Architecture\Services\ServiceContainer;

/**
* @internal
*/
final class ObjectDescription extends \PHPUnit\Architecture\Elements\ObjectDescription // @phpstan-ignore-line
{
/**
* {@inheritDoc}
*/
public static function make(string $path): ?self
{
/** @var ObjectDescription|null $description */
$description = parent::make($path);

if (! $description instanceof \Pest\Arch\Objects\ObjectDescription) {
return null;
}

$description->uses = new ObjectUses(
[
...$description->uses->getIterator(),
...self::retrieveCoreUses($description),
]
);

return $description;
}

/**
* @return array<int, string>
*/
private static function retrieveCoreUses(ObjectDescription $description): array
{

$expressions = [];

foreach (PhpCoreExpressions::$ENABLED as $expression) {
$expressions = [
...$expressions,
...ServiceContainer::$nodeFinder->findInstanceOf(
$description->stmts,
$expression,
),
];
}

/** @var array<int, Expr> $expressions */
return array_filter(array_map(fn (Expr $expression): string => PhpCoreExpressions::getName($expression), $expressions));
}
}
7 changes: 7 additions & 0 deletions src/Repositories/ObjectsRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Pest\Arch\Factories\ObjectDescriptionFactory;
use Pest\Arch\Objects\FunctionDescription;
use Pest\Arch\Support\Composer;
use Pest\Arch\Support\PhpCoreExpressions;
use Pest\Arch\Support\UserDefinedFunctions;
use PHPUnit\Architecture\Elements\ObjectDescription;
use ReflectionFunction;
Expand Down Expand Up @@ -69,6 +70,12 @@ public static function getInstance(): self
*/
public function allByNamespace(string $namespace, bool $onlyUserDefinedUses = true): array
{
if (PhpCoreExpressions::getClass($namespace) !== null) {
return [
FunctionDescription::make($namespace),
];
}

if (function_exists($namespace) && (new ReflectionFunction($namespace))->getName() === $namespace) {
return [
FunctionDescription::make($namespace),
Expand Down
66 changes: 66 additions & 0 deletions src/Support/PhpCoreExpressions.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<?php

declare(strict_types=1);

namespace Pest\Arch\Support;

use Pest\Exceptions\ShouldNotHappen;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\Clone_;
use PhpParser\Node\Expr\Empty_;
use PhpParser\Node\Expr\Eval_;
use PhpParser\Node\Expr\Exit_;
use PhpParser\Node\Expr\Include_;
use PhpParser\Node\Expr\Isset_;
use PhpParser\Node\Expr\Print_;
use PhpParser\Node\Expr\ShellExec;

/**
* @internal
*/
final class PhpCoreExpressions
{
/**
* @var array<int, class-string<Expr>>
*/
public static array $ENABLED = [
Clone_::class,
Empty_::class,
Eval_::class,
Exit_::class,
Isset_::class,
Print_::class,
];

public static function getName(Expr $expression): string
{
return match ($expression::class) {
Clone_::class => 'clone',
Empty_::class => 'empty',
Eval_::class => 'eval',
Exit_::class => match ($expression->getAttribute('kind')) {
Exit_::KIND_EXIT => 'exit',
Exit_::KIND_DIE => 'die',
default => throw ShouldNotHappen::fromMessage('Unhandled Exit expression kind '.$expression->getAttribute('kind')),
},
Isset_::class => 'isset',
Print_::class => 'print',
default => throw ShouldNotHappen::fromMessage('Unsupported Core Expression'),
};
}

public static function getClass(string $name): ?string
{
return match ($name) {
'clone' => Clone_::class,
'empty' => Empty_::class,
'eval' => Eval_::class,
'die', 'exit' => Exit_::class,
'include' => Include_::class,
'isset' => Isset_::class,
'print' => Print_::class,
'shell_exec' => ShellExec::class,
default => null,
};
}
}
49 changes: 49 additions & 0 deletions tests/Fixtures/Misc/HasNativeFunctions.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?php

namespace Tests\Fixtures\Misc;

class HasNativeFunctions
{
public function startSleeping(): void
{
sleep(1);
}

public function dieWithStatus(int $status): void
{
die($status);
}

public function exitWithStatus(int $status): void
{
exit($status);
}

public function evaluateCode(string $code): void
{
eval($code);
}

public function makeAClone(object $object): object
{
return clone $object;
}

public function checkArray(array $array): bool
{
if(empty($array)){
return false;
}

if(!isset($array[1])){
return false;
}

return true;
}

public function printSomething(string $text): void
{
print $text;
}
}
11 changes: 0 additions & 11 deletions tests/Fixtures/Misc/HasSleepFunction.php

This file was deleted.

28 changes: 9 additions & 19 deletions tests/NativeFunctions.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,19 @@

test('native functions', function () {
expect('Tests')->toUse('sleep');
expect('Tests')->not->toUse('die');

expect('die')->not->toBeUsed();
expect('die')->toBeUsedInNothing();
expect('Tests')->toUse('die');

expect('sleep')->toBeUsedIn('Tests')
->and('die')->not->toBeUsedIn('Tests');
->and('die')->toBeUsedIn('Tests');

expect('die')->toOnlyBeUsedIn('Tests');
});

test('failure 1', function () {
expect('Tests')->toUse('die');
expect('Tests')->not->toUse('die');
})->throws(
ExpectationFailedException::class,
"Expecting 'Tests' to use 'die'."
"Expecting 'Tests' not to use 'die'."
);

test('failure 2', function () {
Expand All @@ -33,26 +30,19 @@
expect('sleep')->not->toBeUsed();
})->throws(
\Pest\Arch\Exceptions\ArchExpectationFailedException::class,
'Expecting \'sleep\' not to be used on \'Tests\Fixtures\Misc\HasSleepFunction\'.'
'Expecting \'sleep\' not to be used on \'Tests\Fixtures\Misc\HasNativeFunctions\'.'
);

test('failure 4', function () {
expect('sleep')->toBeUsedInNothing();
})->throws(
\Pest\Arch\Exceptions\ArchExpectationFailedException::class,
'Expecting \'sleep\' not to be used on \'Tests\Fixtures\Misc\HasSleepFunction\'.'
);

test('failure 5', function () {
expect('die')->toBeUsedIn('Tests\Fixtures\Misc\HasSleepFunction');
})->throws(
ExpectationFailedException::class,
'Expecting \'Tests\Fixtures\Misc\HasSleepFunction\' to use \'die\'.',
'Expecting \'sleep\' not to be used on \'Tests\Fixtures\Misc\HasNativeFunctions\'.'
);

test('failure 6', function () {
expect('sleep')->not->toBeUsedIn('Tests\Fixtures\Misc\HasSleepFunction');
expect('sleep')->not->toBeUsedIn('Tests\Fixtures\Misc\HasNativeFunctions');
})->throws(
ExpectationFailedException::class,
'Expecting \'sleep\' not to be used in \'Tests\Fixtures\Misc\HasSleepFunction\'.',
);
'Expecting \'sleep\' not to be used in \'Tests\Fixtures\Misc\HasNativeFunctions\'.',
)->only();
12 changes: 6 additions & 6 deletions tests/ToBeUsedIn.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@
}
});

it('failure with native functions', function () {
expect('sleep')->not->toBeUsedIn('Tests\Fixtures\Misc\HasSleepFunction');
})->throws(
ExpectationFailedException::class,
"Expecting 'sleep' not to be used in 'Tests\Fixtures\Misc\HasSleepFunction'."
);
it('failure with native functions', function (string $function) {
expect(fn () => expect($function)->not->toBeUsedIn('Tests\Fixtures\Misc\HasNativeFunctions'))->toThrow(
ExpectationFailedException::class,
"Expecting '$function' not to be used in 'Tests\Fixtures\Misc\HasNativeFunctions'."
);
})->with(['sleep', 'die', 'eval', 'exit', 'clone', 'empty', 'isset', 'print']);

it('fails 1', function () {
expect([Fooable::class])
Expand Down
Loading

0 comments on commit 042b351

Please sign in to comment.