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

Fix @pest-arch-ignore-line and @pest-arch-ignore-next-line comments #18

Open
wants to merge 12 commits into
base: 3.x
Choose a base branch
from
125 changes: 61 additions & 64 deletions src/Blueprint.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +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;
use PHPUnit\Architecture\Services\ServiceContainer;
use PHPUnit\Framework\Assert;
use PHPUnit\Framework\ExpectationFailedException;

Expand All @@ -40,7 +36,7 @@ final class Blueprint
public function __construct(
private readonly LayerFactory $layerFactory,
private readonly Targets $target,
private readonly Dependencies $dependencies
private readonly Dependencies $dependencies,
) {
// ...
}
Expand All @@ -58,7 +54,7 @@ public static function make(Targets $target, Dependencies $dependencies): self
/**
* Expects the target to use the given dependencies.
*
* @param callable(string, string): mixed $failure
* @param callable(string, string, Violation|null=): mixed $failure
*/
public function expectToUse(LayerOptions $options, callable $failure): void
{
Expand All @@ -67,16 +63,30 @@ public function expectToUse(LayerOptions $options, callable $failure): void
foreach ($this->target->value as $targetValue) {
$targetLayer = $this->layerFactory->make($options, $targetValue, false);

$targetUses = array_merge(...array_map(
static fn (Objects\ObjectDescription|Objects\FunctionDescription $object): array => iterator_to_array($object->usesByLines->getIterator()), // @phpstan-ignore-line
iterator_to_array($targetLayer->getIterator()),
));
$targetUsesNames = array_column($targetUses, 'name');

foreach ($this->dependencies->values as $dependency) {
$dependencyLayer = $this->layerFactory->make($options, $dependency->value);

try {
$this->assertDoesNotDependOn($targetLayer, $dependencyLayer);
} catch (ExpectationFailedException) {
if ($targetLayer->equals($dependencyLayer)) {
continue;
}

$failure($targetValue, $dependency->value);
$expectedUses = array_map(
static fn (ObjectDescription $object): string => $object->name,
iterator_to_array($dependencyLayer->getIterator()),
);

$uses = array_intersect($targetUsesNames, $expectedUses);
if ($uses === []) {
$failure(
$targetValue,
$dependency->value,
);
}
}
}

Expand Down Expand Up @@ -153,21 +163,29 @@ public function expectToOnlyUse(LayerOptions $options, callable $failure): void
foreach ($this->target->value as $targetValue) {
$allowedUses = array_merge(
...array_map(fn (Layer $layer): array => array_map(
fn (ObjectDescription $object): string => $object->name, iterator_to_array($layer->getIterator())), array_map(
fn (ObjectDescription $object): string => $object->name, iterator_to_array($layer->getIterator())),
array_map(
fn (string $dependency): Layer => $this->layerFactory->make($options, $dependency),
[
$targetValue, ...array_map(
fn (Dependency $dependency): string => $dependency->value, $this->dependencies->values
fn (Dependency $dependency): string => $dependency->value, $this->dependencies->values,
),
],
)
));
),
),
);

