From 5bb2a07a869569dae44340af86a259fb2da3b233 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Fri, 20 Apr 2018 16:14:04 +0200 Subject: [PATCH 01/21] processing includes as proposed in #67 --- .../ASTLocator/FileAST.php | 45 ++++++ .../ASTLocator/LocateASTFromFiles.php | 4 +- .../Cli/CheckCommand.php | 6 +- .../LocateDefinedSymbolsFromASTRoots.php | 10 +- .../NodeVisitor/IncludeCollector.php | 129 ++++++++++++++++++ .../LocateUsedSymbolsFromASTRoots.php | 2 +- 6 files changed, 189 insertions(+), 7 deletions(-) create mode 100644 src/ComposerRequireChecker/ASTLocator/FileAST.php create mode 100644 src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php diff --git a/src/ComposerRequireChecker/ASTLocator/FileAST.php b/src/ComposerRequireChecker/ASTLocator/FileAST.php new file mode 100644 index 00000000..20ff3c1e --- /dev/null +++ b/src/ComposerRequireChecker/ASTLocator/FileAST.php @@ -0,0 +1,45 @@ +file = $file; + $this->ast = $ast; + } + + /** + * @return string + */ + public function getFile() + { + return $this->file; + } + + /** + * @return Traversable|array + */ + public function getAst() + { + return $this->ast; + } +} diff --git a/src/ComposerRequireChecker/ASTLocator/LocateASTFromFiles.php b/src/ComposerRequireChecker/ASTLocator/LocateASTFromFiles.php index 7eebc726..1ca45b30 100644 --- a/src/ComposerRequireChecker/ASTLocator/LocateASTFromFiles.php +++ b/src/ComposerRequireChecker/ASTLocator/LocateASTFromFiles.php @@ -27,12 +27,12 @@ public function __construct(Parser $parser, ?ErrorHandler $errorHandler) /** * @param Traversable|string[] $files * - * @return Traversable|array[] a series of AST roots, one for each given file + * @return Traversable|FileAST[] a series of AST roots, one for each given file */ public function __invoke(Traversable $files): Traversable { foreach ($files as $file) { - yield $this->parser->parse(file_get_contents($file), $this->errorHandler); + yield new FileAST($file, $this->parser->parse(file_get_contents($file), $this->errorHandler)); } } } diff --git a/src/ComposerRequireChecker/Cli/CheckCommand.php b/src/ComposerRequireChecker/Cli/CheckCommand.php index 3cc4255f..94edfc05 100644 --- a/src/ComposerRequireChecker/Cli/CheckCommand.php +++ b/src/ComposerRequireChecker/Cli/CheckCommand.php @@ -67,12 +67,16 @@ protected function execute(InputInterface $input, OutputInterface $output): int $sourcesASTs = $this->getASTFromFilesLocator($input); - $definedVendorSymbols = (new LocateDefinedSymbolsFromASTRoots())->__invoke($sourcesASTs( + list($definedVendorSymbols, $additionalFiles) = (new LocateDefinedSymbolsFromASTRoots())->__invoke($sourcesASTs( (new ComposeGenerators())->__invoke( $getPackageSourceFiles($composerJson), (new LocateComposerPackageDirectDependenciesSourceFiles())->__invoke($composerJson) ) )); + while ($additionalFiles && count($additionalFiles)) { + list($vendorSymbols, $additionalFiles) = (new LocateDefinedSymbolsFromASTRoots())->__invoke($sourcesASTs($additionalFiles)); + $definedVendorSymbols = array_unique(array_merge($vendorSymbols, $definedVendorSymbols)); + } $definedExtensionSymbols = (new LocateDefinedSymbolsFromExtensions())->__invoke( (new DefinedExtensionsResolver())->__invoke($composerJson, $options->getPhpCoreExtensions()) diff --git a/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php b/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php index 1e5fef11..561d02ff 100644 --- a/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php +++ b/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php @@ -2,7 +2,9 @@ namespace ComposerRequireChecker\DefinedSymbolsLocator; +use ArrayIterator; use ComposerRequireChecker\NodeVisitor\DefinedSymbolCollector; +use ComposerRequireChecker\NodeVisitor\IncludeCollector; use PhpParser\NodeTraverser; use PhpParser\NodeVisitor\NameResolver; use Traversable; @@ -21,15 +23,17 @@ public function __invoke(Traversable $ASTs): array $traverser->addVisitor(new NameResolver()); $traverser->addVisitor($collector = new DefinedSymbolCollector()); + $traverser->addVisitor($includes = new IncludeCollector()); $astSymbols = []; + $additionalFiles = []; foreach ($ASTs as $astRoot) { - $traverser->traverse($astRoot); - + $traverser->traverse($astRoot->getAst()); $astSymbols[] = $collector->getDefinedSymbols(); + $additionalFiles = array_merge($additionalFiles, $includes->getIncluded($astRoot->getFile())); } - return array_values(array_unique(array_merge([], ...$astSymbols))); + return [array_values(array_unique(array_merge([], ...$astSymbols))), new ArrayIterator($additionalFiles)]; } } diff --git a/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php b/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php new file mode 100644 index 00000000..b039150a --- /dev/null +++ b/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php @@ -0,0 +1,129 @@ +included = []; + return parent::beforeTraverse($nodes); + } + + /** + * @param string $file + * @return string[] + */ + public function getIncluded(string $file): array + { + $included = []; + foreach ($this->included as $exp) { + try { + $this->computePath($included, $this->processIncludePath($exp, $file), $file); + } catch(InvalidArgumentException $x) { + var_dump($x->getMessage()); + } + } + return $included; + } + + /** + * @param array $included + * @param string $path + * @param string $self + * @return void + */ + private function computePath(array &$included, string $path, string $self) + { + if (!preg_match('#^([A-Z]:)?/#i', str_replace('\\', '/', $path))) { + $path = dirname($self).'/'.$path; + } + if (false === strpos($path, '{var}')) { + $included[] = $path; + return; + } + $parts = explode('{var}', $path); + $regex = []; + foreach($parts as $part) { + $regex[] = preg_quote(str_replace('\\', '/', $part), '/'); + } + $regex = '/^'.implode('.+', $regex).'$/'; + $self = str_replace('\\', '/', $self); + foreach (new RecursiveIteratorIterator( + new RecursiveDirectoryIterator( + $parts[0], + FilesystemIterator::CURRENT_AS_PATHNAME | FilesystemIterator::SKIP_DOTS + ) + ) as $file) { + $rfile = str_replace('\\', '/', $file); + if ($rfile !== $self && preg_match('/\\.php$/i', $rfile) && preg_match($regex, $rfile)) { + $included[] = $file; + } + } + } + + /** + * @param string|Exp $exp + * @param string $file + * @return string + * @throws InvalidArgumentException + */ + private function processIncludePath($exp, string $file): string + { + if (is_string($exp)) { + return $exp; + } + if ($exp instanceof Concat) { + return $this->processIncludePath($exp->left, $file).$this->processIncludePath($exp->right, $file); + } + if ($exp instanceof Dir) { + return dirname($file); + } + if ($exp instanceof File) { + return $file; + } + if ($exp instanceof ConstFetch && $exp->name === 'DIRECTORY_SEPARATOR') { + return DIRECTORY_SEPARATOR; + } + if ($exp instanceof String_) { + return $exp->value; + } + if ($exp instanceof Variable || $exp instanceof ConstFetch) { + return '{var}'; + } + throw new InvalidArgumentException('can\'t yet handle '.$exp->getType()); + } + + /** + * {@inheritDoc} + */ + public function enterNode(Node $node) + { + if ($node instanceof Include_) { + $this->included[] = $node->expr; + } + } +} diff --git a/src/ComposerRequireChecker/UsedSymbolsLocator/LocateUsedSymbolsFromASTRoots.php b/src/ComposerRequireChecker/UsedSymbolsLocator/LocateUsedSymbolsFromASTRoots.php index 5d72bd88..315c0685 100644 --- a/src/ComposerRequireChecker/UsedSymbolsLocator/LocateUsedSymbolsFromASTRoots.php +++ b/src/ComposerRequireChecker/UsedSymbolsLocator/LocateUsedSymbolsFromASTRoots.php @@ -25,7 +25,7 @@ public function __invoke(Traversable $ASTs): array $astSymbols = []; foreach ($ASTs as $astRoot) { - $traverser->traverse($astRoot); + $traverser->traverse($astRoot->getAst()); $astSymbols[] = $collector->getCollectedSymbols(); } From 5ee1670656815ef24fe3874fdafc85f69761d0dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 22 Apr 2018 15:03:20 +0200 Subject: [PATCH 02/21] fixing type issues of tests due to changes in codestructure --- .../LocateUsedSymbolsFromASTRootsTest.php | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/test/ComposerRequireCheckerTest/UsedSymbolsLocator/LocateUsedSymbolsFromASTRootsTest.php b/test/ComposerRequireCheckerTest/UsedSymbolsLocator/LocateUsedSymbolsFromASTRootsTest.php index 8f7350f6..69336f68 100644 --- a/test/ComposerRequireCheckerTest/UsedSymbolsLocator/LocateUsedSymbolsFromASTRootsTest.php +++ b/test/ComposerRequireCheckerTest/UsedSymbolsLocator/LocateUsedSymbolsFromASTRootsTest.php @@ -3,6 +3,7 @@ namespace ComposerRequireCheckerTest\UsedSymbolsLocator; use ArrayObject; +use ComposerRequireChecker\ASTLocator\FileAST; use ComposerRequireChecker\UsedSymbolsLocator\LocateUsedSymbolsFromASTRoots; use PhpParser\Node\Name; use PhpParser\Node\Stmt\Class_; @@ -28,7 +29,9 @@ public function testNoAsts() $asts = []; $symbols = $this->locate($asts); - $this->assertCount(0, $symbols); + $this->assertCount(2, $symbols); + $this->assertCount(0, $symbols[0]); + $this->assertCount(0, $symbols[1]); } public function testLocate() @@ -37,8 +40,10 @@ public function testLocate() $node->extends = new Name('Bar'); $symbols = $this->locate([[$node]]); - $this->assertCount(1, $symbols); - $this->assertContains('Bar', $symbols); + $this->assertCount(2, $symbols); + $this->assertCount(1, $symbols[0]); + $this->assertContains('Bar', $symbols[0]); + $this->assertCount(0, $symbols[1]); } /** @@ -46,6 +51,6 @@ public function testLocate() */ private function locate(array $asts): array { - return ($this->locator)(new ArrayObject($asts)); + return ($this->locator)(new FileAST('', new ArrayObject($asts))); } } From 30573de6311055c60b16eba337bda79c1ed2eec4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 3 Jun 2018 23:42:42 +0200 Subject: [PATCH 03/21] properly wrapping asts in object --- .../LocateDefinedSymbolsFromASTRootsTest.php | 4 ++++ .../UsedSymbolsLocator/LocateUsedSymbolsFromASTRootsTest.php | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/test/ComposerRequireCheckerTest/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRootsTest.php b/test/ComposerRequireCheckerTest/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRootsTest.php index b9b29299..cd664732 100644 --- a/test/ComposerRequireCheckerTest/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRootsTest.php +++ b/test/ComposerRequireCheckerTest/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRootsTest.php @@ -3,6 +3,7 @@ namespace ComposerRequireCheckerTest\DefinedSymbolsLocator; use ArrayObject; +use ComposerRequireChecker\ASTLocator\FileAST; use ComposerRequireChecker\DefinedSymbolsLocator\LocateDefinedSymbolsFromASTRoots; use PhpParser\Node\Arg; use PhpParser\Node\Expr\FuncCall; @@ -131,6 +132,9 @@ public function testBasicDoNotLocateNamespacedDefineCalls() private function locate(array $roots): array { + foreach($roots as &$ast) { + $ast = new FileAST('', $ast); + } return ($this->locator)(new ArrayObject($roots)); } } diff --git a/test/ComposerRequireCheckerTest/UsedSymbolsLocator/LocateUsedSymbolsFromASTRootsTest.php b/test/ComposerRequireCheckerTest/UsedSymbolsLocator/LocateUsedSymbolsFromASTRootsTest.php index 69336f68..bedc388e 100644 --- a/test/ComposerRequireCheckerTest/UsedSymbolsLocator/LocateUsedSymbolsFromASTRootsTest.php +++ b/test/ComposerRequireCheckerTest/UsedSymbolsLocator/LocateUsedSymbolsFromASTRootsTest.php @@ -51,6 +51,9 @@ public function testLocate() */ private function locate(array $asts): array { - return ($this->locator)(new FileAST('', new ArrayObject($asts))); + foreach($asts as &$ast) { + $ast = new FileAST('', $ast); + } + return ($this->locator)(new ArrayObject($asts)); } } From c4591c334528aa92cf12d5af06d443f201b72a40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 15 Jul 2018 11:08:04 +0200 Subject: [PATCH 04/21] fixing expected structure in locate tests --- .../LocateDefinedSymbolsFromASTRootsTest.php | 44 ++++++++++++------- .../LocateUsedSymbolsFromASTRootsTest.php | 10 ++--- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/test/ComposerRequireCheckerTest/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRootsTest.php b/test/ComposerRequireCheckerTest/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRootsTest.php index cd664732..6da9923f 100644 --- a/test/ComposerRequireCheckerTest/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRootsTest.php +++ b/test/ComposerRequireCheckerTest/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRootsTest.php @@ -33,7 +33,9 @@ public function testNoRoots() { $symbols = $this->locate([]); - $this->assertCount(0, $symbols); + $this->assertCount(2, $symbols); + $this->assertCount(0, $symbols[0]); + $this->assertCount(0, $symbols[1]); } public function testBasicLocateClass() @@ -46,11 +48,13 @@ public function testBasicLocateClass() $symbols = $this->locate([$roots]); $this->assertInternalType('array', $symbols); - $this->assertCount(3, $symbols); + $this->assertCount(2, $symbols); + $this->assertCount(3, $symbols[0]); + $this->assertCount(0, $symbols[1]); - $this->assertContains('MyClassA', $symbols); - $this->assertContains('MyClassB', $symbols); - $this->assertContains('MyClassC', $symbols); + $this->assertContains('MyClassA', $symbols[0]); + $this->assertContains('MyClassB', $symbols[0]); + $this->assertContains('MyClassC', $symbols[0]); } public function testBasicLocateFunctions() @@ -64,9 +68,11 @@ public function testBasicLocateFunctions() $this->assertInternalType('array', $symbols); $this->assertCount(2, $symbols); + $this->assertCount(2, $symbols[0]); + $this->assertCount(0, $symbols[1]); - $this->assertContains('myFunctionA', $symbols); - $this->assertContains('myFunctionB', $symbols); + $this->assertContains('myFunctionA', $symbols[0]); + $this->assertContains('myFunctionB', $symbols[0]); } public function testBasicLocateTrait() @@ -79,11 +85,13 @@ public function testBasicLocateTrait() $symbols = $this->locate([$roots]); $this->assertInternalType('array', $symbols); - $this->assertCount(3, $symbols); + $this->assertCount(2, $symbols); + $this->assertCount(3, $symbols[0]); + $this->assertCount(0, $symbols[1]); - $this->assertContains('MyTraitA', $symbols); - $this->assertContains('MyTraitB', $symbols); - $this->assertContains('MyTraitC', $symbols); + $this->assertContains('MyTraitA', $symbols[0]); + $this->assertContains('MyTraitB', $symbols[0]); + $this->assertContains('MyTraitC', $symbols[0]); } public function testBasicLocateAnonymous() @@ -95,7 +103,9 @@ public function testBasicLocateAnonymous() $symbols = $this->locate([$roots]); $this->assertInternalType('array', $symbols); - $this->assertCount(0, $symbols); + $this->assertCount(2, $symbols); + $this->assertCount(0, $symbols[0]); + $this->assertCount(0, $symbols[1]); } public function testBasicLocateDefineCalls() @@ -110,8 +120,9 @@ public function testBasicLocateDefineCalls() $symbols = $this->locate([$roots]); $this->assertInternalType('array', $symbols); - $this->assertCount(1, $symbols); - + $this->assertCount(2, $symbols); + $this->assertCount(1, $symbols[0]); + $this->assertCount(0, $symbols[1]); } public function testBasicDoNotLocateNamespacedDefineCalls() @@ -126,8 +137,9 @@ public function testBasicDoNotLocateNamespacedDefineCalls() $symbols = $this->locate([$roots]); $this->assertInternalType('array', $symbols); - $this->assertCount(0, $symbols); - + $this->assertCount(2, $symbols); + $this->assertCount(0, $symbols[0]); + $this->assertCount(0, $symbols[1]); } private function locate(array $roots): array diff --git a/test/ComposerRequireCheckerTest/UsedSymbolsLocator/LocateUsedSymbolsFromASTRootsTest.php b/test/ComposerRequireCheckerTest/UsedSymbolsLocator/LocateUsedSymbolsFromASTRootsTest.php index bedc388e..bc078275 100644 --- a/test/ComposerRequireCheckerTest/UsedSymbolsLocator/LocateUsedSymbolsFromASTRootsTest.php +++ b/test/ComposerRequireCheckerTest/UsedSymbolsLocator/LocateUsedSymbolsFromASTRootsTest.php @@ -29,9 +29,7 @@ public function testNoAsts() $asts = []; $symbols = $this->locate($asts); - $this->assertCount(2, $symbols); - $this->assertCount(0, $symbols[0]); - $this->assertCount(0, $symbols[1]); + $this->assertCount(0, $symbols); } public function testLocate() @@ -40,10 +38,8 @@ public function testLocate() $node->extends = new Name('Bar'); $symbols = $this->locate([[$node]]); - $this->assertCount(2, $symbols); - $this->assertCount(1, $symbols[0]); - $this->assertContains('Bar', $symbols[0]); - $this->assertCount(0, $symbols[1]); + $this->assertCount(1, $symbols); + $this->assertContains('Bar', $symbols); } /** From 62656b1ca9e5d87add0a851ac70b3f2a62f0b0e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 15 Jul 2018 12:12:18 +0200 Subject: [PATCH 05/21] removing overlooked var_dump adjusting Name-handling to actually work fixing typo in param type declaration --- .../NodeVisitor/IncludeCollector.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php b/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php index b039150a..1c9e3235 100644 --- a/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php +++ b/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php @@ -43,8 +43,8 @@ public function getIncluded(string $file): array foreach ($this->included as $exp) { try { $this->computePath($included, $this->processIncludePath($exp, $file), $file); - } catch(InvalidArgumentException $x) { - var_dump($x->getMessage()); + } catch(InvalidArgumentException $exception) { + // not sure there's anything sensible to do here } } return $included; @@ -86,7 +86,7 @@ private function computePath(array &$included, string $path, string $self) } /** - * @param string|Exp $exp + * @param string|Expr $exp * @param string $file * @return string * @throws InvalidArgumentException @@ -105,7 +105,7 @@ private function processIncludePath($exp, string $file): string if ($exp instanceof File) { return $file; } - if ($exp instanceof ConstFetch && $exp->name === 'DIRECTORY_SEPARATOR') { + if ($exp instanceof ConstFetch && "$exp->name" === 'DIRECTORY_SEPARATOR') { return DIRECTORY_SEPARATOR; } if ($exp instanceof String_) { From 274d7b272e55adf19a72665c1755551c237864c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 15 Jul 2018 12:23:38 +0200 Subject: [PATCH 06/21] fixing spacing issues --- .../LocateDefinedSymbolsFromASTRoots.php | 2 +- .../FileLocator/LocateAllFilesByExtension.php | 2 +- .../NodeVisitor/IncludeCollector.php | 12 ++++++------ .../LocateDefinedSymbolsFromASTRootsTest.php | 2 +- .../LocateUsedSymbolsFromASTRootsTest.php | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php b/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php index 561d02ff..f18191a2 100644 --- a/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php +++ b/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php @@ -14,7 +14,7 @@ final class LocateDefinedSymbolsFromASTRoots /** * @param Traversable|array[] $ASTs a series of AST roots * - * @return string[] all the found symbols + * @return array [all the found symbols, includes to be processed] */ public function __invoke(Traversable $ASTs): array { diff --git a/src/ComposerRequireChecker/FileLocator/LocateAllFilesByExtension.php b/src/ComposerRequireChecker/FileLocator/LocateAllFilesByExtension.php index 8e07316c..13d4e926 100644 --- a/src/ComposerRequireChecker/FileLocator/LocateAllFilesByExtension.php +++ b/src/ComposerRequireChecker/FileLocator/LocateAllFilesByExtension.php @@ -28,7 +28,7 @@ private function filterFilesByExtension(Traversable $files, string $fileExtensio /* @var $file \SplFileInfo */ foreach ($files as $file) { - if ($blacklist && preg_match('{('.implode('|', $blacklist).')}', $file->getPathname())) { + if ($blacklist && preg_match('{(' . implode('|', $blacklist) . ')}', $file->getPathname())) { continue; } diff --git a/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php b/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php index 1c9e3235..4d52b25a 100644 --- a/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php +++ b/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php @@ -43,7 +43,7 @@ public function getIncluded(string $file): array foreach ($this->included as $exp) { try { $this->computePath($included, $this->processIncludePath($exp, $file), $file); - } catch(InvalidArgumentException $exception) { + } catch (InvalidArgumentException $exception) { // not sure there's anything sensible to do here } } @@ -59,7 +59,7 @@ public function getIncluded(string $file): array private function computePath(array &$included, string $path, string $self) { if (!preg_match('#^([A-Z]:)?/#i', str_replace('\\', '/', $path))) { - $path = dirname($self).'/'.$path; + $path = dirname($self) . '/' . $path; } if (false === strpos($path, '{var}')) { $included[] = $path; @@ -67,10 +67,10 @@ private function computePath(array &$included, string $path, string $self) } $parts = explode('{var}', $path); $regex = []; - foreach($parts as $part) { + foreach ($parts as $part) { $regex[] = preg_quote(str_replace('\\', '/', $part), '/'); } - $regex = '/^'.implode('.+', $regex).'$/'; + $regex = '/^' . implode('.+', $regex) . '$/'; $self = str_replace('\\', '/', $self); foreach (new RecursiveIteratorIterator( new RecursiveDirectoryIterator( @@ -97,7 +97,7 @@ private function processIncludePath($exp, string $file): string return $exp; } if ($exp instanceof Concat) { - return $this->processIncludePath($exp->left, $file).$this->processIncludePath($exp->right, $file); + return $this->processIncludePath($exp->left, $file) . $this->processIncludePath($exp->right, $file); } if ($exp instanceof Dir) { return dirname($file); @@ -114,7 +114,7 @@ private function processIncludePath($exp, string $file): string if ($exp instanceof Variable || $exp instanceof ConstFetch) { return '{var}'; } - throw new InvalidArgumentException('can\'t yet handle '.$exp->getType()); + throw new InvalidArgumentException('can\'t yet handle ' . $exp->getType()); } /** diff --git a/test/ComposerRequireCheckerTest/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRootsTest.php b/test/ComposerRequireCheckerTest/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRootsTest.php index 6da9923f..d81d7b35 100644 --- a/test/ComposerRequireCheckerTest/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRootsTest.php +++ b/test/ComposerRequireCheckerTest/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRootsTest.php @@ -144,7 +144,7 @@ public function testBasicDoNotLocateNamespacedDefineCalls() private function locate(array $roots): array { - foreach($roots as &$ast) { + foreach ($roots as &$ast) { $ast = new FileAST('', $ast); } return ($this->locator)(new ArrayObject($roots)); diff --git a/test/ComposerRequireCheckerTest/UsedSymbolsLocator/LocateUsedSymbolsFromASTRootsTest.php b/test/ComposerRequireCheckerTest/UsedSymbolsLocator/LocateUsedSymbolsFromASTRootsTest.php index bc078275..ace76fdb 100644 --- a/test/ComposerRequireCheckerTest/UsedSymbolsLocator/LocateUsedSymbolsFromASTRootsTest.php +++ b/test/ComposerRequireCheckerTest/UsedSymbolsLocator/LocateUsedSymbolsFromASTRootsTest.php @@ -47,7 +47,7 @@ public function testLocate() */ private function locate(array $asts): array { - foreach($asts as &$ast) { + foreach ($asts as &$ast) { $ast = new FileAST('', $ast); } return ($this->locator)(new ArrayObject($asts)); From 07c6d3b983db8fd0e64be1b5bde939bcef3273be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 15 Jul 2018 14:22:23 +0200 Subject: [PATCH 07/21] adding tests for include symbol collector functionality --- .../NodeVisitor/IncludeCollectorTest.php | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 test/ComposerRequireCheckerTest/NodeVisitor/IncludeCollectorTest.php diff --git a/test/ComposerRequireCheckerTest/NodeVisitor/IncludeCollectorTest.php b/test/ComposerRequireCheckerTest/NodeVisitor/IncludeCollectorTest.php new file mode 100644 index 00000000..fafc1170 --- /dev/null +++ b/test/ComposerRequireCheckerTest/NodeVisitor/IncludeCollectorTest.php @@ -0,0 +1,146 @@ + [__FILE__, [], []]; + yield "simple include" => [__FILE__, ['anywhere/here.php'], [__DIR__.'/anywhere/here.php']]; + yield "simple absolute include" => [__FILE__, ['/anywhere/here.php'], ['/anywhere/here.php']]; + yield "simple String_ include" => [ + __FILE__, + [new String_('anywhere/here.php')], + [__DIR__.'/anywhere/here.php'] + ]; + yield "absolute include by DIR" => [ + __FILE__, + [ + new Concat( + new Dir(), + new String_('anywhere/here.php') + ) + ], + [__DIR__.'anywhere/here.php'] + ]; + yield "absolute include by FILE" => [ + __FILE__, + [ + new Concat( + new File(), + new String_('anywhere/here.php') + ) + ], + [__FILE__.'anywhere/here.php'] + ]; + yield "includes with variables" => [ + __FILE__, + [ + new Concat( + new ConstFetch(new Name("NAME")), + new String_('.php') + ) + ], + [ + __DIR__.DIRECTORY_SEPARATOR.'DefinedSymbolCollectorFunctionalTest.php', + __DIR__.DIRECTORY_SEPARATOR.'DefinedSymbolCollectorTest.php', + __DIR__.DIRECTORY_SEPARATOR.'UsedSymbolCollectorFunctionalTest.php', + __DIR__.DIRECTORY_SEPARATOR.'UsedSymbolCollectorTest.php' + ] + ]; + yield "includes with constants" => [ + __FILE__, + [ + new Concat( + new Variable(new Name('name')), + new String_('.php') + ) + ], + [ + __DIR__.DIRECTORY_SEPARATOR.'DefinedSymbolCollectorFunctionalTest.php', + __DIR__.DIRECTORY_SEPARATOR.'DefinedSymbolCollectorTest.php', + __DIR__.DIRECTORY_SEPARATOR.'UsedSymbolCollectorFunctionalTest.php', + __DIR__.DIRECTORY_SEPARATOR.'UsedSymbolCollectorTest.php' + ] + ]; + } + + /** + * @test + * @dataProvider provideGetIncluded + * @param string $file + * @param array $included + * @param array $result + * @return void + */ + public function testGetIncluded(string $file, array $included, array $result): void + { + $collector = new IncludeCollector(); + $reflector = new ReflectionProperty($collector, 'included'); + $reflector->setAccessible(true); + $reflector->setValue($collector, $included); + $reflector->setAccessible(false); + $this->assertEquals($result, $collector->getIncluded($file)); + } + + /** + * @test + * @return void + */ + public function testBeforeTraverseEmptiesIncludes(): void + { + $collector = new IncludeCollector(); + $reflector = new ReflectionProperty($collector, 'included'); + $reflector->setAccessible(true); + $reflector->setValue($collector, ['a', '#', 'p']); + $reflector->setAccessible(false); + $this->assertAttributeCount(3, 'included', $collector); + $collector->beforeTraverse([]); + $this->assertAttributeCount(0, 'included', $collector); + } + + /** + * @return iterable + */ + public function provideEnterNode(): iterable + { + $expr = new String_(''); + yield "require" => [new Include_($expr, Include_::TYPE_REQUIRE), 1]; + yield "require_once" => [new Include_($expr, Include_::TYPE_REQUIRE_ONCE), 1]; + yield "include" => [new Include_($expr, Include_::TYPE_INCLUDE), 1]; + yield "include_once" => [new Include_($expr, Include_::TYPE_INCLUDE_ONCE), 1]; + yield "different node" => [$expr, 0]; + } + + /** + * @test + * @dataProvider provideEnterNode + * @param \ComposerRequireCheckerTest\NodeVisitor\Node $node + * @param int $count + * @return void + */ + public function testEnterNode(Node $node, int $count): void + { + $collector = new IncludeCollector(); + $collector->enterNode($node); + $this->assertAttributeCount($count, 'included', $collector); + } +} \ No newline at end of file From 95c3636d6e6126f8901a8b5e683d3c2889dd94ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 15 Jul 2018 14:28:24 +0200 Subject: [PATCH 08/21] cleaning up file --- .../NodeVisitor/IncludeCollectorTest.php | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/test/ComposerRequireCheckerTest/NodeVisitor/IncludeCollectorTest.php b/test/ComposerRequireCheckerTest/NodeVisitor/IncludeCollectorTest.php index fafc1170..bf1d0926 100644 --- a/test/ComposerRequireCheckerTest/NodeVisitor/IncludeCollectorTest.php +++ b/test/ComposerRequireCheckerTest/NodeVisitor/IncludeCollectorTest.php @@ -24,12 +24,12 @@ class IncludeCollectorTest extends TestCase public function provideGetIncluded(): iterable { yield "no includes" => [__FILE__, [], []]; - yield "simple include" => [__FILE__, ['anywhere/here.php'], [__DIR__.'/anywhere/here.php']]; + yield "simple include" => [__FILE__, ['anywhere/here.php'], [__DIR__ . '/anywhere/here.php']]; yield "simple absolute include" => [__FILE__, ['/anywhere/here.php'], ['/anywhere/here.php']]; yield "simple String_ include" => [ __FILE__, [new String_('anywhere/here.php')], - [__DIR__.'/anywhere/here.php'] + [__DIR__ . '/anywhere/here.php'] ]; yield "absolute include by DIR" => [ __FILE__, @@ -39,7 +39,7 @@ public function provideGetIncluded(): iterable new String_('anywhere/here.php') ) ], - [__DIR__.'anywhere/here.php'] + [__DIR__ . 'anywhere/here.php'] ]; yield "absolute include by FILE" => [ __FILE__, @@ -49,7 +49,7 @@ public function provideGetIncluded(): iterable new String_('anywhere/here.php') ) ], - [__FILE__.'anywhere/here.php'] + [__FILE__ . 'anywhere/here.php'] ]; yield "includes with variables" => [ __FILE__, @@ -60,10 +60,10 @@ public function provideGetIncluded(): iterable ) ], [ - __DIR__.DIRECTORY_SEPARATOR.'DefinedSymbolCollectorFunctionalTest.php', - __DIR__.DIRECTORY_SEPARATOR.'DefinedSymbolCollectorTest.php', - __DIR__.DIRECTORY_SEPARATOR.'UsedSymbolCollectorFunctionalTest.php', - __DIR__.DIRECTORY_SEPARATOR.'UsedSymbolCollectorTest.php' + __DIR__ . DIRECTORY_SEPARATOR . 'DefinedSymbolCollectorFunctionalTest.php', + __DIR__ . DIRECTORY_SEPARATOR . 'DefinedSymbolCollectorTest.php', + __DIR__ . DIRECTORY_SEPARATOR . 'UsedSymbolCollectorFunctionalTest.php', + __DIR__ . DIRECTORY_SEPARATOR . 'UsedSymbolCollectorTest.php' ] ]; yield "includes with constants" => [ @@ -75,10 +75,10 @@ public function provideGetIncluded(): iterable ) ], [ - __DIR__.DIRECTORY_SEPARATOR.'DefinedSymbolCollectorFunctionalTest.php', - __DIR__.DIRECTORY_SEPARATOR.'DefinedSymbolCollectorTest.php', - __DIR__.DIRECTORY_SEPARATOR.'UsedSymbolCollectorFunctionalTest.php', - __DIR__.DIRECTORY_SEPARATOR.'UsedSymbolCollectorTest.php' + __DIR__ . DIRECTORY_SEPARATOR . 'DefinedSymbolCollectorFunctionalTest.php', + __DIR__ . DIRECTORY_SEPARATOR . 'DefinedSymbolCollectorTest.php', + __DIR__ . DIRECTORY_SEPARATOR . 'UsedSymbolCollectorFunctionalTest.php', + __DIR__ . DIRECTORY_SEPARATOR . 'UsedSymbolCollectorTest.php' ] ]; } @@ -98,7 +98,10 @@ public function testGetIncluded(string $file, array $included, array $result): v $reflector->setAccessible(true); $reflector->setValue($collector, $included); $reflector->setAccessible(false); - $this->assertEquals($result, $collector->getIncluded($file)); + $collected = $collector->getIncluded($file); + sort($result); + sort($collected); + $this->assertEquals($result, $collected); } /** @@ -133,7 +136,7 @@ public function provideEnterNode(): iterable /** * @test * @dataProvider provideEnterNode - * @param \ComposerRequireCheckerTest\NodeVisitor\Node $node + * @param Node $node * @param int $count * @return void */ @@ -143,4 +146,4 @@ public function testEnterNode(Node $node, int $count): void $collector->enterNode($node); $this->assertAttributeCount($count, 'included', $collector); } -} \ No newline at end of file +} From 321e39b1e44976901729c7b342367c914fae4e35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 15 Jul 2018 14:38:49 +0200 Subject: [PATCH 09/21] adding tests for missing cases --- .../NodeVisitor/IncludeCollectorTest.php | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/ComposerRequireCheckerTest/NodeVisitor/IncludeCollectorTest.php b/test/ComposerRequireCheckerTest/NodeVisitor/IncludeCollectorTest.php index bf1d0926..328bc5c9 100644 --- a/test/ComposerRequireCheckerTest/NodeVisitor/IncludeCollectorTest.php +++ b/test/ComposerRequireCheckerTest/NodeVisitor/IncludeCollectorTest.php @@ -12,6 +12,7 @@ use PhpParser\Node\Scalar\MagicConst\Dir; use PhpParser\Node\Scalar\MagicConst\File; use PhpParser\Node\Scalar\String_; +use PhpParser\Node\Stmt\Expression; use PHPUnit\Framework\TestCase; use ReflectionProperty; @@ -81,6 +82,28 @@ public function provideGetIncluded(): iterable __DIR__ . DIRECTORY_SEPARATOR . 'UsedSymbolCollectorTest.php' ] ]; + yield "includes with DIRECTORY_SEPARATOR" => [ + __FILE__, + [ + new Concat( + new Dir(), + new Concat( + new ConstFetch(new Name('DIRECTORY_SEPARATOR')), + new String_('Example.php') + ) + ) + ], + [ + __DIR__ . DIRECTORY_SEPARATOR . 'Example.php' + ] + ]; + yield "includes with invalid node" => [ + __FILE__, + [ + new Expression(new String_('')) + ], + [] + ]; } /** From 81b7d95ffa08785d8db33da4c76210a0bdf26182 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 15 Jul 2018 15:37:52 +0200 Subject: [PATCH 10/21] replacing array of arrays with object for easier readability --- .gitignore | 21 +++--- .../Cli/CheckCommand.php | 9 ++- .../LocateDefinedSymbolsFromASTRoots.php | 7 +- .../LocatedSymbolsAndIncludes.php | 68 +++++++++++++++++++ .../LocateDefinedSymbolsFromASTRootsTest.php | 44 ++++-------- 5 files changed, 103 insertions(+), 46 deletions(-) create mode 100644 src/ComposerRequireChecker/DefinedSymbolsLocator/LocatedSymbolsAndIncludes.php diff --git a/.gitignore b/.gitignore index 1b2bc847..54d95575 100644 --- a/.gitignore +++ b/.gitignore @@ -1,10 +1,11 @@ -vendor -composer.lock -composer.phar -phpunit.xml -phpmd.xml -phpdox.xml -clover.xml -example/test-data -phing-latest.phar -build/ +vendor +composer.lock +composer.phar +phpunit.xml +phpmd.xml +phpdox.xml +clover.xml +example/test-data +phing-latest.phar +build/ +/nbproject/private/ \ No newline at end of file diff --git a/src/ComposerRequireChecker/Cli/CheckCommand.php b/src/ComposerRequireChecker/Cli/CheckCommand.php index 94edfc05..39af81a1 100644 --- a/src/ComposerRequireChecker/Cli/CheckCommand.php +++ b/src/ComposerRequireChecker/Cli/CheckCommand.php @@ -67,15 +67,14 @@ protected function execute(InputInterface $input, OutputInterface $output): int $sourcesASTs = $this->getASTFromFilesLocator($input); - list($definedVendorSymbols, $additionalFiles) = (new LocateDefinedSymbolsFromASTRoots())->__invoke($sourcesASTs( + $definedVendorSymbols = (new LocateDefinedSymbolsFromASTRoots())->__invoke($sourcesASTs( (new ComposeGenerators())->__invoke( $getPackageSourceFiles($composerJson), (new LocateComposerPackageDirectDependenciesSourceFiles())->__invoke($composerJson) ) )); - while ($additionalFiles && count($additionalFiles)) { - list($vendorSymbols, $additionalFiles) = (new LocateDefinedSymbolsFromASTRoots())->__invoke($sourcesASTs($additionalFiles)); - $definedVendorSymbols = array_unique(array_merge($vendorSymbols, $definedVendorSymbols)); + while (count($definedVendorSymbols->getIncludes())) { + (new LocateDefinedSymbolsFromASTRoots())->__invoke($sourcesASTs($definedVendorSymbols->getIncludes()), $definedVendorSymbols); } $definedExtensionSymbols = (new LocateDefinedSymbolsFromExtensions())->__invoke( @@ -91,7 +90,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $unknownSymbols = array_diff( $usedSymbols, - $definedVendorSymbols, + $definedVendorSymbols->getSymbols(), $definedExtensionSymbols, $options->getSymbolWhitelist() ); diff --git a/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php b/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php index f18191a2..55d30799 100644 --- a/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php +++ b/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php @@ -16,7 +16,7 @@ final class LocateDefinedSymbolsFromASTRoots * * @return array [all the found symbols, includes to be processed] */ - public function __invoke(Traversable $ASTs): array + public function __invoke(Traversable $ASTs, ?LocatedSymbolsAndIncludes $located = null): LocatedSymbolsAndIncludes { // note: dependency injection is not really feasible for these two, as they need to co-exist in parallel $traverser = new NodeTraverser(); @@ -33,7 +33,10 @@ public function __invoke(Traversable $ASTs): array $astSymbols[] = $collector->getDefinedSymbols(); $additionalFiles = array_merge($additionalFiles, $includes->getIncluded($astRoot->getFile())); } + $located = $located ?? new LocatedSymbolsAndIncludes(); - return [array_values(array_unique(array_merge([], ...$astSymbols))), new ArrayIterator($additionalFiles)]; + return $located + ->addSymbols($astSymbols) + ->setIncludes($additionalFiles); } } diff --git a/src/ComposerRequireChecker/DefinedSymbolsLocator/LocatedSymbolsAndIncludes.php b/src/ComposerRequireChecker/DefinedSymbolsLocator/LocatedSymbolsAndIncludes.php new file mode 100644 index 00000000..4a3bdb4b --- /dev/null +++ b/src/ComposerRequireChecker/DefinedSymbolsLocator/LocatedSymbolsAndIncludes.php @@ -0,0 +1,68 @@ +symbols; + } + + /** + * @return string[] + */ + public function getIncludes(): array + { + return $this->includes; + } + + /** + * @param string[] $symbols + * @return LocatedSymbolsAndIncludes + */ + public function addSymbols(array $symbols): LocatedSymbolsAndIncludes + { + $this->symbols = $this->arrayMergeUnique($this->symbols, $symbols); + return $this; + } + + /** + * @param string[] $includes + * @return LocatedSymbolsAndIncludes + */ + public function setIncludes(array $includes): LocatedSymbolsAndIncludes + { + $this->includes = array_diff($includes, $this->previousIncludes); + $this->previousIncludes = $this->arrayMergeUnique($this->previousIncludes, $includes); + return $this; + } + + /** + * @param array $into + * @param array $add + * @return array + */ + private function arrayMergeUnique(array $into, array $add): array + { + return array_values(array_unique(array_merge($into, ...$add))); + } +} \ No newline at end of file diff --git a/test/ComposerRequireCheckerTest/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRootsTest.php b/test/ComposerRequireCheckerTest/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRootsTest.php index d81d7b35..adc777f3 100644 --- a/test/ComposerRequireCheckerTest/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRootsTest.php +++ b/test/ComposerRequireCheckerTest/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRootsTest.php @@ -33,9 +33,7 @@ public function testNoRoots() { $symbols = $this->locate([]); - $this->assertCount(2, $symbols); - $this->assertCount(0, $symbols[0]); - $this->assertCount(0, $symbols[1]); + $this->assertCount(0, $symbols); } public function testBasicLocateClass() @@ -48,13 +46,11 @@ public function testBasicLocateClass() $symbols = $this->locate([$roots]); $this->assertInternalType('array', $symbols); - $this->assertCount(2, $symbols); - $this->assertCount(3, $symbols[0]); - $this->assertCount(0, $symbols[1]); + $this->assertCount(3, $symbols); - $this->assertContains('MyClassA', $symbols[0]); - $this->assertContains('MyClassB', $symbols[0]); - $this->assertContains('MyClassC', $symbols[0]); + $this->assertContains('MyClassA', $symbols); + $this->assertContains('MyClassB', $symbols); + $this->assertContains('MyClassC', $symbols); } public function testBasicLocateFunctions() @@ -68,11 +64,9 @@ public function testBasicLocateFunctions() $this->assertInternalType('array', $symbols); $this->assertCount(2, $symbols); - $this->assertCount(2, $symbols[0]); - $this->assertCount(0, $symbols[1]); - $this->assertContains('myFunctionA', $symbols[0]); - $this->assertContains('myFunctionB', $symbols[0]); + $this->assertContains('myFunctionA', $symbols); + $this->assertContains('myFunctionB', $symbols); } public function testBasicLocateTrait() @@ -85,13 +79,11 @@ public function testBasicLocateTrait() $symbols = $this->locate([$roots]); $this->assertInternalType('array', $symbols); - $this->assertCount(2, $symbols); - $this->assertCount(3, $symbols[0]); - $this->assertCount(0, $symbols[1]); + $this->assertCount(3, $symbols); - $this->assertContains('MyTraitA', $symbols[0]); - $this->assertContains('MyTraitB', $symbols[0]); - $this->assertContains('MyTraitC', $symbols[0]); + $this->assertContains('MyTraitA', $symbols); + $this->assertContains('MyTraitB', $symbols); + $this->assertContains('MyTraitC', $symbols); } public function testBasicLocateAnonymous() @@ -103,9 +95,7 @@ public function testBasicLocateAnonymous() $symbols = $this->locate([$roots]); $this->assertInternalType('array', $symbols); - $this->assertCount(2, $symbols); - $this->assertCount(0, $symbols[0]); - $this->assertCount(0, $symbols[1]); + $this->assertCount(0, $symbols); } public function testBasicLocateDefineCalls() @@ -120,9 +110,7 @@ public function testBasicLocateDefineCalls() $symbols = $this->locate([$roots]); $this->assertInternalType('array', $symbols); - $this->assertCount(2, $symbols); - $this->assertCount(1, $symbols[0]); - $this->assertCount(0, $symbols[1]); + $this->assertCount(1, $symbols); } public function testBasicDoNotLocateNamespacedDefineCalls() @@ -137,9 +125,7 @@ public function testBasicDoNotLocateNamespacedDefineCalls() $symbols = $this->locate([$roots]); $this->assertInternalType('array', $symbols); - $this->assertCount(2, $symbols); - $this->assertCount(0, $symbols[0]); - $this->assertCount(0, $symbols[1]); + $this->assertCount(0, $symbols); } private function locate(array $roots): array @@ -147,6 +133,6 @@ private function locate(array $roots): array foreach ($roots as &$ast) { $ast = new FileAST('', $ast); } - return ($this->locator)(new ArrayObject($roots)); + return ($this->locator)(new ArrayObject($roots))->getSymbols(); } } From 87db0e45897e3090716dbb13f2176b54c5ad8618 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 15 Jul 2018 15:55:12 +0200 Subject: [PATCH 11/21] correcting phpdoc --- .../LocateDefinedSymbolsFromASTRoots.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php b/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php index 55d30799..c3328d7b 100644 --- a/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php +++ b/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php @@ -2,7 +2,6 @@ namespace ComposerRequireChecker\DefinedSymbolsLocator; -use ArrayIterator; use ComposerRequireChecker\NodeVisitor\DefinedSymbolCollector; use ComposerRequireChecker\NodeVisitor\IncludeCollector; use PhpParser\NodeTraverser; @@ -13,8 +12,9 @@ final class LocateDefinedSymbolsFromASTRoots { /** * @param Traversable|array[] $ASTs a series of AST roots + * @param LocatedSymbolsAndIncludes|null previously found data if exists * - * @return array [all the found symbols, includes to be processed] + * @return LocatedSymbolsAndIncludes */ public function __invoke(Traversable $ASTs, ?LocatedSymbolsAndIncludes $located = null): LocatedSymbolsAndIncludes { From eb16a9a7c47b8ae75e6e38945ea8d3e975c5c37c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 15 Jul 2018 15:56:47 +0200 Subject: [PATCH 12/21] restoring gitignore --- .gitignore | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 54d95575..8ef3aacd 100644 --- a/.gitignore +++ b/.gitignore @@ -7,5 +7,4 @@ phpdox.xml clover.xml example/test-data phing-latest.phar -build/ -/nbproject/private/ \ No newline at end of file +build/ \ No newline at end of file From 14b0691456b3abb08952e92c07a77d903ffcf681 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 15 Jul 2018 16:30:59 +0200 Subject: [PATCH 13/21] extracting regex creation from IncludeCollector::computePath() --- .../LocateDefinedSymbolsFromASTRoots.php | 2 +- .../NodeVisitor/IncludeCollector.php | 23 +++++++++++++------ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php b/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php index c3328d7b..70b61aee 100644 --- a/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php +++ b/src/ComposerRequireChecker/DefinedSymbolsLocator/LocateDefinedSymbolsFromASTRoots.php @@ -12,7 +12,7 @@ final class LocateDefinedSymbolsFromASTRoots { /** * @param Traversable|array[] $ASTs a series of AST roots - * @param LocatedSymbolsAndIncludes|null previously found data if exists + * @param LocatedSymbolsAndIncludes|null $located previously found data if exists * * @return LocatedSymbolsAndIncludes */ diff --git a/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php b/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php index 4d52b25a..6f990100 100644 --- a/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php +++ b/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php @@ -65,16 +65,11 @@ private function computePath(array &$included, string $path, string $self) $included[] = $path; return; } - $parts = explode('{var}', $path); - $regex = []; - foreach ($parts as $part) { - $regex[] = preg_quote(str_replace('\\', '/', $part), '/'); - } - $regex = '/^' . implode('.+', $regex) . '$/'; + $regex = $this->pathWithVarToRegex($path); $self = str_replace('\\', '/', $self); foreach (new RecursiveIteratorIterator( new RecursiveDirectoryIterator( - $parts[0], + explode('{var}', $path)[0], FilesystemIterator::CURRENT_AS_PATHNAME | FilesystemIterator::SKIP_DOTS ) ) as $file) { @@ -85,6 +80,20 @@ private function computePath(array &$included, string $path, string $self) } } + /** + * @param string $path + * @return string + */ + private function pathWithVarToRegex(string $path): string + { + $parts = explode('{var}', $path); + $regex = []; + foreach ($parts as $part) { + $regex[] = preg_quote(str_replace('\\', '/', $part), '/'); + } + return '/^' . implode('.+', $regex) . '$/'; + } + /** * @param string|Expr $exp * @param string $file From 7d966b9f8be49a0307c52106ebb4cba3b05b43c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 15 Jul 2018 16:58:27 +0200 Subject: [PATCH 14/21] separating contant and variable handling from simple value returns --- .../NodeVisitor/IncludeCollector.php | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php b/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php index 6f990100..d3acf41b 100644 --- a/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php +++ b/src/ComposerRequireChecker/NodeVisitor/IncludeCollector.php @@ -105,9 +105,23 @@ private function processIncludePath($exp, string $file): string if (is_string($exp)) { return $exp; } + if ($exp instanceof String_) { + return $exp->value; + } if ($exp instanceof Concat) { return $this->processIncludePath($exp->left, $file) . $this->processIncludePath($exp->right, $file); } + return $this->replaceInIncludePath($exp, $file); + } + + /** + * @param Expr $exp + * @param string $file + * @return string + * @throws InvalidArgumentException + */ + private function replaceInIncludePath($exp, string $file) + { if ($exp instanceof Dir) { return dirname($file); } @@ -117,9 +131,6 @@ private function processIncludePath($exp, string $file): string if ($exp instanceof ConstFetch && "$exp->name" === 'DIRECTORY_SEPARATOR') { return DIRECTORY_SEPARATOR; } - if ($exp instanceof String_) { - return $exp->value; - } if ($exp instanceof Variable || $exp instanceof ConstFetch) { return '{var}'; } From 7669763b4401d42255c09c87045e6287933fbc73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 15 Jul 2018 17:51:45 +0200 Subject: [PATCH 15/21] checking table output for failures --- .gitignore | 2 +- .../Cli/CheckCommand.php | 25 ++++++++++--- .../Cli/CheckCommandTest.php | 37 +++++++++++++++++++ 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/.gitignore b/.gitignore index 8ef3aacd..2f020922 100644 --- a/.gitignore +++ b/.gitignore @@ -7,4 +7,4 @@ phpdox.xml clover.xml example/test-data phing-latest.phar -build/ \ No newline at end of file +build/ diff --git a/src/ComposerRequireChecker/Cli/CheckCommand.php b/src/ComposerRequireChecker/Cli/CheckCommand.php index 39af81a1..333a410b 100644 --- a/src/ComposerRequireChecker/Cli/CheckCommand.php +++ b/src/ComposerRequireChecker/Cli/CheckCommand.php @@ -88,13 +88,24 @@ protected function execute(InputInterface $input, OutputInterface $output): int throw new \LogicException('There were no symbols found, please check your configuration.'); } - $unknownSymbols = array_diff( - $usedSymbols, - $definedVendorSymbols->getSymbols(), - $definedExtensionSymbols, - $options->getSymbolWhitelist() + return $this->handleResult( + array_diff( + $usedSymbols, + $definedVendorSymbols->getSymbols(), + $definedExtensionSymbols, + $options->getSymbolWhitelist() + ), + $output ); + } + /** + * @param array $unknownSymbols + * @param OutputInterface $output + * @return int + */ + private function handleResult(?array $unknownSymbols, OutputInterface $output): int + { if (!$unknownSymbols) { $output->writeln("There were no unknown symbols found."); return 0; @@ -116,6 +127,10 @@ protected function execute(InputInterface $input, OutputInterface $output): int return ((int)(bool)$unknownSymbols); } + /** + * @param InputInterface $input + * @return Options + */ private function getCheckOptions(InputInterface $input): Options { $fileName = $input->getOption('config-file'); diff --git a/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php b/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php index c955f5ba..a67ae68c 100644 --- a/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php +++ b/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php @@ -3,7 +3,11 @@ namespace ComposerRequireCheckerTest\Cli; use ComposerRequireChecker\Cli\Application; +use ComposerRequireChecker\Cli\CheckCommand; use PHPUnit\Framework\TestCase; +use ReflectionMethod; +use Symfony\Component\Console\Formatter\OutputFormatterInterface; +use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Tester\CommandTester; class CheckCommandTest extends TestCase @@ -41,4 +45,37 @@ public function testSelfCheckShowsNoErrors() $this->assertSame(0, $this->commandTester->getStatusCode()); $this->assertContains('no unknown symbols found', $this->commandTester->getDisplay()); } + + /** + * @test + * @return void + */ + public function testHandleResultForFailure(): void + { + $command = new CheckCommand(); + $method = new ReflectionMethod($command, 'handleResult'); + $method->setAccessible(true); + $output = $this->getMockBuilder(OutputInterface::class)->getMock(); + $output->expects($this->exactly(8)) + ->method('writeln') + ->withConsecutive( + ["The following unknown symbols were found:"], + ["+--+--+"], + ["| unknown symbol | guessed dependency |"], + ["+--+--+"], + ["| A_Symbol | |"], + ["| A\\Nother\\Symbol | |"], + ["| A_Third\\Symbol | |"], + ["+--+--+"] + ); + $output->expects($this->any()) + ->method('getFormatter') + ->with() + ->willReturn($this->getMockBuilder(OutputFormatterInterface::class)->getMock()); + $this->assertEquals(1, $method->invoke( + $command, + ['A_Symbol', 'A\\Nother\\Symbol', 'A_Third\\Symbol'], + $output + )); + } } From 354b648fbee6f0f81806982a44e1bfad8f2f97dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 15 Jul 2018 18:26:42 +0200 Subject: [PATCH 16/21] ignoring empty lines in output --- .../Cli/CheckCommandTest.php | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php b/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php index a67ae68c..67c705c9 100644 --- a/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php +++ b/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php @@ -28,6 +28,7 @@ public function setUp() public function testExceptionIfComposerJsonNotFound() { + $this->markTestSkipped(); self::expectException(\InvalidArgumentException::class); $this->commandTester->execute([ @@ -37,6 +38,7 @@ public function testExceptionIfComposerJsonNotFound() public function testSelfCheckShowsNoErrors() { + $this->markTestSkipped(); $this->commandTester->execute([ // that's our own composer.json, lets be sure our self check does not throw errors 'composer-json' => dirname(__DIR__, 3) . '/composer.json' @@ -56,18 +58,14 @@ public function testHandleResultForFailure(): void $method = new ReflectionMethod($command, 'handleResult'); $method->setAccessible(true); $output = $this->getMockBuilder(OutputInterface::class)->getMock(); - $output->expects($this->exactly(8)) + $printed = []; + $output->expects($this->any()) ->method('writeln') - ->withConsecutive( - ["The following unknown symbols were found:"], - ["+--+--+"], - ["| unknown symbol | guessed dependency |"], - ["+--+--+"], - ["| A_Symbol | |"], - ["| A\\Nother\\Symbol | |"], - ["| A_Third\\Symbol | |"], - ["+--+--+"] - ); + ->willReturnCallback(function ($line) use (&$printed) { + if ($line) { + $printed[] = $line; + } + }); $output->expects($this->any()) ->method('getFormatter') ->with() @@ -77,5 +75,18 @@ public function testHandleResultForFailure(): void ['A_Symbol', 'A\\Nother\\Symbol', 'A_Third\\Symbol'], $output )); + $this->assertEquals( + [ + "The following unknown symbols were found:", + "+--+--+", + "| unknown symbol | guessed dependency |", + "+--+--+", + "| A_Symbol | |", + "| A\\Nother\\Symbol | |", + "| A_Third\\Symbol | |", + "+--+--+" + ], + $printed + ); } } From 7fb8fc8c9339b13e23984a4d274da7f11db33636 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 15 Jul 2018 18:34:48 +0200 Subject: [PATCH 17/21] reverting skip --- .../Cli/CheckCommandTest.php | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php b/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php index 67c705c9..e6bab70f 100644 --- a/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php +++ b/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php @@ -28,7 +28,6 @@ public function setUp() public function testExceptionIfComposerJsonNotFound() { - $this->markTestSkipped(); self::expectException(\InvalidArgumentException::class); $this->commandTester->execute([ @@ -38,7 +37,6 @@ public function testExceptionIfComposerJsonNotFound() public function testSelfCheckShowsNoErrors() { - $this->markTestSkipped(); $this->commandTester->execute([ // that's our own composer.json, lets be sure our self check does not throw errors 'composer-json' => dirname(__DIR__, 3) . '/composer.json' @@ -59,13 +57,17 @@ public function testHandleResultForFailure(): void $method->setAccessible(true); $output = $this->getMockBuilder(OutputInterface::class)->getMock(); $printed = []; + $collect = function ($line) use (&$printed) { + if ($line) { + $printed[] = $line; + } + }; $output->expects($this->any()) ->method('writeln') - ->willReturnCallback(function ($line) use (&$printed) { - if ($line) { - $printed[] = $line; - } - }); + ->willReturnCallback($collect); + $output->expects($this->any()) + ->method('write') + ->willReturnCallback($collect); $output->expects($this->any()) ->method('getFormatter') ->with() From c2ee536f3e1ff2d3dc9560e115f88311a8a7ad9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 15 Jul 2018 19:22:19 +0200 Subject: [PATCH 18/21] hopefully handling weird array differences for output --- test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php b/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php index e6bab70f..c85b2d56 100644 --- a/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php +++ b/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php @@ -78,7 +78,7 @@ public function testHandleResultForFailure(): void $output )); $this->assertEquals( - [ + implode("", [ "The following unknown symbols were found:", "+--+--+", "| unknown symbol | guessed dependency |", @@ -87,8 +87,8 @@ public function testHandleResultForFailure(): void "| A\\Nother\\Symbol | |", "| A_Third\\Symbol | |", "+--+--+" - ], - $printed + ]), + implode("", $printed) ); } } From 2a82f26b66c154803a0bde6918e48693a083caef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 15 Jul 2018 19:49:35 +0200 Subject: [PATCH 19/21] fixing indentation --- test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php b/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php index c85b2d56..fea3075c 100644 --- a/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php +++ b/test/ComposerRequireCheckerTest/Cli/CheckCommandTest.php @@ -59,7 +59,7 @@ public function testHandleResultForFailure(): void $printed = []; $collect = function ($line) use (&$printed) { if ($line) { - $printed[] = $line; + $printed[] = $line; } }; $output->expects($this->any()) From 482c363e1a1ca786c62da0f3509bc0eb6ca05577 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 15 Jul 2018 20:45:42 +0200 Subject: [PATCH 20/21] completing types where missing --- .../ASTLocator/FileAST.php | 16 +- .../LocatedSymbolsAndIncludes.php | 140 +++++++++--------- 2 files changed, 81 insertions(+), 75 deletions(-) diff --git a/src/ComposerRequireChecker/ASTLocator/FileAST.php b/src/ComposerRequireChecker/ASTLocator/FileAST.php index 20ff3c1e..9eb8e5ce 100644 --- a/src/ComposerRequireChecker/ASTLocator/FileAST.php +++ b/src/ComposerRequireChecker/ASTLocator/FileAST.php @@ -2,7 +2,7 @@ namespace ComposerRequireChecker\ASTLocator; -use Traversable; +use PhpParser\Node; class FileAST { @@ -12,33 +12,33 @@ class FileAST private $file; /** - * @var Traversable + * @var array */ private $ast; /** * @param string $file - * @param Traversable|array|null $ast + * @param array|null $ast * */ - public function __construct(string $file, $ast) + public function __construct(string $file, ?array $ast) { $this->file = $file; - $this->ast = $ast; + $this->ast = $ast ?? []; } /** * @return string */ - public function getFile() + public function getFile(): string { return $this->file; } /** - * @return Traversable|array + * @return Node[] */ - public function getAst() + public function getAst(): array { return $this->ast; } diff --git a/src/ComposerRequireChecker/DefinedSymbolsLocator/LocatedSymbolsAndIncludes.php b/src/ComposerRequireChecker/DefinedSymbolsLocator/LocatedSymbolsAndIncludes.php index 4a3bdb4b..c1e08683 100644 --- a/src/ComposerRequireChecker/DefinedSymbolsLocator/LocatedSymbolsAndIncludes.php +++ b/src/ComposerRequireChecker/DefinedSymbolsLocator/LocatedSymbolsAndIncludes.php @@ -1,68 +1,74 @@ -symbols; - } - - /** - * @return string[] - */ - public function getIncludes(): array - { - return $this->includes; - } - - /** - * @param string[] $symbols - * @return LocatedSymbolsAndIncludes - */ - public function addSymbols(array $symbols): LocatedSymbolsAndIncludes - { - $this->symbols = $this->arrayMergeUnique($this->symbols, $symbols); - return $this; - } - - /** - * @param string[] $includes - * @return LocatedSymbolsAndIncludes - */ - public function setIncludes(array $includes): LocatedSymbolsAndIncludes - { - $this->includes = array_diff($includes, $this->previousIncludes); - $this->previousIncludes = $this->arrayMergeUnique($this->previousIncludes, $includes); - return $this; - } - - /** - * @param array $into - * @param array $add - * @return array - */ - private function arrayMergeUnique(array $into, array $add): array - { - return array_values(array_unique(array_merge($into, ...$add))); - } +symbols; + } + + /** + * @return Traversable|string[] + */ + public function getIncludes(): Traversable + { + return new ArrayIterator($this->includes); + } + + /** + * @param string[] $symbols + * @return LocatedSymbolsAndIncludes + */ + public function addSymbols(array $symbols): LocatedSymbolsAndIncludes + { + $this->symbols = $this->arrayMergeUnique($this->symbols, $symbols); + return $this; + } + + /** + * @param string[] $includes + * @return LocatedSymbolsAndIncludes + */ + public function setIncludes(array $includes): LocatedSymbolsAndIncludes + { + $this->includes = array_diff($includes, $this->previousIncludes); + $this->previousIncludes = $this->arrayMergeUnique($this->previousIncludes, [$includes]); + return $this; + } + + /** + * @param array $into + * @param array $add + * @return array + */ + private function arrayMergeUnique(array $into, array $add): array + { + if (empty($add)) { + return $into; + } + return array_values(array_unique(array_merge($into, ...$add))); + } } \ No newline at end of file From ca53f9631d743cf2b2e303aaa284ff3d89a9e823 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20B=C3=BCttner?= Date: Sun, 15 Jul 2018 23:46:40 +0200 Subject: [PATCH 21/21] reverting --- .gitignore | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/.gitignore b/.gitignore index 2f020922..1b2bc847 100644 --- a/.gitignore +++ b/.gitignore @@ -1,10 +1,10 @@ -vendor -composer.lock -composer.phar -phpunit.xml -phpmd.xml -phpdox.xml -clover.xml -example/test-data -phing-latest.phar -build/ +vendor +composer.lock +composer.phar +phpunit.xml +phpmd.xml +phpdox.xml +clover.xml +example/test-data +phing-latest.phar +build/