akeneo / php-coupling-detector Goto Github PK

View Code? Open in Web Editor NEW

42.0 42.0 6.0 304 KB

[Experimental] Detect all the coupling issues of your project with respect to the coupling rules you have defined.

License: MIT License

PHP 99.03% Dockerfile 0.71% Makefile 0.26%

php-coupling-detector's People

Contributors

Stargazers

Watchers

Forkers

jjanvier nyholm gplanchat seldaek isabella232 dfayet

php-coupling-detector's Issues

Warning output style is not interpreted

For example when displaying violations of a discouraged rule, the <warning> tag is not interpreted to style the output and is displayed as is.

. is not recognized as current dir

jjanvier:lambdas$ vendor/bin/php-coupling-detector detect .                                                                           

In DetectCommand.php line 146:
                                                                                                         
  The configuration file "/home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/./.php_cd" does not exit  
                                                                                                         
detect [-c|--config-file CONFIG-FILE] [-f|--format FORMAT] [--] [<path>]

Can't define an only rule that defines nothing (except itself)

the conf

$builder->only([])->in('Jjanvier\C2P\Common'),

the error

jjanvier:lambdas$ vendor/bin/php-coupling-detector detect --config-file=.phpcd                                                        

In RuleBuilder.php line 56:
                                                                     
  Can not create a rule without any requirement defined previously.  

detect [-c|--config-file CONFIG-FILE] [-f|--format FORMAT] [--] [<path>]

Remove useless interfaces

Especially on domain objects (Rule, Node, etc.).

Split Rule class

Currently the same Rule class is used for every rule type. As we move more logic to the domain it would be clearer to have a dedicated class by type.

Be able to use a config file (like in php-cs-fixer) and decouple the lib from the PIM

I'm on it

Re-work display modes of the detect command

remove display mode of the detect command?

Handle rules of parent nodes in a sub node

Example :

$rules = [
    $builder->only([
        'Akeneo\Pim\ApiClient',
        'Akeneo\PimEnterprise\ApiClient',
        'Symfony',
        'Doctrine',
        'PHPUnit',
        'OldSound\RabbitMqBundle',
        'PhpAmqpLib',
        'Psr',
        'Prophecy',
        'JMS\Serializer',
        'Http\Discovery',
        'GuzzleHttp\Psr7',
    ])->in('Smallable\Produit\AkeneoBundle'),
    $builder->only([
        'Smallable\Produit\ProduitBundle\Entity' // this should be possible, and then it should allow to use everything that is in the parent
    ])->in('Smallable\Produit\AkeneoBundle\Migration'),
];

Package coupling-detector as a PHAR

How to do it manually (to understand):

How to do it automatically:

https://github.com/clue/phar-composer

Issue with dependency to php-cs-fixer

bin/php-coupling-detector detect --config-file=./.php_cd src/

Parsing 1819 nodes...
                                                           
  [Symfony\Component\Debug\Exception\FatalThrowableError]  
  Class 'Symfony\CS\Tokenizer\Tokens' not found            
                                                           

detect [-c|--config-file CONFIG-FILE] [-f|--format FORMAT] [--] [<path>]

Since our last bump here 53021d3

Due to the following change between v1 a v2 of php-cs-fixer PHP-CS-Fixer/PHP-CS-Fixer@4cccddb#diff-b853b43b9aa6d26771db195ab8a079c8

If i use v0.1.0 using php-cs-fixer 1.x here akeneo/pim-community-dev@6f6ffcc it properly run php-coupling-detector but fails on cs-fixer step

If i use dev-master using php-cs-fixer here akeneo/pim-community-dev@6f6ffcc it fails on php-coupling-detector step (Class 'Symfony\CS\Tokenizer\Tokens' not found) but cs-fixer step runs well.

