carlosas / phpat Goto Github PK

The regex implemented in #130 requires the filename length (without extension) to be even. File A.php will not be loaded, while Ab.php will be.

This can be fixed with proper grouping and quantifier - *.

In 0.2.2:

• include_only configuration broken

In master:

• exclusions from rules broken on windows (due to slash direction)

Enhancement description
At the moment, every assertion requires specifying another selector for a relation. There are no assertions which can assert a certain characteristic of the selected classes. For example, one could want to assert that every class matching a selector is final.

Example usage:

 <?php

use PhpAT\Rule\Rule;
use PhpAT\Selector\Selector;
use PhpAT\Test\ArchitectureTest;

class ExampleTest extends ArchitectureTest
{
    public function testEntitesAreFinal(): Rule
    {
        return $this->newRule
            ->classesThat(Selector::haveClassName('App\Entity\*'))
            ->mustBeFinal()
            ->build();
    }
}

Would such assertions be accepted as pull requests?
Do you think phpat is the best tool for that (vs rector, php-cs-fixer and tests)?

Suggested approach or solution

Creating a new collection of assertions which look only at the AST of the matched classes and not depend on its relations. Perhaps for these assertions a simple AST processing would be needed.

Enhancement description
Currently Selector::haveClassName does not allow using classes from composer, e.g. a test like:

    public function testDomainDoesNotDependOnAnythingElse(): Rule
    {
        return $this
            ->newRule
            ->classesThat(Selector::haveClassName('App\\*\\Domain\\*'))
            ->canOnlyDependOn()
            ->classesThat(Selector::haveClassName('App\\*\\Domain\\*'))
            ->andClassesThat(Selector::haveClassName(\Ramsey\Uuid\UuidInterface::class))
            ->build();
    }

Will result in:

WARNING: PhpAT\Selector\ClassNameSelector (Ramsey\Uuid\UuidInterface) could not find any class

Suggested approach or solution
I think this could also be solved by allowing a filter on ComposerDependencySelector, e.g. something like:

Selector::areDependenciesFromComposer('main')->only('\\Ramsey\\Uuid\\UuidInterface')

Bug Description

Exclude folders has no effect.

Example Config:

src:
    path: src/
    exclude:
        - 'Tests'
tests:
    path: tests/architecture/

Additional context

I have found a place here, if this is adjusted it works.

application/vendor/carlosas/phpat/src/File/FileFinder.php:38

public function findAllFiles(string $path): array
{
     return $this->finder->find($path, '*.php', [], []);
}

change to

public function findAllFiles(string $path): array
{
     return $this->finder->find($path, '*.php', [], Configuration::getSrcExcluded());
}

Bug Description
The return type of StatementBuilder#parseFile is declared as array, but the underlying method Parser#parse may return null.

This may result in type errors down the road.

Hi! I can't think up how to check, for example, infrastructure layer can only dependent on all composer packages.

What do you guys think about using a dedicated cli error handling lib/tool, e.g. https://laravel-news.com/collision-error-handler-framework

ClassNameExtractor already covers what NamespaceExtractor does

Currently the OutputInterface consists of 2 methods: write and writeLn. While this works I would suggest a interface that is more tailored to the project in order to enable more structured outputs in the future (e.g. XML for build systems).

Bug Description