$layer = $this->layerFactory->make($options, $targetValue);
/** @var Objects\ObjectDescription $object */
foreach ($layer as $object) {
foreach ($object->uses as $use) {
if (! in_array($use, $allowedUses, true)) {
$failure($targetValue, $this->dependencies->__toString(), $use, $this->getUsagePathAndLines($layer, $targetValue, $use));
foreach ($object->usesByLines as $use) {
if (! in_array($use['name'], $allowedUses, true)) {
$failure(
$targetValue,
$this->dependencies->__toString(),
$use['name'],
new Violation($this->normalizePath($object->path), $use['startLine'], $use['endLine']),
);

return;
}
Expand Down Expand Up @@ -197,13 +215,28 @@ public function expectToOnlyBeUsedIn(LayerOptions $options, callable $failure):
foreach ($this->target->value as $targetValue) {
$dependencyLayer = $this->layerFactory->make($options, $targetValue);

try {
$this->assertDoesNotDependOn($namespaceLayer, $dependencyLayer);
} catch (ExpectationFailedException) {
$objects = $this->getObjectsWhichUsesOnLayerAFromLayerB($namespaceLayer, $dependencyLayer);
[$dependOn, $target] = explode(' <- ', $objects[0]);
if ($namespaceLayer->equals($dependencyLayer)) {
continue;
}

$failure($target, $dependOn, $this->getUsagePathAndLines($namespaceLayer, $dependOn, $target));
$disallowedUses = array_map(
static fn (ObjectDescription $object): string => $object->name,
iterator_to_array($dependencyLayer->getIterator()),
);

/** @var Objects\ObjectDescription $object */
foreach ($namespaceLayer as $object) {
foreach ($object->usesByLines as $use) {
if (in_array($use['name'], $disallowedUses, true)) {
$failure(
$targetValue,
$object->name,
new Violation($this->normalizePath($object->path), $use['startLine'], $use['endLine']),
);

return;
}
}
}
}
}
Expand Down Expand Up @@ -241,47 +274,11 @@ public static function assertEquals(mixed $expected, mixed $actual, string $mess
Assert::assertEquals($expected, $actual, $message);
}

private function getUsagePathAndLines(Layer $layer, string $objectName, string $target): ?Violation
private function normalizePath(string $path): string
{
$dependOnObjects = array_filter(
$layer->getIterator()->getArrayCopy(), //@phpstan-ignore-line
fn (ObjectDescription $objectDescription): bool => $objectDescription->name === $objectName
);

/** @var ObjectDescription $dependOnObject */
$dependOnObject = array_pop($dependOnObjects);

/** @var class-string<\PhpParser\Node> $class */
$class = PhpCoreExpressions::getClass($target) ?? Name::class;

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

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

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

if ($names === []) {
return null;
}

$startLine = $names[0]->getAttribute('startLine');
assert(is_int($startLine));

$endLine = $names[0]->getAttribute('endLine');
assert(is_int($endLine));

$path = preg_replace('/[\/\\\\]vendor[\/\\\\]composer[\/\\\\]\.\.[\/\\\\]\.\./', '', $dependOnObject->path);

assert($path !== null);
$normalized = preg_replace('/[\/\\\\]vendor[\/\\\\]composer[\/\\\\]\.\.[\/\\\\]\.\./', '', $path);
assert($normalized !== null);

return new Violation($path, $startLine, $endLine);
return $normalized;
}
}
8 changes: 4 additions & 4 deletions src/Expectations/ToOnlyBeUsedIn.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@ static function (LayerOptions $options) use ($blueprint): void {
$blueprint->expectToOnlyBeUsedIn(
$options,
static function (string $value, string $notAllowedDependOn, ?Violation $violation): void {
$message = "Expecting '$value' not to be used on '$notAllowedDependOn'.";

if (! $violation instanceof Violation) {
throw new ExpectationFailedException(
"Expecting '$value' not to be used on '$notAllowedDependOn'.",
);
throw new ExpectationFailedException($message);
}

throw new ArchExpectationFailedException($violation, "Expecting '$value' not to be used on '$notAllowedDependOn'.");
throw new ArchExpectationFailedException($violation, $message);
},
);
},
Expand Down
12 changes: 9 additions & 3 deletions src/Expectations/ToUse.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@

use Pest\Arch\Blueprint;
use Pest\Arch\Collections\Dependencies;
use Pest\Arch\Exceptions\ArchExpectationFailedException;
use Pest\Arch\Options\LayerOptions;
use Pest\Arch\SingleArchExpectation;
use Pest\Arch\ValueObjects\Targets;
use Pest\Arch\ValueObjects\Violation;
use Pest\Expectation;
use PHPUnit\Framework\ExpectationFailedException;

Expand Down Expand Up @@ -36,9 +38,13 @@ public static function make(Expectation $expectation, array|string $dependencies
static function (LayerOptions $options) use ($blueprint): void {
$blueprint->expectToUse(
$options,
static fn (string $value, string $dependOn) => throw new ExpectationFailedException(
"Expecting '{$value}' to use '{$dependOn}'.",
),
static function (string $value, string $dependOn, ?Violation $violation = null): never {
$message = "Expecting '$value' to use '$dependOn'.";
if (! $violation instanceof Violation) {
throw new ExpectationFailedException($message);
}
throw new ArchExpectationFailedException($violation, $message);
},
);
},
);
Expand Down
11 changes: 11 additions & 0 deletions src/Factories/LayerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,17 @@ public function make(LayerOptions $options, string $name, bool $onlyUserDefinedU
return true;
}))
);
if ($object instanceof \Pest\Arch\Objects\ObjectDescription) {
$object->usesByLines = $object->usesByLines->cloneFiltered(static function (array $use) use ($options): bool {
foreach ($options->exclude as $exclude) {
if (str_starts_with((string) $use['name'], $exclude)) {
return false;
}
}

return true;
});
}