We would have used v0.1.0 everywhere in our composer.json to avoid such issue on the PIM, imho we should fix it with proper coupling-detector versions, for instance 1.0 using fabpot cs fixer, 1.0 using cs fixer 2.0 + updating our code base to update not existing classes and methods issues (btw, we're using very internal api of cs fixer so we should carefully fix this deps and not expect much stability on it ;) )

Weird behavior with only rule

    $builder->only([
        'Smallable\Produit\ProduitBundle\Entity',
        'Smallable\Produit\AkeneoBundle',
        'Symfony',
        'Doctrine',
    ])->in('Smallable\Produit\AkeneoBundle\Migration'),

result in the error

Node Smallable\Produit\AkeneoBundle\Migration\ProductTermes\ProductTermsBuilder does not respect the rule Smallable\Produit\AkeneoBundle because of the tokens:
    * Smallable\Produit\ProduitBundle\Entity\Lang
    * Smallable\Produit\ProduitBundle\Entity\Product

Introduce layers

We already have a Domain namespace.
Could be interesting to also have Application (especially if we use a message bus at some point) and Infrastructure (Commands, Formatters, Parsers...)

.php_cd for sylius

Can't launch detect without parameter

Default dir should be .

jjanvier:lambdas$ vendor/bin/php-coupling-detector detect                                                                             

PHP Fatal error:  Uncaught TypeError: is_file() expects parameter 1 to be a valid path, null given in /home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/vendor/akeneo/php-coupling-detector/src/Console/Command/DetectCommand.php:92
Stack trace:
#0 /home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/vendor/akeneo/php-coupling-detector/src/Console/Command/DetectCommand.php(92): is_file(NULL)
#1 /home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/vendor/symfony/console/Command/Command.php(255): Akeneo\CouplingDetector\Console\Command\DetectCommand->execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#2 /home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/vendor/symfony/console/Application.php(912): Symfony\Component\Console\Command\Command->run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#3 /home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/vendor/symfony/console/Application.php(264): Symfony\Component in /home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/vendor/akeneo/php-coupling-detector/src/Console/Command/DetectCommand.php on line 92

Use regexp instead of simple string to define rule patterns

Missing fancy API for only rules with no Rules

Imagine a namespace Acme\Domain.
I'd like to have something that tells I can use only Acme\Domain in Acme\Domain.

PHP files without namespaces raise an exception

for example:

an empty php file
a procedural php script
a class without namespace

In NamespaceExtractor.php line 35:
                                               
  No way to parse the namespace of this class

we don't know which file, which class, etc...

such nodes are completely valid
they should be ignored and should not stop the program

.php_cd for pim-community-dev

set owner to Akeneo -> done
set the .php_cd for PIM community dev -> https://github.com/akeneo/pim-community-dev/pull/3687/files
remove the example folder and add the link to PIM community dev in the README

it should not be possible to create a rule with an empty string as pattern

the config

$builder->only([''])->in('Foo'),

the error

jjanvier:lambdas$ vendor/bin/php-coupling-detector detect --config-file=.phpcd                                                        ≠ ? {master}

Parsing 11 nodes...
Checking 3 rules...
PHP Warning:  strpos(): Empty needle in /home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/vendor/akeneo/php-coupling-detector/src/RuleChecker.php on line 122
PHP Stack trace:
PHP   1. {main}() /home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/vendor/akeneo/php-coupling-detector/bin/php-coupling-detector:0
PHP   2. Akeneo\CouplingDetector\Console\Application->run() /home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/vendor/akeneo/php-coupling-detector/bin/php-coupling-detector:18
PHP   3. Akeneo\CouplingDetector\Console\Application->doRun() /home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/vendor/symfony/console/Application.php:140
PHP   4. Akeneo\CouplingDetector\Console\Application->doRunCommand() /home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/vendor/symfony/console/Application.php:264
PHP   5. Akeneo\CouplingDetector\Console\Command\DetectCommand->run() /home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/vendor/symfony/console/Application.php:912
PHP   6. Akeneo\CouplingDetector\Console\Command\DetectCommand->execute() /home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/vendor/symfony/console/Command/Command.php:255
PHP   7. Akeneo\CouplingDetector\CouplingDetector->detect() /home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/vendor/akeneo/php-coupling-detector/src/Console/Command/DetectCommand.php:111
PHP   8. Akeneo\CouplingDetector\RuleChecker->check() /home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/vendor/akeneo/php-coupling-detector/src/CouplingDetector.php:78
PHP   9. Akeneo\CouplingDetector\RuleChecker->checkOnlyRule() /home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/vendor/akeneo/php-coupling-detector/src/RuleChecker.php:43
PHP  10. Akeneo\CouplingDetector\RuleChecker->doesTokenRespectOnlyRule() /home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/vendor/akeneo/php-coupling-detector/src/RuleChecker.php:102
PHP  11. strpos() /home/jjanvier/workspaces/phpstorm/perso/c2p/lambdas/vendor/akeneo/php-coupling-detector/src/RuleChecker.php:122

Move logic to the domain

Currently, most of the application logic is located in service-like classes (CouplingDetector and RuleChecker) while the domain is dumb.
It would make sense to let the domain be responsible for domain logic and use it directly from the commands without services in the middle.

It would be a good occasion to use more value objects instead of scalars too.

Since some events are dispatched by that logic through Symfony's EventDispatcher we may want to use a message bus instead (something like https://symfony.com/doc/current/components/messenger.html)