/app/web/vendor/bin/phpat /app/phpat.yaml
Deprecated: Array and string offset access syntax with curly braces is deprecated in /app/web/vendor/sentry/sentry/lib/Raven/Client.php on line 331
Notice: Undefined property: PhpParser\Node\Stmt\Class_::$namespacedName in /app/web/vendor/carlosas/phpat/src/Parser/NameCollector.php on line 23


	An error occurred while running phpat :(
        Please consider opening an issue: http://github.com/carlosas/phpat/issues


Call to a member function toString() on null

#0 /app/web/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(153): PhpAT\Parser\NameCollector->leaveNode(Object(PhpParser\Node\Stmt\Class_))
#1 /app/web/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(146): PhpParser\NodeTraverser->traverseNode(Object(PhpParser\Node\Expr\New_))
#2 /app/web/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(223): PhpParser\NodeTraverser->traverseNode(Object(PhpParser\Node\Stmt\Return_))
#3 /app/web/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(114): PhpParser\NodeTraverser->traverseArray(Array)
#4 /app/web/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(223): PhpParser\NodeTraverser->traverseNode(Object(PhpParser\Node\Stmt\ClassMethod))
#5 /app/web/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(114): PhpParser\NodeTraverser->traverseArray(Array)
#6 /app/web/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(223): PhpParser\NodeTraverser->traverseNode(Object(PhpParser\Node\Stmt\Class_))
#7 /app/web/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(114): PhpParser\NodeTraverser->traverseArray(Array)
#8 /app/web/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(223): PhpParser\NodeTraverser->traverseNode(Object(PhpParser\Node\Stmt\Namespace_))
#9 /app/web/vendor/nikic/php-parser/lib/PhpParser/NodeTraverser.php(91): PhpParser\NodeTraverser->traverseArray(Array)
#10 /app/web/vendor/carlosas/phpat/src/Parser/MapBuilder.php(70): PhpParser\NodeTraverser->traverse(Array)
#11 /app/web/vendor/carlosas/phpat/src/App.php(63): PhpAT\Parser\MapBuilder->build()
#12 /app/web/vendor/carlosas/phpat/phpat(47): PhpAT\App->execute()

Additional context
My config

src:
  path: web/src/
tests:
  path: tests/architecture/

My test:

<?php

use PhpAT\Selector\Selector;

class OnlyRepositoriesDependOnModelsTest extends \PhpAT\Test\ArchitectureTest
{
    /**
     * @return \PhpAT\Rule\Rule
     */
    public function testNobodyExceptRepositoriesDependOnModels(): \PhpAT\Rule\Rule
    {
        return $this->newRule
            ->classesThat(Selector::haveClassName('App\Domain\*'))
            ->canOnlyDependOn()
            ->classesThat(Selector::havePath('Domain/*'))
            ->andClassesThat(Selector::haveClassName('App\Application\Shared\Service\KnownBadApproach'))
            ->build();
    }
}

Bug Description
Docblocks like this are not resolved as a SelectorInterface dependency

/**
 * @return SelectorInterface[]
 */

/** @var SelectorInterface[] $a */

Bug Description
The file exclusion from the configuration file only gets applied when the selector used in the rule is a PathSelector, if the rule is using other selectors, those files are not excluded.

Approach
Configuration::getSrcExcluded entries need to be resolved to classnames in StatementBuilder (while building origins) and treated as the excluded selectors.

Bug Description
Classes excluded from selectors that can not be resolved using the AST (out of src) are not being excluded from the checks.

Additional context
Rules like this one would still check the excluded classes:

return $this->newRule
    ->classesThat(Selector::haveClassName('App\Something\*'))
    ->mustNotDependOn()
    ->classesThat(Selector::haveClassName('Symfony\*'))
    ->excludingClassesThat(Selector::haveClassName('Symfony\*\Exception\*'))
    ->build();

Enhancement description
It would be great to provide a phar version of the tool to avoid dependency collisions with other projects.

Suggested approach or solution
Investigate and create a script to generate updated versions every time a new release is published.

GithubActions supports annotations within changed files.
These can be achieved with https://github.com/staabm/annotate-pull-request-from-checkstyle when the tool in question supports output in the checkstyle format.

That means one can actually see errors detected by the tool on a per line basis in the „files changed“ tab of a pull request (see screenshots in the repo)

See https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/acc2aad7ceb544a414c9c87587d688393649a9c4/src/Report/CheckstyleReporter.php#L41 as a reference implementation

Enhancement description
Now phpat is compiled in a phar but the tests need to be defined extending from the class ArchitectureTest. It would be necessary to develop a way to define tests using yaml files.

Suggested approach or solution
It would need to search for .yml/.yaml files in the defined tests directory and be able to create statements based on a file that would look like:

rules:
  - testUseCasesExtendAbstractUseCaseHandler:
    - classes:
      - havePath: Application/UseCase/*/*UseCase.php
      - havePath: Legacy/*/*Handler.php
    - excluding:
      - havePath: Application/UseCase/*/*TaskUseCase.php
    - assert: mustExtend
    - classes:
      - havePath: Application/UseCase/AbstractUseCaseHandler.php

Bug Description
Sometimes native types like scalars or self get resolved using the class namespace. It also happens with native functions when used with their fully qualified name like \is_null.

Enhancement description
In a Mono-Repo setup it is common that each module has it's own composer.json file that defines the dependencies. However it is easy to depend on a dependency from another module.
Also dependencies between modules in the same mono-repo should also be defined inside each modules composer.json.

It would be great if we don't have to replicate the whole information from the composer.json, but rather use the composer.json as a source for our architecture testing.

**Suggested approach or solution **
Didn't look at the source yet, however phparch has a similar feature: https://github.com/j6s/phparch#shorthand-for-monorepos-addcomposerbasedcomponent

The phparch solution misses one important thing, as it expects a composer.lock file inside each module, in a mono-repo setup especially for development purposes you mostly just have one lock file in the mono-repo root containing the dependencies of all modules.
https://github.com/symfony/symfony being a good example of such setup

In a rule like

$this->newRule
  ->classesThat(Selector::havePathname('Application/*/UseCase/*Handler.php'))
  ->shouldExtend()
  ->classesThat(Selector::havePathname('Application/Shared/UseCase/AbstractCommandHandler.php'))
  ->build();

will the rule type try to compare AbstractCommandHandler with itself?

It sounds funny (and no matter why I did it) It took me a while to figure out what the problem was.

Additional context
Create test class like this (_03_ArchitectureTest):

class _03_ArchitectureTest extends ArchitectureTest
{
    public function testCoreDependency(): Rule
    {
        return $this->newRule
            ->classesThat(Selector::haveClassName('Domain\*'))
            ->mustOnlyDependOn()
            ->classesThat(Selector::havePath('src'))
            ->build();
    }
}

Output:

vendor/bin/phpat

---/-------\------|-----\---/--
--/-PHP Architecture Tester/---
-/-----------\----|-------X----


phpat | 2.41s | TESTS PASSED

Unfortunately, however, these test was not started.

Composition has files as a param but treats the variable as a string.
It should be treated as an array of file names.

First of all, thank you very much for this tool. I have tried many but settled for this one in the end. Great work!

I am sporting phpat on the localhost for some time, but now, I have a weird behaviour in the Gitlab CI. First of all, the whole application is dockerized and it runs on the same image on localhost as it is in Gitlab CI.

This is the output on my localhost:

---/-------\------|-----\---/--
--/-PHP Architecture Tester/---
-/-----------\----|-------X----


---------
| Infra |
---------
................................
OK

--------
| Impl |
--------
...............................................................................
OK

----------
| Domain |
----------
.....................................................
OK

phpat | 1.48s | TESTS PASSED

And this is the output in Gitlab CI:

---/-------\------|-----\---/--
--/-PHP Architecture Tester/---
-/-----------\----|-------X----

Yeah, thats it :-) phpat just prints the welcome banner and then nothing. Also, the CI JOB is failed so I guess it exited with code != 0

Question
Is there a way to make the phpat run in some verbose mode so that I can see what is wrong? Where should I look for logs? How to debug this?

Now that #67 is merged, can phpat be changed to work like PHPStan's PHAR shim mechanism (info here https://github.com/phpstan/phpstan-shim, it's now in phpstan core)?

Enhancement description
It would be great to have the option of checking if a class has some dependency/inheritance/mixin/composition not only by itself but coming from a parent.

Suggested approach or solution
The feature should be enabled by default, but with the ability to disable it with ignore-indirect-behaviour. This check should be performed by the parser.

Hello there.

I am the author of phparch - a testing library that does essentially the same thing as phpat. I wrote phparch more as a proof of concept that evolved.

It is good to see that there are other people interested enough in architectural testing in PHP to create libraries. Since the name phparch was very unfortunately chosen and since I quite like your API from looking at the examples I would propose deprecating phparch in order to help build phpat. There is no sense in having 2 competing libraries when there could be 1 good one.

Seeing that you are from barcelona and that I will be in barcelona soon leading up to php.barcelona we can meet up to discuss this in person if you are cool with this.

Enhancement description

phpat provides assertion on dependencies which is very useful for ensuring loose coupling between different layers or domains in a project.

I think would be useful to have a more specific dependency assertion only on the constructor dependencies. E.g. mustHaveConstructorDependency and mustNotHaveConstructorDependency.

This would allow to test a given selection of classes inject some specific interfaces. It would enable validation of dependency injection principle exactly how one would expect.

To validate dependency inversion, we might need additional selectors specifying the dependency not just implement an interface, but is also typed as the interface itself or its child.

For example:

<?php

namespace App\Database;

use Doctrine\ORM\EntityManager;

final class ExampleService
{
	// This would fail as it depends on a concrete class
	// If we have used implementInterface selector it would pass dependency injection, but not dependency inversion
	public function __construct(EntityManager $entityManager)
	{
		// ...
	}
}

<?php
namespace Tests\Architecture;

use PhpAT\Rule\Rule;
use PhpAT\Selector\Selector;
use PhpAT\Test\ArchitectureTest;

final class DependencyInversionTest extends ArchitectureTest
{
	public function testDatabaseServicesDependOnEntityManagerInterface(): Rule
	{
		return $this->newRule
            ->classesThat(Selector::haveClassName('App\Database\*'))
            ->mustHaveConstructorDependency()
            ->classesThat(Selector::isInterface('Doctrine\ORM\EntityManagerInterface'))
            ->build();
	}
}

Suggested approach or solution

I think building such an assertion is not that hard as getting constructor types is relatively easy compared to some of the others.

Harder is to find the right naming and structure to provide complementing selectors so it's easier to enforce dependency inversion.

Enhancement description

It would be great to have a sample test to test standard architecture patterns. In this issue, the Layered Architecture

After testing with a canOnlyDependOn rule, we found out that vendor / libary classes obviously also get resolved. In other words, the rule (if I'm correct ? ) should be an exclusion rule using mustNotDependOn.

https://www.oreilly.com/library/view/domain-driven-design-in/9781787284944/00830b50-6403-4fba-8139-9ec64f18d756.xhtml

Suggested approach or solution

What about adding a folder with samples like the following:

<?php

use PhpAT\Rule\Rule;
use PhpAT\Selector\Selector;
use PhpAT\Test\ArchitectureTest;

final class LayeredArchitectureTest extends ArchitectureTest
{
    public function testUserInterface(): Rule
    {
        return $this->newRule
            ->classesThat(Selector::havePath('UserInterface/*'))
            ->mustNotDependOn()
            ->classesThat(Selector::havePath('Infra/*'))
            ->build();
    }

    public function testApplication(): Rule
    {
        return $this->newRule
            ->classesThat(Selector::havePath('Application/*'))
            ->mustNotDependOn()
            ->classesThat(Selector::havePath('UserInterface/*'))
            ->build();
    }

    public function testDomain(): Rule
    {
        return $this->newRule
            ->classesThat(Selector::havePath('Domain/*'))
            ->mustNotDependOn()
            ->classesThat(Selector::havePath('Application/*'))
            ->andClassesThat(Selector::havePath('UserInterface/*'))
            ->build();
    }

    public function testInfra()
    {
        return $this->newRule
            ->classesThat(Selector::havePath('Domain/*'))
            ->mustNotDependOn()
            ->classesThat(Selector::havePath('UserInterface/*'))
            ->build();
    }
}

Bug Description
Namespace\ClassName and \Namespace\ClassName are treated as different FQCNs
Initial backslash should be removed if present while saving relations in the reference map.

Enhancement description
I've used this package recently and diving into the code I discovered that it's using its own I/O implementation for the CLI.

It would be very useful to have all the power of Symfony commands, which between other things it allows you to have multiple commands, adjust verbosity etc.

Verbosity is another thing I would like to have the option to tweak because IMHO the output is too verbose and under some environments you want to turn it off.

This refactoring will also probably reduce the size of the project, making it more lightweight and easier to maintain.

Suggested approach or solution
Migrate the CLI related classes (Input, Output, etc.) to symfony/console and create the main command of phpat with it.

The current Selector only allows path based (so filename/structre convention based) assumptions.

I feel it would be great to have a similar selector but based on php namespaces instead.
That way the rules can be expressed on a more cenceptual level instead of filestructure which is more like a implementation detail of the app being tested

Bug Description
We have a Symfony project, and sometimes we use classes defined outside the project directory.
Since we would like to avoid this dependency we wrote some rules.

PHPAT was able to find these errors:

ERROR: App\Entity\Sales\OrderItemMerchantProduct depends on Legacy\Shared\MerchantProduct\MerchantProductConstants
ERROR: App\Entity\Customer depends on Legacy\Shared\Customer\CustomerConstants
ERROR: App\Entity\Company depends on Legacy\Shared\MerchantProduct\MerchantProductConstants

But when I try to exclude CustomerConstants class from the rule

    public function testNoReferenceToLegacy()
    {
        return $this->newRule
            ->classesThat(Selector::haveClassName('App\Ecommerce\Yves\*'))
            ->mustNotDependOn()
            ->classesThat(Selector::haveClassName('Legacy\*'))
            ->andExcludingClassesThat(Selector::haveClassName(Client::class))
            ->build();
    }

phpat shows the following warning

WARNING: PhpAT\Selector\ClassNameSelector (Legacy\Shared\Customer\CustomerConstants) could not find any class

Additional context
I suspect the problem is linked to the configuration file phpat.yml, where we can specify only one src, and our legacy classes are in a parent directory of src.

src:
    path: src/
tests:
    path: tests/architecture/

Bug Description

The test output seems to get shown before the actual test was run.

We would expect the output with the green/red marker to get shown right below the test where the errors originate from (at the bottom green/red marker).

Additional context

We run phpat using Docker, with the basic config yaml from the README

$ docker run --rm -v ${PWD}:/project -w /project jakzal/phpqa phpat

Enhancement description
It would be really useful if you can point phpat to certain classes like entities, repositories etc. when using Doctrine or another data mapper. It's usually easy with active record ORM implementations as models would usually extend a base model class. However, Doctrine could have only configuration via XML, PHP or annotations considering specific classes as Doctrine entities.

Some use cases for that would be enforcing entities to not extend classes or implement certain interfaces, require some traits for timestamping etc.

Suggested approach or solution

At first, I was thinking these selectors would be based on PHPDoc annotations. There are many tools relying on annotations like PHPUnit and especially in the Symfony ecosystem like validators, routing and entities. So such selectors could still be useful.

However, Doctrine could have those configurations combined from annotations, XML, YAML and PHP configuration files. So it'd be better to use the Doctrine metadata specifically for more useful Doctrine selectors. Similar to https://github.com/phpstan/phpstan-doctrine phpat (or an extension) could load the Doctrine metadata from a provided entity manager or a configuration instance allowing for very quick and easy detection of which classes are considered entities by Doctrine.

• DocBlock dependencies

Inspecting code like following:

namespace Shared\Domain;

interface EventSourcedAggregateRoot extends AggregateRoot
{
    /**
     * @param iterable<DomainEvent> $history
     *
     * @return static
     *
     * @psalm-pure
     */
    public static function reconstituteFromHistory(iterable $history) : self;
    // ... snip ...
}