return $object;
}, $this->objectsStorage->allByNamespace($name, $onlyUserDefinedUses));
Expand Down
33 changes: 31 additions & 2 deletions src/Factories/ObjectDescriptionFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ public static function make(string $filename, bool $onlyUserDefinedUses = true):
{
self::ensureServiceContainerIsInitialized();

$isFromVendor = str_contains((string) realpath($filename), DIRECTORY_SEPARATOR.'vendor'.DIRECTORY_SEPARATOR);
$path = (string) realpath($filename);

$isFromVendor = str_contains($path, DIRECTORY_SEPARATOR.'vendor'.DIRECTORY_SEPARATOR);
$originalErrorReportingLevel = error_reporting();
error_reporting($originalErrorReportingLevel & ~E_USER_DEPRECATED);

Expand All @@ -47,13 +48,19 @@ public static function make(string $filename, bool $onlyUserDefinedUses = true):
return null;
}

if (! $isFromVendor) {
if ($object instanceof ObjectDescription) {
$object->uses = new ObjectUses(array_values(
array_filter(
iterator_to_array($object->uses->getIterator()),
static fn (string $use): bool => (! $onlyUserDefinedUses || self::isUserDefined($use)) && ! self::isSameLayer($object, $use),
)
));
$ignoredLines = self::getIgnoredLines($path);
$object->usesByLines->filter(
static fn (array $use): bool => ! in_array($use['startLine'], $ignoredLines, true)
&& (! $onlyUserDefinedUses || self::isUserDefined($use['name']))
&& ! self::isSameLayer($object, $use['name']),
);
}

return $object;
Expand Down Expand Up @@ -101,4 +108,26 @@ private static function isSameLayer(\PHPUnit\Architecture\Elements\ObjectDescrip
|| $use === 'parent'
|| $object->reflectionClass->getNamespaceName() === $use;
}

/**
* Scans file for ignored lines (@pest-arch-ignore-line and @pest-arch-ignore-next-line)
*
* @return int[]
*/
private static function getIgnoredLines(string $filename): array
{
$ignoredLines = [];
$lines = file($filename);
if (is_array($lines)) {
foreach ($lines as $lineNo => $line) {
if (str_contains($line, '@pest-arch-ignore-line')) {
$ignoredLines[] = $lineNo + 1;
} elseif (str_contains($line, '@pest-arch-ignore-next-line')) {
$ignoredLines[] = $lineNo + 2;
}
}
}

return array_values(array_unique($ignoredLines, SORT_NUMERIC));
}
}
3 changes: 3 additions & 0 deletions src/Objects/FunctionDescription.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
*/
final class FunctionDescription extends ObjectDescription // @phpstan-ignore-line
{
public ObjectUsesByLines $usesByLines;

/**
* {@inheritDoc}
*/
Expand All @@ -29,6 +31,7 @@ public static function make(string $path): self
/** @var class-string<mixed> $path */
$description->name = $path;
$description->uses = new ObjectUses([]);
$description->usesByLines = new ObjectUsesByLines([]);
// $description->reflectionClass = new ReflectionFunction($path);

return $description;
Expand Down
Loading