Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Assertion on constructor dependencies #99

Open
hkdobrev opened this issue Mar 15, 2020 · 1 comment
Open

Assertion on constructor dependencies #99

hkdobrev opened this issue Mar 15, 2020 · 1 comment
Labels
enhancement New feature or request 👍 Approved

Comments

@hkdobrev
Copy link
Contributor

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.

@hkdobrev hkdobrev added the enhancement New feature or request label Mar 15, 2020
@carlosas
Copy link
Owner

carlosas commented Mar 15, 2020

Hello! Thanks for such a committed description 😃

This is really close to one of the enhancements I have in mind for the future. Be able to assert what kind of dependency you want to enforce or forbid. This would for instance allow to forbid instances of classes but allow using their public constants. The feature you are proposing would be a good start.

Note for contributions: It could be done by adding a type property in Parser/Relation/Dependency.php and then, in Parser/Ast/DependencyCollector.php specify the type based on the node type (nikic/php-parser). Then a new assertion based on Rule/Assertion/Dependency/MustDepend.php just filtering that type of dependencies.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request 👍 Approved
Projects
None yet
Development

No branches or pull requests

2 participants