Current tooling detects self and static as class names, while they should probably be resolved as self in the context of dependency tracking:

X Shared\Domain\EventSourcedAggregateRoot depends on Shared\Domain\static
X Shared\Domain\EventSourcedAggregateRoot depends on Shared\Domain\self

Enhancement description
My code is structured in such a way that the top level namespaces do not depend on each other, except for some strictly defined exceptions, e.g. assuming I have the following classes/interfaces:

• App\Weather\Infrastructure\WeatherController
• Graphs\Infrastructure\Renderer
• Graphs\Api\RendererInterface

I would like to define rules such that WeatherController cannot depend on Renderer, but can depend on RendererInterface. This can be currently done like this:

return $this
            ->newRule
            ->classesThat(Selector::haveClassName('App\\Weather\\*'))
            ->canOnlyDependOn()
            ->classesThat(Selector::haveClassName('App\\Weather\\*'))
            ->andClassesThat(Selector::haveClassName('App\\*\\Api\\*'))
            ->build();

The issue with that approach is that it requires repetition for each top-level namespace.

Suggested approach or solution
My dream API would be something like this, altough I'm not sure if that's really the best approach:

return $this
            ->newRule
            ->classesThat(Selector::haveClassName('App\\{level1}\\*'))
            ->canOnlyDependOn()
            ->classesThat(Selector::haveClassName('App\\{level1}\\*'))
            ->andClassesThat(Selector::haveClassName('App\\*\\Api\\*'))
            ->build();

