diff --git a/doc/LIST_UNUSED_REQUIREMENTS.md b/doc/LIST_UNUSED_REQUIREMENTS.md new file mode 100644 index 0000000..cfe5580 --- /dev/null +++ b/doc/LIST_UNUSED_REQUIREMENTS.md @@ -0,0 +1,16 @@ +# List unused requirements + +The _list-unused-requirements_ command is a complementary tool to the _detect_ command that allows you to easily clean +your configuration. It lists the requirements that are not necessary anymore for each rule : + +```bash + php bin/php-coupling-detector list-unused-requirements +``` + +Like the _detect_ command, you can specify the path to the configuration file with the ``--config-file`` option. + +The exit status of the _list-unused-requirements_ command can be: ``10`` if some unused requirements have been found, +or ``0`` otherwise. + +Please note that for the moment this is relevant only for rules of type ``ONLY`` as the notion of "unused requirement" +in a ``DISCOURAGED`` or ``FORBIDDEN`` rule makes no sense. diff --git a/spec/Domain/RuleSpec.php b/spec/Domain/RuleSpec.php new file mode 100644 index 0000000..26b7d0e --- /dev/null +++ b/spec/Domain/RuleSpec.php @@ -0,0 +1,81 @@ +beConstructedWith( + 'Namespace\To\Check', + [ + 'Authorized\Namespace\One', + 'Authorized\Namespace\Two', + 'Authorized\Namespace\Three', + 'Authorized\Namespace\Four', + ], + RuleInterface::TYPE_ONLY + ); + } + + function it_is_initializable() + { + $this->shouldBeAnInstanceOf(Rule::class); + } + + function it_matches_a_node(NodeInterface $node) + { + $node->getSubject()->willReturn('Namespace\To\Check\SomeClass'); + $this->matches($node)->shouldReturn(true); + + $node->getSubject()->willReturn('Another\Namespace\SomeClass'); + $this->matches($node)->shouldReturn(false); + } + + function it_finds_which_of_its_requirements_are_unused_in_a_set_of_nodes() + { + // We only test this case for Rules of type "ONLY". It is not applicable for other types. + // The fact that this method has a very different behavior depending on an immutable property (the type) and not + // on the inputs probably shows that the class should be split (one class per type). + $this->getUnusedRequirements( + [ + new Node( + [ + 'Authorized\Namespace\One', + 'Authorized\Namespace\Three', + ], + 'Namespace\To\Check\ClassOne', + '/path/to/file', + RuleInterface::TYPE_ONLY + ), + new Node( + [ + 'Authorized\Namespace\One', + ], + 'Namespace\To\Check\ClassTwo', + '/path/to/file', + RuleInterface::TYPE_ONLY + ), + new Node( + [ + 'Authorized\Namespace\Two', + ], + 'Namespace\Not\To\Check\ClassThree', + '/path/to/file', + RuleInterface::TYPE_ONLY + ), + ] + )->shouldReturn( + [ + 1 => 'Authorized\Namespace\Two', + 3 => 'Authorized\Namespace\Four', + ] + ); + } +} diff --git a/spec/NodeParser/NodeParserResolverSpec.php b/spec/NodeParser/NodeParserResolverSpec.php index d75b8c5..b80b3f9 100644 --- a/spec/NodeParser/NodeParserResolverSpec.php +++ b/spec/NodeParser/NodeParserResolverSpec.php @@ -3,7 +3,6 @@ namespace spec\Akeneo\CouplingDetector\NodeParser; use PhpSpec\ObjectBehavior; -use Prophecy\Argument; class NodeParserResolverSpec extends ObjectBehavior { diff --git a/spec/NodeParser/PhpClass/UseDeclarationsExtractorSpec.php b/spec/NodeParser/PhpClass/UseDeclarationsExtractorSpec.php index 0d9aa9b..17b61bb 100644 --- a/spec/NodeParser/PhpClass/UseDeclarationsExtractorSpec.php +++ b/spec/NodeParser/PhpClass/UseDeclarationsExtractorSpec.php @@ -3,7 +3,6 @@ namespace spec\Akeneo\CouplingDetector\NodeParser\PhpClass; use PhpSpec\ObjectBehavior; -use Prophecy\Argument; use PhpCsFixer\Tokenizer\Tokens; class UseDeclarationsExtractorSpec extends ObjectBehavior diff --git a/spec/RuleCheckerSpec.php b/spec/RuleCheckerSpec.php index 97f5312..a3e508a 100644 --- a/spec/RuleCheckerSpec.php +++ b/spec/RuleCheckerSpec.php @@ -2,125 +2,136 @@ namespace spec\Akeneo\CouplingDetector; +use Akeneo\CouplingDetector\Domain\Node; use Akeneo\CouplingDetector\Domain\NodeInterface; +use Akeneo\CouplingDetector\Domain\Rule; use Akeneo\CouplingDetector\Domain\RuleInterface; +use Akeneo\CouplingDetector\Domain\Violation; use Akeneo\CouplingDetector\Domain\ViolationInterface; use PhpSpec\ObjectBehavior; -use Prophecy\Argument; class RuleCheckerSpec extends ObjectBehavior { - function it_matches_a_node(RuleInterface $rule, NodeInterface $node) + function it_checks_a_valid_node_with_forbidden_rule() { - $rule->getSubject()->willReturn('foo\bar'); - $node->getSubject()->willReturn('foo\bar\baz'); - $this->match($rule, $node)->shouldReturn(true); + $rule = new Rule( + 'foo\bar', + ['blu', 'bla', 'bli'], + RuleInterface::TYPE_FORBIDDEN + ); + + $node = new Node( + ['blo', 'bly'], + 'foo\bar\baz', + '/path/to/file', + NodeInterface::TYPE_PHP_USE + ); - $rule->getSubject()->willReturn('foo\bar'); - $node->getSubject()->willReturn('blu\bla\blo'); - $this->match($rule, $node)->shouldReturn(false); + $this->check($rule, $node)->shouldReturn(null); } - function it_checks_a_valid_node_with_forbidden_rule(RuleInterface $rule, NodeInterface $node) + function it_checks_an_invalid_node_with_forbidden_rule() { - $rule->getSubject()->willReturn('foo\bar'); - $rule->getRequirements()->willReturn(array('blu', 'bla', 'bli')); - $rule->getType()->willReturn(RuleInterface::TYPE_FORBIDDEN); - $node->getSubject()->willReturn('foo\bar\baz'); - $node->getTokens()->willReturn(array('blo', 'bly')); + $rule = new Rule( + 'foo\bar', + ['blu', 'bla', 'bli'], + RuleInterface::TYPE_FORBIDDEN + ); - $this->check($rule, $node)->shouldReturn(null); - } + $node = new Node( + ['blu', 'bla', 'blo', 'bly'], + 'foo\bar\baz', + '/path/to/file', + NodeInterface::TYPE_PHP_USE + ); - function it_checks_an_invalid_node_with_forbidden_rule( - RuleInterface $rule, - NodeInterface $node, - ViolationInterface $violation - ) { - $rule->getSubject()->willReturn('foo\bar'); - $rule->getRequirements()->willReturn(array('blu', 'bla', 'bli')); - $rule->getType()->willReturn(RuleInterface::TYPE_FORBIDDEN); - $node->getSubject()->willReturn('foo\bar\baz'); - $node->getTokens()->willReturn(array('blu', 'bla', 'blo', 'bly')); - - $violation->getNode()->willReturn($node); - $violation->getRule()->willReturn($rule); - $violation->getType()->willReturn(ViolationInterface::TYPE_ERROR); - $violation->getTokenViolations()->willReturn(array('blu', 'bla')); - - $this->check($rule, $node)->shouldBeLikeExpectedViolation($violation); + $this->check($rule, $node)->shouldBeLike(new Violation( + $node, + $rule, + ['blu', 'bla'], + ViolationInterface::TYPE_ERROR + )); } - function it_checks_a_valid_node_with_discouraged_rule(RuleInterface $rule, NodeInterface $node) + function it_checks_a_valid_node_with_discouraged_rule() { - $rule->getSubject()->willReturn('foo\bar'); - $rule->getRequirements()->willReturn(array('blu', 'bla', 'bli')); - $rule->getType()->willReturn(RuleInterface::TYPE_DISCOURAGED); - $node->getSubject()->willReturn('foo\bar\baz'); - $node->getTokens()->willReturn(array('blo', 'bly')); + $rule = new Rule( + 'foo\bar', + ['blu', 'bla', 'bli'], + RuleInterface::TYPE_DISCOURAGED + ); + + $node = new Node( + ['blo', 'bly'], + 'foo\bar\baz', + '/path/to/file', + NodeInterface::TYPE_PHP_USE + ); $this->check($rule, $node)->shouldReturn(null); } - function it_checks_an_invalid_node_with_discouraged_rule( - RuleInterface $rule, - NodeInterface $node, - ViolationInterface $violation - ) { - $rule->getSubject()->willReturn('foo\bar'); - $rule->getRequirements()->willReturn(array('blu', 'bla', 'bli')); - $rule->getType()->willReturn(RuleInterface::TYPE_DISCOURAGED); - $node->getSubject()->willReturn('foo\bar\baz'); - $node->getTokens()->willReturn(array('blu', 'bla', 'blo', 'bly')); - - $violation->getNode()->willReturn($node); - $violation->getRule()->willReturn($rule); - $violation->getType()->willReturn(ViolationInterface::TYPE_WARNING); - $violation->getTokenViolations()->willReturn(array('blu', 'bla')); - - $this->check($rule, $node)->shouldBeLikeExpectedViolation($violation); + function it_checks_an_invalid_node_with_discouraged_rule() + { + $rule = new Rule( + 'foo\bar', + ['blu', 'bla', 'bli'], + RuleInterface::TYPE_DISCOURAGED + ); + + $node = new Node( + ['blu', 'bla', 'blo', 'bly'], + 'foo\bar\baz', + '/path/to/file', + NodeInterface::TYPE_PHP_USE + ); + + $this->check($rule, $node)->shouldBeLike(new Violation( + $node, + $rule, + ['blu', 'bla'], + ViolationInterface::TYPE_WARNING + )); } - function it_checks_a_valid_node_with_only_rule(RuleInterface $rule, NodeInterface $node) + function it_checks_a_valid_node_with_only_rule() { - $rule->getSubject()->willReturn('foo\bar'); - $rule->getRequirements()->willReturn(array('blu', 'bla', 'bli')); - $rule->getType()->willReturn(RuleInterface::TYPE_ONLY); - $node->getSubject()->willReturn('foo\bar\baz'); - $node->getTokens()->willReturn(array('blu', 'bla')); + $rule = new Rule( + 'foo\bar', + ['blu', 'bla', 'bli'], + RuleInterface::TYPE_ONLY + ); - $this->check($rule, $node)->shouldReturn(null); - } + $node = new Node( + ['blu', 'bla'], + 'foo\bar\baz', + '/path/to/file', + NodeInterface::TYPE_PHP_USE + ); - function it_checks_an_invalid_node_with_only_rule( - RuleInterface $rule, - NodeInterface $node, - ViolationInterface $violation - ) { - $rule->getSubject()->willReturn('foo\bar'); - $rule->getRequirements()->willReturn(array('blu', 'bla', 'bli')); - $rule->getType()->willReturn(RuleInterface::TYPE_ONLY); - $node->getSubject()->willReturn('foo\bar\baz'); - $node->getTokens()->willReturn(array('blu', 'bla', 'blo', 'bly')); - - $violation->getNode()->willReturn($node); - $violation->getRule()->willReturn($rule); - $violation->getType()->willReturn(ViolationInterface::TYPE_ERROR); - $violation->getTokenViolations()->willReturn(array('blo', 'bly')); - - $this->check($rule, $node)->shouldBeLikeExpectedViolation($violation); + $this->check($rule, $node)->shouldReturn(null); } - public function getMatchers(): array + function it_checks_an_invalid_node_with_only_rule() { - return array( - 'beLikeExpectedViolation' => function ($subject, $expected) { - return - $subject->getNode() === $expected->getNode() && - $subject->getRule() === $expected->getRule() && - $subject->getTokenViolations() === $expected->getTokenViolations() && - $subject->getType() === $expected->getType(); - }, + $rule = new Rule( + 'foo\bar', + ['blu', 'bla', 'bli'], + RuleInterface::TYPE_ONLY ); + + $node = new Node( + ['blu', 'bla', 'blo', 'bly'], + 'foo\bar\baz', + '/path/to/file', + NodeInterface::TYPE_PHP_USE + ); + + $this->check($rule, $node)->shouldBeLike(new Violation( + $node, + $rule, + ['blo', 'bly'], + ViolationInterface::TYPE_ERROR + )); } } diff --git a/src/Console/Application.php b/src/Console/Application.php index 8b18f7d..630c9ad 100644 --- a/src/Console/Application.php +++ b/src/Console/Application.php @@ -5,6 +5,7 @@ namespace Akeneo\CouplingDetector\Console; use Akeneo\CouplingDetector\Console\Command\DetectCommand; +use Akeneo\CouplingDetector\Console\Command\ListUnusedRequirementsCommand; use Akeneo\CouplingDetector\CouplingDetector; use Symfony\Component\Console\Application as BaseApplication; @@ -23,5 +24,6 @@ public function __construct() error_reporting(-1); parent::__construct('Akeneo coupling detector', CouplingDetector::VERSION); $this->add(new DetectCommand()); + $this->add(new ListUnusedRequirementsCommand()); } } diff --git a/src/Console/Command/DetectCommand.php b/src/Console/Command/DetectCommand.php index bb965e9..3243974 100644 --- a/src/Console/Command/DetectCommand.php +++ b/src/Console/Command/DetectCommand.php @@ -151,8 +151,9 @@ private function loadConfiguration($filePath) if (!$config instanceof Configuration) { throw new \InvalidArgumentException( sprintf( - 'The configuration file "%s" must return a "Akeneo\CouplingDetector\Configuration\Configuration"', - $filePath + 'The configuration file "%s" must return a "%s" instance.', + $filePath, + Configuration::class ) ); } diff --git a/src/Console/Command/ListUnusedRequirementsCommand.php b/src/Console/Command/ListUnusedRequirementsCommand.php new file mode 100644 index 0000000..8cbdc81 --- /dev/null +++ b/src/Console/Command/ListUnusedRequirementsCommand.php @@ -0,0 +1,189 @@ + + * @copyright 2019 Akeneo SAS (http://www.akeneo.com) + * @license http://opensource.org/licenses/MIT MIT + */ +final class ListUnusedRequirementsCommand extends Command +{ + private const EXIT_WITH_WARNINGS = 10; + + protected function configure(): void + { + $this + ->setName('list-unused-requirements') + ->setDefinition( + [ + new InputArgument('path', InputArgument::OPTIONAL, 'path of the project', null), + new InputOption( + 'config-file', + 'c', + InputOption::VALUE_REQUIRED, + 'File path of the configuration file' + ), + ] + ) + ->setDescription('List rule requirements that are not needed anymore.') + ->setHelp($this->loadHelpContent()) + ; + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $output->setFormatter(new OutputFormatter($output->isDecorated())); + + if (null !== $path = $input->getArgument('path')) { + $filesystem = new Filesystem(); + if (!$filesystem->isAbsolutePath($path)) { + $path = getcwd() . DIRECTORY_SEPARATOR . $path; + } + } + + if (null === $configFile = $input->getOption('config-file')) { + $configDir = $path; + if (is_file($path) && $dirName = pathinfo($path, PATHINFO_DIRNAME)) { + $configDir = $dirName; + } elseif (null === $path) { + $configDir = getcwd(); + // path is directory + } + $configFile = $configDir . DIRECTORY_SEPARATOR . '.php_cd'; + } + + $config = $this->loadConfiguration($configFile); + $rules = $config->getRules(); + $finder = $config->getFinder(); + $finder->in($path); + + $nodeParserResolver = new NodeParserResolver(); + $eventDispatcher = new EventDispatcher(); + + $nodes = $this->parseNodes($finder, $nodeParserResolver, $eventDispatcher); + + $output->writeln(sprintf('Parsing %s nodes...', count($nodes))); + $output->writeln(sprintf('Checking %s rules...', count($rules))); + + $exitCode = 0; + foreach ($rules as $rule) { + $ruleUnusedRequirements = $rule->getUnusedRequirements($nodes); + if (count($ruleUnusedRequirements) > 0) { + $exitCode = self::EXIT_WITH_WARNINGS; + } + + $this->displayRuleUnusedRequirements($output, $rule, $ruleUnusedRequirements); + } + + return $exitCode; + } + + private function loadHelpContent(): string + { + $content = file_get_contents(__DIR__ . '/../../../doc/LIST_UNUSED_REQUIREMENTS.md'); + if (false === $content) { + throw new \RuntimeException('Unable to load the help content.'); + } + + $content = preg_replace('/^.+\n/', '', $content); + $content = str_replace('bin/php-coupling-detector', '%command.full_name%', $content); + $content = str_replace('_list-unused-requirements_ command', '%command.name% command', $content); + $content = preg_replace('/```bash(.*?)```/s', '$1', $content); + $content = preg_replace('/```php(.*?)```/s', '$1', $content); + $content = preg_replace('/``(.*?)``/s', '$1', $content); + + return $content; + } + + /** + * @return NodeInterface[] + */ + private function parseNodes( + Finder $finder, + NodeParserResolver $nodeParserResolver, + EventDispatcher $eventDispatcher + ): array { + $eventDispatcher->dispatch(Events::PRE_NODES_PARSED, new PreNodesParsedEvent($finder)); + + $nodes = array(); + foreach ($finder as $file) { + $parser = $nodeParserResolver->resolve($file); + if (null !== $parser) { + try { + $node = $parser->parse($file); + $nodes[] = $node; + $eventDispatcher->dispatch(Events::NODE_PARSED, new NodeParsedEvent($node)); + } catch (ExtractionException $e) { + // at the moment, let's just ignore invalid node + // need to fix that with a better design + } + } + } + + $eventDispatcher->dispatch(Events::POST_NODES_PARSED, new PostNodesParsedEvent($nodes)); + + return $nodes; + } + + private function loadConfiguration(string $filePath): Configuration + { + if (!is_file($filePath)) { + throw new \InvalidArgumentException(sprintf('The configuration file "%s" does not exit', $filePath)); + } + + $config = include $filePath; + + if (!$config instanceof Configuration) { + throw new \InvalidArgumentException( + sprintf( + 'The configuration file "%s" must return a "%s" instance.', + $filePath, + Configuration::class + ) + ); + } + + return $config; + } + + private function displayRuleUnusedRequirements( + OutputInterface $output, + RuleInterface $rule, + array $ruleUnusedRequirements + ): void { + if (count($ruleUnusedRequirements) < 1) { + return; + } + + $output->writeln(''); + $output->writeln(sprintf('Rule %s has unused requirements:', $rule->getSubject())); + + foreach ($ruleUnusedRequirements as $ruleUnusedRequirement) { + $output->writeln(sprintf(' * %s', $ruleUnusedRequirement)); + } + } +} diff --git a/src/Domain/Rule.php b/src/Domain/Rule.php index 973a96a..a6d808d 100644 --- a/src/Domain/Rule.php +++ b/src/Domain/Rule.php @@ -67,4 +67,37 @@ public function getDescription(): ?string { return $this->description; } + + public function matches(NodeInterface $node): bool + { + return false !== strpos($node->getSubject(), $this->subject); + } + + public function getUnusedRequirements(array $nodes): array + { + // Not relevant for other types of rules + if (RuleInterface::TYPE_ONLY !== $this->type) { + return []; + } + + $matchingNodes = array_filter($nodes, function (NodeInterface $node) { + return $this->matches($node); + }); + + return array_filter($this->requirements, function (string $requirement) use ($matchingNodes) { + if ($this->subject === $requirement) { + return false; + } + + foreach ($matchingNodes as $node) { + foreach ($node->getTokens() as $token) { + if (false !== strpos($token, $requirement)) { + return false; + } + } + } + + return true; + }); + } } diff --git a/src/Domain/RuleInterface.php b/src/Domain/RuleInterface.php index 7c39fa6..3f351fe 100644 --- a/src/Domain/RuleInterface.php +++ b/src/Domain/RuleInterface.php @@ -23,4 +23,8 @@ public function getRequirements(): array; public function getType(): string; public function getDescription(): ?string; + + public function matches(NodeInterface $node): bool; + + public function getUnusedRequirements(array $nodes): array; } diff --git a/src/RuleChecker.php b/src/RuleChecker.php index 05cb366..97d11a1 100644 --- a/src/RuleChecker.php +++ b/src/RuleChecker.php @@ -25,24 +25,12 @@ */ class RuleChecker { - /** - * Does a node match a rule? - */ - public function match(RuleInterface $rule, NodeInterface $node): bool - { - if (false !== strpos($node->getSubject(), $rule->getSubject())) { - return true; - } - - return false; - } - /** * Checks if a node respect a rule. */ public function check(RuleInterface $rule, NodeInterface $node): ?ViolationInterface { - if (!$this->match($rule, $node)) { + if (!$rule->matches($node)) { return null; } @@ -70,7 +58,7 @@ private function checkForbiddenOrDiscouragedRule(RuleInterface $rule, NodeInterf $errors = array(); foreach ($node->getTokens() as $token) { - if (!$this->checkTokenForForbiddenOrDiscouragedRule($rule, $token) && + if (!$this->doesTokenRespectForbiddenOrDiscouragedRule($rule, $token) && !in_array($token, $errors)) { $errors[] = $token; } @@ -91,7 +79,7 @@ private function checkForbiddenOrDiscouragedRule(RuleInterface $rule, NodeInterf /** * Checks if a token fits a "forbidden" / "discouraged" rule or not. */ - private function checkTokenForForbiddenOrDiscouragedRule(RuleInterface $rule, string $token): bool + private function doesTokenRespectForbiddenOrDiscouragedRule(RuleInterface $rule, string $token): bool { foreach ($rule->getRequirements() as $req) { if (strpos($token, $req) !== false) { @@ -111,7 +99,7 @@ private function checkOnlyRule(RuleInterface $rule, NodeInterface $node): ?Viola $errors = array(); foreach ($node->getTokens() as $token) { - if (!$this->checkTokenForOnlyRule($rule, $token) && + if (!$this->doesTokenRespectOnlyRule($rule, $token) && !in_array($token, $errors)) { $errors[] = $token; } @@ -127,7 +115,7 @@ private function checkOnlyRule(RuleInterface $rule, NodeInterface $node): ?Viola /** * Checks if a token fits a "only" rule or not. */ - private function checkTokenForOnlyRule(RuleInterface $rule, $token): bool + private function doesTokenRespectOnlyRule(RuleInterface $rule, $token): bool { $fitRuleRequirements = false; foreach ($rule->getRequirements() as $req) {