The option to test only certain files is broken in version 0.5.3.

Currently running the phpat command with no arguments, or with -h/--help makes the tool to crash with a fatal error. It would be more user friendly if a suggestion what to do was displayed instead.

Looks like the config file is not checked for existence and Yaml::parse() is called with a non-string argument.

Option to run the script without error exit code

Similar to psalm and phpstan it would be helpfull to integrate the tool in already existing code bases by providing a baseline feature.

A baseline allows you to define all rules and make sure all newly written code will obey these rules, without the need to fix the whole codebase for violations

See details in

https://medium.com/@ondrejmirtes/phpstans-baseline-feature-lets-you-hold-new-code-to-a-higher-standard-e77d815a5dff
https://psalm.dev/docs/running_psalm/dealing_with_code_issues/#using-a-baseline-file

Bug Description
If I use a path selector(like in the Docs example) Selector::havePath('Application/*/UseCase/*Handler.php' and it matches nothing. It will crash phpat instead of producing a warning like other selectors.

OUTPUT:

---/-------\------|-----\---/--
--/-PHP Architecture Tester/---
-/-----------\----|-------X----


------------------------------------------
| Domain Does Not Depend On Other Layers |
------------------------------------------

WARNING: PhpAT\Selector\ImplementSelector (App\Domain\BlackMagicInterface) could not find any class
WARNING: PhpAT\Selector\PathSelector (libs) could not find any class
.
.
.
OK

------------------------------------------------
| All Handlers Extend Abstract Command Handler |
------------------------------------------------


        An error occurred while running phpat :(
        Please consider opening an issue: http://github.com/carlosas/phpat/issues


The "/home/patrick/PhpstormProjects/sfengineer/app/src/Application/*/UseCase/" directory does not exist.

#0 /home/patrick/PhpstormProjects/sfengineer/app/vendor/phpat/phpat/src/File/SymfonyFinderAdapter.php(23): Symfony\Component\Finder\Finder->in()
#1 /home/patrick/PhpstormProjects/sfengineer/app/vendor/phpat/phpat/src/File/FileFinder.php(43): PhpAT\File\SymfonyFinderAdapter->find()
#2 /home/patrick/PhpstormProjects/sfengineer/app/vendor/phpat/phpat/src/Selector/PathSelector.php(71): PhpAT\File\FileFinder->findSrcFiles()
#3 /home/patrick/PhpstormProjects/sfengineer/app/vendor/phpat/phpat/src/Selector/SelectorResolver.php(55): PhpAT\Selector\PathSelector->select()
#4 /home/patrick/PhpstormProjects/sfengineer/app/vendor/phpat/phpat/src/Statement/StatementBuilder.php(94): PhpAT\Selector\SelectorResolver->resolve()
#5 /home/patrick/PhpstormProjects/sfengineer/app/vendor/phpat/phpat/src/Statement/StatementBuilder.php(60): PhpAT\Statement\StatementBuilder->selectOrigins()
#6 /home/patrick/PhpstormProjects/sfengineer/app/vendor/phpat/phpat/src/App.php(87): PhpAT\Statement\StatementBuilder->build()
#7 /home/patrick/PhpstormProjects/sfengineer/app/vendor/phpat/phpat/phpat(48): PhpAT\App->execute()

Additional context
Just run the test file from docs on an empty project (or in my case on a project with a different structure)

Use PhpParser/FindingVisitor for extractors instead of the custom one